Skip to content

Commit

Permalink
fix: simplify channels logic in slack app integration (#4756)
Browse files Browse the repository at this point in the history
https://linear.app/unleash/issue/2-1393/drop-the-always-post-to-default-channels-field

This drops the "Always post to default channels" field in the Slack App
integration in favor of always posting to the configured channels. This
should simplify the configuration of this integration.

Here's a breakdown of the logic with this change:
 - Always post to the configured Slack channels, regardless of tags;
- Tags are still respected. E.g. if we have a configured channel
"channel-1" and a tag for "channel-2", then we post to both channels;
- As channels are optional, if you would like to skip default channels
for certain events and handle everything through tags, you can just
create a new configuration without any default channels;

This also updates the labels and changes the tests to better reflect the
intended behavior.


![image](https://github.com/Unleash/unleash/assets/14320932/a2427bdd-4b92-44b3-9bad-8adb0f94c34d)
  • Loading branch information
nunogois committed Sep 18, 2023
1 parent 055cf15 commit 1dcb33d
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 46 deletions.
12 changes: 2 additions & 10 deletions src/lib/addons/slack-app-definition.ts
Expand Up @@ -41,17 +41,9 @@ const slackAppDefinition: IAddonDefinition = {
},
{
name: 'defaultChannels',
displayName: 'Default channels',
displayName: 'Channels',
description:
'A comma-separated list of channels to post to if no tagged channels are found (e.g. a toggle without tags, or an event with no tags associated).',
type: 'text',
required: false,
sensitive: false,
},
{
name: 'alwaysPostToDefault',
displayName: 'Always post to default channels',
description: `If set to 'true' or 'yes', the app will always post events to the default channels, even if the feature toggle has slack tags`,
'A comma-separated list of channels to post the configured events to. These channels are always notified, regardless of the event type or the presence of a slack tag.',
type: 'text',
required: false,
sensitive: false,
Expand Down
53 changes: 34 additions & 19 deletions src/lib/addons/slack-app.test.ts
Expand Up @@ -54,7 +54,6 @@ describe('SlackAppAddon', () => {
type: 'release',
strategies: [{ name: 'default' }],
},
tags: [{ type: 'slack', value: 'general' }],
};

beforeEach(() => {
Expand All @@ -72,12 +71,26 @@ describe('SlackAppAddon', () => {
});

it('should post message when feature is toggled', async () => {
await addon.handleEvent(event, { accessToken });
await addon.handleEvent(event, {
accessToken,
defaultChannels: 'general',
});

expect(slackApiCalls.length).toBe(1);
expect(slackApiCalls[0].channel).toBe('general');
});

it('should post to all channels in defaultChannels', async () => {
await addon.handleEvent(event, {
accessToken,
defaultChannels: 'general, another-channel-1',
});

expect(slackApiCalls.length).toBe(2);
expect(slackApiCalls[0].channel).toBe('general');
expect(slackApiCalls[1].channel).toBe('another-channel-1');
});

it('should post to all channels in tags', async () => {
const eventWith2Tags: IEvent = {
...event,
Expand All @@ -94,39 +107,41 @@ describe('SlackAppAddon', () => {
expect(slackApiCalls[1].channel).toBe('another-channel-1');
});

it('should not post a message if there are no tagged channels and no defaultChannels', async () => {
const eventWithoutTags: IEvent = {
it('should concatenate defaultChannels and channels in tags to post to all unique channels found', async () => {
const eventWith2Tags: IEvent = {
...event,
tags: [],
tags: [
{ type: 'slack', value: 'general' },
{ type: 'slack', value: 'another-channel-1' },
],
};

await addon.handleEvent(eventWithoutTags, {
await addon.handleEvent(eventWith2Tags, {
accessToken,
defaultChannels: 'another-channel-1, another-channel-2',
});

expect(slackApiCalls.length).toBe(0);
expect(slackApiCalls.length).toBe(3);
expect(slackApiCalls[0].channel).toBe('general');
expect(slackApiCalls[1].channel).toBe('another-channel-1');
expect(slackApiCalls[2].channel).toBe('another-channel-2');
});

it('should use defaultChannels if no tagged channels are found', async () => {
const eventWithoutTags: IEvent = {
...event,
tags: [],
};

await addon.handleEvent(eventWithoutTags, {
it('should not post a message if there are no tagged channels and no defaultChannels', async () => {
await addon.handleEvent(event, {
accessToken,
defaultChannels: 'general, another-channel-1',
});

expect(slackApiCalls.length).toBe(2);
expect(slackApiCalls[0].channel).toBe('general');
expect(slackApiCalls[1].channel).toBe('another-channel-1');
expect(slackApiCalls.length).toBe(0);
});

it('should log error when an API call fails', async () => {
postMessage = jest.fn().mockRejectedValue(mockError);

await addon.handleEvent(event, { accessToken });
await addon.handleEvent(event, {
accessToken,
defaultChannels: 'general',
});

expect(loggerMock.warn).toHaveBeenCalledWith(
`Error handling event ${event.type}. A platform error occurred: Platform error message`,
Expand Down
25 changes: 8 additions & 17 deletions src/lib/addons/slack-app.ts
Expand Up @@ -22,7 +22,6 @@ import { IEvent } from '../types/events';
interface ISlackAppAddonParameters {
accessToken: string;
defaultChannels: string;
alwaysPostToDefault: string;
}

export default class SlackAppAddon extends Addon {
Expand All @@ -45,28 +44,20 @@ export default class SlackAppAddon extends Addon {
parameters: ISlackAppAddonParameters,
): Promise<void> {
try {
const { accessToken, defaultChannels, alwaysPostToDefault } =
parameters;
const { accessToken, defaultChannels } = parameters;
if (!accessToken) {
this.logger.warn('No access token provided.');
return;
}

const postToDefault =
alwaysPostToDefault === 'true' || alwaysPostToDefault === 'yes';
this.logger.debug(`Post to default was set to ${postToDefault}`);

const taggedChannels = this.findTaggedChannels(event);
let eventChannels: string[];
if (postToDefault) {
eventChannels = taggedChannels.concat(
this.getDefaultChannels(defaultChannels),
);
} else {
eventChannels = taggedChannels.length
? taggedChannels
: this.getDefaultChannels(defaultChannels);
}
const eventChannels = [
...new Set(
taggedChannels.concat(
this.getDefaultChannels(defaultChannels),
),
),
];

if (!eventChannels.length) {
this.logger.debug(
Expand Down

0 comments on commit 1dcb33d

Please sign in to comment.