diff --git a/.changeset/lovely-planes-shop.md b/.changeset/lovely-planes-shop.md new file mode 100644 index 0000000000..3ee5a57e75 --- /dev/null +++ b/.changeset/lovely-planes-shop.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications-backend': patch +--- + +fix: consider broadcast union with user diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts index c6df60c56e..d4e678c8f1 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.test.ts @@ -42,6 +42,7 @@ async function createStore(databaseId: TestDatabaseId) { const idOnly = (notification: Notification) => notification.id; const user = 'user:default/john.doe'; +const otherUser = 'user:default/jane.doe'; const id0 = '08e0871e-e60a-4f68-8110-5ae3513f992e'; const id1 = '01e0871e-e60a-4f68-8110-5ae3513f992e'; @@ -140,7 +141,7 @@ const testNotification7: Notification = { }; const otherUserNotification: Notification = { id: id0, - user: 'user:default/jane.doe', + user: otherUser, created: new Date(now), origin: 'plugin-test', payload: { @@ -246,6 +247,34 @@ describe.each(databases.eachSupportedId())( expect(notifications.map(idOnly)).toEqual([id2, id3, id1]); }); + it('should return correct broadcast notifications for different users', async () => { + await storage.saveNotification(testNotification1); + await storage.saveBroadcast(testNotification2); + await storage.saveNotification(testNotification3); + await storage.saveNotification(otherUserNotification); + + await storage.markRead({ ids: [id1, id2], user }); + + const notifications = await storage.getNotifications({ + user, + }); + expect(notifications.map(idOnly)).toEqual([id2, id3, id1]); + expect(notifications[1].user).toBe(user); + + let otherUserNotifications = await storage.getNotifications({ + user: otherUser, + }); + expect(otherUserNotifications.map(idOnly)).toEqual([id0, id2]); + expect(otherUserNotifications[1].user).toBeNull(); + + await storage.markRead({ ids: [id0, id2], user: otherUser }); + otherUserNotifications = await storage.getNotifications({ + user: otherUser, + }); + expect(otherUserNotifications.map(idOnly)).toEqual([id0, id2]); + expect(otherUserNotifications[1].user).toBe(otherUser); + }); + it('should allow searching for notifications', async () => { await storage.saveNotification(testNotification2); await storage.saveBroadcast(testNotification1); @@ -571,6 +600,23 @@ describe.each(databases.eachSupportedId())( const notification = await storage.getNotification({ id: id2 }); expect(notification?.id).toEqual(id2); }); + + it('should consider user for broadcast by id', async () => { + await storage.saveBroadcast(testNotification1); + + let notification = await storage.getNotification({ id: id1, user }); + expect(notification?.id).toEqual(id1); + expect(notification?.user).toBeNull(); + await storage.markRead({ ids: [id1], user }); + notification = await storage.getNotification({ id: id1, user }); + expect(notification?.user).toBe(user); + + const otherNotification = await storage.getNotification({ + id: id1, + user: otherUser, + }); + expect(otherNotification?.user).toBeNull(); + }); }); describe('markRead', () => { diff --git a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts index 3d027128ec..b23f5254f4 100644 --- a/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts +++ b/plugins/notifications-backend/src/database/DatabaseNotificationsStore.ts @@ -138,14 +138,14 @@ export class DatabaseNotificationsStore implements NotificationsStore { }; }; - private getBroadcastUnion = () => { + private getBroadcastUnion = (user?: string | null) => { return this.db('broadcast') - .leftJoin( - 'broadcast_user_status', - 'id', - '=', - 'broadcast_user_status.broadcast_id', - ) + .leftJoin('broadcast_user_status', function clause() { + const join = this.on('id', '=', 'broadcast_user_status.broadcast_id'); + if (user !== null && user !== undefined) { + join.andOnVal('user', '=', user); + } + }) .select(NOTIFICATION_COLUMNS); }; @@ -156,7 +156,7 @@ export class DatabaseNotificationsStore implements NotificationsStore { const subQuery = this.db('notification') .select(NOTIFICATION_COLUMNS) - .unionAll([this.getBroadcastUnion()]) + .unionAll([this.getBroadcastUnion(user)]) .as('notifications'); const query = this.db.from(subQuery).where(q => { @@ -345,16 +345,19 @@ export class DatabaseNotificationsStore implements NotificationsStore { broadcastQuery.update({ ...updateColumns, read: undefined }), ]); - return await this.getNotification({ id }); + return await this.getNotification({ id, user: notification.user }); } - async getNotification(options: { id: string }): Promise { + async getNotification(options: { + id: string; + user?: string | null; + }): Promise { const rows = await this.db .select('*') .from( this.db('notification') .select(NOTIFICATION_COLUMNS) - .unionAll([this.getBroadcastUnion()]) + .unionAll([this.getBroadcastUnion(options.user)]) .as('notifications'), ) .where('id', options.id)