diff --git a/apps/meteor/app/api/server/v1/banners.ts b/apps/meteor/app/api/server/v1/banners.ts index 48c94d3711bd..598497fcb837 100644 --- a/apps/meteor/app/api/server/v1/banners.ts +++ b/apps/meteor/app/api/server/v1/banners.ts @@ -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'; @@ -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); @@ -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; @@ -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); @@ -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); diff --git a/apps/meteor/tests/end-to-end/api/banners.ts b/apps/meteor/tests/end-to-end/api/banners.ts index a4fd2638e7dd..054f48532f68 100644 --- a/apps/meteor/tests/end-to-end/api/banners.ts +++ b/apps/meteor/tests/end-to-end/api/banners.ts @@ -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) @@ -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) @@ -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'); + }); + }); + }); }); diff --git a/packages/rest-typings/src/index.ts b/packages/rest-typings/src/index.ts index 0b2e73a7f25b..c6fb7a3943e2 100644 --- a/packages/rest-typings/src/index.ts +++ b/packages/rest-typings/src/index.ts @@ -268,3 +268,4 @@ export * from './v1/groups'; export * from './v1/chat'; export * from './v1/auth'; export * from './v1/cloud'; +export * from './v1/banners'; diff --git a/packages/rest-typings/src/v1/banners.ts b/packages/rest-typings/src/v1/banners.ts index 5ab1fc09c8f8..d3cedbcef75f 100644 --- a/packages/rest-typings/src/v1/banners.ts +++ b/packages/rest-typings/src/v1/banners.ts @@ -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, }; @@ -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(BannersIdSchema); - type Banners = { platform: BannerPlatform; }; @@ -53,6 +40,7 @@ const BannersSchema = { properties: { platform: { type: 'string', + enum: ['web', 'mobile'], }, }, required: ['platform'], @@ -70,6 +58,7 @@ const BannersDismissSchema = { properties: { bannerId: { type: 'string', + minLength: 1, }, }, required: ['bannerId'],