From cbab8bd61e1fb62543271c5a8d86e51fa18ec8af Mon Sep 17 00:00:00 2001 From: Michael Poutre Date: Tue, 2 Nov 2021 02:44:29 +0000 Subject: [PATCH] Fix #1148 - Adjust the app.message listener interface in TypeScript to compile the examples in documents (#1185) To open this method up to TypeScript users a required either removing the two existing overloads, or creating new overloads. Hopefully by providing additional overloads as well as inline documentation, this can provide assistance as well as control -- Fixes #1148 --- src/App.spec.ts | 219 +++++++++++++++++++++++++++++++++++++++++++++++- src/App.ts | 46 ++++++++-- 2 files changed, 259 insertions(+), 6 deletions(-) diff --git a/src/App.spec.ts b/src/App.spec.ts index 6e43377da..3ce6c4a9c 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -6,7 +6,12 @@ import { LogLevel } from '@slack/logger'; import { WebClientOptions, WebClient } from '@slack/web-api'; import { Override, mergeOverrides, createFakeLogger, delay } from './test-helpers'; import { ErrorCode, UnknownError, AuthorizationError, CodedError } from './errors'; -import { Receiver, ReceiverEvent, SayFn, NextFn } from './types'; +import { + Receiver, + ReceiverEvent, + SayFn, + NextFn, +} from './types'; import { ConversationStore } from './conversation-store'; import App, { ExtendedErrorHandlerArgs, ViewConstraints } from './App'; import { WorkflowStep } from './WorkflowStep'; @@ -2139,6 +2144,218 @@ describe('App', () => { assert.equal(fakeErrorHandler.callCount, dummyReceiverEvents.length); }); }); + + describe('App#message', () => { + let fakeMiddleware1: sinon.SinonSpy; + let fakeMiddleware2: sinon.SinonSpy; + let fakeMiddlewares: sinon.SinonSpy[]; + let passFilter: sinon.SinonSpy; + let failFilter: sinon.SinonSpy; + let MockApp: typeof import('./App').default; + let app: App; + + const callNextMiddleware = () => async ({ next }: { next?: NextFn }) => { + if (next) { + await next(); + } + }; + + const fakeMessageEvent = (receiver: FakeReceiver, message: string): Promise => receiver.sendEvent({ + body: { + type: 'event_callback', + event: { + type: 'message', + text: message, + }, + }, + ack: noop, + }); + + const controlledMiddleware = (shouldCallNext: boolean) => async ({ next }: { next?: NextFn }) => { + if (next && shouldCallNext) { + await next(); + } + }; + + const assertMiddlewaresCalledOnce = () => { + assert(fakeMiddleware1.calledOnce); + assert(fakeMiddleware2.calledOnce); + }; + + const assertMiddlewaresCalledOrder = () => { + sinon.assert.callOrder(...fakeMiddlewares); + }; + + const assertMiddlewaresNotCalled = () => { + assert(fakeMiddleware1.notCalled); + assert(fakeMiddleware2.notCalled); + }; + + const message = 'val - pass-string - val'; + const PASS_STRING = 'pass-string'; + const PASS_PATTERN = /.*pass-string.*/; + const FAIL_STRING = 'fail-string'; + const FAIL_PATTERN = /.*fail-string.*/; + + beforeEach(async () => { + sinon.restore(); + MockApp = await importApp(); + app = new MockApp({ receiver: fakeReceiver, authorize: sinon.fake.resolves(dummyAuthorizationResult) }); + fakeMiddleware1 = sinon.spy(callNextMiddleware()); + fakeMiddleware2 = sinon.spy(callNextMiddleware()); + fakeMiddlewares = [ + fakeMiddleware1, + fakeMiddleware2, + ]; + + passFilter = sinon.spy(controlledMiddleware(true)); + failFilter = sinon.spy(controlledMiddleware(false)); + }); + + // public message(...listeners: MessageEventMiddleware[]): void; + it('overload1 - should accept list of listeners and call each one', async () => { + // Act + app.message(...fakeMiddlewares); + await fakeMessageEvent(fakeReceiver, 'testing message'); + + // Assert + assertMiddlewaresCalledOnce(); + }); + + it('overload1 - should not call second listener if first does not pass', async () => { + // Act + app.message(controlledMiddleware(false), fakeMiddleware1); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assert(fakeMiddleware1.notCalled); + }); + + // public message(pattern: string | RegExp, ...listeners: MessageEventMiddleware[]): void; + it('overload2 - should call listeners if message contains string', async () => { + // Act + app.message(PASS_STRING, ...fakeMiddlewares); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresCalledOnce(); + assertMiddlewaresCalledOrder(); + }); + + it('overload2 - should not call listeners if message does not contain string', async () => { + // Act + app.message(FAIL_STRING, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresNotCalled(); + }); + + it('overload2 - should call listeners if message matches pattern', async () => { + // Act + app.message(PASS_PATTERN, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresCalledOnce(); + assertMiddlewaresCalledOrder(); + }); + + it('overload2 - should not call listeners if message does not match pattern', async () => { + // Act + app.message(FAIL_PATTERN, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresNotCalled(); + }); + + it('overload3 - should call listeners if filter and string match', async () => { + // Act + app.message(passFilter, PASS_STRING, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresCalledOnce(); + assertMiddlewaresCalledOrder(); + }); + + it('overload3 - should not call listeners if filter does not pass', async () => { + // Act + app.message(failFilter, PASS_STRING, ...fakeMiddlewares); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresNotCalled(); + }); + + it('overload3 - should not call listeners if string does not match', async () => { + // Act + app.message(passFilter, FAIL_STRING, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresNotCalled(); + }); + + it('overload3 - should not call listeners if message does not match pattern', async () => { + // Act + app.message(passFilter, FAIL_PATTERN, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresNotCalled(); + }); + + it('overload4 - should call listeners if filter passes', async () => { + // Act + app.message(passFilter, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresCalledOrder(); + assertMiddlewaresCalledOnce(); + }); + + it('overload4 - should not call listeners if filter fails', async () => { + // Act + app.message(failFilter, fakeMiddleware1, fakeMiddleware2); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresNotCalled(); + }); + + it('should accept multiple strings', async () => { + // Act + app.message(PASS_STRING, '- val', ...fakeMiddlewares); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresCalledOnce(); + assertMiddlewaresCalledOrder(); + }); + + it('should accept string and pattern', async () => { + // Act + app.message(PASS_STRING, PASS_PATTERN, ...fakeMiddlewares); + await fakeMessageEvent(fakeReceiver, message); + // Assert + assertMiddlewaresCalledOnce(); + assertMiddlewaresCalledOrder(); + }); + + it('should not call listeners after fail', async () => { + // Act + app.message(PASS_STRING, FAIL_PATTERN, ...fakeMiddlewares); + app.message(FAIL_STRING, PASS_PATTERN, ...fakeMiddlewares); + app.message(passFilter, failFilter, ...fakeMiddlewares); + await fakeMessageEvent(fakeReceiver, message); + + // Assert + assertMiddlewaresNotCalled(); + }); + }); }); }); }); diff --git a/src/App.ts b/src/App.ts index 54718f6e9..8932a23cb 100644 --- a/src/App.ts +++ b/src/App.ts @@ -180,6 +180,9 @@ export interface ExtendedErrorHandler { export interface AnyErrorHandler extends ErrorHandler, ExtendedErrorHandler { } +// Used only in this file +type MessageEventMiddleware = Middleware>; + class WebClientPool { private pool: { [token: string]: WebClient } = {}; @@ -549,11 +552,44 @@ export default class App { ] as Middleware[]); } - // TODO: just make a type alias for Middleware> - // TODO: maybe remove the first two overloads - public message(...listeners: Middleware>[]): void; - public message(pattern: string | RegExp, ...listeners: Middleware>[]): void; - public message(...patternsOrMiddleware: (string | RegExp | Middleware>)[]): void { + /** + * + * @param listeners Middlewares that process and react to a message event + */ + public message(...listeners: MessageEventMiddleware[]): void; + /** + * + * @param pattern Used for filtering out messages that don't match. + * Strings match via {@link String.prototype.includes}. + * @param listeners Middlewares that process and react to the message events that matched the provided patterns. + */ + public message(pattern: string | RegExp, ...listeners: MessageEventMiddleware[]): void; + /** + * + * @param filter Middleware that can filter out messages. Generally this is done by returning before + * calling {@link AllMiddlewareArgs.next} if there is no match. See {@link directMention} for an example. + * @param pattern Used for filtering out messages that don't match the pattern. Strings match + * via {@link String.prototype.includes}. + * @param listeners Middlewares that process and react to the message events that matched the provided pattern. + */ + public message( + filter: MessageEventMiddleware, pattern: string | RegExp, ...listeners: MessageEventMiddleware[] + ): void; + /** + * + * @param filter Middleware that can filter out messages. Generally this is done by returning before calling + * {@link AllMiddlewareArgs.next} if there is no match. See {@link directMention} for an example. + * @param listeners Middlewares that process and react to the message events that matched the provided patterns. + */ + public message(filter: MessageEventMiddleware, ...listeners: MessageEventMiddleware[]): void; + /** + * This allows for further control of the filtering and response logic. Patterns and middlewares are processed in + * the order provided. If any patterns do not match, or a middleware does not call {@link AllMiddlewareArgs.next}, + * all remaining patterns and middlewares will be skipped. + * @param patternsOrMiddleware A mix of patterns and/or middlewares. + */ + public message(...patternsOrMiddleware: (string | RegExp | MessageEventMiddleware)[]): void; + public message(...patternsOrMiddleware: (string | RegExp | MessageEventMiddleware)[]): void { const messageMiddleware = patternsOrMiddleware.map((patternOrMiddleware) => { if (typeof patternOrMiddleware === 'string' || util.types.isRegExp(patternOrMiddleware)) { return matchMessage(patternOrMiddleware);