From 3b8e156c140ca73fca70c77323ebf290fcca76cb Mon Sep 17 00:00:00 2001 From: Hellgren Heikki Date: Tue, 23 Sep 2025 20:43:05 +0300 Subject: [PATCH] fix: default notification recipient resolver exclusion Signed-off-by: Hellgren Heikki --- .changeset/slimy-signs-agree.md | 5 ++ ...faultNotificationRecipientResolver.test.ts | 10 +-- .../DefaultNotificationRecipientResolver.ts | 16 ++--- .../src/service/router.test.ts | 69 +++++++++++++++++++ 4 files changed, 87 insertions(+), 13 deletions(-) create mode 100644 .changeset/slimy-signs-agree.md diff --git a/.changeset/slimy-signs-agree.md b/.changeset/slimy-signs-agree.md new file mode 100644 index 0000000000..c4307f0bd7 --- /dev/null +++ b/.changeset/slimy-signs-agree.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications-backend': patch +--- + +Fixed exclude entity reference not working in notification sending diff --git a/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.test.ts b/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.test.ts index cd17db3680..7faec0c23c 100644 --- a/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.test.ts +++ b/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.test.ts @@ -34,7 +34,7 @@ describe('getUsersForEntityRef', () => { await expect( resolver.resolveNotificationRecipients({ entityRefs: ['user:foo', 'user:ignored'], - excludeEntityRefs: ['user:ignored'], + excludedEntityRefs: ['user:ignored'], }), ).resolves.toEqual({ userEntityRefs: ['user:foo'] }); expect(catalog.getEntitiesByRefs).not.toHaveBeenCalled(); @@ -87,7 +87,7 @@ describe('getUsersForEntityRef', () => { await expect( resolver.resolveNotificationRecipients({ entityRefs: ['group:default/parent_group'], - excludeEntityRefs: ['user:default/ignored'], + excludedEntityRefs: ['user:default/ignored'], }), ).resolves.toEqual({ userEntityRefs: ['user:default/foo', 'user:default/bar'], @@ -120,7 +120,7 @@ describe('getUsersForEntityRef', () => { await expect( resolver.resolveNotificationRecipients({ entityRefs: ['component:default/test_component'], - excludeEntityRefs: [], + excludedEntityRefs: [], }), ).resolves.toEqual({ userEntityRefs: ['user:default/foo'] }); }); @@ -164,7 +164,7 @@ describe('getUsersForEntityRef', () => { await expect( resolver.resolveNotificationRecipients({ entityRefs: ['component:default/test_component'], - excludeEntityRefs: [], + excludedEntityRefs: [], }), ).resolves.toEqual({ userEntityRefs: ['user:default/foo'] }); }); @@ -208,7 +208,7 @@ describe('getUsersForEntityRef', () => { await expect( resolver.resolveNotificationRecipients({ entityRefs: ['component:default/test_component'], - excludeEntityRefs: ['user:default/foo'], + excludedEntityRefs: ['user:default/foo'], }), ).resolves.toEqual({ userEntityRefs: [] }); }); diff --git a/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.ts b/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.ts index 2401ec7c1f..200b5dc4ea 100644 --- a/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.ts +++ b/plugins/notifications-backend/src/service/DefaultNotificationRecipientResolver.ts @@ -54,16 +54,16 @@ export class DefaultNotificationRecipientResolver async resolveNotificationRecipients(options: { entityRefs: string[]; - excludeEntityRefs?: string[]; + excludedEntityRefs?: string[]; }): Promise<{ userEntityRefs: string[] }> { - const { entityRefs, excludeEntityRefs = [] } = options; + const { entityRefs, excludedEntityRefs = [] } = options; const [userEntityRefs, otherEntityRefs] = partitionEntityRefs(entityRefs); const users: string[] = userEntityRefs.filter( - ref => !excludeEntityRefs.includes(ref), + ref => !excludedEntityRefs.includes(ref), ); const filtered = otherEntityRefs.filter( - ref => !excludeEntityRefs.includes(ref), + ref => !excludedEntityRefs.includes(ref), ); const fields = ['kind', 'metadata.name', 'metadata.namespace', 'relations']; @@ -87,7 +87,7 @@ export class DefaultNotificationRecipientResolver } const currentEntityRef = stringifyEntityRef(entity); - if (excludeEntityRefs.includes(currentEntityRef)) { + if (excludedEntityRefs.includes(currentEntityRef)) { return []; } @@ -130,7 +130,7 @@ export class DefaultNotificationRecipientResolver const ret = [ ...new Set([...groupUsers, ...childGroupUsers.flat(2)]), - ].filter(ref => !excludeEntityRefs.includes(ref)); + ].filter(ref => !excludedEntityRefs.includes(ref)); cachedEntityRefs.set(currentEntityRef, ret); return ret; } @@ -145,7 +145,7 @@ export class DefaultNotificationRecipientResolver } if (isUserEntityRef(ownerRef)) { - if (excludeEntityRefs.includes(ownerRef)) { + if (excludedEntityRefs.includes(ownerRef)) { return []; } return [ownerRef]; @@ -171,7 +171,7 @@ export class DefaultNotificationRecipientResolver userEntityRefs: [...new Set(users)] .filter(Boolean) // Need to filter again after resolving users - .filter(ref => !excludeEntityRefs.includes(ref)), + .filter(ref => !excludedEntityRefs.includes(ref)), }; } } diff --git a/plugins/notifications-backend/src/service/router.test.ts b/plugins/notifications-backend/src/service/router.test.ts index 08aa021014..f42a7b4513 100644 --- a/plugins/notifications-backend/src/service/router.test.ts +++ b/plugins/notifications-backend/src/service/router.test.ts @@ -274,6 +274,31 @@ describe.each(databases.eachSupportedId())('createRouter (%s)', databaseId => { expect(notifications).toHaveLength(1); }); + it('should not send to user entity if excluded', async () => { + const response = await sendNotification({ + recipients: { + type: 'entity', + entityRef: ['user:default/mock'], + excludeEntityRef: 'user:default/mock', + }, + payload: { + title: 'test notification', + metadata: { + attr: 1, + }, + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([]); + + const client = await database.getClient(); + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(0); + }); + it('should send to group entity', async () => { const response = await sendNotification({ recipients: { @@ -306,6 +331,50 @@ describe.each(databases.eachSupportedId())('createRouter (%s)', databaseId => { expect(notifications).toHaveLength(1); }); + it('should send not send to group entity if excluded', async () => { + const response = await sendNotification({ + recipients: { + type: 'entity', + entityRef: ['group:default/mock'], + excludeEntityRef: 'group:default/mock', + }, + payload: { + title: 'test notification', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([]); + + const client = await database.getClient(); + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(0); + }); + + it('should send not send to user entity if excluded', async () => { + const response = await sendNotification({ + recipients: { + type: 'entity', + entityRef: ['group:default/mock'], + excludeEntityRef: 'user:default/mock', + }, + payload: { + title: 'test notification', + }, + }); + + expect(response.status).toEqual(200); + expect(response.body).toEqual([]); + + const client = await database.getClient(); + const notifications = await client('notification') + .where('user', 'user:default/mock') + .select(); + expect(notifications).toHaveLength(0); + }); + it('should only send one notification per user', async () => { const response = await sendNotification({ recipients: {