Merge pull request #29710 from backstage/notifications-slack-user-email
notifications-slack-module: use user email if slack annotation is missing
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'@backstage/plugin-notifications-backend-module-slack': patch
|
||||
---
|
||||
|
||||
Added email-based Slack User ID lookup if `metadata.annotations.slack.com/bot-notify` is missing from user entity
|
||||
+175
@@ -50,6 +50,7 @@ jest.mock('@slack/web-api', () => {
|
||||
},
|
||||
],
|
||||
})),
|
||||
lookupByEmail: jest.fn(),
|
||||
},
|
||||
};
|
||||
return { WebClient: jest.fn(() => mockSlack) };
|
||||
@@ -81,6 +82,27 @@ const DEFAULT_ENTITIES_RESPONSE = {
|
||||
},
|
||||
},
|
||||
} as unknown as Entity,
|
||||
{
|
||||
kind: 'User',
|
||||
metadata: {
|
||||
name: 'mock-without-slack-annotation',
|
||||
namespace: 'default',
|
||||
annotations: {},
|
||||
},
|
||||
spec: {
|
||||
profile: {
|
||||
email: 'test@example.com',
|
||||
},
|
||||
},
|
||||
} as unknown as Entity,
|
||||
{
|
||||
kind: 'Group',
|
||||
metadata: {
|
||||
name: 'mock-without-slack-annotation',
|
||||
namespace: 'default',
|
||||
annotations: {},
|
||||
},
|
||||
} as unknown as Entity,
|
||||
],
|
||||
};
|
||||
|
||||
@@ -368,4 +390,157 @@ describe('SlackNotificationProcessor', () => {
|
||||
expect(slack.chat.postMessage).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('when slack.com/bot-notify annotation is missing', () => {
|
||||
it('should not send notification to a group without annotation', async () => {
|
||||
const slack = new WebClient();
|
||||
|
||||
const processor = SlackNotificationProcessor.fromConfig(config, {
|
||||
auth,
|
||||
discovery,
|
||||
logger,
|
||||
catalog: catalogServiceMock({
|
||||
entities: DEFAULT_ENTITIES_RESPONSE.items,
|
||||
}),
|
||||
slack,
|
||||
})[0];
|
||||
|
||||
await processor.processOptions({
|
||||
recipients: {
|
||||
type: 'entity',
|
||||
entityRef: 'group:default/mock-without-slack-annotation',
|
||||
},
|
||||
payload: { title: 'notification' },
|
||||
});
|
||||
|
||||
expect(slack.chat.postMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not send notification to a user without annotation and email', async () => {
|
||||
const slack = new WebClient();
|
||||
|
||||
const processor = SlackNotificationProcessor.fromConfig(config, {
|
||||
auth,
|
||||
discovery,
|
||||
logger,
|
||||
catalog: catalogServiceMock({
|
||||
entities: [DEFAULT_ENTITIES_RESPONSE.items[2]],
|
||||
}),
|
||||
slack,
|
||||
})[0];
|
||||
|
||||
await processor.postProcess(
|
||||
{
|
||||
origin: 'plugin',
|
||||
id: '1234',
|
||||
user: 'user:default/mock-without-slack-annotation',
|
||||
created: new Date(),
|
||||
payload: {
|
||||
title: 'notification',
|
||||
link: '/catalog/user/default/jane.doe',
|
||||
},
|
||||
},
|
||||
{
|
||||
recipients: {
|
||||
type: 'entity',
|
||||
entityRef: 'user:default/mock-without-slack-annotation',
|
||||
},
|
||||
payload: { title: 'notification' },
|
||||
},
|
||||
);
|
||||
|
||||
expect(slack.chat.postMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should try to find user by email when annotation is missing', async () => {
|
||||
const slack = new WebClient();
|
||||
(slack.users.lookupByEmail as jest.Mock).mockResolvedValueOnce({
|
||||
ok: true,
|
||||
user: { id: 'U12345678' },
|
||||
});
|
||||
|
||||
const processor = SlackNotificationProcessor.fromConfig(config, {
|
||||
auth,
|
||||
discovery,
|
||||
logger,
|
||||
catalog: catalogServiceMock({
|
||||
entities: DEFAULT_ENTITIES_RESPONSE.items,
|
||||
}),
|
||||
slack,
|
||||
})[0];
|
||||
|
||||
await processor.postProcess(
|
||||
{
|
||||
origin: 'plugin',
|
||||
id: '1234',
|
||||
user: 'user:default/mock-without-slack-annotation',
|
||||
created: new Date(),
|
||||
payload: {
|
||||
title: 'notification',
|
||||
link: '/catalog/user/default/jane.doe',
|
||||
},
|
||||
},
|
||||
{
|
||||
recipients: {
|
||||
type: 'entity',
|
||||
entityRef: 'user:default/mock-without-slack-annotation',
|
||||
},
|
||||
payload: { title: 'notification' },
|
||||
},
|
||||
);
|
||||
|
||||
expect(slack.users.lookupByEmail).toHaveBeenCalledWith({
|
||||
email: 'test@example.com',
|
||||
});
|
||||
expect(slack.chat.postMessage).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
channel: 'U12345678',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should log warning when email lookup fails', async () => {
|
||||
const slack = new WebClient();
|
||||
(slack.users.lookupByEmail as jest.Mock).mockRejectedValueOnce(
|
||||
new Error('User not found'),
|
||||
);
|
||||
|
||||
const processor = SlackNotificationProcessor.fromConfig(config, {
|
||||
auth,
|
||||
discovery,
|
||||
logger,
|
||||
catalog: catalogServiceMock({
|
||||
entities: DEFAULT_ENTITIES_RESPONSE.items,
|
||||
}),
|
||||
slack,
|
||||
})[0];
|
||||
|
||||
await processor.postProcess(
|
||||
{
|
||||
origin: 'plugin',
|
||||
id: '1234',
|
||||
user: 'user:default/mock-without-slack-annotation',
|
||||
created: new Date(),
|
||||
payload: {
|
||||
title: 'notification',
|
||||
link: '/catalog/user/default/jane.doe',
|
||||
},
|
||||
},
|
||||
{
|
||||
recipients: {
|
||||
type: 'entity',
|
||||
entityRef: 'user:default/mock-without-slack-annotation',
|
||||
},
|
||||
payload: { title: 'notification' },
|
||||
},
|
||||
);
|
||||
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
'Failed to lookup Slack user by email test@example.com',
|
||||
),
|
||||
);
|
||||
expect(slack.chat.postMessage).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,7 +20,12 @@ import {
|
||||
LoggerService,
|
||||
} from '@backstage/backend-plugin-api';
|
||||
import { CatalogApi } from '@backstage/catalog-client';
|
||||
import { Entity, parseEntityRef } from '@backstage/catalog-model';
|
||||
import {
|
||||
Entity,
|
||||
isUserEntity,
|
||||
parseEntityRef,
|
||||
UserEntity,
|
||||
} from '@backstage/catalog-model';
|
||||
import { Config } from '@backstage/config';
|
||||
import { NotFoundError } from '@backstage/errors';
|
||||
import { Notification } from '@backstage/plugin-notifications-common';
|
||||
@@ -255,7 +260,11 @@ export class SlackNotificationProcessor implements NotificationProcessor {
|
||||
const response = await this.catalog.getEntitiesByRefs(
|
||||
{
|
||||
entityRefs: entityRefs.slice(),
|
||||
fields: [`metadata.annotations.${ANNOTATION_SLACK_BOT_NOTIFY}`],
|
||||
fields: [
|
||||
`kind`,
|
||||
`spec.profile.email`,
|
||||
`metadata.annotations.${ANNOTATION_SLACK_BOT_NOTIFY}`,
|
||||
],
|
||||
},
|
||||
{
|
||||
token,
|
||||
@@ -278,7 +287,42 @@ export class SlackNotificationProcessor implements NotificationProcessor {
|
||||
throw new NotFoundError(`Entity not found: ${entityRef}`);
|
||||
}
|
||||
|
||||
return entity?.metadata?.annotations?.[ANNOTATION_SLACK_BOT_NOTIFY];
|
||||
const slackId = await this.resolveSlackId(entity);
|
||||
return slackId;
|
||||
}
|
||||
|
||||
private async resolveSlackId(entity: Entity): Promise<string | undefined> {
|
||||
// First try to get Slack ID from annotations
|
||||
const slackId = entity.metadata?.annotations?.[ANNOTATION_SLACK_BOT_NOTIFY];
|
||||
if (slackId) {
|
||||
return slackId;
|
||||
}
|
||||
|
||||
// If no Slack ID in annotations and entity is a User, try to find by email
|
||||
if (isUserEntity(entity)) {
|
||||
return this.findSlackIdByEmail(entity);
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
private async findSlackIdByEmail(
|
||||
entity: UserEntity,
|
||||
): Promise<string | undefined> {
|
||||
const email = entity.spec?.profile?.email;
|
||||
if (!email) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
try {
|
||||
const user = await this.slack.users.lookupByEmail({ email });
|
||||
return user.user?.id;
|
||||
} catch (error) {
|
||||
this.logger.warn(
|
||||
`Failed to lookup Slack user by email ${email}: ${error}`,
|
||||
);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
async sendNotification(args: ChatPostMessageArguments): Promise<void> {
|
||||
|
||||
Reference in New Issue
Block a user