From dff7a7e9e2c90510b7fdc92eb0953b946c1d37d8 Mon Sep 17 00:00:00 2001 From: Marek Libra Date: Sun, 3 Mar 2024 09:15:37 +0100 Subject: [PATCH] feat(notifications): implement severity of notifications Signed-off-by: Marek Libra --- .changeset/curvy-tigers-cross.md | 6 ++ .../migrations/20240302_numericSeverity.js | 28 ++++++++ .../DatabaseNotificationsStore.test.ts | 72 ++++++++++++++++++- .../database/DatabaseNotificationsStore.ts | 45 ++++++++---- .../src/database/NotificationsStore.ts | 1 + .../src/service/router.ts | 6 ++ plugins/notifications/api-report.md | 2 + .../notifications/src/api/NotificationsApi.ts | 2 + .../src/api/NotificationsClient.ts | 3 + .../NotificationsFilters.tsx | 47 +++++++++++- .../NotificationsPage/NotificationsPage.tsx | 7 ++ .../NotificationsTable/NotificationsTable.tsx | 7 ++ .../NotificationsTable/SeverityIcon.tsx | 39 ++++++++++ 13 files changed, 249 insertions(+), 16 deletions(-) create mode 100644 .changeset/curvy-tigers-cross.md create mode 100644 plugins/notifications-backend/migrations/20240302_numericSeverity.js create mode 100644 plugins/notifications/src/components/NotificationsTable/SeverityIcon.tsx diff --git a/.changeset/curvy-tigers-cross.md b/.changeset/curvy-tigers-cross.md new file mode 100644 index 0000000000..51b1c1ab66 --- /dev/null +++ b/.changeset/curvy-tigers-cross.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-notifications-backend': patch +'@backstage/plugin-notifications': patch +--- + +all notifications can be marked and filtered by severity critical, high, normal or low, the default is 'normal' diff --git a/plugins/notifications-backend/migrations/20240302_numericSeverity.js b/plugins/notifications-backend/migrations/20240302_numericSeverity.js new file mode 100644 index 0000000000..6f71fd49d9 --- /dev/null +++ b/plugins/notifications-backend/migrations/20240302_numericSeverity.js @@ -0,0 +1,28 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +exports.up = async function up(knex) { + await knex.schema.alterTable('notification', table => { + table.tinyint('severity').notNullable().alter(); + // we do not need to migrate data since there are not real deployments so far + }); +}; + +exports.down = async function down(knex) { + await knex.schema.alterTable('notification', table => { + table.string('severity').nullable().alter(); + }); +}; diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts index c4798cfa2e..f532f2873a 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts @@ -14,9 +14,15 @@ * limitations under the License. */ import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; -import { DatabaseNotificationsStore } from './DatabaseNotificationsStore'; +import { + DatabaseNotificationsStore, + getNumericSeverity, +} from './DatabaseNotificationsStore'; import { Knex } from 'knex'; -import { Notification } from '@backstage/plugin-notifications-common'; +import { + Notification, + NotificationSeverity, +} from '@backstage/plugin-notifications-common'; jest.setTimeout(60_000); @@ -48,6 +54,7 @@ const id4 = '04e0871e-e60a-4f68-8110-5ae3513f992e'; const id5 = '05e0871e-e60a-4f68-8110-5ae3513f992e'; const id6 = '06e0871e-e60a-4f68-8110-5ae3513f992e'; const id7 = '07e0871e-e60a-4f68-8110-5ae3513f992e'; +const ids = [id1, id2, id3, id4, id5, id6, id7]; const now = Date.now(); const timeDelay = 5 * 1000; /* 5 secs */ @@ -266,6 +273,64 @@ describe.each(databases.eachSupportedId())( }); }); + describe('getNotifications filters on severity', () => { + beforeEach(async () => { + const severities: (NotificationSeverity | undefined)[] = [ + 'normal', + undefined, + 'critical', + 'high', + 'low', + ]; + await Promise.all( + severities.map((severity, idx) => + storage.saveNotification({ + id: ids[idx], + user, + origin: 'test-origin', + created: new Date(now - idx * timeDelay), + payload: { + title: severity || 'default', + severity, + }, + }), + ), + ); + }); + it('normal', async () => { + const normal = await storage.getNotifications({ + user, + minimalSeverity: getNumericSeverity('normal'), + }); + expect(normal.map(idOnly)).toEqual([id1, id2, id3, id4]); + }); + + it('critical', async () => { + const critical = await storage.getNotifications({ + user, + minimalSeverity: getNumericSeverity('critical'), + }); + expect(critical.length).toBe(1); + expect(critical.at(0)?.id).toEqual(id3); + }); + + it('high', async () => { + const high = await storage.getNotifications({ + user, + minimalSeverity: getNumericSeverity('high'), + }); + expect(high.map(idOnly)).toEqual([id3, id4]); + }); + + it('low', async () => { + const low = await storage.getNotifications({ + user, + minimalSeverity: getNumericSeverity('low'), + }); + expect(low.map(idOnly)).toEqual([id1, id2, id3, id4, id5]); + }); + }); + describe('getNotifications pagination', () => { beforeEach(async () => { await storage.saveNotification(testNotification1); @@ -439,13 +504,14 @@ describe.each(databases.eachSupportedId())( payload: { title: 'New notification', link: '/scaffolder/task/1234', - severity: 'normal', + severity: 'low', }, } as any, }); expect(existing).not.toBeNull(); expect(existing?.id).toEqual(id2); expect(existing?.payload.title).toEqual('New notification'); + expect(existing?.payload.severity).toEqual('low'); expect(existing?.read).toBeNull(); }); }); diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts index 9ffb4c769c..6d0d8bb638 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts @@ -22,7 +22,10 @@ import { NotificationModifyOptions, NotificationsStore, } from './NotificationsStore'; -import { Notification } from '@backstage/plugin-notifications-common'; +import { + Notification, + NotificationSeverity, +} from '@backstage/plugin-notifications-common'; import { Knex } from 'knex'; const migrationsDir = resolvePackagePath( @@ -30,6 +33,17 @@ const migrationsDir = resolvePackagePath( 'migrations', ); +const severities: NotificationSeverity[] = [ + 'critical', + 'high', + 'normal', + 'low', +]; +export const getNumericSeverity = (severity: string): Number => { + const idx = severities.indexOf(severity as NotificationSeverity); + return idx >= 0 ? idx : 2 /* normal */; +}; + /** @internal */ export class DatabaseNotificationsStore implements NotificationsStore { private constructor(private readonly db: Knex) {} @@ -70,7 +84,7 @@ export class DatabaseNotificationsStore implements NotificationsStore { description: row.description, link: row.link, topic: row.topic, - severity: row.severity, + severity: severities[row.severity], scope: row.scope, icon: row.icon, }, @@ -87,7 +101,7 @@ export class DatabaseNotificationsStore implements NotificationsStore { link: notification.payload?.link, title: notification.payload?.title, description: notification.payload?.description, - severity: notification.payload?.severity, + severity: getNumericSeverity(notification.payload?.severity ?? 'normal'), scope: notification.payload?.scope, saved: notification.saved, read: notification.read, @@ -155,6 +169,10 @@ export class DatabaseNotificationsStore implements NotificationsStore { query.whereNull('notification.saved'); } // or match both if undefined + if (options.minimalSeverity !== undefined) { + query.where('notification.severity', '<=', options.minimalSeverity); + } + return query; }; @@ -225,25 +243,28 @@ export class DatabaseNotificationsStore implements NotificationsStore { return rows[0] as Notification; } - async restoreExistingNotification(options: { + async restoreExistingNotification({ + id, + notification, + }: { id: string; notification: Notification; }) { const query = this.db('notification') - .where('id', options.id) - .where('user', options.notification.user); + .where('id', id) + .where('user', notification.user); await query.update({ - title: options.notification.payload.title, - description: options.notification.payload.description, - link: options.notification.payload.link, - topic: options.notification.payload.topic, + title: notification.payload.title, + description: notification.payload.description, + link: notification.payload.link, + topic: notification.payload.topic, updated: new Date(), - severity: options.notification.payload.severity, + severity: getNumericSeverity(notification.payload?.severity ?? 'normal'), read: null, }); - return await this.getNotification(options); + return await this.getNotification({ id }); } async getNotification(options: { id: string }): Promise { diff --git a/plugins/notifications-backend/src/database/NotificationsStore.ts b/plugins/notifications-backend/src/database/NotificationsStore.ts index 92d9883255..b95f504322 100644 --- a/plugins/notifications-backend/src/database/NotificationsStore.ts +++ b/plugins/notifications-backend/src/database/NotificationsStore.ts @@ -32,6 +32,7 @@ export type NotificationGetOptions = { read?: boolean; saved?: boolean; createdAfter?: Date; + minimalSeverity?: Number; }; /** @internal */ diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 51dcff07af..baab9fd4ae 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -18,6 +18,7 @@ import express, { Request } from 'express'; import Router from 'express-promise-router'; import { DatabaseNotificationsStore, + getNumericSeverity, NotificationGetOptions, } from '../database'; import { v4 as uuid } from 'uuid'; @@ -237,6 +238,11 @@ export async function createRouter( } opts.createdAfter = new Date(sinceEpoch); } + if (req.query.minimal_severity) { + opts.minimalSeverity = getNumericSeverity( + req.query.minimal_severity.toString(), + ); + } const [notifications, totalCount] = await Promise.all([ store.getNotifications(opts), diff --git a/plugins/notifications/api-report.md b/plugins/notifications/api-report.md index be57d9da73..93a418b27b 100644 --- a/plugins/notifications/api-report.md +++ b/plugins/notifications/api-report.md @@ -11,6 +11,7 @@ import { DiscoveryApi } from '@backstage/core-plugin-api'; import { FetchApi } from '@backstage/core-plugin-api'; import { JSX as JSX_2 } from 'react'; import { Notification as Notification_2 } from '@backstage/plugin-notifications-common'; +import { NotificationSeverity } from '@backstage/plugin-notifications-common'; import { NotificationStatus } from '@backstage/plugin-notifications-common'; import { default as React_2 } from 'react'; import { RouteRef } from '@backstage/core-plugin-api'; @@ -26,6 +27,7 @@ export type GetNotificationsOptions = { createdAfter?: Date; sort?: 'created' | 'topic' | 'origin'; sortOrder?: 'asc' | 'desc'; + minimalSeverity?: NotificationSeverity; }; // @public (undocumented) diff --git a/plugins/notifications/src/api/NotificationsApi.ts b/plugins/notifications/src/api/NotificationsApi.ts index 8e80f5908c..461c38537c 100644 --- a/plugins/notifications/src/api/NotificationsApi.ts +++ b/plugins/notifications/src/api/NotificationsApi.ts @@ -16,6 +16,7 @@ import { createApiRef } from '@backstage/core-plugin-api'; import { Notification, + NotificationSeverity, NotificationStatus, } from '@backstage/plugin-notifications-common'; @@ -34,6 +35,7 @@ export type GetNotificationsOptions = { createdAfter?: Date; sort?: 'created' | 'topic' | 'origin'; sortOrder?: 'asc' | 'desc'; + minimalSeverity?: NotificationSeverity; }; /** @public */ diff --git a/plugins/notifications/src/api/NotificationsClient.ts b/plugins/notifications/src/api/NotificationsClient.ts index d98e0080e8..ea809658f9 100644 --- a/plugins/notifications/src/api/NotificationsClient.ts +++ b/plugins/notifications/src/api/NotificationsClient.ts @@ -67,6 +67,9 @@ export class NotificationsClient implements NotificationsApi { if (options?.createdAfter !== undefined) { queryString.append('created_after', options.createdAfter.toISOString()); } + if (options?.minimalSeverity !== undefined) { + queryString.append('minimal_severity', options.minimalSeverity); + } const urlSegment = `?${queryString}`; return await this.request(urlSegment); diff --git a/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx b/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx index ad42c03d16..44617ea72c 100644 --- a/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx +++ b/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx @@ -25,6 +25,7 @@ import { Typography, } from '@material-ui/core'; import { GetNotificationsOptions } from '../../api'; +import { NotificationSeverity } from '@backstage/plugin-notifications-common'; export type SortBy = Required< Pick @@ -39,6 +40,8 @@ export type NotificationsFiltersProps = { onSortingChanged: (sortBy: SortBy) => void; saved?: boolean; onSavedChanged: (checked: boolean | undefined) => void; + severity: NotificationSeverity; + onSeverityChanged: (severity: NotificationSeverity) => void; }; export const CreatedAfterOptions: { @@ -108,6 +111,13 @@ const getSortByText = (sortBy?: SortBy): string => { return 'newest'; }; +const AllSeverityOptions: { [key in NotificationSeverity]: string } = { + critical: 'Critical', + high: 'High', + normal: 'Normal', + low: 'Low', +}; + export const NotificationsFilters = ({ sorting, onSortingChanged, @@ -117,6 +127,8 @@ export const NotificationsFilters = ({ onCreatedAfterChanged, saved, onSavedChanged, + severity, + onSeverityChanged, }: NotificationsFiltersProps) => { const sortByText = getSortByText(sorting); @@ -162,6 +174,14 @@ export const NotificationsFilters = ({ viewValue = 'read'; } + const handleOnSeverityChanged = ( + event: React.ChangeEvent<{ name?: string; value: unknown }>, + ) => { + const value: NotificationSeverity = + (event.target.value as NotificationSeverity) || 'normal'; + onSeverityChanged(value); + }; + return ( <> @@ -169,6 +189,7 @@ export const NotificationsFilters = ({ Filters + View @@ -185,14 +206,16 @@ export const NotificationsFilters = ({ + - + Created after + {Object.keys(AllSeverityOptions).map((key: string) => ( + + {AllSeverityOptions[key as NotificationSeverity]} + + ))} + + + ); diff --git a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx index 54acc5d7bc..5708c99bc4 100644 --- a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx +++ b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx @@ -32,6 +32,7 @@ import { SortByOptions, } from '../NotificationsFilters'; import { GetNotificationsOptions } from '../../api'; +import { NotificationSeverity } from '@backstage/plugin-notifications-common'; export const NotificationsPage = () => { const [refresh, setRefresh] = React.useState(false); @@ -45,6 +46,8 @@ export const NotificationsPage = () => { const [sorting, setSorting] = React.useState( SortByOptions.newest.sortBy, ); + const [severity, setSeverity] = + React.useState('normal'); const { error, value, retry, loading } = useNotificationsApi( api => { @@ -52,6 +55,7 @@ export const NotificationsPage = () => { search: containsText, limit: pageSize, offset: pageNumber * pageSize, + minimalSeverity: severity, ...(sorting || {}), }; if (unreadOnly !== undefined) { @@ -76,6 +80,7 @@ export const NotificationsPage = () => { pageSize, sorting, saved, + severity, ], ); @@ -114,6 +119,8 @@ export const NotificationsPage = () => { sorting={sorting} saved={saved} onSavedChanged={setSaved} + severity={severity} + onSeverityChanged={setSeverity} /> diff --git a/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx b/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx index acbeef8769..8012451cc9 100644 --- a/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx +++ b/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx @@ -33,6 +33,7 @@ import MarkAsUnreadIcon from '@material-ui/icons/Markunread' /* TODO: use Drafts import MarkAsReadIcon from '@material-ui/icons/CheckCircle'; import MarkAsUnsavedIcon from '@material-ui/icons/LabelOff' /* TODO: use BookmarkRemove and BookmarkAdd once we have mui 5 icons */; import MarkAsSavedIcon from '@material-ui/icons/Label'; +import { SeverityIcon } from './SeverityIcon'; const ThrottleDelayMs = 1000; @@ -93,6 +94,12 @@ export const NotificationsTable = ({ const compactColumns = React.useMemo( (): TableColumn[] => [ + { + width: '1rem', + render: (notification: Notification) => ( + + ), + }, { customFilterAndSearch: () => true /* Keep it on backend due to pagination. If recent flickering is an issue, implement search here as well. */, diff --git a/plugins/notifications/src/components/NotificationsTable/SeverityIcon.tsx b/plugins/notifications/src/components/NotificationsTable/SeverityIcon.tsx new file mode 100644 index 0000000000..37680db3c2 --- /dev/null +++ b/plugins/notifications/src/components/NotificationsTable/SeverityIcon.tsx @@ -0,0 +1,39 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import React from 'react'; +import { NotificationSeverity } from '@backstage/plugin-notifications-common'; +import NormalIcon from '@material-ui/icons/CheckOutlined'; +import CriticalIcon from '@material-ui/icons/ErrorOutline'; +import HighIcon from '@material-ui/icons/WarningOutlined'; +import LowIcon from '@material-ui/icons/InfoOutlined'; + +export const SeverityIcon = ({ + severity, +}: { + severity?: NotificationSeverity; +}) => { + switch (severity) { + case 'critical': + return ; + case 'high': + return ; + case 'low': + return ; + case 'normal': + default: + return ; + } +};