From ca5747e40379cecaabe1c6e14ec15b33f00ed5cb Mon Sep 17 00:00:00 2001 From: Marek Libra Date: Thu, 15 Feb 2024 12:16:04 +0100 Subject: [PATCH] feat: Update Notifications front-end Signed-off-by: Marek Libra --- .changeset/tender-carrots-care.md | 7 + .../migrations/20231215_init.js | 2 +- .../migrations/20240221_removeDone.js | 25 ++ .../DatabaseNotificationsStore.test.ts | 101 +++-- .../database/DatabaseNotificationsStore.ts | 56 ++- .../src/database/NotificationsStore.ts | 9 +- .../src/service/router.ts | 12 +- plugins/notifications-common/src/types.ts | 5 +- plugins/notifications/package.json | 1 + .../notifications/src/api/NotificationsApi.ts | 3 +- .../src/api/NotificationsClient.ts | 3 + .../NotificationsFilters.tsx | 214 +++++++++ .../components/NotificationsFilters/index.ts | 16 + .../NotificationsPage/NotificationsPage.tsx | 75 ++-- .../NotificationsTable/NotificationsTable.tsx | 410 ++++++------------ yarn.lock | 1 + 16 files changed, 537 insertions(+), 403 deletions(-) create mode 100644 .changeset/tender-carrots-care.md create mode 100644 plugins/notifications-backend/migrations/20240221_removeDone.js create mode 100644 plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx create mode 100644 plugins/notifications/src/components/NotificationsFilters/index.ts diff --git a/.changeset/tender-carrots-care.md b/.changeset/tender-carrots-care.md new file mode 100644 index 0000000000000..45db612f7f6b7 --- /dev/null +++ b/.changeset/tender-carrots-care.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-notifications-backend': minor +'@backstage/plugin-notifications': minor +'@backstage/plugin-notifications-common': patch +--- + +Notifications frontend has been updated towards expectations of the BEP 001 diff --git a/plugins/notifications-backend/migrations/20231215_init.js b/plugins/notifications-backend/migrations/20231215_init.js index a1e57237b6b21..b00f49c9c7819 100644 --- a/plugins/notifications-backend/migrations/20231215_init.js +++ b/plugins/notifications-backend/migrations/20231215_init.js @@ -21,7 +21,7 @@ exports.up = async function up(knex) { table.string('title').notNullable(); table.text('description').nullable(); table.string('severity', 8).notNullable(); - table.text('link').notNullable(); + table.text('link').nullable(); table.string('origin', 255).notNullable(); table.string('scope', 255).nullable(); table.string('topic', 255).nullable(); diff --git a/plugins/notifications-backend/migrations/20240221_removeDone.js b/plugins/notifications-backend/migrations/20240221_removeDone.js new file mode 100644 index 0000000000000..fb34f76083daf --- /dev/null +++ b/plugins/notifications-backend/migrations/20240221_removeDone.js @@ -0,0 +1,25 @@ +/* + * 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.dropColumn('done'); + }); +}; + +exports.down = async function down(knex) { + await knex.schema.dropTable('notification'); +}; diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts index 35964ff5826c5..c678f92a77dd7 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts @@ -62,7 +62,6 @@ describe.each(databases.eachSupportedId())( const insertNotification = async ( notification: Partial & { id: string; - done?: Date; saved?: Date; read?: Date; }, @@ -78,7 +77,6 @@ describe.each(databases.eachSupportedId())( title: notification.payload?.title, severity: notification.payload?.severity, scope: notification.payload?.scope, - done: notification.done, saved: notification.saved, read: notification.read, }) @@ -106,42 +104,64 @@ describe.each(databases.eachSupportedId())( expect(notifications.length).toBe(2); }); - it('should return undone notifications for user', async () => { + it('should return read notifications for user', async () => { const id1 = uuid(); const id2 = uuid(); - await insertNotification({ - id: id1, - ...testNotification, - done: new Date(), + const id3 = uuid(); + await insertNotification({ id: id1, ...testNotification }); + await insertNotification({ id: id2, ...testNotification }); + await insertNotification({ id: id3, ...testNotification }); + await insertNotification({ id: uuid(), ...otherUserNotification }); + + await storage.markRead({ ids: [id1, id3], user }); + + const notifications = await storage.getNotifications({ + user, + read: true, }); + expect(notifications.length).toBe(2); + expect(notifications.at(0)?.id).toEqual(id1); + expect(notifications.at(1)?.id).toEqual(id3); + }); + + it('should return unread notifications for user', async () => { + const id1 = uuid(); + const id2 = uuid(); + const id3 = uuid(); + await insertNotification({ id: id1, ...testNotification }); await insertNotification({ id: id2, ...testNotification }); + await insertNotification({ id: id3, ...testNotification }); await insertNotification({ id: uuid(), ...otherUserNotification }); + await storage.markRead({ ids: [id1, id3], user }); + const notifications = await storage.getNotifications({ user, - type: 'undone', + read: false, }); expect(notifications.length).toBe(1); expect(notifications.at(0)?.id).toEqual(id2); }); - it('should return done notifications for user', async () => { + it('should return both read and unread notifications for user', async () => { const id1 = uuid(); const id2 = uuid(); - await insertNotification({ - id: id1, - ...testNotification, - done: new Date(), - }); + const id3 = uuid(); + await insertNotification({ id: id1, ...testNotification }); await insertNotification({ id: id2, ...testNotification }); + await insertNotification({ id: id3, ...testNotification }); await insertNotification({ id: uuid(), ...otherUserNotification }); + await storage.markRead({ ids: [id1, id3], user }); + const notifications = await storage.getNotifications({ user, - type: 'done', + read: undefined, }); - expect(notifications.length).toBe(1); + expect(notifications.length).toBe(3); expect(notifications.at(0)?.id).toEqual(id1); + expect(notifications.at(1)?.id).toEqual(id2); + expect(notifications.at(2)?.id).toEqual(id3); }); it('should allow searching for notifications', async () => { @@ -218,7 +238,6 @@ describe.each(databases.eachSupportedId())( ...testNotification, id: id1, read: new Date(), - done: new Date(), payload: { title: 'Notification', link: '/scaffolder/task/1234', @@ -242,7 +261,6 @@ describe.each(databases.eachSupportedId())( expect(existing).not.toBeNull(); expect(existing?.id).toEqual(id1); expect(existing?.payload.title).toEqual('New notification'); - expect(existing?.done).toBeNull(); expect(existing?.read).toBeNull(); }); }); @@ -283,32 +301,6 @@ describe.each(databases.eachSupportedId())( }); }); - describe('markDone', () => { - it('should mark notification done', async () => { - const id1 = uuid(); - await insertNotification({ id: id1, ...testNotification }); - - await storage.markDone({ ids: [id1], user }); - const notification = await storage.getNotification({ id: id1 }); - expect(notification?.done).not.toBeNull(); - }); - }); - - describe('markUndone', () => { - it('should mark notification undone', async () => { - const id1 = uuid(); - await insertNotification({ - id: id1, - ...testNotification, - done: new Date(), - }); - - await storage.markUndone({ ids: [id1], user }); - const notification = await storage.getNotification({ id: id1 }); - expect(notification?.done).toBeNull(); - }); - }); - describe('markSaved', () => { it('should mark notification saved', async () => { const id1 = uuid(); @@ -334,5 +326,26 @@ describe.each(databases.eachSupportedId())( expect(notification?.saved).toBeNull(); }); }); + + describe('saveNotification', () => { + it('should store a notification', async () => { + const id1 = uuid(); + await storage.saveNotification({ + id: id1, + user, + created: new Date(), + origin: 'my-origin', + payload: { + title: 'My title One', + description: 'a description of the notification', + link: 'http://foo.bar', + severity: 'normal', + topic: 'my-topic', + }, + }); + const notification = await storage.getNotification({ id: id1 }); + expect(notification?.payload?.title).toBe('My title One'); + }); + }); }, ); diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts index 7e5060106b62d..ef2f38de7d951 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts @@ -78,10 +78,27 @@ export class DatabaseNotificationsStore implements NotificationsStore { })); }; + private mapNotificationToDbRow = (notification: Notification) => { + return { + id: notification.id, + user: notification.user, + origin: notification.origin, + created: notification.created, + topic: notification.payload?.topic, + link: notification.payload?.link, + title: notification.payload?.title, + description: notification.payload?.description, + severity: notification.payload?.severity, + scope: notification.payload?.scope, + saved: notification.saved, + read: notification.read, + }; + }; + private getNotificationsBaseQuery = ( options: NotificationGetOptions | NotificationModifyOptions, ) => { - const { user, type } = options; + const { user } = options; const query = this.db('notification').where('user', user); if (options.sort !== undefined && options.sort !== null) { @@ -90,14 +107,6 @@ export class DatabaseNotificationsStore implements NotificationsStore { query.orderBy('created', options.sortOrder ?? 'desc'); } - if (type === 'undone') { - query.whereNull('done'); - } else if (type === 'done') { - query.whereNotNull('done'); - } else if (type === 'saved') { - query.whereNotNull('saved'); - } - if (options.limit) { query.limit(options.limit); } @@ -117,6 +126,18 @@ export class DatabaseNotificationsStore implements NotificationsStore { query.whereIn('notification.id', options.ids); } + if (options.read) { + query.whereNotNull('notification.read'); + } else if (options.read === false) { + query.whereNull('notification.read'); + } // or match both if undefined + + if (options.saved) { + query.whereNotNull('notification.saved'); + } else if (options.saved === false) { + query.whereNull('notification.saved'); + } // or match both if undefined + return query; }; @@ -127,7 +148,9 @@ export class DatabaseNotificationsStore implements NotificationsStore { } async saveNotification(notification: Notification) { - await this.db.insert(notification).into('notification'); + await this.db + .insert(this.mapNotificationToDbRow(notification)) + .into('notification'); } async getStatus(options: NotificationGetOptions) { @@ -188,10 +211,9 @@ export class DatabaseNotificationsStore implements NotificationsStore { description: options.notification.payload.description, link: options.notification.payload.link, topic: options.notification.payload.topic, - updated: options.notification.created, + updated: new Date(), severity: options.notification.payload.severity, read: null, - done: null, }); return await this.getNotification(options); @@ -218,16 +240,6 @@ export class DatabaseNotificationsStore implements NotificationsStore { await notificationQuery.update({ read: null }); } - async markDone(options: NotificationModifyOptions): Promise { - const notificationQuery = this.getNotificationsBaseQuery(options); - await notificationQuery.update({ done: new Date(), read: new Date() }); - } - - async markUndone(options: NotificationModifyOptions): Promise { - const notificationQuery = this.getNotificationsBaseQuery(options); - await notificationQuery.update({ done: null, read: null }); - } - async markSaved(options: NotificationModifyOptions): Promise { const notificationQuery = this.getNotificationsBaseQuery(options); await notificationQuery.update({ saved: new Date() }); diff --git a/plugins/notifications-backend/src/database/NotificationsStore.ts b/plugins/notifications-backend/src/database/NotificationsStore.ts index 4e3a2fa547625..285f609446598 100644 --- a/plugins/notifications-backend/src/database/NotificationsStore.ts +++ b/plugins/notifications-backend/src/database/NotificationsStore.ts @@ -17,19 +17,20 @@ import { Notification, NotificationStatus, - NotificationType, } from '@backstage/plugin-notifications-common'; +// TODO: reuse the common part of the type with front-end /** @internal */ export type NotificationGetOptions = { user: string; ids?: string[]; - type?: NotificationType; offset?: number; limit?: number; search?: string; sort?: 'created' | 'read' | 'updated' | null; sortOrder?: 'asc' | 'desc'; + read?: boolean; + saved?: boolean; }; /** @internal */ @@ -62,10 +63,6 @@ export interface NotificationsStore { markUnread(options: NotificationModifyOptions): Promise; - markDone(options: NotificationModifyOptions): Promise; - - markUndone(options: NotificationModifyOptions): Promise; - markSaved(options: NotificationModifyOptions): Promise; markUnsaved(options: NotificationModifyOptions): Promise; diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 6c745e158e720..2759adc21c73b 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -45,7 +45,6 @@ import { NewNotificationSignal, Notification, NotificationReadSignal, - NotificationType, } from '@backstage/plugin-notifications-common'; /** @internal */ @@ -191,9 +190,6 @@ export async function createRouter( const opts: NotificationGetOptions = { user: user, }; - if (req.query.type) { - opts.type = req.query.type.toString() as NotificationType; - } if (req.query.offset) { opts.offset = Number.parseInt(req.query.offset.toString(), 10); } @@ -203,6 +199,12 @@ export async function createRouter( if (req.query.search) { opts.search = req.query.search.toString(); } + if (req.query.read === 'true') { + opts.read = true; + } else if (req.query.read === 'false') { + opts.read = false; + // or keep undefined + } const notifications = await store.getNotifications(opts); res.send(notifications); @@ -225,7 +227,7 @@ export async function createRouter( router.get('/status', async (req, res) => { const user = await getUser(req); - const status = await store.getStatus({ user, type: 'undone' }); + const status = await store.getStatus({ user }); res.send(status); }); diff --git a/plugins/notifications-common/src/types.ts b/plugins/notifications-common/src/types.ts index 4d710042656f1..34ca17e2bb8e6 100644 --- a/plugins/notifications-common/src/types.ts +++ b/plugins/notifications-common/src/types.ts @@ -14,9 +14,6 @@ * limitations under the License. */ -/** @public */ -export type NotificationType = 'undone' | 'done' | 'saved'; - /** @public */ export type NotificationSeverity = 'critical' | 'high' | 'normal' | 'low'; @@ -24,7 +21,7 @@ export type NotificationSeverity = 'critical' | 'high' | 'normal' | 'low'; export type NotificationPayload = { title: string; description?: string; - link: string; + link?: string; // TODO: Add support for additional links // additionalLinks?: string[]; severity: NotificationSeverity; diff --git a/plugins/notifications/package.json b/plugins/notifications/package.json index d6efb58c9e96f..13a565113bdc0 100644 --- a/plugins/notifications/package.json +++ b/plugins/notifications/package.json @@ -39,6 +39,7 @@ "@material-ui/icons": "^4.9.1", "@material-ui/lab": "^4.0.0-alpha.61", "@types/react": "^16.13.1 || ^17.0.0", + "lodash": "^4.17.21", "react-relative-time": "^0.0.9", "react-use": "^17.2.4" }, diff --git a/plugins/notifications/src/api/NotificationsApi.ts b/plugins/notifications/src/api/NotificationsApi.ts index 266c8a1ee5033..1b65831e708b2 100644 --- a/plugins/notifications/src/api/NotificationsApi.ts +++ b/plugins/notifications/src/api/NotificationsApi.ts @@ -17,7 +17,6 @@ import { createApiRef } from '@backstage/core-plugin-api'; import { Notification, NotificationStatus, - NotificationType, } from '@backstage/plugin-notifications-common'; /** @public */ @@ -27,10 +26,10 @@ export const notificationsApiRef = createApiRef({ /** @public */ export type GetNotificationsOptions = { - type?: NotificationType; offset?: number; limit?: number; search?: string; + read?: boolean; }; /** @public */ diff --git a/plugins/notifications/src/api/NotificationsClient.ts b/plugins/notifications/src/api/NotificationsClient.ts index 9ef7b287abe8c..998659e4772bc 100644 --- a/plugins/notifications/src/api/NotificationsClient.ts +++ b/plugins/notifications/src/api/NotificationsClient.ts @@ -54,6 +54,9 @@ export class NotificationsClient implements NotificationsApi { if (options?.search) { queryString.append('search', options.search); } + if (options?.read !== undefined) { + queryString.append('read', options.read ? 'true' : 'false'); + } const urlSegment = `?${queryString}`; diff --git a/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx b/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx new file mode 100644 index 0000000000000..ac4b02307a635 --- /dev/null +++ b/plugins/notifications/src/components/NotificationsFilters/NotificationsFilters.tsx @@ -0,0 +1,214 @@ +/* + * 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 { + Divider, + FormControl, + Grid, + InputLabel, + MenuItem, + Select, + Typography, +} from '@material-ui/core'; + +export type NotificationsFiltersProps = { + unreadOnly?: boolean; + onUnreadOnlyChanged: (checked: boolean | undefined) => void; + // createdAfter?: string; + // sorting?: { + // orderBy: GetNotificationsOrderByEnum; + // orderByDirec: GetNotificationsOrderByDirecEnum; + // }; + // onCreatedAfterChanged: (value: string) => void; + // setSorting: ({ + // orderBy, + // orderByDirec, + // }: { + // orderBy: GetNotificationsOrderByEnum; + // orderByDirec: GetNotificationsOrderByDirecEnum; + // }) => void; +}; + +// export const CreatedAfterOptions: { +// [key: string]: { label: string; getDate: () => Date }; +// } = { +// last24h: { +// label: 'Last 24h', +// getDate: () => new Date(Date.now() - 24 * 3600 * 1000), +// }, +// lastWeek: { +// label: 'Last week', +// getDate: () => new Date(Date.now() - 7 * 24 * 3600 * 1000), +// }, +// all: { +// label: 'Any time', +// getDate: () => new Date(0), +// }, +// }; + +// export const SortByOptions: { +// [key: string]: { +// label: string; +// orderBy: GetNotificationsOrderByEnum; +// orderByDirec: GetNotificationsOrderByDirecEnum; +// }; +// } = { +// newest: { +// label: 'Newest on top', +// orderBy: GetNotificationsOrderByEnum.Created, +// orderByDirec: GetNotificationsOrderByDirecEnum.Asc, +// }, +// oldest: { +// label: 'Oldest on top', +// orderBy: GetNotificationsOrderByEnum.Created, +// orderByDirec: GetNotificationsOrderByDirecEnum.Desc, +// }, +// topic: { +// label: 'Topic', +// orderBy: GetNotificationsOrderByEnum.Topic, +// orderByDirec: GetNotificationsOrderByDirecEnum.Asc, +// }, +// origin: { +// label: 'Origin', +// orderBy: GetNotificationsOrderByEnum.Origin, +// orderByDirec: GetNotificationsOrderByDirecEnum.Asc, +// }, +// }; + +// TODO: Implement sorting on server (to work with pagination) +// const getSortBy = (sorting: NotificationsFiltersProps['sorting']): string => { +// if ( +// sorting?.orderBy === GetNotificationsOrderByEnum.Created && +// sorting.orderByDirec === GetNotificationsOrderByDirecEnum.Desc +// ) { +// return 'oldest'; +// } +// if (sorting?.orderBy === GetNotificationsOrderByEnum.Topic) { +// return 'topic'; +// } +// if (sorting?.orderBy === GetNotificationsOrderByEnum.Origin) { +// return 'origin'; +// } + +// return 'newest'; +// }; + +export const NotificationsFilters = ({ + unreadOnly, + // createdAfter, + // sorting, + // onCreatedAfterChanged, + onUnreadOnlyChanged, +}: // setSorting, +NotificationsFiltersProps) => { + // const sortBy = getSortBy(sorting); + + // const handleOnCreatedAfterChanged = ( + // event: React.ChangeEvent<{ name?: string; value: unknown }>, + // ) => { + // onCreatedAfterChanged(event.target.value as string); + // }; + + const handleOnUnreadOnlyChanged = ( + event: React.ChangeEvent<{ name?: string; value: unknown }>, + ) => { + let value = undefined; + if (event.target.value === 'unread') value = true; + if (event.target.value === 'read') value = false; + onUnreadOnlyChanged(value); + }; + + // const handleOnSortByChanged = ( + // event: React.ChangeEvent<{ name?: string; value: unknown }>, + // ) => { + // const idx = (event.target.value as string) || 'newest'; + // const option = SortByOptions[idx]; + // setSorting({ + // orderBy: option.orderBy, + // orderByDirec: option.orderByDirec, + // }); + // }; + + let unreadOnlyValue = 'all'; + if (unreadOnly) unreadOnlyValue = 'unread'; + if (unreadOnly === false) unreadOnlyValue = 'read'; + + return ( + <> + + + Filters + + + + + View + + + + {/* TODO: extend BE to support following: + + + + Created after + + + + + + + + Sort by + + + + */} + + + ); +}; diff --git a/plugins/notifications/src/components/NotificationsFilters/index.ts b/plugins/notifications/src/components/NotificationsFilters/index.ts new file mode 100644 index 0000000000000..196aef61e0981 --- /dev/null +++ b/plugins/notifications/src/components/NotificationsFilters/index.ts @@ -0,0 +1,16 @@ +/* + * 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. + */ +export * from './NotificationsFilters'; diff --git a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx index 85e0937ca3b6a..43a402ac73d35 100644 --- a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx +++ b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx @@ -14,35 +14,35 @@ * limitations under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useEffect } from 'react'; import { Content, - ErrorPanel, PageWithHeader, + ResponseErrorPanel, } from '@backstage/core-components'; import { NotificationsTable } from '../NotificationsTable'; import { useNotificationsApi } from '../../hooks'; -import { Button, Grid, makeStyles } from '@material-ui/core'; -import Bookmark from '@material-ui/icons/Bookmark'; -import Check from '@material-ui/icons/Check'; -import Inbox from '@material-ui/icons/Inbox'; -import { NotificationType } from '@backstage/plugin-notifications-common'; +import { Grid } from '@material-ui/core'; import { useSignal } from '@backstage/plugin-signals-react'; - -const useStyles = makeStyles(_theme => ({ - filterButton: { - width: '100%', - justifyContent: 'start', - }, -})); +import { NotificationsFilters } from '../NotificationsFilters'; +import { GetNotificationsOptions } from '../../api'; export const NotificationsPage = () => { - const [type, setType] = useState('undone'); const [refresh, setRefresh] = React.useState(false); + const { lastSignal } = useSignal('notifications'); + const [unreadOnly, setUnreadOnly] = React.useState(true); + const [containsText, setContainsText] = React.useState(); - const { error, value, retry } = useNotificationsApi( - api => api.getNotifications({ type }), - [type], + const { error, value, retry, loading } = useNotificationsApi( + // TODO: add pagination and other filters + api => { + const options: GetNotificationsOptions = { search: containsText }; + if (unreadOnly !== undefined) { + options.read = !unreadOnly; + } + return api.getNotifications(options); + }, + [containsText, unreadOnly], ); useEffect(() => { @@ -52,7 +52,6 @@ export const NotificationsPage = () => { } }, [refresh, setRefresh, retry]); - const { lastSignal } = useSignal('notifications'); useEffect(() => { if (lastSignal && lastSignal.action) { setRefresh(true); @@ -63,9 +62,8 @@ export const NotificationsPage = () => { setRefresh(true); }; - const styles = useStyles(); if (error) { - return ; + return ; } return ( @@ -73,36 +71,21 @@ export const NotificationsPage = () => { - - - + diff --git a/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx b/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx index 8328da1c6dc5d..d7851154cc57c 100644 --- a/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx +++ b/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx @@ -13,296 +13,160 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import React, { useEffect, useState } from 'react'; -import { - Box, - Button, - IconButton, - makeStyles, - Table, - TableBody, - TableCell, - TableHead, - TableRow, - Tooltip, - Typography, -} from '@material-ui/core'; -import { - Notification, - NotificationType, -} from '@backstage/plugin-notifications-common'; -import { useNavigate } from 'react-router-dom'; -import Checkbox from '@material-ui/core/Checkbox'; -import Check from '@material-ui/icons/Check'; -import Bookmark from '@material-ui/icons/Bookmark'; +import React, { useMemo } from 'react'; +import throttle from 'lodash/throttle'; +import { Box, IconButton, Tooltip, Typography } from '@material-ui/core'; +import { Notification } from '@backstage/plugin-notifications-common'; import { notificationsApiRef } from '../../api'; import { useApi } from '@backstage/core-plugin-api'; -import Inbox from '@material-ui/icons/Inbox'; -import CloseIcon from '@material-ui/icons/Close'; +import MarkAsUnreadIcon from '@material-ui/icons/Markunread'; +import MarkAsReadIcon from '@material-ui/icons/CheckCircle'; + // @ts-ignore import RelativeTime from 'react-relative-time'; -import ArrowForwardIcon from '@material-ui/icons/ArrowForward'; +import { Link, Table, TableColumn } from '@backstage/core-components'; -const useStyles = makeStyles(theme => ({ - table: { - border: `1px solid ${theme.palette.divider}`, - }, - header: { - borderBottom: `1px solid ${theme.palette.divider}`, - }, +const ThrottleDelayMs = 1000; - notificationRow: { - cursor: 'pointer', - '&.unread': { - border: '1px solid rgba(255, 255, 255, .3)', - }, - '& .hideOnHover': { - display: 'initial', - }, - '& .showOnHover': { - display: 'none', - }, - '&:hover': { - '& .hideOnHover': { - display: 'none', - }, - '& .showOnHover': { - display: 'initial', - }, - }, - }, - actionButton: { - padding: '9px', - }, - checkBox: { - padding: '0 10px 10px 0', - }, -})); - -/** @public */ -export const NotificationsTable = (props: { - onUpdate: () => void; - type: NotificationType; +export type NotificationsTableProps = { + isLoading?: boolean; notifications?: Notification[]; -}) => { - const { notifications, type } = props; - const navigate = useNavigate(); - const styles = useStyles(); - const [selected, setSelected] = useState([]); - const notificationsApi = useApi(notificationsApiRef); - - const onCheckBoxClick = (id: string) => { - const index = selected.indexOf(id); - if (index !== -1) { - setSelected(selected.filter(s => s !== id)); - } else { - setSelected([...selected, id]); - } - }; - - useEffect(() => { - setSelected([]); - }, [type]); + onUpdate: () => void; + setContainsText: (search: string) => void; +}; - const isChecked = (id: string) => { - return selected.indexOf(id) !== -1; - }; +export const NotificationsTable = ({ + isLoading, + notifications = [], + onUpdate, + setContainsText, +}: NotificationsTableProps) => { + const notificationsApi = useApi(notificationsApiRef); - const isAllSelected = () => { - return ( - selected.length === notifications?.length && notifications.length > 0 - ); - }; + const onSwitchReadStatus = React.useCallback( + (notification: Notification) => { + notificationsApi + .updateNotifications({ + ids: [notification.id], + read: !notification.read, + }) + .then(() => onUpdate()); + }, + [notificationsApi, onUpdate], + ); - return ( - - - - - {type !== 'saved' && !notifications?.length && 'No notifications'} - {type !== 'saved' && !!notifications?.length && ( - { - if (isAllSelected()) { - setSelected([]); - } else { - setSelected( - notifications ? notifications.map(n => n.id) : [], - ); - } - }} - /> - )} - {type === 'saved' && - `${notifications?.length ?? 0} saved notifications`} - {selected.length === 0 && - !!notifications?.length && - type !== 'saved' && - 'Select all'} - {selected.length > 0 && `${selected.length} selected`} - {type === 'done' && selected.length > 0 && ( - - )} + const throttledContainsTextHandler = useMemo( + () => throttle(setContainsText, ThrottleDelayMs), + [setContainsText], + ); - {type === 'undone' && selected.length > 0 && ( - - )} - - - - - {props.notifications?.map(notification => { + const compactColumns = React.useMemo( + (): TableColumn[] => [ + { + customFilterAndSearch: () => + true /* Keep it on backend due to pagination. If recent flickering is an issue, implement search here as well. */, + render: (notification: Notification) => { + // Compact content return ( - - - onCheckBoxClick(notification.id)} - /> - - - notificationsApi - .updateNotifications({ ids: [notification.id], read: true }) - .then(() => navigate(notification.payload.link)) - } - style={{ paddingLeft: 0 }} - > + <> + - {notification.payload.title} + {notification.payload.link ? ( + + {notification.payload.title} + + ) : ( + notification.payload.title + )} {notification.payload.description} - - - - - - - - - notificationsApi - .updateNotifications({ - ids: [notification.id], - read: true, - }) - .then(() => navigate(notification.payload.link)) - } - > - - - - - { - if (notification.read) { - notificationsApi - .updateNotifications({ - ids: [notification.id], - done: false, - }) - .then(() => { - props.onUpdate(); - }); - } else { - notificationsApi - .updateNotifications({ - ids: [notification.id], - done: true, - }) - .then(() => { - props.onUpdate(); - }); - } - }} - > - {notification.read ? ( - - ) : ( - - )} - - - - { - if (notification.saved) { - notificationsApi - .updateNotifications({ - ids: [notification.id], - saved: false, - }) - .then(() => { - props.onUpdate(); - }); - } else { - notificationsApi - .updateNotifications({ - ids: [notification.id], - saved: true, - }) - .then(() => { - props.onUpdate(); - }); - } - }} - > - {notification.saved ? ( - - ) : ( - - )} - - - - - + + {notification.origin && ( + <>{notification.origin} •  + )} + {notification.payload.topic && ( + <>{notification.payload.topic} •  + )} + {notification.created && ( + + )} + + + + ); + }, + }, + // { + // // TODO: additional action links + // width: '25%', + // render: (notification: Notification) => { + // return ( + // notification.payload.link && ( + // + // {/* TODO: render additionalLinks of different titles */} + // + // + //  More info + // + // + // + // ) + // ); + // }, + // }, + { + // TODO: action for saving notifications + // actions + width: '1rem', + render: (notification: Notification) => { + const markAsReadText = !!notification.read + ? 'Return among unread' + : 'Mark as read'; + const IconComponent = !!notification.read + ? MarkAsUnreadIcon + : MarkAsReadIcon; + + return ( + + { + onSwitchReadStatus(notification); + }} + > + + + ); - })} - -
+ }, + }, + ], + [onSwitchReadStatus], + ); + + // TODO: render "Saved notifications" as "Pinned" + return ( + + isLoading={isLoading} + options={{ + search: true, + // TODO: add pagination + // paging: true, + // pageSize, + header: false, + sorting: false, + }} + // onPageChange={setPageNumber} + // onRowsPerPageChange={setPageSize} + // page={offset} + // totalCount={value?.totalCount} + onSearchChange={throttledContainsTextHandler} + data={notifications} + columns={compactColumns} + /> ); }; diff --git a/yarn.lock b/yarn.lock index 6947f988b0d7b..4740a54da77a7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7696,6 +7696,7 @@ __metadata: "@testing-library/react": ^14.0.0 "@testing-library/user-event": ^14.0.0 "@types/react": ^16.13.1 || ^17.0.0 + lodash: ^4.17.21 msw: ^1.0.0 react-relative-time: ^0.0.9 react-use: ^17.2.4