From 438c36c55496b3e907f28d15a692bd09a902755c Mon Sep 17 00:00:00 2001 From: Marek Libra Date: Sat, 21 Dec 2024 21:53:05 +0100 Subject: [PATCH] feat(notifications): added the topic filter for notifications Signed-off-by: Marek Libra --- .changeset/clean-squids-build.md | 6 ++ .../DatabaseNotificationsStore.test.ts | 48 +++++++++++++ .../database/DatabaseNotificationsStore.ts | 10 +++ .../src/database/NotificationsStore.ts | 12 ++++ .../src/service/router.ts | 68 +++++++++++++------ plugins/notifications/report.api.md | 25 +++++-- .../notifications/src/api/NotificationsApi.ts | 25 +++++-- .../src/api/NotificationsClient.test.ts | 51 ++++++++++++++ .../src/api/NotificationsClient.ts | 50 ++++++++++---- .../NotificationsFilters.tsx | 40 +++++++++++ .../NotificationsPage/NotificationsPage.tsx | 23 ++++++- 11 files changed, 310 insertions(+), 48 deletions(-) create mode 100644 .changeset/clean-squids-build.md diff --git a/.changeset/clean-squids-build.md b/.changeset/clean-squids-build.md new file mode 100644 index 0000000000..3c1e0b56a7 --- /dev/null +++ b/.changeset/clean-squids-build.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-notifications-backend': patch +'@backstage/plugin-notifications': patch +--- + +added topic filter for notifications diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts index e0536c50ac..d4edcd1e89 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts @@ -742,5 +742,53 @@ describe.each(databases.eachSupportedId())( expect(settings).toEqual(notificationSettings); }); }); + + describe('topics', () => { + it('should return all topics for user', async () => { + await storage.saveNotification(testNotification1); + await storage.saveNotification(testNotification2); + await storage.saveBroadcast(testNotification3); + await storage.saveNotification(otherUserNotification); + + const topics = await storage.getTopics({ user }); + expect(topics.topics.sort()).toEqual([ + testNotification1.payload.topic, + testNotification3.payload.topic, + testNotification2.payload.topic, + ]); + }); + + it('should return filtered topics for user by title', async () => { + await storage.saveNotification(testNotification1); + await storage.saveNotification(testNotification2); + await storage.saveBroadcast(testNotification3); + await storage.saveBroadcast(testNotification4); + await storage.saveNotification(otherUserNotification); + + const topics = await storage.getTopics({ + user, + search: 'Notification 3', + }); + expect(topics).toEqual({ + topics: [testNotification3.payload.topic], + }); + }); + + it('should return filtered topics for user by severity', async () => { + await storage.saveNotification(testNotification1); + await storage.saveNotification(testNotification2); + await storage.saveBroadcast(testNotification3); + await storage.saveBroadcast(testNotification4); + await storage.saveNotification(otherUserNotification); + + const topics = await storage.getTopics({ + user, + minimumSeverity: 'critical', + }); + expect(topics).toEqual({ + topics: [testNotification1.payload.topic], + }); + }); + }); }, ); diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts index 3a00d1c9b5..cf19874ff2 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts @@ -21,6 +21,7 @@ import { NotificationGetOptions, NotificationModifyOptions, NotificationsStore, + TopicGetOptions, } from './NotificationsStore'; import { Notification, @@ -563,4 +564,13 @@ export class DatabaseNotificationsStore implements NotificationsStore { .delete(); await this.db('user_settings').insert(rows); } + + async getTopics(options: TopicGetOptions): Promise<{ topics: string[] }> { + const notificationQuery = this.getNotificationsBaseQuery({ + ...options, + orderField: [{ field: 'topic', order: 'asc' }], + }); + const topics = await notificationQuery.distinct(['topic']); + return { topics: topics.map(row => row.topic) }; + } } diff --git a/plugins/notifications-backend/src/database/NotificationsStore.ts b/plugins/notifications-backend/src/database/NotificationsStore.ts index 2560406e20..a1f4eb6999 100644 --- a/plugins/notifications-backend/src/database/NotificationsStore.ts +++ b/plugins/notifications-backend/src/database/NotificationsStore.ts @@ -48,6 +48,16 @@ export type NotificationModifyOptions = { ids: string[]; } & NotificationGetOptions; +/** @internal */ +export type TopicGetOptions = { + user: string; + search?: string; + read?: boolean; + saved?: boolean; + createdAfter?: Date; + minimumSeverity?: NotificationSeverity; +}; + /** @internal */ export interface NotificationsStore { getNotifications(options: NotificationGetOptions): Promise; @@ -97,4 +107,6 @@ export interface NotificationsStore { user: string; settings: NotificationSettings; }): Promise; + + getTopics(options: TopicGetOptions): Promise<{ topics: string[] }>; } diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 8a12a53be5..06a0a5ebb0 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -20,6 +20,7 @@ import { DatabaseNotificationsStore, normalizeSeverity, NotificationGetOptions, + TopicGetOptions, } from '../database'; import { v4 as uuid } from 'uuid'; import { CatalogApi } from '@backstage/catalog-client'; @@ -246,24 +247,10 @@ export async function createRouter( } }; - // TODO: Move to use OpenAPI router instead - const router = Router(); - router.use(express.json()); - - const listNotificationsHandler = async (req: Request, res: Response) => { - const user = await getUser(req); - const opts: NotificationGetOptions = { - user: user, - }; - if (req.query.offset) { - opts.offset = Number.parseInt(req.query.offset.toString(), 10); - } - if (req.query.limit) { - opts.limit = Number.parseInt(req.query.limit.toString(), 10); - } - if (req.query.orderField) { - opts.orderField = parseEntityOrderFieldParams(req.query); - } + const appendCommonOptions = ( + req: Request, + opts: NotificationGetOptions | TopicGetOptions, + ) => { if (req.query.search) { opts.search = req.query.search.toString(); } @@ -274,10 +261,6 @@ export async function createRouter( // or keep undefined } - if (req.query.topic) { - opts.topic = req.query.topic.toString(); - } - if (req.query.saved === 'true') { opts.saved = true; } else if (req.query.saved === 'false') { @@ -296,6 +279,32 @@ export async function createRouter( req.query.minimumSeverity.toString(), ); } + }; + + // TODO: Move to use OpenAPI router instead + const router = Router(); + router.use(express.json()); + + const listNotificationsHandler = async (req: Request, res: Response) => { + const user = await getUser(req); + const opts: NotificationGetOptions = { + user: user, + }; + if (req.query.offset) { + opts.offset = Number.parseInt(req.query.offset.toString(), 10); + } + if (req.query.limit) { + opts.limit = Number.parseInt(req.query.limit.toString(), 10); + } + if (req.query.orderField) { + opts.orderField = parseEntityOrderFieldParams(req.query); + } + + if (req.query.topic) { + opts.topic = req.query.topic.toString(); + } + + appendCommonOptions(req, opts); const [notifications, totalCount] = await Promise.all([ store.getNotifications(opts), @@ -357,6 +366,21 @@ export async function createRouter( res.json(notifications[0]); }; + // Get topics + const listTopicsHandler = async (req: Request, res: Response) => { + const user = await getUser(req); + const opts: TopicGetOptions = { + user: user, + }; + + appendCommonOptions(req, opts); + + const topics = await store.getTopics(opts); + res.json(topics); + }; + + router.get('/topics', listTopicsHandler); + // Make sure this is the last "GET" handler router.get('/:id', getNotificationHandler); // Deprecated endpoint router.get('/notifications/:id', getNotificationHandler); diff --git a/plugins/notifications/report.api.md b/plugins/notifications/report.api.md index 17c1e7e338..8b4553cf42 100644 --- a/plugins/notifications/report.api.md +++ b/plugins/notifications/report.api.md @@ -20,16 +20,21 @@ import { RouteRef } from '@backstage/core-plugin-api'; import { TableProps } from '@backstage/core-components'; // @public (undocumented) -export type GetNotificationsOptions = { - offset?: number; - limit?: number; +export type GetNotificationsCommonOptions = { search?: string; read?: boolean; saved?: boolean; createdAfter?: Date; + minimumSeverity?: NotificationSeverity; +}; + +// @public (undocumented) +export type GetNotificationsOptions = GetNotificationsCommonOptions & { + offset?: number; + limit?: number; sort?: 'created' | 'topic' | 'origin'; sortOrder?: 'asc' | 'desc'; - minimumSeverity?: NotificationSeverity; + topic?: string; }; // @public (undocumented) @@ -38,6 +43,14 @@ export type GetNotificationsResponse = { totalCount: number; }; +// @public (undocumented) +export type GetTopicsOptions = GetNotificationsCommonOptions; + +// @public (undocumented) +export type GetTopicsResponse = { + topics: string[]; +}; + // @public (undocumented) export interface NotificationsApi { // (undocumented) @@ -51,6 +64,8 @@ export interface NotificationsApi { // (undocumented) getStatus(): Promise; // (undocumented) + getTopics(options?: GetTopicsOptions): Promise; + // (undocumented) updateNotifications( options: UpdateNotificationsOptions, ): Promise; @@ -77,6 +92,8 @@ export class NotificationsClient implements NotificationsApi { // (undocumented) getStatus(): Promise; // (undocumented) + getTopics(options?: GetTopicsOptions): Promise; + // (undocumented) updateNotifications( options: UpdateNotificationsOptions, ): Promise; diff --git a/plugins/notifications/src/api/NotificationsApi.ts b/plugins/notifications/src/api/NotificationsApi.ts index e24e5cad64..6bf53c2864 100644 --- a/plugins/notifications/src/api/NotificationsApi.ts +++ b/plugins/notifications/src/api/NotificationsApi.ts @@ -27,18 +27,26 @@ export const notificationsApiRef = createApiRef({ }); /** @public */ -export type GetNotificationsOptions = { - offset?: number; - limit?: number; +export type GetNotificationsCommonOptions = { search?: string; read?: boolean; saved?: boolean; createdAfter?: Date; - sort?: 'created' | 'topic' | 'origin'; - sortOrder?: 'asc' | 'desc'; minimumSeverity?: NotificationSeverity; }; +/** @public */ +export type GetNotificationsOptions = GetNotificationsCommonOptions & { + offset?: number; + limit?: number; + sort?: 'created' | 'topic' | 'origin'; + sortOrder?: 'asc' | 'desc'; + topic?: string; +}; + +/** @public */ +export type GetTopicsOptions = GetNotificationsCommonOptions; + /** @public */ export type UpdateNotificationsOptions = { ids: string[]; @@ -52,6 +60,11 @@ export type GetNotificationsResponse = { totalCount: number; }; +/** @public */ +export type GetTopicsResponse = { + topics: string[]; +}; + /** @public */ export interface NotificationsApi { getNotifications( @@ -71,4 +84,6 @@ export interface NotificationsApi { updateNotificationSettings( settings: NotificationSettings, ): Promise; + + getTopics(options?: GetTopicsOptions): Promise; } diff --git a/plugins/notifications/src/api/NotificationsClient.test.ts b/plugins/notifications/src/api/NotificationsClient.test.ts index 8248ea9331..8856e529e6 100644 --- a/plugins/notifications/src/api/NotificationsClient.test.ts +++ b/plugins/notifications/src/api/NotificationsClient.test.ts @@ -22,6 +22,8 @@ import { Notification } from '@backstage/plugin-notifications-common'; const server = setupServer(); +const testTopic = 'test-topic'; + const testNotification: Partial = { user: 'user:default/john.doe', origin: 'plugin-test', @@ -29,6 +31,7 @@ const testNotification: Partial = { title: 'Notification 1', link: '/catalog', severity: 'normal', + topic: testTopic, }, }; @@ -75,6 +78,22 @@ describe('NotificationsClient', () => { expect(response).toEqual(expectedResp); }); + it('should fetch notifications of the topic', async () => { + server.use( + rest.get(`${mockBaseUrl}/notifications`, (req, res, ctx) => { + expect(req.url.search).toBe(`?limit=10&offset=0&topic=${testTopic}`); + return res(ctx.json(expectedResp)); + }), + ); + + const response = await client.getNotifications({ + limit: 10, + offset: 0, + topic: testTopic, + }); + expect(response).toEqual(expectedResp); + }); + it('should omit unselected fetch options', async () => { server.use( rest.get(`${mockBaseUrl}/notifications`, (req, res, ctx) => { @@ -131,4 +150,36 @@ describe('NotificationsClient', () => { expect(response).toEqual(expectedResp); }); }); + + describe('getTopics', () => { + const expectedResp = [testTopic]; + + it('should fetch topics from correct endpoint', async () => { + server.use( + rest.get(`${mockBaseUrl}/topics`, (_, res, ctx) => + res(ctx.json(expectedResp)), + ), + ); + const response = await client.getTopics(); + expect(response).toEqual(expectedResp); + }); + + it('should fetch topics with options', async () => { + server.use( + rest.get(`${mockBaseUrl}/topics`, (req, res, ctx) => { + expect(req.url.search).toBe( + '?search=find+me&read=true&createdAfter=1970-01-01T00%3A00%3A00.005Z', + ); + return res(ctx.json(expectedResp)); + }), + ); + + const response = await client.getTopics({ + search: 'find me', + read: true, + createdAfter: new Date(5), + }); + expect(response).toEqual(expectedResp); + }); + }); }); diff --git a/plugins/notifications/src/api/NotificationsClient.ts b/plugins/notifications/src/api/NotificationsClient.ts index 5e763ecb8c..002553ad22 100644 --- a/plugins/notifications/src/api/NotificationsClient.ts +++ b/plugins/notifications/src/api/NotificationsClient.ts @@ -14,8 +14,11 @@ * limitations under the License. */ import { + GetNotificationsCommonOptions, GetNotificationsOptions, GetNotificationsResponse, + GetTopicsOptions, + GetTopicsResponse, NotificationsApi, UpdateNotificationsOptions, } from './NotificationsApi'; @@ -56,20 +59,11 @@ export class NotificationsClient implements NotificationsApi { `${options.sort},${options?.sortOrder ?? 'desc'}`, ); } - if (options?.search) { - queryString.append('search', options.search); - } - if (options?.read !== undefined) { - queryString.append('read', options.read ? 'true' : 'false'); - } - if (options?.saved !== undefined) { - queryString.append('saved', options.saved ? 'true' : 'false'); - } - if (options?.createdAfter !== undefined) { - queryString.append('createdAfter', options.createdAfter.toISOString()); - } - if (options?.minimumSeverity !== undefined) { - queryString.append('minimumSeverity', options.minimumSeverity); + + this.appendCommonQueryStrings(queryString, options); + + if (options?.topic !== undefined) { + queryString.append('topic', options.topic); } return await this.request( @@ -111,6 +105,34 @@ export class NotificationsClient implements NotificationsApi { }); } + async getTopics(options?: GetTopicsOptions): Promise { + const queryString = new URLSearchParams(); + this.appendCommonQueryStrings(queryString, options); + + return await this.request(`/topics?${queryString}`); + } + + private appendCommonQueryStrings( + queryString: URLSearchParams, + options?: GetNotificationsCommonOptions, + ) { + if (options?.search) { + queryString.append('search', options.search); + } + if (options?.read !== undefined) { + queryString.append('read', options.read ? 'true' : 'false'); + } + if (options?.saved !== undefined) { + queryString.append('saved', options.saved ? 'true' : 'false'); + } + if (options?.createdAfter !== undefined) { + queryString.append('createdAfter', options.createdAfter.toISOString()); + } + if (options?.minimumSeverity !== undefined) { + queryString.append('minimumSeverity', options.minimumSeverity); + } + } + private async request(path: string, init?: RequestInit): Promise { const baseUrl = await this.discoveryApi.getBaseUrl('notifications'); const res = await this.fetchApi.fetch(`${baseUrl}${path}`, init); diff --git a/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx b/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx index 0cb88bb3f8..300c56bdee 100644 --- a/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx +++ b/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx @@ -40,8 +40,13 @@ export type NotificationsFiltersProps = { onSavedChanged: (checked: boolean | undefined) => void; severity: NotificationSeverity; onSeverityChanged: (severity: NotificationSeverity) => void; + topic?: string; + onTopicChanged: (value: string | undefined) => void; + allTopics?: string[]; }; +const ALL = '___all___'; + export const CreatedAfterOptions: { [key: string]: { label: string; getDate: () => Date }; } = { @@ -127,6 +132,9 @@ export const NotificationsFilters = ({ onSavedChanged, severity, onSeverityChanged, + topic, + onTopicChanged, + allTopics, }: NotificationsFiltersProps) => { const sortByText = getSortByText(sorting); @@ -180,6 +188,15 @@ export const NotificationsFilters = ({ onSeverityChanged(value); }; + const handleOnTopicChanged = ( + event: React.ChangeEvent<{ name?: string; value: unknown }>, + ) => { + const value = event.target.value as string; + onTopicChanged(value === ALL ? undefined : value); + }; + + const sortedAllTopics = (allTopics || []).sort((a, b) => a.localeCompare(b)); + return ( <> @@ -265,6 +282,29 @@ export const NotificationsFilters = ({ + + + + Topic + + + + ); diff --git a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx index 0d48a064e5..194033a697 100644 --- a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx +++ b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx @@ -33,7 +33,11 @@ import { SortBy, SortByOptions, } from '../NotificationsFilters'; -import { GetNotificationsOptions, GetNotificationsResponse } from '../../api'; +import { + GetNotificationsOptions, + GetNotificationsResponse, + GetTopicsResponse, +} from '../../api'; import { NotificationSeverity, NotificationStatus, @@ -76,9 +80,10 @@ export const NotificationsPage = (props?: NotificationsPageProps) => { SortByOptions.newest.sortBy, ); const [severity, setSeverity] = React.useState('low'); + const [topic, setTopic] = React.useState(); const { error, value, retry, loading } = useNotificationsApi< - [GetNotificationsResponse, NotificationStatus] + [GetNotificationsResponse, NotificationStatus, GetTopicsResponse] >( api => { const options: GetNotificationsOptions = { @@ -94,13 +99,20 @@ export const NotificationsPage = (props?: NotificationsPageProps) => { if (saved !== undefined) { options.saved = saved; } + if (topic !== undefined) { + options.topic = topic; + } const createdAfterDate = CreatedAfterOptions[createdAfter].getDate(); if (createdAfterDate.valueOf() > 0) { options.createdAfter = createdAfterDate; } - return Promise.all([api.getNotifications(options), api.getStatus()]); + return Promise.all([ + api.getNotifications(options), + api.getStatus(), + api.getTopics(options), + ]); }, [ containsText, @@ -111,6 +123,7 @@ export const NotificationsPage = (props?: NotificationsPageProps) => { sorting, saved, severity, + topic, ], ); @@ -143,6 +156,7 @@ export const NotificationsPage = (props?: NotificationsPageProps) => { const notifications = value?.[0]?.notifications; const totalCount = value?.[0]?.totalCount; const isUnread = !!value?.[1]?.unread; + const allTopics = value?.[2]?.topics; let tableTitle = `All notifications (${totalCount})`; if (saved) { @@ -177,6 +191,9 @@ export const NotificationsPage = (props?: NotificationsPageProps) => { onSavedChanged={setSaved} severity={severity} onSeverityChanged={setSeverity} + topic={topic} + onTopicChanged={setTopic} + allTopics={allTopics} />