From ea329727f98555ddb9f0c28a69b8690d21bc121d Mon Sep 17 00:00:00 2001 From: Kai Dubauskas Date: Wed, 18 Feb 2026 16:28:39 -0500 Subject: [PATCH] check explicit user refs from options.recipients Signed-off-by: Kai Dubauskas --- .../lib/SlackNotificationProcessor.test.ts | 58 +++++-------------- .../src/lib/SlackNotificationProcessor.ts | 45 ++++---------- .../src/lib/constants.ts | 6 -- 3 files changed, 25 insertions(+), 84 deletions(-) diff --git a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts index e5dfc22754..41c5939532 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts @@ -16,7 +16,6 @@ import { mockServices } from '@backstage/backend-test-utils'; import { SlackNotificationProcessor } from './SlackNotificationProcessor'; -import { USER_REFS_FROM_REQUEST_KEY } from './constants'; import { catalogServiceMock } from '@backstage/plugin-catalog-node/testUtils'; import { WebClient } from '@slack/web-api'; import { Entity } from '@backstage/catalog-model'; @@ -319,32 +318,20 @@ describe('SlackNotificationProcessor', () => { }); }); - describe('when group and user recipients are both provided', () => { - it('should send a group message and only DM explicit users', async () => { + describe('when recipients include both users and a group', () => { + it('should still DM explicit user recipients', async () => { const slack = new WebClient(); - const extraUser: Entity = { - apiVersion: 'backstage.io/v1alpha1', - kind: 'User', - metadata: { - name: 'other', - namespace: 'default', - annotations: { - 'slack.com/bot-notify': 'U99999999', - }, - }, - spec: {}, - }; const processor = SlackNotificationProcessor.fromConfig(config, { auth, logger, catalog: catalogServiceMock({ - entities: [...DEFAULT_ENTITIES_RESPONSE.items, extraUser], + entities: DEFAULT_ENTITIES_RESPONSE.items, }), slack, })[0]; - const processedOptions = await processor.processOptions({ + await processor.processOptions({ recipients: { type: 'entity', entityRef: ['group:default/mock', 'user:default/mock'], @@ -352,44 +339,25 @@ describe('SlackNotificationProcessor', () => { payload: { title: 'notification' }, }); - expect(processedOptions.payload.metadata).toEqual( - expect.objectContaining({ - [USER_REFS_FROM_REQUEST_KEY]: ['user:default/mock'], - }), - ); - - await processor.postProcess( - { - origin: 'plugin', - id: 'group-user-1', - user: 'user:default/other', - created: new Date(), - payload: { - title: 'notification', - link: '/catalog/user/default/other', - }, - }, - processedOptions, - ); - - expect(slack.chat.postMessage).toHaveBeenCalledTimes(1); - await processor.postProcess( { origin: 'plugin', id: 'explicit-user-1', user: 'user:default/mock', created: new Date(), - payload: { - title: 'notification', - link: '/catalog/user/default/mock', - }, + payload: { title: 'notification' }, + }, + { + recipients: { + type: 'entity', + entityRef: ['group:default/mock', 'user:default/mock'], + }, + payload: { title: 'notification' }, }, - processedOptions, ); expect(slack.chat.postMessage).toHaveBeenCalledTimes(2); - expect(slack.chat.postMessage).toHaveBeenLastCalledWith( + expect(slack.chat.postMessage).toHaveBeenCalledWith( expect.objectContaining({ channel: 'U12345678' }), ); }); diff --git a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts index e1df3b0db9..648bc5aa80 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts @@ -34,10 +34,7 @@ import { Counter, metrics } from '@opentelemetry/api'; import { ChatPostMessageArguments, WebClient } from '@slack/web-api'; import DataLoader from 'dataloader'; import pThrottle from 'p-throttle'; -import { - ANNOTATION_SLACK_BOT_NOTIFY, - USER_REFS_FROM_REQUEST_KEY, -} from './constants'; +import { ANNOTATION_SLACK_BOT_NOTIFY } from './constants'; import { BroadcastRoute } from './types'; import { ExpiryMap, toChatPostMessageArgs } from './util'; import { CatalogService } from '@backstage/plugin-catalog-node'; @@ -214,14 +211,12 @@ export class SlackNotificationProcessor implements NotificationProcessor { const entityRefs = [options.recipients.entityRef].flat(); - const userEntityRefs: string[] = []; const outbound: ChatPostMessageArguments[] = []; await Promise.all( entityRefs.map(async entityRef => { const compoundEntityRef = parseEntityRef(entityRef); // skip users as they are sent direct messages if (compoundEntityRef.kind === 'user') { - userEntityRefs.push(stringifyEntityRef(compoundEntityRef)); return; } @@ -263,18 +258,7 @@ export class SlackNotificationProcessor implements NotificationProcessor { await this.sendNotifications(outbound); - const enrichedPayload = { - ...options.payload, - metadata: { - ...options.payload.metadata, - [USER_REFS_FROM_REQUEST_KEY]: userEntityRefs, - }, - }; - - return { - ...options, - payload: enrichedPayload, - }; + return options; } async postProcess( @@ -289,25 +273,20 @@ export class SlackNotificationProcessor implements NotificationProcessor { destinations.push(...routedChannels); } else if (options.recipients.type === 'entity') { // Handle user-specific notification - const rawUserRefs = - options.payload.metadata?.[USER_REFS_FROM_REQUEST_KEY]; - + const entityRefs = [options.recipients.entityRef].flat(); + const explicitUserEntityRefs = entityRefs + .filter(entityRef => parseEntityRef(entityRef).kind === 'user') + .map(entityRef => stringifyEntityRef(parseEntityRef(entityRef))); const normalizedUserRef = stringifyEntityRef( parseEntityRef(notification.user), ); - const hasValidUserRefs = - Array.isArray(rawUserRefs) && - rawUserRefs.every(ref => typeof ref === 'string'); - const explicitUserEntityRefs = hasValidUserRefs - ? rawUserRefs.map(ref => stringifyEntityRef(parseEntityRef(ref))) - : undefined; - if (!explicitUserEntityRefs?.includes(normalizedUserRef)) { - const entityRefs = [options.recipients.entityRef].flat(); - if (entityRefs.some(e => parseEntityRef(e).kind === 'group')) { - // This user was resolved from a non-user entity and we've already send a channel DM so let's not send another. - return; - } + if ( + entityRefs.some(e => parseEntityRef(e).kind === 'group') && + !explicitUserEntityRefs.includes(normalizedUserRef) + ) { + // This user was resolved from a non-user entity and we've already sent a group channel message. + return; } const destination = await this.getSlackNotificationTarget( diff --git a/plugins/notifications-backend-module-slack/src/lib/constants.ts b/plugins/notifications-backend-module-slack/src/lib/constants.ts index 2ad07af73b..cdf052da7e 100644 --- a/plugins/notifications-backend-module-slack/src/lib/constants.ts +++ b/plugins/notifications-backend-module-slack/src/lib/constants.ts @@ -27,9 +27,3 @@ * however IDs are preferred. */ export const ANNOTATION_SLACK_BOT_NOTIFY = 'slack.com/bot-notify'; - -/** - * @public - * Metadata key containing a list of all user entities before resolution - */ -export const USER_REFS_FROM_REQUEST_KEY = 'userRefsFromRequest';