From cd62d78b78d26747363d0a0193fdb29cb60e68f0 Mon Sep 17 00:00:00 2001 From: Kai Dubauskas Date: Sat, 31 Jan 2026 13:08:10 -0500 Subject: [PATCH] fix: skipped user DMs when group is present Closes #32584 Signed-off-by: Kai Dubauskas --- .changeset/curvy-socks-punch.md | 5 ++ .../lib/SlackNotificationProcessor.test.ts | 77 +++++++++++++++++++ .../src/lib/SlackNotificationProcessor.ts | 44 +++++++++-- .../src/lib/constants.ts | 6 ++ 4 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 .changeset/curvy-socks-punch.md diff --git a/.changeset/curvy-socks-punch.md b/.changeset/curvy-socks-punch.md new file mode 100644 index 0000000000..6933ad8847 --- /dev/null +++ b/.changeset/curvy-socks-punch.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-notifications-backend-module-slack': patch +--- + +Fix skipped slack DMS for users when a group entity is a recipient 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 9490ff7dde..e5dfc22754 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.test.ts @@ -16,6 +16,7 @@ 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'; @@ -318,6 +319,82 @@ describe('SlackNotificationProcessor', () => { }); }); + describe('when group and user recipients are both provided', () => { + it('should send a group message and only DM explicit users', 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], + }), + slack, + })[0]; + + const processedOptions = await processor.processOptions({ + recipients: { + type: 'entity', + entityRef: ['group:default/mock', 'user:default/mock'], + }, + 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', + }, + }, + processedOptions, + ); + + expect(slack.chat.postMessage).toHaveBeenCalledTimes(2); + expect(slack.chat.postMessage).toHaveBeenLastCalledWith( + expect.objectContaining({ channel: 'U12345678' }), + ); + }); + }); + describe('when broadcast channels are not configured', () => { it('should not send broadcast messages', async () => { const slack = new WebClient(); diff --git a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts index e8f8d9eda4..e1df3b0db9 100644 --- a/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts +++ b/plugins/notifications-backend-module-slack/src/lib/SlackNotificationProcessor.ts @@ -19,6 +19,7 @@ import { Entity, isUserEntity, parseEntityRef, + stringifyEntityRef, UserEntity, } from '@backstage/catalog-model'; import { Config, readDurationFromConfig } from '@backstage/config'; @@ -33,7 +34,10 @@ 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 } from './constants'; +import { + ANNOTATION_SLACK_BOT_NOTIFY, + USER_REFS_FROM_REQUEST_KEY, +} from './constants'; import { BroadcastRoute } from './types'; import { ExpiryMap, toChatPostMessageArgs } from './util'; import { CatalogService } from '@backstage/plugin-catalog-node'; @@ -210,12 +214,14 @@ 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; } @@ -257,7 +263,18 @@ export class SlackNotificationProcessor implements NotificationProcessor { await this.sendNotifications(outbound); - return options; + const enrichedPayload = { + ...options.payload, + metadata: { + ...options.payload.metadata, + [USER_REFS_FROM_REQUEST_KEY]: userEntityRefs, + }, + }; + + return { + ...options, + payload: enrichedPayload, + }; } async postProcess( @@ -272,10 +289,25 @@ export class SlackNotificationProcessor implements NotificationProcessor { destinations.push(...routedChannels); } else if (options.recipients.type === 'entity') { // Handle user-specific notification - const entityRefs = [options.recipients.entityRef].flat(); - if (entityRefs.some(e => parseEntityRef(e).kind === 'group')) { - // We've already dispatched a slack channel message, so let's not send a DM. - return; + const rawUserRefs = + options.payload.metadata?.[USER_REFS_FROM_REQUEST_KEY]; + + 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; + } } 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 cdf052da7e..1fd81c8f61 100644 --- a/plugins/notifications-backend-module-slack/src/lib/constants.ts +++ b/plugins/notifications-backend-module-slack/src/lib/constants.ts @@ -27,3 +27,9 @@ * however IDs are preferred. */ export const ANNOTATION_SLACK_BOT_NOTIFY = 'slack.com/bot-notify'; + +/** + * @public + * Metadata key containing user entities before resolution + */ +export const USER_REFS_FROM_REQUEST_KEY = 'userRefsFromRequest';