From 1dcb33d4e7f01f0dfe98274ad93c7638c7e8f7b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nuno=20G=C3=B3is?= Date: Mon, 18 Sep 2023 08:32:37 +0100 Subject: [PATCH] fix: simplify channels logic in slack app integration (#4756) 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) --- src/lib/addons/slack-app-definition.ts | 12 +---- src/lib/addons/slack-app.test.ts | 61 ++++++++++++++++---------- src/lib/addons/slack-app.ts | 25 ++++------- 3 files changed, 48 insertions(+), 50 deletions(-) diff --git a/src/lib/addons/slack-app-definition.ts b/src/lib/addons/slack-app-definition.ts index 43ce4aafcd..e246f25690 100644 --- a/src/lib/addons/slack-app-definition.ts +++ b/src/lib/addons/slack-app-definition.ts @@ -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, diff --git a/src/lib/addons/slack-app.test.ts b/src/lib/addons/slack-app.test.ts index ffd56e944c..abfaa69a30 100644 --- a/src/lib/addons/slack-app.test.ts +++ b/src/lib/addons/slack-app.test.ts @@ -54,7 +54,6 @@ describe('SlackAppAddon', () => { type: 'release', strategies: [{ name: 'default' }], }, - tags: [{ type: 'slack', value: 'general' }], }; beforeEach(() => { @@ -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, @@ -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(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 not post a message if there are no tagged channels and no defaultChannels', async () => { + await addon.handleEvent(event, { accessToken, }); expect(slackApiCalls.length).toBe(0); }); - it('should use defaultChannels if no tagged channels are found', async () => { - const eventWithoutTags: IEvent = { - ...event, - tags: [], - }; - - await addon.handleEvent(eventWithoutTags, { - 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 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`, diff --git a/src/lib/addons/slack-app.ts b/src/lib/addons/slack-app.ts index 5727ff4d6d..e2b9fe0579 100644 --- a/src/lib/addons/slack-app.ts +++ b/src/lib/addons/slack-app.ts @@ -22,7 +22,6 @@ import { IEvent } from '../types/events'; interface ISlackAppAddonParameters { accessToken: string; defaultChannels: string; - alwaysPostToDefault: string; } export default class SlackAppAddon extends Addon { @@ -45,28 +44,20 @@ export default class SlackAppAddon extends Addon { parameters: ISlackAppAddonParameters, ): Promise { 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(