diff --git a/.changeset/mighty-birds-rest.md b/.changeset/mighty-birds-rest.md new file mode 100644 index 0000000000..79f1bdd075 --- /dev/null +++ b/.changeset/mighty-birds-rest.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications': minor +--- + +Switched to using the new `/notifications` endpoints. Be sure to update the `notifications` plugin backend before deploying this frontend plugin change. diff --git a/.changeset/odd-days-push.md b/.changeset/odd-days-push.md new file mode 100644 index 0000000000..fd46de029f --- /dev/null +++ b/.changeset/odd-days-push.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications-backend': patch +--- + +Deprecated root '/' endpoints, moving them under `/notifications` instead. diff --git a/.changeset/perfect-fishes-kick.md b/.changeset/perfect-fishes-kick.md new file mode 100644 index 0000000000..b9e7ade993 --- /dev/null +++ b/.changeset/perfect-fishes-kick.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications-backend': minor +--- + +**BREAKING**: Removed redundant `/health` endpoint, switch to using [the built-in endpoint instead](https://backstage.io/docs/backend-system/core-services/root-health). diff --git a/docs/notifications/index.md b/docs/notifications/index.md index b44b04ef4e..bb8a7dc246 100644 --- a/docs/notifications/index.md +++ b/docs/notifications/index.md @@ -385,7 +385,7 @@ Refer to the [service-to-service auth documentation](https://backstage.io/docs/a An example request for creating a broadcast notification might look like: ```bash -curl -X POST https://[BACKSTAGE_BACKEND]/api/notifications -H "Content-Type: application/json" -H "Authorization: Bearer YOUR_BASE64_SHARED_KEY_TOKEN" -d '{"recipients":{"type":"broadcast"},"payload": {"title": "Title of broadcast message","link": "http://foo.com/bar","severity": "high","topic": "The topic"}}' +curl -X POST https://[BACKSTAGE_BACKEND]/api/notifications/notifications -H "Content-Type: application/json" -H "Authorization: Bearer YOUR_BASE64_SHARED_KEY_TOKEN" -d '{"recipients":{"type":"broadcast"},"payload": {"title": "Title of broadcast message","link": "http://foo.com/bar","severity": "high","topic": "The topic"}}' ``` ## Additional info diff --git a/plugins/notifications-backend/README.md b/plugins/notifications-backend/README.md index a6b6615d9f..080f1e746b 100644 --- a/plugins/notifications-backend/README.md +++ b/plugins/notifications-backend/README.md @@ -29,5 +29,5 @@ To be able to do so, `external access` needs to be enabled as described in the [ Once the API can be accessed, the request can look like: ``` -curl -X POST [YOUR_SERVER_URL]/api/notifications -H "Content-Type: application/json" -H "Authorization: Bearer [BASE64_ENCODED_ACCESS_TOKEN]" -d '{"recipients":{"type":"entity","entityRef":"user:development/guest"},"payload": {"title": "Title of user-targeted external message","description": "The description","link": "http://foo.com/bar","severity": "high","topic": "The topic"}}' +curl -X POST [YOUR_SERVER_URL]/api/notifications/notifications -H "Content-Type: application/json" -H "Authorization: Bearer [BASE64_ENCODED_ACCESS_TOKEN]" -d '{"recipients":{"type":"entity","entityRef":"user:development/guest"},"payload": {"title": "Title of user-targeted external message","description": "The description","link": "http://foo.com/bar","severity": "high","topic": "The topic"}}' ``` diff --git a/plugins/notifications-backend/src/service/router.test.ts b/plugins/notifications-backend/src/service/router.test.ts index 470532b2dc..5126181c4f 100644 --- a/plugins/notifications-backend/src/service/router.test.ts +++ b/plugins/notifications-backend/src/service/router.test.ts @@ -23,7 +23,11 @@ import request from 'supertest'; import { createRouter } from './router'; import { ConfigReader } from '@backstage/config'; import { SignalsService } from '@backstage/plugin-signals-node'; -import { mockCredentials, mockServices } from '@backstage/backend-test-utils'; +import { + mockCredentials, + mockErrorHandler, + mockServices, +} from '@backstage/backend-test-utils'; import { NotificationSendOptions } from '@backstage/plugin-notifications-node'; import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; @@ -68,26 +72,17 @@ describe('createRouter', () => { auth, catalog, }); - app = express().use(router); + app = express().use(router).use(mockErrorHandler()); }); beforeEach(() => { jest.resetAllMocks(); }); - describe('GET /health', () => { - it('returns ok', async () => { - const response = await request(app).get('/health'); - - expect(response.status).toEqual(200); - expect(response.body).toEqual({ status: 'ok' }); - }); - }); - - describe('POST /', () => { + describe('POST /notifications', () => { const sendNotification = async (data: NotificationSendOptions) => request(app) - .post('/') + .post('/notifications') .send(data) .set('Content-Type', 'application/json') .set('Accept', 'application/json'); diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 88cd1fd337..8a12a53be5 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -14,8 +14,7 @@ * limitations under the License. */ -import { errorHandler, PluginDatabaseManager } from '@backstage/backend-common'; -import express, { Request } from 'express'; +import express, { Request, Response } from 'express'; import Router from 'express-promise-router'; import { DatabaseNotificationsStore, @@ -31,6 +30,7 @@ import { import { InputError, NotFoundError } from '@backstage/errors'; import { AuthService, + DatabaseService, HttpAuthService, LoggerService, UserInfoService, @@ -53,7 +53,7 @@ import { Config } from '@backstage/config'; export interface RouterOptions { logger: LoggerService; config: Config; - database: PluginDatabaseManager; + database: DatabaseService; auth: AuthService; httpAuth: HttpAuthService; userInfo: UserInfoService; @@ -250,12 +250,7 @@ export async function createRouter( const router = Router(); router.use(express.json()); - router.get('/health', (_, response) => { - logger.info('PONG!'); - response.json({ status: 'ok' }); - }); - - router.get('/', async (req, res) => { + const listNotificationsHandler = async (req: Request, res: Response) => { const user = await getUser(req); const opts: NotificationGetOptions = { user: user, @@ -310,7 +305,10 @@ export async function createRouter( totalCount, notifications, }); - }); + }; + + router.get('/', listNotificationsHandler); // Deprecated endpoint + router.get('/notifications', listNotificationsHandler); router.get('/status', async (req: Request, res) => { const user = await getUser(req); @@ -345,8 +343,7 @@ export async function createRouter( }, ); - // Make sure this is the last "GET" handler - router.get('/:id', async (req, res) => { + const getNotificationHandler = async (req: Request, res: Response) => { const user = await getUser(req); const opts: NotificationGetOptions = { user: user, @@ -358,9 +355,13 @@ export async function createRouter( throw new NotFoundError('Not found'); } res.json(notifications[0]); - }); + }; - router.post('/update', async (req, res) => { + // Make sure this is the last "GET" handler + router.get('/:id', getNotificationHandler); // Deprecated endpoint + router.get('/notifications/:id', getNotificationHandler); + + const updateNotificationsHandler = async (req: Request, res: Response) => { const user = await getUser(req); const { ids, read, saved } = req.body; if (!ids || !Array.isArray(ids)) { @@ -397,7 +398,10 @@ export async function createRouter( const notifications = await store.getNotifications({ ids, user: user }); res.json(notifications); - }); + }; + + router.post('/update', updateNotificationsHandler); // Deprecated endpoint + router.post('/notifications/update', updateNotificationsHandler); const sendBroadcastNotification = async ( baseNotification: Omit, @@ -509,77 +513,79 @@ export async function createRouter( return notifications; }; - // Add new notification - router.post( - '/', - async (req: Request, res) => { - const credentials = await httpAuth.credentials(req, { - allow: ['service'], - }); + const createNotificationHandler = async ( + req: Request, + res: Response, + ) => { + const credentials = await httpAuth.credentials(req, { + allow: ['service'], + }); - const origin = credentials.principal.subject; - const opts = await processOptions(req.body, origin); - const { recipients, payload } = opts; - const { title, link } = payload; - const notifications: Notification[] = []; - let users = []; + const origin = credentials.principal.subject; + const opts = await processOptions(req.body, origin); + const { recipients, payload } = opts; + const { title, link } = payload; + const notifications: Notification[] = []; + let users = []; - if (!recipients || !title) { - logger.error(`Invalid notification request received`); - throw new InputError(`Invalid notification request received`); + if (!recipients || !title) { + logger.error(`Invalid notification request received`); + throw new InputError(`Invalid notification request received`); + } + + if (link) { + try { + validateLink(link); + } catch (e) { + throw new InputError('Invalid link provided', e); } + } - if (link) { - try { - validateLink(link); - } catch (e) { - throw new InputError('Invalid link provided', e); - } - } + const baseNotification = { + payload: { + ...payload, + severity: payload.severity ?? 'normal', + }, + origin, + created: new Date(), + }; - const baseNotification = { - payload: { - ...payload, - severity: payload.severity ?? 'normal', - }, + if (recipients.type === 'broadcast') { + const broadcast = await sendBroadcastNotification( + baseNotification, + opts, origin, - created: new Date(), - }; + ); + notifications.push(broadcast); + } else { + const entityRef = recipients.entityRef; - if (recipients.type === 'broadcast') { - const broadcast = await sendBroadcastNotification( - baseNotification, - opts, - origin, + try { + users = await getUsersForEntityRef( + entityRef, + recipients.excludeEntityRef ?? [], + { auth, catalogClient: catalog }, ); - notifications.push(broadcast); - } else { - const entityRef = recipients.entityRef; - - try { - users = await getUsersForEntityRef( - entityRef, - recipients.excludeEntityRef ?? [], - { auth, catalogClient: catalog }, - ); - } catch (e) { - logger.error(`Failed to resolve notification receivers: ${e}`); - throw new InputError('Failed to resolve notification receivers', e); - } - - const userNotifications = await sendUserNotifications( - baseNotification, - users, - opts, - origin, - ); - notifications.push(...userNotifications); + } catch (e) { + logger.error(`Failed to resolve notification receivers: ${e}`); + throw new InputError('Failed to resolve notification receivers', e); } - res.json(notifications); - }, - ); + const userNotifications = await sendUserNotifications( + baseNotification, + users, + opts, + origin, + ); + notifications.push(...userNotifications); + } + + res.json(notifications); + }; + + // Add new notification + router.post('/', createNotificationHandler); + router.post('/notifications', createNotificationHandler); - router.use(errorHandler()); return router; } diff --git a/plugins/notifications/src/api/NotificationsClient.test.ts b/plugins/notifications/src/api/NotificationsClient.test.ts index f378269e45..8248ea9331 100644 --- a/plugins/notifications/src/api/NotificationsClient.test.ts +++ b/plugins/notifications/src/api/NotificationsClient.test.ts @@ -48,7 +48,7 @@ describe('NotificationsClient', () => { it('should fetch notifications from correct endpoint', async () => { server.use( - rest.get(`${mockBaseUrl}/`, (_, res, ctx) => + rest.get(`${mockBaseUrl}/notifications`, (_, res, ctx) => res(ctx.json(expectedResp)), ), ); @@ -58,7 +58,7 @@ describe('NotificationsClient', () => { it('should fetch notifications with options', async () => { server.use( - rest.get(`${mockBaseUrl}/`, (req, res, ctx) => { + rest.get(`${mockBaseUrl}/notifications`, (req, res, ctx) => { expect(req.url.search).toBe( '?limit=10&offset=0&search=find+me&read=true&createdAfter=1970-01-01T00%3A00%3A00.005Z', ); @@ -77,7 +77,7 @@ describe('NotificationsClient', () => { it('should omit unselected fetch options', async () => { server.use( - rest.get(`${mockBaseUrl}/`, (req, res, ctx) => { + rest.get(`${mockBaseUrl}/notifications`, (req, res, ctx) => { expect(req.url.search).toBe('?limit=10'); return res(ctx.json(expectedResp)); }), @@ -91,7 +91,7 @@ describe('NotificationsClient', () => { it('should fetch single notification', async () => { server.use( - rest.get(`${mockBaseUrl}/:id`, (req, res, ctx) => { + rest.get(`${mockBaseUrl}/notifications/:id`, (req, res, ctx) => { expect(req.params.id).toBe('acdaa8ca-262b-43c1-b74b-de06e5f3b3c7'); return res(ctx.json(testNotification)); }), @@ -115,12 +115,15 @@ describe('NotificationsClient', () => { it('should update notifications', async () => { server.use( - rest.post(`${mockBaseUrl}/update`, async (req, res, ctx) => { - expect(await req.json()).toEqual({ - ids: ['acdaa8ca-262b-43c1-b74b-de06e5f3b3c7'], - }); - return res(ctx.json(expectedResp)); - }), + rest.post( + `${mockBaseUrl}/notifications/update`, + async (req, res, ctx) => { + expect(await req.json()).toEqual({ + ids: ['acdaa8ca-262b-43c1-b74b-de06e5f3b3c7'], + }); + return res(ctx.json(expectedResp)); + }, + ), ); const response = await client.updateNotifications({ ids: ['acdaa8ca-262b-43c1-b74b-de06e5f3b3c7'], diff --git a/plugins/notifications/src/api/NotificationsClient.ts b/plugins/notifications/src/api/NotificationsClient.ts index 983065f96e..5e763ecb8c 100644 --- a/plugins/notifications/src/api/NotificationsClient.ts +++ b/plugins/notifications/src/api/NotificationsClient.ts @@ -71,23 +71,26 @@ export class NotificationsClient implements NotificationsApi { if (options?.minimumSeverity !== undefined) { queryString.append('minimumSeverity', options.minimumSeverity); } - const urlSegment = `?${queryString}`; - return await this.request(urlSegment); + return await this.request( + `/notifications?${queryString}`, + ); } async getNotification(id: string): Promise { - return await this.request(`${id}`); + return await this.request( + `/notifications/${encodeURIComponent(id)}`, + ); } async getStatus(): Promise { - return await this.request('status'); + return await this.request('/status'); } async updateNotifications( options: UpdateNotificationsOptions, ): Promise { - return await this.request('update', { + return await this.request('/notifications/update', { method: 'POST', body: JSON.stringify(options), headers: { 'Content-Type': 'application/json' }, @@ -95,29 +98,27 @@ export class NotificationsClient implements NotificationsApi { } async getNotificationSettings(): Promise { - return await this.request('settings'); + return await this.request('/settings'); } async updateNotificationSettings( settings: NotificationSettings, ): Promise { - return await this.request('settings', { + return await this.request('/settings', { method: 'POST', body: JSON.stringify(settings), headers: { 'Content-Type': 'application/json' }, }); } - private async request(path: string, init?: any): Promise { - const baseUrl = `${await this.discoveryApi.getBaseUrl('notifications')}/`; - const url = new URL(path, baseUrl); + private async request(path: string, init?: RequestInit): Promise { + const baseUrl = await this.discoveryApi.getBaseUrl('notifications'); + const res = await this.fetchApi.fetch(`${baseUrl}${path}`, init); - const response = await this.fetchApi.fetch(url.toString(), init); - - if (!response.ok) { - throw await ResponseError.fromResponse(response); + if (!res.ok) { + throw await ResponseError.fromResponse(res); } - return response.json() as Promise; + return res.json() as Promise; } }