From 9873c44c2493d70a3d970adad0d904fdf0959196 Mon Sep 17 00:00:00 2001 From: Heikki Hellgren Date: Thu, 15 Feb 2024 14:39:44 +0200 Subject: [PATCH] feat: define signal type for notifications Signed-off-by: Heikki Hellgren --- .changeset/honest-pets-judge.md | 7 ++ .../src/service/router.ts | 69 +++++++++---------- plugins/notifications-common/api-report.md | 15 ++++ plugins/notifications-common/src/types.ts | 15 ++++ plugins/notifications/api-report.md | 5 ++ .../notifications/src/api/NotificationsApi.ts | 2 + .../src/api/NotificationsClient.test.ts | 14 ++++ .../src/api/NotificationsClient.ts | 4 ++ .../NotificationsSideBarItem.tsx | 53 +++++++------- .../src/hooks/useWebNotifications.ts | 11 ++- 10 files changed, 130 insertions(+), 65 deletions(-) create mode 100644 .changeset/honest-pets-judge.md diff --git a/.changeset/honest-pets-judge.md b/.changeset/honest-pets-judge.md new file mode 100644 index 0000000000..468fc1b6d5 --- /dev/null +++ b/.changeset/honest-pets-judge.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-notifications-backend': patch +'@backstage/plugin-notifications-common': patch +'@backstage/plugin-notifications': patch +--- + +Add support for signal type in notifications diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index b293ea4a1f..6c745e158e 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -42,7 +42,9 @@ import { AuthenticationError, InputError } from '@backstage/errors'; import { DiscoveryService, LoggerService } from '@backstage/backend-plugin-api'; import { SignalService } from '@backstage/plugin-signals-node'; import { + NewNotificationSignal, Notification, + NotificationReadSignal, NotificationType, } from '@backstage/plugin-notifications-common'; @@ -206,6 +208,21 @@ export async function createRouter( res.send(notifications); }); + router.get('/:id', async (req, res) => { + const user = await getUser(req); + const opts: NotificationGetOptions = { + user: user, + limit: 1, + ids: [req.params.id], + }; + const notifications = await store.getNotifications(opts); + if (notifications.length !== 1) { + res.status(404).send({ error: 'Not found' }); + return; + } + res.send(notifications[0]); + }); + router.get('/status', async (req, res) => { const user = await getUser(req); const status = await store.getStatus({ user, type: 'undone' }); @@ -214,38 +231,18 @@ export async function createRouter( router.post('/update', async (req, res) => { const user = await getUser(req); - const { ids, done, read, saved } = req.body; + const { ids, read, saved } = req.body; if (!ids || !Array.isArray(ids)) { throw new InputError(); } - if (done === true) { - await store.markDone({ user, ids }); - if (signalService) { - await signalService.publish({ - recipients: [user], - message: { action: 'done', notification_ids: ids }, - channel: 'notifications', - }); - } - } else if (done === false) { - await store.markUndone({ user, ids }); - if (signalService) { - await signalService.publish({ - recipients: [user], - message: { action: 'undone', notification_ids: ids }, - channel: 'notifications', - }); - } - } - if (read === true) { await store.markRead({ user, ids }); if (signalService) { - await signalService.publish({ + await signalService.publish({ recipients: [user], - message: { action: 'mark_read', notification_ids: ids }, + message: { action: 'notification_read', notification_ids: ids }, channel: 'notifications', }); } @@ -253,9 +250,9 @@ export async function createRouter( await store.markUnread({ user: user, ids }); if (signalService) { - await signalService.publish({ + await signalService.publish({ recipients: [user], - message: { action: 'mark_unread', notification_ids: ids }, + message: { action: 'notification_unread', notification_ids: ids }, channel: 'notifications', }); } @@ -285,7 +282,7 @@ export async function createRouter( throw new AuthenticationError(); } - const { title, link, description, scope } = payload; + const { title, link, scope } = payload; if (!recipients || !title || !origin || !link) { logger.error(`Invalid notification request received`); @@ -344,17 +341,17 @@ export async function createRouter( processorSendNotification(ret); notifications.push(ret); - } - if (signalService) { - await signalService.publish({ - recipients: entityRef === null ? null : uniqueUsers, - message: { - action: 'new_notification', - notification: { title, description, link }, - }, - channel: 'notifications', - }); + if (signalService) { + await signalService.publish({ + recipients: user, + message: { + action: 'new_notification', + notification_id: ret.id, + }, + channel: 'notifications', + }); + } } res.json(notifications); diff --git a/plugins/notifications-common/api-report.md b/plugins/notifications-common/api-report.md index d2dc1afb9d..6f40c76e58 100644 --- a/plugins/notifications-common/api-report.md +++ b/plugins/notifications-common/api-report.md @@ -3,6 +3,12 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts +// @public (undocumented) +export type NewNotificationSignal = { + action: 'new_notification'; + notification_id: string; +}; + // @public (undocumented) type Notification_2 = { id: string; @@ -28,9 +34,18 @@ export type NotificationPayload = { icon?: string; }; +// @public (undocumented) +export type NotificationReadSignal = { + action: 'notification_read' | 'notification_unread'; + notification_ids: string[]; +}; + // @public (undocumented) export type NotificationSeverity = 'critical' | 'high' | 'normal' | 'low'; +// @public (undocumented) +export type NotificationSignal = NewNotificationSignal | NotificationReadSignal; + // @public (undocumented) export type NotificationStatus = { unread: number; diff --git a/plugins/notifications-common/src/types.ts b/plugins/notifications-common/src/types.ts index 1e7469244f..4d71004265 100644 --- a/plugins/notifications-common/src/types.ts +++ b/plugins/notifications-common/src/types.ts @@ -51,3 +51,18 @@ export type NotificationStatus = { unread: number; read: number; }; + +/** @public */ +export type NewNotificationSignal = { + action: 'new_notification'; + notification_id: string; +}; + +/** @public */ +export type NotificationReadSignal = { + action: 'notification_read' | 'notification_unread'; + notification_ids: string[]; +}; + +/** @public */ +export type NotificationSignal = NewNotificationSignal | NotificationReadSignal; diff --git a/plugins/notifications/api-report.md b/plugins/notifications/api-report.md index e2071dad14..0f5d7a0049 100644 --- a/plugins/notifications/api-report.md +++ b/plugins/notifications/api-report.md @@ -26,6 +26,8 @@ export type GetNotificationsOptions = { // @public (undocumented) export interface NotificationsApi { + // (undocumented) + getNotification(id: string): Promise; // (undocumented) getNotifications( options?: GetNotificationsOptions, @@ -45,6 +47,8 @@ export const notificationsApiRef: ApiRef; export class NotificationsClient implements NotificationsApi { constructor(options: { discoveryApi: DiscoveryApi; fetchApi: FetchApi }); // (undocumented) + getNotification(id: string): Promise; + // (undocumented) getNotifications( options?: GetNotificationsOptions, ): Promise; @@ -128,6 +132,7 @@ export function useWebNotifications(): { sendWebNotification: (options: { title: string; description: string; + link?: string; }) => Notification | null; }; diff --git a/plugins/notifications/src/api/NotificationsApi.ts b/plugins/notifications/src/api/NotificationsApi.ts index 775dc6b244..266c8a1ee5 100644 --- a/plugins/notifications/src/api/NotificationsApi.ts +++ b/plugins/notifications/src/api/NotificationsApi.ts @@ -45,6 +45,8 @@ export type UpdateNotificationsOptions = { export interface NotificationsApi { getNotifications(options?: GetNotificationsOptions): Promise; + getNotification(id: string): Promise; + getStatus(): Promise; updateNotifications( diff --git a/plugins/notifications/src/api/NotificationsClient.test.ts b/plugins/notifications/src/api/NotificationsClient.test.ts index a7a2d1e5a7..1312b99cf4 100644 --- a/plugins/notifications/src/api/NotificationsClient.test.ts +++ b/plugins/notifications/src/api/NotificationsClient.test.ts @@ -74,6 +74,20 @@ describe('NotificationsClient', () => { expect(response).toEqual(expectedResp); }); + it('should fetch single notification', async () => { + server.use( + rest.get(`${mockBaseUrl}/:id`, (req, res, ctx) => { + expect(req.params.id).toBe('acdaa8ca-262b-43c1-b74b-de06e5f3b3c7'); + return res(ctx.json(testNotification)); + }), + ); + + const response = await client.getNotification( + 'acdaa8ca-262b-43c1-b74b-de06e5f3b3c7', + ); + expect(response).toEqual(testNotification); + }); + it('should fetch status from correct endpoint', async () => { server.use( rest.get(`${mockBaseUrl}/status`, (_, res, ctx) => diff --git a/plugins/notifications/src/api/NotificationsClient.ts b/plugins/notifications/src/api/NotificationsClient.ts index fe7a90d3e8..9ef7b287ab 100644 --- a/plugins/notifications/src/api/NotificationsClient.ts +++ b/plugins/notifications/src/api/NotificationsClient.ts @@ -60,6 +60,10 @@ export class NotificationsClient implements NotificationsApi { return await this.request(urlSegment); } + async getNotification(id: string): Promise { + return await this.request(`${id}`); + } + async getStatus(): Promise { return await this.request('status'); } diff --git a/plugins/notifications/src/components/NotificationsSideBarItem/NotificationsSideBarItem.tsx b/plugins/notifications/src/components/NotificationsSideBarItem/NotificationsSideBarItem.tsx index 90fba009a3..2163c6660f 100644 --- a/plugins/notifications/src/components/NotificationsSideBarItem/NotificationsSideBarItem.tsx +++ b/plugins/notifications/src/components/NotificationsSideBarItem/NotificationsSideBarItem.tsx @@ -17,12 +17,13 @@ import React, { useEffect } from 'react'; import { useNotificationsApi } from '../../hooks'; import { SidebarItem } from '@backstage/core-components'; import NotificationsIcon from '@material-ui/icons/Notifications'; -import { useRouteRef } from '@backstage/core-plugin-api'; +import { useApi, useRouteRef } from '@backstage/core-plugin-api'; import { rootRouteRef } from '../../routes'; import { useSignal } from '@backstage/plugin-signals-react'; +import { NotificationSignal } from '@backstage/plugin-notifications-common'; import { useWebNotifications } from '../../hooks/useWebNotifications'; import { useTitleCounter } from '../../hooks/useTitleCounter'; -import { JsonObject } from '@backstage/types'; +import { notificationsApiRef } from '../../api'; /** @public */ export const NotificationsSidebarItem = (props?: { @@ -35,11 +36,11 @@ export const NotificationsSidebarItem = (props?: { const { loading, error, value, retry } = useNotificationsApi(api => api.getStatus(), ); + const notificationsApi = useApi(notificationsApiRef); const [unreadCount, setUnreadCount] = React.useState(0); const notificationsRoute = useRouteRef(rootRouteRef); - // TODO: Add signal type support to `useSignal` to make it a bit easier to use // TODO: Do we want to add long polling in case signals are not available - const { lastSignal } = useSignal('notifications'); + const { lastSignal } = useSignal('notifications'); const { sendWebNotification } = useWebNotifications(); const [refresh, setRefresh] = React.useState(false); const { setNotificationCount } = useTitleCounter(); @@ -52,39 +53,35 @@ export const NotificationsSidebarItem = (props?: { }, [refresh, retry]); useEffect(() => { - const handleWebNotification = (signal: JsonObject) => { - if (!webNotificationsEnabled || !('notification' in signal)) { + const handleWebNotification = (signal: NotificationSignal) => { + if (!webNotificationsEnabled || signal.action !== 'new_notification') { return; } - const notificationData = signal.notification as JsonObject; - - if ( - !notificationData || - !('title' in notificationData) || - !('description' in notificationData) || - !('title' in notificationData) - ) { - return; - } - const notification = sendWebNotification({ - title: notificationData.title as string, - description: notificationData.description as string, - }); - if (notification) { - notification.onclick = event => { - event.preventDefault(); - notification.close(); - window.open(notificationData.link as string, '_blank'); - }; - } + notificationsApi + .getNotification(signal.notification_id) + .then(notification => { + if (!notification) { + return; + } + sendWebNotification({ + title: notification.payload.title, + description: notification.payload.description ?? '', + link: notification.payload.link, + }); + }); }; if (lastSignal && lastSignal.action) { handleWebNotification(lastSignal); setRefresh(true); } - }, [lastSignal, sendWebNotification, webNotificationsEnabled]); + }, [ + lastSignal, + sendWebNotification, + webNotificationsEnabled, + notificationsApi, + ]); useEffect(() => { if (!loading && !error && value) { diff --git a/plugins/notifications/src/hooks/useWebNotifications.ts b/plugins/notifications/src/hooks/useWebNotifications.ts index 7e34172a97..bf3b5269f5 100644 --- a/plugins/notifications/src/hooks/useWebNotifications.ts +++ b/plugins/notifications/src/hooks/useWebNotifications.ts @@ -37,7 +37,7 @@ export function useWebNotifications() { }); const sendWebNotification = useCallback( - (options: { title: string; description: string }) => { + (options: { title: string; description: string; link?: string }) => { if (webNotificationPermission !== 'granted') { return null; } @@ -45,6 +45,15 @@ export function useWebNotifications() { const notification = new Notification(options.title, { body: options.description, }); + + notification.onclick = event => { + event.preventDefault(); + notification.close(); + if (options.link) { + window.open(options.link, '_blank'); + } + }; + return notification; }, [webNotificationPermission],