paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 16 Packages Projects Releases Wiki Activity

Merge pull request #23246 from mareklibra/FLPATH-1003.pagination

Browse Source 
feat(notifications): use pagination on the backend layer
This commit is contained in:
Patrik Oldsberg
2024-02-29 22:58:22 +01:00
committed by GitHub
parent 72968bb1ca c0f059728a
commit 80bcb8a84d
10 changed files with 192 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/silent-elephants-reflect.md
+6
View File
@@ -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
plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts
+96 -33
View File
@@ -16,7 +16,6 @@
import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils';
import { DatabaseNotificationsStore } from './DatabaseNotificationsStore';
import { Knex } from 'knex';
import { v4 as uuid } from 'uuid';
import { Notification } from '@backstage/plugin-notifications-common';
jest.setTimeout(60_000);
@@ -54,6 +53,15 @@ const otherUserNotification: Partial<Notification> = {
user: 'user:default/jane.doe',
};
const id1 = '01e0871e-e60a-4f68-8110-5ae3513f992e';
const id2 = '02e0871e-e60a-4f68-8110-5ae3513f992e';
const id3 = '03e0871e-e60a-4f68-8110-5ae3513f992e';
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 id8 = '08e0871e-e60a-4f68-8110-5ae3513f992e';
describe.each(databases.eachSupportedId())(
'DatabaseNotificationsStore (%s)',
databaseId => {
@@ -94,11 +102,9 @@ describe.each(databases.eachSupportedId())(
describe('getNotifications', () => {
it('should return all notifications for user', async () => {
const id1 = uuid();
const id2 = uuid();
await insertNotification({ id: id1, ...testNotification });
await insertNotification({ id: id2, ...testNotification });
await insertNotification({ id: uuid(), ...otherUserNotification });
await insertNotification({ id: id3, ...otherUserNotification });
const notifications = await storage.getNotifications({ user });
expect(notifications.length).toBe(2);
@@ -107,13 +113,10 @@ describe.each(databases.eachSupportedId())(
});
it('should return read 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 insertNotification({ id: id4, ...otherUserNotification });
await storage.markRead({ ids: [id1, id3], user });
@@ -127,13 +130,10 @@ describe.each(databases.eachSupportedId())(
});
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 insertNotification({ id: id4, ...otherUserNotification });
await storage.markRead({ ids: [id1, id3], user });
@@ -146,13 +146,10 @@ describe.each(databases.eachSupportedId())(
});
it('should return both read and 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 insertNotification({ id: id4, ...otherUserNotification });
await storage.markRead({ ids: [id1, id3], user });
@@ -167,8 +164,6 @@ describe.each(databases.eachSupportedId())(
});
it('should allow searching for notifications', async () => {
const id1 = uuid();
const id2 = uuid();
await insertNotification({
id: id1,
...testNotification,
@@ -179,7 +174,7 @@ describe.each(databases.eachSupportedId())(
},
});
await insertNotification({ id: id2, ...testNotification });
await insertNotification({ id: uuid(), ...otherUserNotification });
await insertNotification({ id: id3, ...otherUserNotification });
const notifications = await storage.getNotifications({
user,
@@ -190,8 +185,6 @@ describe.each(databases.eachSupportedId())(
});
it('should filter notifications based on created date', async () => {
const id1 = uuid();
const id2 = uuid();
await insertNotification({
id: id1,
...testNotification,
@@ -206,7 +199,7 @@ describe.each(databases.eachSupportedId())(
},
created: new Date() /* now */,
});
await insertNotification({ id: uuid(), ...otherUserNotification });
await insertNotification({ id: id3, ...otherUserNotification });
const notifications = await storage.getNotifications({
user,
@@ -215,19 +208,97 @@ 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 timeDelay = 5 * 1000; /* 5 secs */
await insertNotification({
id: id1,
...testNotification,
created: new 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 - 5 * timeDelay),
});
await insertNotification({
id: id4,
...testNotification,
created: new Date(now - 4 * timeDelay),
});
await insertNotification({
id: id5,
...testNotification,
created: new Date(now - 3 * timeDelay),
});
await insertNotification({
id: id6,
...testNotification,
created: new Date(now - 2 * timeDelay),
});
await insertNotification({
id: id7,
...testNotification,
created: new Date(now - 1 * timeDelay),
});
await insertNotification({ id: id8, ...otherUserNotification });
const allUserNotifications = await storage.getNotifications({
user,
});
expect(allUserNotifications.length).toBe(7);
const notifications = await storage.getNotifications({
user,
createdAfter: new Date(now - 5 * 60 * 1000 /* 5 mins */),
// so far no pagination
});
expect(notifications.length).toBe(6);
expect(notifications.at(0)?.id).toEqual(id2);
expect(notifications.at(1)?.id).toEqual(id7);
expect(notifications.at(2)?.id).toEqual(id6);
expect(notifications.at(3)?.id).toEqual(id5);
expect(notifications.at(4)?.id).toEqual(id4);
const allUserNotificationsPageOne = await storage.getNotifications({
user,
limit: 3,
offset: 0,
});
expect(allUserNotificationsPageOne.length).toBe(3);
expect(allUserNotificationsPageOne.at(0)?.id).toEqual(id2);
expect(allUserNotificationsPageOne.at(1)?.id).toEqual(id7);
expect(allUserNotificationsPageOne.at(2)?.id).toEqual(id6);
const allUserNotificationsPageTwo = await storage.getNotifications({
user,
limit: 3,
offset: 3,
});
expect(allUserNotificationsPageTwo.length).toBe(3);
expect(allUserNotificationsPageTwo.at(0)?.id).toEqual(id5);
expect(allUserNotificationsPageTwo.at(1)?.id).toEqual(id4);
expect(allUserNotificationsPageTwo.at(2)?.id).toEqual(id3);
});
});
describe('getStatus', () => {
it('should return status for user', async () => {
const id1 = uuid();
const id2 = uuid();
await insertNotification({
id: id1,
...testNotification,
read: new Date(),
});
await insertNotification({ id: id2, ...testNotification });
await insertNotification({ id: uuid(), ...otherUserNotification });
await insertNotification({ id: id3, ...otherUserNotification });
const status = await storage.getStatus({ user });
expect(status.read).toEqual(1);
@@ -237,7 +308,6 @@ describe.each(databases.eachSupportedId())(
describe('getExistingScopeNotification', () => {
it('should return existing scope notification', async () => {
const id1 = uuid();
const notification: any = {
...testNotification,
id: id1,
@@ -262,7 +332,6 @@ describe.each(databases.eachSupportedId())(
describe('restoreExistingNotification', () => {
it('should return restore existing scope notification', async () => {
const id1 = uuid();
const notification: any = {
...testNotification,
id: id1,
@@ -296,7 +365,6 @@ describe.each(databases.eachSupportedId())(
describe('getNotification', () => {
it('should return notification by id', async () => {
const id1 = uuid();
await insertNotification({ id: id1, ...testNotification });
const notification = await storage.getNotification({ id: id1 });
@@ -306,7 +374,6 @@ describe.each(databases.eachSupportedId())(
describe('markRead', () => {
it('should mark notification read', async () => {
const id1 = uuid();
await insertNotification({ id: id1, ...testNotification });
await storage.markRead({ ids: [id1], user });
@@ -317,7 +384,6 @@ describe.each(databases.eachSupportedId())(
describe('markUnread', () => {
it('should mark notification unread', async () => {
const id1 = uuid();
await insertNotification({
id: id1,
...testNotification,
@@ -332,7 +398,6 @@ describe.each(databases.eachSupportedId())(
describe('markSaved', () => {
it('should mark notification saved', async () => {
const id1 = uuid();
await insertNotification({ id: id1, ...testNotification });
await storage.markSaved({ ids: [id1], user });
@@ -343,7 +408,6 @@ describe.each(databases.eachSupportedId())(
describe('markUnsaved', () => {
it('should mark notification not saved', async () => {
const id1 = uuid();
await insertNotification({
id: id1,
...testNotification,
@@ -358,7 +422,6 @@ describe.each(databases.eachSupportedId())(
describe('saveNotification', () => {
it('should store a notification', async () => {
const id1 = uuid();
await storage.saveNotification({
id: id1,
user,
plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts
+10 -1
View File
@@ -99,7 +99,6 @@ export class DatabaseNotificationsStore implements NotificationsStore {
) => {
const { user } = options;
const isSQLite = this.db.client.config.client.includes('sqlite3');
// const isPsql = this.db.client.config.client.includes('pg');
const query = this.db('notification').where('user', user);
@@ -165,6 +164,16 @@ 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');
return Number(response[0].CNT);
}
async saveNotification(notification: Notification) {
await this.db
.insert(this.mapNotificationToDbRow(notification))
plugins/notifications-backend/src/database/NotificationsStore.ts
+1
View File
@@ -42,6 +42,7 @@ export type NotificationModifyOptions = {
/** @internal */
export interface NotificationsStore {
getNotifications(options: NotificationGetOptions): Promise<Notification[]>;
getNotificationsCount(options: NotificationGetOptions): Promise<number>;
saveNotification(notification: Notification): Promise<void>;
plugins/notifications-backend/src/service/router.ts
+9 -3
View File
@@ -197,15 +197,21 @@ export async function createRouter(
// or keep undefined
}
if (req.query.created_after) {
const sinceEpoch = Date.parse(req.query.created_after.toString());
const sinceEpoch = Date.parse(String(req.query.created_after));
if (isNaN(sinceEpoch)) {
throw new InputError('Unexpected date format');
}
opts.createdAfter = new Date(sinceEpoch);
}
const notifications = await store.getNotifications(opts);
res.send(notifications);
const [notifications, totalCount] = await Promise.all([
store.getNotifications(opts),
store.getNotificationsCount(opts),
]);
res.send({
totalCount,
notifications,
});
});
router.get('/:id', async (req, res) => {
plugins/notifications/api-report.md
+19 -3
View File
@@ -14,6 +14,7 @@ import { Notification as Notification_2 } from '@backstage/plugin-notifications-
import { NotificationStatus } from '@backstage/plugin-notifications-common';
import { default as React_2 } from 'react';
import { RouteRef } from '@backstage/core-plugin-api';
import { TableProps } from '@backstage/core-components';
// @public (undocumented)
export type GetNotificationsOptions = {
@@ -24,6 +25,12 @@ export type GetNotificationsOptions = {
createdAfter?: Date;
};
// @public (undocumented)
export type GetNotificationsResponse = {
notifications: Notification_2[];
totalCount: number;
};
// @public (undocumented)
export interface NotificationsApi {
// (undocumented)
@@ -31,7 +38,7 @@ export interface NotificationsApi {
// (undocumented)
getNotifications(
options?: GetNotificationsOptions,
): Promise<Notification_2[]>;
): Promise<GetNotificationsResponse>;
// (undocumented)
getStatus(): Promise<NotificationStatus>;
// (undocumented)
@@ -51,7 +58,7 @@ export class NotificationsClient implements NotificationsApi {
// (undocumented)
getNotifications(
options?: GetNotificationsOptions,
): Promise<Notification_2[]>;
): Promise<GetNotificationsResponse>;
// (undocumented)
getStatus(): Promise<NotificationStatus>;
// (undocumented)
@@ -83,14 +90,23 @@ export const NotificationsTable: ({
notifications,
onUpdate,
setContainsText,
onPageChange,
onRowsPerPageChange,
page,
pageSize,
totalCount,
}: NotificationsTableProps) => React_2.JSX.Element;
// @public (undocumented)
export type NotificationsTableProps = {
export type NotificationsTableProps = Pick<
TableProps,
'onPageChange' | 'onRowsPerPageChange' | 'page' | 'totalCount'
> & {
isLoading?: boolean;
notifications?: Notification_2[];
onUpdate: () => void;
setContainsText: (search: string) => void;
pageSize: number;
};
// @public (undocumented)
plugins/notifications/src/api/NotificationsApi.ts
+9 -1
View File
@@ -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<Notification[]>;
getNotifications(
options?: GetNotificationsOptions,
): Promise<GetNotificationsResponse>;
getNotification(id: string): Promise<Notification>;
plugins/notifications/src/api/NotificationsClient.ts
+3 -2
View File
@@ -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<Notification[]> {
): Promise<GetNotificationsResponse> {
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<Notification[]>(urlSegment);
return await this.request<GetNotificationsResponse>(urlSegment);
}
async getNotification(id: string): Promise<Notification> {
plugins/notifications/src/components/NotificationsPage/NotificationsPage.tsx
+14 -4
View File
@@ -35,13 +35,18 @@ export const NotificationsPage = () => {
const [refresh, setRefresh] = React.useState(false);
const { lastSignal } = useSignal('notifications');
const [unreadOnly, setUnreadOnly] = React.useState<boolean | undefined>(true);
const [pageNumber, setPageNumber] = React.useState(0);
const [pageSize, setPageSize] = React.useState(5);
const [containsText, setContainsText] = React.useState<string>();
const [createdAfter, setCreatedAfter] = React.useState<string>('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 = () => {
<Grid item xs={10}>
<NotificationsTable
isLoading={loading}
notifications={value}
notifications={value?.notifications}
onUpdate={onUpdate}
setContainsText={setContainsText}
onPageChange={setPageNumber}
onRowsPerPageChange={setPageSize}
page={pageNumber}
pageSize={pageSize}
totalCount={value?.totalCount}
/>
</Grid>
</Grid>
plugins/notifications/src/components/NotificationsTable/NotificationsTable.tsx
+25 -12
View File
@@ -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}