mirror of
				https://github.com/Unleash/unleash.git
				synced 2025-10-27 11:02:16 +01:00 
			
		
		
		
	fix: use postmessage in slack app addon (#4688)
https://linear.app/unleash/issue/2-1392/fix-flaky-unhandled-events - Reverts part of https://github.com/Unleash/unleash/pull/4490 and uses `postMessage` instead. This prevents an error where scheduling the message was flaky and some events ended up not being handled at all (see screenshot below). As a bonus, this simplifies our code and prevents having a delay. It seems like this method still works with the channel name instead of needing an id, which was the main motivation towards the changes in the aforementioned PR; - Removes `FEATURE_UPDATED` from the event options for the Slack App addon, as [this event is deprecated](https://docs.getunleash.io/reference/environments#addons); - Adds support for events without a specific project, including a test, similar to https://github.com/Unleash/unleash/pull/4672 - Misc cleanups; 
This commit is contained in:
		
							parent
							
								
									772f9850ba
								
							
						
					
					
						commit
						ba416e1656
					
				| @ -1,6 +1,5 @@ | ||||
| import { | ||||
|     FEATURE_CREATED, | ||||
|     FEATURE_UPDATED, | ||||
|     FEATURE_ARCHIVED, | ||||
|     FEATURE_REVIVED, | ||||
|     FEATURE_STALE_ON, | ||||
| @ -70,7 +69,6 @@ const slackAppDefinition: IAddonDefinition = { | ||||
|     ], | ||||
|     events: [ | ||||
|         FEATURE_CREATED, | ||||
|         FEATURE_UPDATED, | ||||
|         FEATURE_ARCHIVED, | ||||
|         FEATURE_REVIVED, | ||||
|         FEATURE_STALE_ON, | ||||
|  | ||||
| @ -4,7 +4,7 @@ import { ChatPostMessageArguments, ErrorCode } from '@slack/web-api'; | ||||
| 
 | ||||
| const slackApiCalls: ChatPostMessageArguments[] = []; | ||||
| 
 | ||||
| let scheduleMessage = jest.fn().mockImplementation((options) => { | ||||
| let postMessage = jest.fn().mockImplementation((options) => { | ||||
|     slackApiCalls.push(options); | ||||
|     return Promise.resolve(); | ||||
| }); | ||||
| @ -12,7 +12,7 @@ let scheduleMessage = jest.fn().mockImplementation((options) => { | ||||
| jest.mock('@slack/web-api', () => ({ | ||||
|     WebClient: jest.fn().mockImplementation(() => ({ | ||||
|         chat: { | ||||
|             scheduleMessage, | ||||
|             postMessage, | ||||
|         }, | ||||
|         on: jest.fn(), | ||||
|     })), | ||||
| @ -60,7 +60,7 @@ describe('SlackAppAddon', () => { | ||||
|     beforeEach(() => { | ||||
|         jest.useFakeTimers(); | ||||
|         slackApiCalls.length = 0; | ||||
|         scheduleMessage.mockClear(); | ||||
|         postMessage.mockClear(); | ||||
|         addon = new SlackAppAddon({ | ||||
|             getLogger, | ||||
|             unleashUrl: 'http://some-url.com', | ||||
| @ -124,7 +124,7 @@ describe('SlackAppAddon', () => { | ||||
|     }); | ||||
| 
 | ||||
|     it('should log error when an API call fails', async () => { | ||||
|         scheduleMessage = jest.fn().mockRejectedValue(mockError); | ||||
|         postMessage = jest.fn().mockRejectedValue(mockError); | ||||
| 
 | ||||
|         await addon.handleEvent(event, { accessToken }); | ||||
| 
 | ||||
| @ -134,7 +134,7 @@ describe('SlackAppAddon', () => { | ||||
|         ); | ||||
|     }); | ||||
| 
 | ||||
|     it('should handle rejections in chat.scheduleMessage', async () => { | ||||
|     it('should handle rejections in chat.postMessage', async () => { | ||||
|         const eventWith3Tags: IEvent = { | ||||
|             ...event, | ||||
|             tags: [ | ||||
| @ -144,7 +144,7 @@ describe('SlackAppAddon', () => { | ||||
|             ], | ||||
|         }; | ||||
| 
 | ||||
|         scheduleMessage = jest | ||||
|         postMessage = jest | ||||
|             .fn() | ||||
|             .mockResolvedValueOnce({ ok: true }) | ||||
|             .mockResolvedValueOnce({ ok: true }) | ||||
| @ -152,7 +152,7 @@ describe('SlackAppAddon', () => { | ||||
| 
 | ||||
|         await addon.handleEvent(eventWith3Tags, { accessToken }); | ||||
| 
 | ||||
|         expect(scheduleMessage).toHaveBeenCalledTimes(3); | ||||
|         expect(postMessage).toHaveBeenCalledTimes(3); | ||||
|         expect(loggerMock.warn).toHaveBeenCalledWith( | ||||
|             `Error handling event ${FEATURE_ENVIRONMENT_ENABLED}. A platform error occurred: Platform error message`, | ||||
|             expect.any(Object), | ||||
|  | ||||
| @ -12,7 +12,6 @@ import Addon from './addon'; | ||||
| 
 | ||||
| import slackAppDefinition from './slack-app-definition'; | ||||
| import { IAddonConfig } from '../types/model'; | ||||
| const SCHEDULE_MESSAGE_DELAY_IN_SECONDS = 10; | ||||
| import { | ||||
|     FeatureEventFormatter, | ||||
|     FeatureEventFormatterMd, | ||||
| @ -52,9 +51,11 @@ export default class SlackAppAddon extends Addon { | ||||
|                 this.logger.warn('No access token provided.'); | ||||
|                 return; | ||||
|             } | ||||
|             let postToDefault = | ||||
| 
 | ||||
|             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) { | ||||
| @ -89,9 +90,7 @@ export default class SlackAppAddon extends Addon { | ||||
|             const text = this.msgFormatter.format(event); | ||||
|             const url = this.msgFormatter.featureLink(event); | ||||
|             const requests = eventChannels.map((name) => { | ||||
|                 const now = Math.floor(new Date().getTime() / 1000); | ||||
|                 const postAt = now + SCHEDULE_MESSAGE_DELAY_IN_SECONDS; | ||||
|                 return this.slackClient!.chat.scheduleMessage({ | ||||
|                 return this.slackClient!.chat.postMessage({ | ||||
|                     channel: name, | ||||
|                     text, | ||||
|                     blocks: [ | ||||
| @ -118,7 +117,6 @@ export default class SlackAppAddon extends Addon { | ||||
|                             ], | ||||
|                         }, | ||||
|                     ], | ||||
|                     post_at: postAt, | ||||
|                 }); | ||||
|             }); | ||||
| 
 | ||||
|  | ||||
| @ -329,6 +329,42 @@ test('should not filter out global events (no specific environment) even if addo | ||||
|     expect(events[0].event.data.name).toBe('some-toggle'); | ||||
| }); | ||||
| 
 | ||||
| test('should not filter out global events (no specific project) even if addon is setup to filter for projects', async () => { | ||||
|     const { addonService, stores } = getSetup(); | ||||
|     const filteredProject = 'filtered'; | ||||
|     const config = { | ||||
|         provider: 'simple', | ||||
|         enabled: true, | ||||
|         events: [FEATURE_CREATED], | ||||
|         projects: [filteredProject], | ||||
|         environments: [], | ||||
|         description: '', | ||||
|         parameters: { | ||||
|             url: 'http://localhost:wh', | ||||
|         }, | ||||
|     }; | ||||
| 
 | ||||
|     const globalEventWithNoProject = { | ||||
|         type: FEATURE_CREATED, | ||||
|         createdBy: 'some@user.com', | ||||
|         data: { | ||||
|             name: 'some-toggle', | ||||
|             enabled: false, | ||||
|             strategies: [{ name: 'default' }], | ||||
|         }, | ||||
|     }; | ||||
| 
 | ||||
|     await addonService.createAddon(config, 'me@mail.com'); | ||||
|     await stores.eventStore.store(globalEventWithNoProject); | ||||
|     const simpleProvider = addonService.addonProviders.simple; | ||||
|     // @ts-expect-error
 | ||||
|     const events = simpleProvider.getEvents(); | ||||
| 
 | ||||
|     expect(events.length).toBe(1); | ||||
|     expect(events[0].event.type).toBe(FEATURE_CREATED); | ||||
|     expect(events[0].event.data.name).toBe('some-toggle'); | ||||
| }); | ||||
| 
 | ||||
| test('should support wildcard option for filtering addons', async () => { | ||||
|     const { addonService, stores } = getSetup(); | ||||
|     const desiredProjects = ['desired', 'desired2']; | ||||
|  | ||||
| @ -114,6 +114,7 @@ export default class AddonService { | ||||
|                     .filter((addon) => addon.events.includes(eventName)) | ||||
|                     .filter( | ||||
|                         (addon) => | ||||
|                             !event.project || | ||||
|                             !addon.projects || | ||||
|                             addon.projects.length == 0 || | ||||
|                             addon.projects[0] === WILDCARD_OPTION || | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user