Skip to content

Commit

Permalink
chore!: Remove meteor/check from banners endpoints (#32509)
Browse files Browse the repository at this point in the history
  • Loading branch information
@matheusbsilva137 @ggazzo
matheusbsilva137 authored and ggazzo committed Sep 11, 2024
1 parent 944e0fa commit 3bb5e6e
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 59 deletions.
47 changes: 6 additions & 41 deletions apps/meteor/app/api/server/v1/banners.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Banner } from '@rocket.chat/core-services';
import { BannerPlatform } from '@rocket.chat/core-typings';
import { Match, check } from 'meteor/check';
import { isBannersDismissProps, isBannersGetNewProps, isBannersProps } from '@rocket.chat/rest-typings';

import { API } from '../api';

Expand Down Expand Up @@ -52,17 +51,10 @@ import { API } from '../api';
*/
API.v1.addRoute(
'banners.getNew',
{ authRequired: true, deprecation: { version: '8.0.0', alternatives: ['banners/:id', 'banners'] } },
{ authRequired: true, validateParams: isBannersGetNewProps, deprecation: { version: '8.0.0', alternatives: ['banners/:id', 'banners'] } },
{
// deprecated
async get() {
check(
this.queryParams,
Match.ObjectIncluding({
platform: Match.OneOf(...Object.values(BannerPlatform)),
bid: Match.Maybe(String),
}),
);

const { platform, bid: bannerId } = this.queryParams;

const banners = await Banner.getBannersForUser(this.userId, platform, bannerId ?? undefined);
Expand Down Expand Up @@ -120,23 +112,10 @@ API.v1.addRoute(
*/
API.v1.addRoute(
'banners/:id',
{ authRequired: true },
{ authRequired: true, validateParams: isBannersProps },
{
// TODO: move to users/:id/banners
async get() {
check(
this.urlParams,
Match.ObjectIncluding({
id: Match.Where((id: unknown): id is string => typeof id === 'string' && Boolean(id.trim())),
}),
);
check(
this.queryParams,
Match.ObjectIncluding({
platform: Match.OneOf(...Object.values(BannerPlatform)),
}),
);

const { platform } = this.queryParams;
const { id } = this.urlParams;

Expand Down Expand Up @@ -186,16 +165,9 @@ API.v1.addRoute(
*/
API.v1.addRoute(
'banners',
{ authRequired: true },
{ authRequired: true, validateParams: isBannersProps },
{
async get() {
check(
this.queryParams,
Match.ObjectIncluding({
platform: Match.OneOf(...Object.values(BannerPlatform)),
}),
);

const { platform } = this.queryParams;

const banners = await Banner.getBannersForUser(this.userId, platform);
Expand Down Expand Up @@ -240,16 +212,9 @@ API.v1.addRoute(
*/
API.v1.addRoute(
'banners.dismiss',
{ authRequired: true },
{ authRequired: true, validateParams: isBannersDismissProps },
{
async post() {
check(
this.bodyParams,
Match.ObjectIncluding({
bannerId: Match.Where((id: unknown): id is string => typeof id === 'string' && Boolean(id.trim())),
}),
);

const { bannerId } = this.bodyParams;

await Banner.dismiss(this.userId, bannerId);
Expand Down
144 changes: 141 additions & 3 deletions apps/meteor/tests/end-to-end/api/banners.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ describe('banners', () => {
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', "Match error: Missing key 'bannerId'");
expect(res.body).to.have.property('errorType', 'invalid-params');
})
.end(done);
});

it('should fail if missing bannerId is empty', (done) => {
it('should fail if bannerId is empty', (done) => {
void request
.post(api('banners.dismiss'))
.set(credentials)
Expand All @@ -118,7 +118,7 @@ describe('banners', () => {
.end(done);
});

it('should fail if missing bannerId is invalid', (done) => {
it('should fail if bannerId is invalid', (done) => {
void request
.post(api('banners.dismiss'))
.set(credentials)
Expand All @@ -132,4 +132,142 @@ describe('banners', () => {
.end(done);
});
});

describe('[/banners]', () => {
it('should fail if not logged in', async () => {
return request
.get(api('banners'))
.query({
platform: 'web',
})
.expect(401)
.expect((res) => {
expect(res.body).to.have.property('status', 'error');
expect(res.body).to.have.property('message');
});
});

it('should fail if missing platform', async () => {
return request
.get(api('banners'))
.set(credentials)
.query({})
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'invalid-params');
});
});

it('should fail if platform is invalid', async () => {
return request
.get(api('banners'))
.set(credentials)
.query({
platform: 'invalid-platform',
})
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'invalid-params');
});
});

it('should succesfully return web banners', async () => {
return request
.get(api('banners'))
.set(credentials)
.query({
platform: 'web',
})
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('banners').that.is.an('array');
});
});

it('should succesfully return mobile banners', async () => {
return request
.get(api('banners'))
.set(credentials)
.query({
platform: 'mobile',
})
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('banners').that.is.an('array');
});
});
});

describe('[/banners/:id]', () => {
it('should fail if not logged in', async () => {
return request
.get(api('banners/some-id'))
.query({
platform: 'web',
})
.expect(401)
.expect((res) => {
expect(res.body).to.have.property('status', 'error');
expect(res.body).to.have.property('message');
});
});

it('should fail if missing platform', async () => {
return request
.get(api('banners/some-id'))
.set(credentials)
.query({})
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'invalid-params');
});
});

it('should fail if platform is invalid', async () => {
return request
.get(api('banners/some-id'))
.set(credentials)
.query({
platform: 'invalid-platform',
})
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'invalid-params');
});
});

it('should succesfully return a web banner by id', async () => {
return request
.get(api('banners/some-id'))
.set(credentials)
.query({
platform: 'web',
})
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('banners').that.is.an('array');
});
});

it('should succesfully return a mobile banner by id', async () => {
return request
.get(api('banners/some-id'))
.set(credentials)
.query({
platform: 'mobile',
})
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('banners').that.is.an('array');
});
});
});
});
1 change: 1 addition & 0 deletions packages/rest-typings/src/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,4 @@ export * from './v1/groups';
export * from './v1/chat';
export * from './v1/auth';
export * from './v1/cloud';
export * from './v1/banners';
19 changes: 4 additions & 15 deletions packages/rest-typings/src/v1/banners.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ const BannersGetNewSchema = {
properties: {
platform: {
type: 'string',
enum: ['1', '2'],
enum: ['web', 'mobile'],
},
bid: {
type: 'string',
},
},
required: ['platform', 'bid'],
required: ['platform'],
additionalProperties: false,
};

Expand All @@ -31,19 +31,6 @@ type BannersId = {
platform: BannerPlatform;
};

const BannersIdSchema = {
type: 'object',
properties: {
platform: {
type: 'string',
},
},
required: ['platform'],
additionalProperties: false,
};

export const isBannersIdProps = ajv.compile<BannersId>(BannersIdSchema);

type Banners = {
platform: BannerPlatform;
};
Expand All @@ -53,6 +40,7 @@ const BannersSchema = {
properties: {
platform: {
type: 'string',
enum: ['web', 'mobile'],
},
},
required: ['platform'],
Expand All @@ -70,6 +58,7 @@ const BannersDismissSchema = {
properties: {
bannerId: {
type: 'string',
minLength: 1,
},
},
required: ['bannerId'],
Expand Down

0 comments on commit 3bb5e6e

Please sign in to comment.