Skip to content

Commit 80bcb8a

Browse files
RugvipRugvip
authored
Merge pull request backstage#23246 from mareklibra/FLPATH-1003.pagination
feat(notifications): use pagination on the backend layer
2 parents 72968bb + c0f0597 commit 80bcb8a

File tree

10 files changed

+192
-59
lines changed

10 files changed

+192
-59
lines changed

‎.changeset/silent-elephants-reflect.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@backstage/plugin-notifications-backend': minor
3+
'@backstage/plugin-notifications': minor
4+
---
5+
6+
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
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils';
1717
import { DatabaseNotificationsStore } from './DatabaseNotificationsStore';
1818
import { Knex } from 'knex';
19-
import { v4 as uuid } from 'uuid';
2019
import { Notification } from '@backstage/plugin-notifications-common';
2120

2221
jest.setTimeout(60_000);
@@ -54,6 +53,15 @@ const otherUserNotification: Partial<Notification> = {
5453
user: 'user:default/jane.doe',
5554
};
5655

56+
const id1 = '01e0871e-e60a-4f68-8110-5ae3513f992e';
57+
const id2 = '02e0871e-e60a-4f68-8110-5ae3513f992e';
58+
const id3 = '03e0871e-e60a-4f68-8110-5ae3513f992e';
59+
const id4 = '04e0871e-e60a-4f68-8110-5ae3513f992e';
60+
const id5 = '05e0871e-e60a-4f68-8110-5ae3513f992e';
61+
const id6 = '06e0871e-e60a-4f68-8110-5ae3513f992e';
62+
const id7 = '07e0871e-e60a-4f68-8110-5ae3513f992e';
63+
const id8 = '08e0871e-e60a-4f68-8110-5ae3513f992e';
64+
5765
describe.each(databases.eachSupportedId())(
5866
'DatabaseNotificationsStore (%s)',
5967
databaseId => {
@@ -94,11 +102,9 @@ describe.each(databases.eachSupportedId())(
94102

95103
describe('getNotifications', () => {
96104
it('should return all notifications for user', async () => {
97-
const id1 = uuid();
98-
const id2 = uuid();
99105
await insertNotification({ id: id1, ...testNotification });
100106
await insertNotification({ id: id2, ...testNotification });
101-
await insertNotification({ id: uuid(), ...otherUserNotification });
107+
await insertNotification({ id: id3, ...otherUserNotification });
102108

103109
const notifications = await storage.getNotifications({ user });
104110
expect(notifications.length).toBe(2);
@@ -107,13 +113,10 @@ describe.each(databases.eachSupportedId())(
107113
});
108114

109115
it('should return read notifications for user', async () => {
110-
const id1 = uuid();
111-
const id2 = uuid();
112-
const id3 = uuid();
113116
await insertNotification({ id: id1, ...testNotification });
114117
await insertNotification({ id: id2, ...testNotification });
115118
await insertNotification({ id: id3, ...testNotification });
116-
await insertNotification({ id: uuid(), ...otherUserNotification });
119+
await insertNotification({ id: id4, ...otherUserNotification });
117120

118121
await storage.markRead({ ids: [id1, id3], user });
119122

@@ -127,13 +130,10 @@ describe.each(databases.eachSupportedId())(
127130
});
128131

129132
it('should return unread notifications for user', async () => {
130-
const id1 = uuid();
131-
const id2 = uuid();
132-
const id3 = uuid();
133133
await insertNotification({ id: id1, ...testNotification });
134134
await insertNotification({ id: id2, ...testNotification });
135135
await insertNotification({ id: id3, ...testNotification });
136-
await insertNotification({ id: uuid(), ...otherUserNotification });
136+
await insertNotification({ id: id4, ...otherUserNotification });
137137

138138
await storage.markRead({ ids: [id1, id3], user });
139139

@@ -146,13 +146,10 @@ describe.each(databases.eachSupportedId())(
146146
});
147147

148148
it('should return both read and unread notifications for user', async () => {
149-
const id1 = uuid();
150-
const id2 = uuid();
151-
const id3 = uuid();
152149
await insertNotification({ id: id1, ...testNotification });
153150
await insertNotification({ id: id2, ...testNotification });
154151
await insertNotification({ id: id3, ...testNotification });
155-
await insertNotification({ id: uuid(), ...otherUserNotification });
152+
await insertNotification({ id: id4, ...otherUserNotification });
156153

157154
await storage.markRead({ ids: [id1, id3], user });
158155

@@ -167,8 +164,6 @@ describe.each(databases.eachSupportedId())(
167164
});
168165

169166
it('should allow searching for notifications', async () => {
170-
const id1 = uuid();
171-
const id2 = uuid();
172167
await insertNotification({
173168
id: id1,
174169
...testNotification,
@@ -179,7 +174,7 @@ describe.each(databases.eachSupportedId())(
179174
},
180175
});
181176
await insertNotification({ id: id2, ...testNotification });
182-
await insertNotification({ id: uuid(), ...otherUserNotification });
177+
await insertNotification({ id: id3, ...otherUserNotification });
183178

184179
const notifications = await storage.getNotifications({
185180
user,
@@ -190,8 +185,6 @@ describe.each(databases.eachSupportedId())(
190185
});
191186

192187
it('should filter notifications based on created date', async () => {
193-
const id1 = uuid();
194-
const id2 = uuid();
195188
await insertNotification({
196189
id: id1,
197190
...testNotification,
@@ -206,7 +199,7 @@ describe.each(databases.eachSupportedId())(
206199
},
207200
created: new Date() /* now */,
208201
});
209-
await insertNotification({ id: uuid(), ...otherUserNotification });
202+
await insertNotification({ id: id3, ...otherUserNotification });
210203

211204
const notifications = await storage.getNotifications({
212205
user,
@@ -215,19 +208,97 @@ describe.each(databases.eachSupportedId())(
215208
expect(notifications.length).toBe(1);
216209
expect(notifications.at(0)?.id).toEqual(id2);
217210
});
211+
212+
it('should apply pagination', async () => {
213+
const now = Date.now();
214+
const timeDelay = 5 * 1000; /* 5 secs */
215+
216+
await insertNotification({
217+
id: id1,
218+
...testNotification,
219+
created: new Date(now - 1 * 60 * 60 * 1000 /* an hour ago */),
220+
});
221+
await insertNotification({
222+
id: id2,
223+
...testNotification,
224+
created: new Date(now),
225+
});
226+
await insertNotification({
227+
id: id3,
228+
...testNotification,
229+
created: new Date(now - 5 * timeDelay),
230+
});
231+
await insertNotification({
232+
id: id4,
233+
...testNotification,
234+
created: new Date(now - 4 * timeDelay),
235+
});
236+
await insertNotification({
237+
id: id5,
238+
...testNotification,
239+
created: new Date(now - 3 * timeDelay),
240+
});
241+
await insertNotification({
242+
id: id6,
243+
...testNotification,
244+
created: new Date(now - 2 * timeDelay),
245+
});
246+
await insertNotification({
247+
id: id7,
248+
...testNotification,
249+
created: new Date(now - 1 * timeDelay),
250+
});
251+
252+
await insertNotification({ id: id8, ...otherUserNotification });
253+
254+
const allUserNotifications = await storage.getNotifications({
255+
user,
256+
});
257+
expect(allUserNotifications.length).toBe(7);
258+
259+
const notifications = await storage.getNotifications({
260+
user,
261+
createdAfter: new Date(now - 5 * 60 * 1000 /* 5 mins */),
262+
// so far no pagination
263+
});
264+
expect(notifications.length).toBe(6);
265+
expect(notifications.at(0)?.id).toEqual(id2);
266+
expect(notifications.at(1)?.id).toEqual(id7);
267+
expect(notifications.at(2)?.id).toEqual(id6);
268+
expect(notifications.at(3)?.id).toEqual(id5);
269+
expect(notifications.at(4)?.id).toEqual(id4);
270+
271+
const allUserNotificationsPageOne = await storage.getNotifications({
272+
user,
273+
limit: 3,
274+
offset: 0,
275+
});
276+
expect(allUserNotificationsPageOne.length).toBe(3);
277+
expect(allUserNotificationsPageOne.at(0)?.id).toEqual(id2);
278+
expect(allUserNotificationsPageOne.at(1)?.id).toEqual(id7);
279+
expect(allUserNotificationsPageOne.at(2)?.id).toEqual(id6);
280+
281+
const allUserNotificationsPageTwo = await storage.getNotifications({
282+
user,
283+
limit: 3,
284+
offset: 3,
285+
});
286+
expect(allUserNotificationsPageTwo.length).toBe(3);
287+
expect(allUserNotificationsPageTwo.at(0)?.id).toEqual(id5);
288+
expect(allUserNotificationsPageTwo.at(1)?.id).toEqual(id4);
289+
expect(allUserNotificationsPageTwo.at(2)?.id).toEqual(id3);
290+
});
218291
});
219292

220293
describe('getStatus', () => {
221294
it('should return status for user', async () => {
222-
const id1 = uuid();
223-
const id2 = uuid();
224295
await insertNotification({
225296
id: id1,
226297
...testNotification,
227298
read: new Date(),
228299
});
229300
await insertNotification({ id: id2, ...testNotification });
230-
await insertNotification({ id: uuid(), ...otherUserNotification });
301+
await insertNotification({ id: id3, ...otherUserNotification });
231302

232303
const status = await storage.getStatus({ user });
233304
expect(status.read).toEqual(1);
@@ -237,7 +308,6 @@ describe.each(databases.eachSupportedId())(
237308

238309
describe('getExistingScopeNotification', () => {
239310
it('should return existing scope notification', async () => {
240-
const id1 = uuid();
241311
const notification: any = {
242312
...testNotification,
243313
id: id1,
@@ -262,7 +332,6 @@ describe.each(databases.eachSupportedId())(
262332

263333
describe('restoreExistingNotification', () => {
264334
it('should return restore existing scope notification', async () => {
265-
const id1 = uuid();
266335
const notification: any = {
267336
...testNotification,
268337
id: id1,
@@ -296,7 +365,6 @@ describe.each(databases.eachSupportedId())(
296365

297366
describe('getNotification', () => {
298367
it('should return notification by id', async () => {
299-
const id1 = uuid();
300368
await insertNotification({ id: id1, ...testNotification });
301369

302370
const notification = await storage.getNotification({ id: id1 });
@@ -306,7 +374,6 @@ describe.each(databases.eachSupportedId())(
306374

307375
describe('markRead', () => {
308376
it('should mark notification read', async () => {
309-
const id1 = uuid();
310377
await insertNotification({ id: id1, ...testNotification });
311378

312379
await storage.markRead({ ids: [id1], user });
@@ -317,7 +384,6 @@ describe.each(databases.eachSupportedId())(
317384

318385
describe('markUnread', () => {
319386
it('should mark notification unread', async () => {
320-
const id1 = uuid();
321387
await insertNotification({
322388
id: id1,
323389
...testNotification,
@@ -332,7 +398,6 @@ describe.each(databases.eachSupportedId())(
332398

333399
describe('markSaved', () => {
334400
it('should mark notification saved', async () => {
335-
const id1 = uuid();
336401
await insertNotification({ id: id1, ...testNotification });
337402

338403
await storage.markSaved({ ids: [id1], user });
@@ -343,7 +408,6 @@ describe.each(databases.eachSupportedId())(
343408

344409
describe('markUnsaved', () => {
345410
it('should mark notification not saved', async () => {
346-
const id1 = uuid();
347411
await insertNotification({
348412
id: id1,
349413
...testNotification,
@@ -358,7 +422,6 @@ describe.each(databases.eachSupportedId())(
358422

359423
describe('saveNotification', () => {
360424
it('should store a notification', async () => {
361-
const id1 = uuid();
362425
await storage.saveNotification({
363426
id: id1,
364427
user,

‎plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts

+10-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ export class DatabaseNotificationsStore implements NotificationsStore {
9999
) => {
100100
const { user } = options;
101101
const isSQLite = this.db.client.config.client.includes('sqlite3');
102-
// const isPsql = this.db.client.config.client.includes('pg');
103102

104103
const query = this.db('notification').where('user', user);
105104

@@ -165,6 +164,16 @@ export class DatabaseNotificationsStore implements NotificationsStore {
165164
return this.mapToNotifications(notifications);
166165
}
167166

167+
async getNotificationsCount(options: NotificationGetOptions) {
168+
const countOptions: NotificationGetOptions = { ...options };
169+
countOptions.limit = undefined;
170+
countOptions.offset = undefined;
171+
countOptions.sort = null;
172+
const notificationQuery = this.getNotificationsBaseQuery(countOptions);
173+
const response = await notificationQuery.count('* as CNT');
174+
return Number(response[0].CNT);
175+
}
176+
168177
async saveNotification(notification: Notification) {
169178
await this.db
170179
.insert(this.mapNotificationToDbRow(notification))

‎plugins/notifications-backend/src/database/NotificationsStore.ts

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export type NotificationModifyOptions = {
4242
/** @internal */
4343
export interface NotificationsStore {
4444
getNotifications(options: NotificationGetOptions): Promise<Notification[]>;
45+
getNotificationsCount(options: NotificationGetOptions): Promise<number>;
4546

4647
saveNotification(notification: Notification): Promise<void>;
4748

‎plugins/notifications-backend/src/service/router.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -197,15 +197,21 @@ export async function createRouter(
197197
// or keep undefined
198198
}
199199
if (req.query.created_after) {
200-
const sinceEpoch = Date.parse(req.query.created_after.toString());
200+
const sinceEpoch = Date.parse(String(req.query.created_after));
201201
if (isNaN(sinceEpoch)) {
202202
throw new InputError('Unexpected date format');
203203
}
204204
opts.createdAfter = new Date(sinceEpoch);
205205
}
206206

207-
const notifications = await store.getNotifications(opts);
208-
res.send(notifications);
207+
const [notifications, totalCount] = await Promise.all([
208+
store.getNotifications(opts),
209+
store.getNotificationsCount(opts),
210+
]);
211+
res.send({
212+
totalCount,
213+
notifications,
214+
});
209215
});
210216

211217
router.get('/:id', async (req, res) => {

0 commit comments

Comments
 (0)