From 425a61d311a68f9715e5e0d4ca148d0463e36757 Mon Sep 17 00:00:00 2001 From: Heikki Hellgren Date: Mon, 9 Dec 2024 12:54:59 +0200 Subject: [PATCH] test(notifications): improve router tests improve the existing router tests for notifications to handle different endpoints. Signed-off-by: Heikki Hellgren --- .changeset/weak-lemons-sing.md | 5 + .../src/service/router.test.ts | 483 +++++++++++++++++- .../src/service/router.ts | 19 +- 3 files changed, 479 insertions(+), 28 deletions(-) create mode 100644 .changeset/weak-lemons-sing.md diff --git a/.changeset/weak-lemons-sing.md b/.changeset/weak-lemons-sing.md new file mode 100644 index 0000000000..e1a349b85e --- /dev/null +++ b/.changeset/weak-lemons-sing.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications-backend': patch +--- + +Improved notifications router tests diff --git a/plugins/notifications-backend/src/service/router.test.ts b/plugins/notifications-backend/src/service/router.test.ts index e557e5818d..46ee00c554 100644 --- a/plugins/notifications-backend/src/service/router.test.ts +++ b/plugins/notifications-backend/src/service/router.test.ts @@ -19,53 +19,99 @@ import request from 'supertest'; import { createRouter } from './router'; import { SignalsService } from '@backstage/plugin-signals-node'; import { - TestDatabases, mockCredentials, mockErrorHandler, mockServices, + TestDatabaseId, + TestDatabases, } from '@backstage/backend-test-utils'; import { NotificationSendOptions } from '@backstage/plugin-notifications-node'; import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; +import { DatabaseService } from '@backstage/backend-plugin-api'; +import { v4 as uuid } from 'uuid'; -describe('createRouter', () => { - const databases = TestDatabases.create(); +const databases = TestDatabases.create(); +async function createDatabase( + databaseId: TestDatabaseId, +): Promise { + const knex = await databases.init(databaseId); + return mockServices.database({ knex, migrations: { skip: false } }); +} + +describe.each(databases.eachSupportedId())('createRouter (%s)', databaseId => { let app: express.Express; + let database: DatabaseService; const signalService: jest.Mocked = { publish: jest.fn(), }; const userInfo = mockServices.userInfo(); - const httpAuth = mockServices.httpAuth({ - defaultCredentials: mockCredentials.service(), - }); + const auth = mockServices.auth(); const config = mockServices.rootConfig({ data: { app: { baseUrl: 'http://localhost' } }, }); - const catalog = catalogServiceMock(); - beforeAll(async () => { - const knex = await databases.init('SQLITE_3'); - const router = await createRouter({ - logger: mockServices.logger.mock(), - database: { getClient: async () => knex }, - signals: signalService, - userInfo, - config, - httpAuth, - auth, - catalog, - }); - app = express().use(router).use(mockErrorHandler()); + const catalog = catalogServiceMock({ + entities: [ + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'User', + metadata: { + name: 'mock', + namespace: 'default', + }, + }, + { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Group', + metadata: { + name: 'mock', + namespace: 'default', + }, + relations: [ + { + type: 'hasMember', + targetRef: 'user:default/mock', + }, + ], + }, + ], }); - beforeEach(() => { - jest.resetAllMocks(); + beforeAll(async () => { + database = await createDatabase(databaseId); }); describe('POST /notifications', () => { + const httpAuth = mockServices.httpAuth({ + defaultCredentials: mockCredentials.service(), + }); + + beforeAll(async () => { + const router = await createRouter({ + logger: mockServices.logger.mock(), + database, + signals: signalService, + userInfo, + config, + httpAuth, + auth, + catalog, + }); + app = express().use(router).use(mockErrorHandler()); + }); + + beforeEach(async () => { + jest.resetAllMocks(); + const client = await database.getClient(); + await client('notification').del(); + await client('broadcast').del(); + await client('user_settings').del(); + }); + const sendNotification = async (data: NotificationSendOptions) => request(app) .post('/notifications') @@ -84,7 +130,6 @@ describe('createRouter', () => { link: 'javascript:alert(document.domain)', }, }); - expect(javascriptXSS.status).toEqual(400); const ftpLink = await sendNotification({ @@ -124,6 +169,19 @@ describe('createRouter', () => { }); expect(httpsLink.status).toEqual(200); + expect(httpsLink.body).toEqual([ + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + severity: 'normal', + title: 'test notification', + link: 'https://example.com', + }, + user: null, + }, + ]); }); it('should accept relative links', async () => { @@ -138,6 +196,385 @@ describe('createRouter', () => { }); expect(catalogLink.status).toEqual(200); + expect(catalogLink.body).toEqual([ + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + severity: 'normal', + title: 'test notification', + link: '/catalog', + }, + user: null, + }, + ]); + }); + + it('should send to user entity', async () => { + const response = await sendNotification({ + recipients: { + type: 'entity', + entityRef: ['user:default/mock'], + }, + payload: { + title: 'test notification', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + severity: 'normal', + title: 'test notification', + }, + user: 'user:default/mock', + }, + ]); + + const client = await database.getClient(); + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(1); + }); + + it('should send to group entity', async () => { + const response = await sendNotification({ + recipients: { + type: 'entity', + entityRef: ['group:default/mock'], + }, + payload: { + title: 'test notification', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + severity: 'normal', + title: 'test notification', + }, + user: 'user:default/mock', + }, + ]); + + const client = await database.getClient(); + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(1); + }); + + it('should only send one notification per user', async () => { + const response = await sendNotification({ + recipients: { + type: 'entity', + entityRef: ['group:default/mock', 'user:default/mock'], + }, + payload: { + title: 'test notification', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([ + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + severity: 'normal', + title: 'test notification', + }, + user: 'user:default/mock', + }, + ]); + + const client = await database.getClient(); + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(1); + }); + + it('should not send to user entity if disabled in settings', async () => { + const client = await database.getClient(); + await client('user_settings').insert({ + user: 'user:default/mock', + channel: 'Web', + origin: 'external:test-service', + enabled: false, + }); + + const response = await sendNotification({ + recipients: { + type: 'entity', + entityRef: ['user:default/mock'], + }, + payload: { + title: 'test notification', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([]); + + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(0); + }); + + it('should fail without recipients', async () => { + const response = await sendNotification({ + payload: { + title: 'test notification', + }, + } as unknown as NotificationSendOptions); + + expect(response.status).toEqual(400); + }); + + it('should fail with invalid recipients', async () => { + const response = await sendNotification({ + recipients: { + type: 'invalid', + }, + payload: { + title: 'test notification', + }, + } as unknown as NotificationSendOptions); + + expect(response.status).toEqual(400); + }); + + it('should fail without title', async () => { + const response = await sendNotification({ + recipients: { + type: 'broadcast', + }, + payload: { + description: 'test notification', + }, + } as unknown as NotificationSendOptions); + + expect(response.status).toEqual(400); + }); + }); + + describe('GET /', () => { + const httpAuth = mockServices.httpAuth({ + defaultCredentials: mockCredentials.user(), + }); + + beforeAll(async () => { + const router = await createRouter({ + logger: mockServices.logger.mock(), + database, + signals: signalService, + userInfo, + config, + httpAuth, + auth, + catalog, + }); + app = express().use(router).use(mockErrorHandler()); + }); + + beforeEach(async () => { + jest.resetAllMocks(); + const client = await database.getClient(); + await client('notification').del(); + await client('broadcast').del(); + }); + + it('should return notifications', async () => { + const client = await database.getClient(); + await client('broadcast').insert({ + id: uuid(), + origin: 'external:test-service', + title: 'Test broadcast notification', + created: new Date(), + severity: 'high', + }); + await client('notification').insert({ + id: uuid(), + user: 'user:default/mock', + origin: 'external:test-service', + title: 'Test notification', + created: new Date(), + severity: 'normal', + }); + + const response = await request(app).get('/'); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + notifications: [ + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + description: null, + icon: null, + link: null, + scope: null, + severity: 'normal', + title: 'Test notification', + topic: null, + }, + read: null, + saved: null, + updated: null, + user: 'user:default/mock', + }, + { + created: expect.any(String), + id: expect.any(String), + origin: 'external:test-service', + payload: { + description: null, + icon: null, + link: null, + scope: null, + severity: 'high', + title: 'Test broadcast notification', + topic: null, + }, + read: null, + saved: null, + updated: null, + user: null, + }, + ], + totalCount: 2, + }); + }); + }); + + describe('GET /settings', () => { + const httpAuth = mockServices.httpAuth({ + defaultCredentials: mockCredentials.user(), + }); + + beforeAll(async () => { + const router = await createRouter({ + logger: mockServices.logger.mock(), + database, + signals: signalService, + userInfo, + config, + httpAuth, + auth, + catalog, + }); + app = express().use(router).use(mockErrorHandler()); + }); + + beforeEach(async () => { + jest.resetAllMocks(); + const client = await database.getClient(); + await client('user_settings').del(); + }); + + it('should return user settings', async () => { + const client = await database.getClient(); + await client('user_settings').insert({ + user: 'user:default/mock', + channel: 'Web', + origin: 'external:test-service', + enabled: false, + }); + + const response = await request(app).get('/settings'); + expect(response.status).toEqual(200); + expect(response.body).toEqual({ + channels: [ + { + id: 'Web', + origins: [{ enabled: false, id: 'external:test-service' }], + }, + ], + }); + }); + }); + + describe('POST /settings', () => { + const httpAuth = mockServices.httpAuth({ + defaultCredentials: mockCredentials.user(), + }); + + beforeAll(async () => { + const router = await createRouter({ + logger: mockServices.logger.mock(), + database, + signals: signalService, + userInfo, + config, + httpAuth, + auth, + catalog, + }); + app = express().use(router).use(mockErrorHandler()); + }); + + beforeEach(async () => { + jest.resetAllMocks(); + const client = await database.getClient(); + await client('user_settings').del(); + }); + + it('should save user settings', async () => { + await request(app) + .post('/settings') + .send({ + channels: [ + { + id: 'Web', + origins: [{ enabled: false, id: 'external:test-service' }], + }, + ], + }) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + + const client = await database.getClient(); + const settings = await client('user_settings').select(); + expect(settings.length).toEqual(1); + expect(settings[0].user).toEqual('user:default/mock'); + expect(settings[0].channel).toEqual('Web'); + expect(settings[0].origin).toEqual('external:test-service'); + expect(Boolean(settings[0].enabled)).toEqual(false); + }); + + it('should fail to save user settings with invalid channel', async () => { + const response = await request(app) + .post('/settings') + .send({ + channels: [ + { + id: 'Invalid', + origins: [{ enabled: false, id: 'external:test-service' }], + }, + ], + }) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + expect(response.status).toEqual(400); + + const client = await database.getClient(); + const settings = await client('user_settings').select(); + expect(settings.length).toEqual(0); }); }); }); diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 06a0a5ebb0..368974b6ca 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -180,7 +180,7 @@ export async function createRouter( const processOptions = async ( opts: NotificationSendOptions, origin: string, - ) => { + ): Promise => { const filtered = await filterProcessors({ ...opts, origin, user: null }); let ret = opts; for (const processor of filtered) { @@ -553,8 +553,14 @@ export async function createRouter( let users = []; if (!recipients || !title) { - logger.error(`Invalid notification request received`); - throw new InputError(`Invalid notification request received`); + const missing = [ + !title ? 'title' : null, + !recipients ? 'recipients' : null, + ].filter(Boolean); + const err = `Invalid notification request received: missing ${missing.join( + ', ', + )}`; + throw new InputError(err); } if (link) { @@ -581,7 +587,7 @@ export async function createRouter( origin, ); notifications.push(broadcast); - } else { + } else if (recipients.type === 'entity') { const entityRef = recipients.entityRef; try { @@ -591,7 +597,6 @@ export async function createRouter( { auth, catalogClient: catalog }, ); } catch (e) { - logger.error(`Failed to resolve notification receivers: ${e}`); throw new InputError('Failed to resolve notification receivers', e); } @@ -602,6 +607,10 @@ export async function createRouter( origin, ); notifications.push(...userNotifications); + } else { + throw new InputError( + `Invalid recipients type, please use either 'broadcast' or 'entity'`, + ); } res.json(notifications);