diff --git a/.changeset/silent-elephants-reflect.md b/.changeset/silent-elephants-reflect.md new file mode 100644 index 0000000000..f9f35421bc --- /dev/null +++ b/.changeset/silent-elephants-reflect.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-notifications-backend': minor +'@backstage/plugin-notifications': minor +--- + +The NotificationsPage newly uses pagination implemented on the backend layer to avoid large dataset transfers diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts index e0af93a1d6..8abb0f1c06 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts @@ -215,6 +215,87 @@ describe.each(databases.eachSupportedId())( expect(notifications.length).toBe(1); expect(notifications.at(0)?.id).toEqual(id2); }); + + it('should apply pagination', async () => { + const now = Date.now(); + const id1 = uuid(); + const id2 = uuid(); + const id3 = uuid(); + const id4 = uuid(); + const id5 = uuid(); + const id6 = uuid(); + const id7 = uuid(); + await insertNotification({ + id: id1, + ...testNotification, + created: new Date(Date.now() - 1 * 60 * 60 * 1000 /* an hour ago */), + }); + await insertNotification({ + id: id2, + ...testNotification, + created: new Date(now), + }); + await insertNotification({ + id: id3, + ...testNotification, + created: new Date(now + 1), + }); + await insertNotification({ + id: id4, + ...testNotification, + created: new Date(now + 2), + }); + await insertNotification({ + id: id5, + ...testNotification, + created: new Date(now + 3), + }); + await insertNotification({ + id: id6, + ...testNotification, + created: new Date(now + 4), + }); + await insertNotification({ + id: id7, + ...testNotification, + created: new Date(now + 5), + }); + + await insertNotification({ id: uuid(), ...otherUserNotification }); + + const allUserNotifications = await storage.getNotifications({ + user, + }); + expect(allUserNotifications.length).toBe(7); + + const notifications = await storage.getNotifications({ + user, + createdAfter: new Date(Date.now() - 5 * 60 * 1000 /* 5mins */), + }); + expect(notifications.length).toBe(6); + expect(notifications.at(0)?.id).toEqual(id7); + expect(notifications.at(1)?.id).toEqual(id6); + + const allUserNotificationsPageOne = await storage.getNotifications({ + user, + limit: 3, + offset: 0, + }); + expect(allUserNotificationsPageOne.length).toBe(3); + expect(allUserNotificationsPageOne.at(0)?.id).toEqual(id7); + expect(allUserNotificationsPageOne.at(1)?.id).toEqual(id6); + expect(allUserNotificationsPageOne.at(2)?.id).toEqual(id5); + + const allUserNotificationsPageTwo = await storage.getNotifications({ + user, + limit: 3, + offset: 3, + }); + expect(allUserNotificationsPageTwo.length).toBe(3); + expect(allUserNotificationsPageTwo.at(0)?.id).toEqual(id4); + expect(allUserNotificationsPageTwo.at(1)?.id).toEqual(id3); + expect(allUserNotificationsPageTwo.at(2)?.id).toEqual(id2); + }); }); describe('getStatus', () => { diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts index 9b654cfd04..afd94fd69d 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts @@ -165,6 +165,17 @@ export class DatabaseNotificationsStore implements NotificationsStore { return this.mapToNotifications(notifications); } + async getNotificationsCount(options: NotificationGetOptions) { + const countOptions: NotificationGetOptions = { ...options }; + countOptions.limit = undefined; + countOptions.offset = undefined; + countOptions.sort = null; + const notificationQuery = this.getNotificationsBaseQuery(countOptions); + const response = await notificationQuery.count('* as CNT'); + const totalCount = Number.parseInt(response[0].CNT.toString(), 10); + return totalCount; + } + async saveNotification(notification: Notification) { await this.db .insert(this.mapNotificationToDbRow(notification)) diff --git a/plugins/notifications-backend/src/database/NotificationsStore.ts b/plugins/notifications-backend/src/database/NotificationsStore.ts index 0a7df92f03..03f47263f1 100644 --- a/plugins/notifications-backend/src/database/NotificationsStore.ts +++ b/plugins/notifications-backend/src/database/NotificationsStore.ts @@ -42,6 +42,7 @@ export type NotificationModifyOptions = { /** @internal */ export interface NotificationsStore { getNotifications(options: NotificationGetOptions): Promise; + getNotificationsCount(options: NotificationGetOptions): Promise; saveNotification(notification: Notification): Promise; diff --git a/plugins/notifications-backend/src/service/router.ts b/plugins/notifications-backend/src/service/router.ts index 78d44dcd44..45a9c56e59 100644 --- a/plugins/notifications-backend/src/service/router.ts +++ b/plugins/notifications-backend/src/service/router.ts @@ -213,7 +213,11 @@ export async function createRouter( } const notifications = await store.getNotifications(opts); - res.send(notifications); + const totalCount = await store.getNotificationsCount(opts); + res.send({ + totalCount, + notifications, + }); }); router.get('/:id', async (req, res) => { diff --git a/plugins/notifications/src/api/NotificationsApi.ts b/plugins/notifications/src/api/NotificationsApi.ts index 4a1c792012..da7d88dee1 100644 --- a/plugins/notifications/src/api/NotificationsApi.ts +++ b/plugins/notifications/src/api/NotificationsApi.ts @@ -40,9 +40,17 @@ export type UpdateNotificationsOptions = { saved?: boolean; }; +/** @public */ +export type GetNotificationsResponse = { + notifications: Notification[]; + totalCount: number; +}; + /** @public */ export interface NotificationsApi { - getNotifications(options?: GetNotificationsOptions): Promise; + getNotifications( + options?: GetNotificationsOptions, + ): Promise; getNotification(id: string): Promise; diff --git a/plugins/notifications/src/api/NotificationsClient.ts b/plugins/notifications/src/api/NotificationsClient.ts index 1013497b3d..ba0782fe6f 100644 --- a/plugins/notifications/src/api/NotificationsClient.ts +++ b/plugins/notifications/src/api/NotificationsClient.ts @@ -15,6 +15,7 @@ */ import { GetNotificationsOptions, + GetNotificationsResponse, NotificationsApi, UpdateNotificationsOptions, } from './NotificationsApi'; @@ -40,7 +41,7 @@ export class NotificationsClient implements NotificationsApi { async getNotifications( options?: GetNotificationsOptions, - ): Promise { + ): Promise { const queryString = new URLSearchParams(); if (options?.limit !== undefined) { queryString.append('limit', options.limit.toString(10)); @@ -59,7 +60,7 @@ export class NotificationsClient implements NotificationsApi { } const urlSegment = `?${queryString}`; - return await this.request(urlSegment); + return await this.request(urlSegment); } async getNotification(id: string): Promise { diff --git a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx index 1f8b141608..81c991798d 100644 --- a/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx +++ b/plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx @@ -35,13 +35,18 @@ export const NotificationsPage = () => { const [refresh, setRefresh] = React.useState(false); const { lastSignal } = useSignal('notifications'); const [unreadOnly, setUnreadOnly] = React.useState(true); + const [pageNumber, setPageNumber] = React.useState(0); + const [pageSize, setPageSize] = React.useState(5); const [containsText, setContainsText] = React.useState(); const [createdAfter, setCreatedAfter] = React.useState('lastWeek'); const { error, value, retry, loading } = useNotificationsApi( - // TODO: add pagination and other filters api => { - const options: GetNotificationsOptions = { search: containsText }; + const options: GetNotificationsOptions = { + search: containsText, + limit: pageSize, + offset: pageNumber * pageSize, + }; if (unreadOnly !== undefined) { options.read = !unreadOnly; } @@ -53,7 +58,7 @@ export const NotificationsPage = () => { return api.getNotifications(options); }, - [containsText, unreadOnly, createdAfter], + [containsText, unreadOnly, createdAfter, pageNumber, pageSize], ); useEffect(() => { @@ -94,9 +99,14 @@ export const NotificationsPage = () => { diff --git a/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx b/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx index 6bcb6badc7..3452a618fb 100644 --- a/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx +++ b/plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx @@ -15,25 +15,34 @@ */ import React, { useMemo } from 'react'; import throttle from 'lodash/throttle'; +// @ts-ignore +import RelativeTime from 'react-relative-time'; 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 MarkAsUnreadIcon from '@material-ui/icons/Markunread'; import MarkAsReadIcon from '@material-ui/icons/CheckCircle'; - -// @ts-ignore -import RelativeTime from 'react-relative-time'; -import { Link, Table, TableColumn } from '@backstage/core-components'; +import { + Link, + Table, + TableProps, + TableColumn, +} from '@backstage/core-components'; const ThrottleDelayMs = 1000; /** @public */ -export type NotificationsTableProps = { +export type NotificationsTableProps = Pick< + TableProps, + 'onPageChange' | 'onRowsPerPageChange' | 'page' | 'totalCount' +> & { isLoading?: boolean; notifications?: Notification[]; onUpdate: () => void; setContainsText: (search: string) => void; + pageSize: number; }; /** @public */ @@ -42,6 +51,11 @@ export const NotificationsTable = ({ notifications = [], onUpdate, setContainsText, + onPageChange, + onRowsPerPageChange, + page, + pageSize, + totalCount, }: NotificationsTableProps) => { const notificationsApi = useApi(notificationsApiRef); @@ -156,16 +170,15 @@ export const NotificationsTable = ({ isLoading={isLoading} options={{ search: true, - // TODO: add pagination - // paging: true, - // pageSize, + paging: true, + pageSize, header: false, sorting: false, }} - // onPageChange={setPageNumber} - // onRowsPerPageChange={setPageSize} - // page={offset} - // totalCount={value?.totalCount} + onPageChange={onPageChange} + onRowsPerPageChange={onRowsPerPageChange} + page={page} + totalCount={totalCount} onSearchChange={throttledContainsTextHandler} data={notifications} columns={compactColumns}