From f470c4af0ae5f2012ffe3258d89a933eab7ca831 Mon Sep 17 00:00:00 2001 From: Filip Maj Date: Thu, 19 Sep 2024 16:25:52 -0700 Subject: [PATCH] todos, linting, and setting up routing options unit tests. --- src/App.ts | 30 +++++------ src/types/options/index.ts | 2 +- src/types/utilities.ts | 3 +- test/unit/App-routes.spec.ts | 29 ---------- test/unit/App/routing-options.spec.ts | 76 +++++++++++++++++++++++++++ test/unit/helpers/events.ts | 31 +++++++++++ 6 files changed, 125 insertions(+), 46 deletions(-) create mode 100644 test/unit/App/routing-options.spec.ts diff --git a/src/App.ts b/src/App.ts index 7a587c97d..bdcb87b81 100644 --- a/src/App.ts +++ b/src/App.ts @@ -541,6 +541,7 @@ export default class App return this.receiver.start(...args) as ReturnType; } + // biome-ignore lint/suspicious/noExplicitAny: receivers could accept anything as arguments for stop public stop(...args: any[]): Promise { return this.receiver.stop(...args); } @@ -641,7 +642,7 @@ export default class App return matchMessage(patternOrMiddleware); } return patternOrMiddleware; - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // biome-ignore lint/suspicious/noExplicitAny: FIXME: workaround for TypeScript 4.7 breaking changes }) as any; // FIXME: workaround for TypeScript 4.7 breaking changes this.listeners.push([ @@ -900,12 +901,10 @@ export default class App try { authorizeResult = await this.authorize(source, bodyArg); } catch (error) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // biome-ignore lint/suspicious/noExplicitAny: errors can be anything const e = error as any; this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.'); e.code = ErrorCode.AuthorizationError; - // disabling due to https://github.com/typescript-eslint/typescript-eslint/issues/1277 - // eslint-disable-next-line consistent-return return this.handleError({ error: e, logger: this.logger, @@ -1002,6 +1001,7 @@ export default class App case IncomingEventType.Shortcut: payload = bodyArg as SlackShortcutMiddlewareArgs['body']; break; + // biome-ignore lint/suspicious/noFallthroughSwitchClause: usually not great, but we do it here case IncomingEventType.Action: if (isBlockActionOrInteractiveMessageBody(bodyArg as SlackActionMiddlewareArgs['body'])) { const { actions } = bodyArg as SlackActionMiddlewareArgs['body']; @@ -1024,7 +1024,7 @@ export default class App /** Respond function might be set below */ respond?: RespondFn; /** Ack function might be set below */ - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // biome-ignore lint/suspicious/noExplicitAny: different kinds of acks accept different arguments, TODO: revisit this to see if we can type better ack?: AckFn; complete?: FunctionCompleteFn; fail?: FunctionFailFn; @@ -1097,12 +1097,11 @@ export default class App } if (token !== undefined) { - let pool; + let pool: WebClientPool | undefined = undefined; const clientOptionsCopy = { ...this.clientOptions }; if (authorizeResult.teamId !== undefined) { pool = this.clients[authorizeResult.teamId]; if (pool === undefined) { - // eslint-disable-next-line no-multi-assign pool = this.clients[authorizeResult.teamId] = new WebClientPool(); } // Add teamId to clientOptions so it can be automatically added to web-api calls @@ -1110,7 +1109,6 @@ export default class App } else if (authorizeResult.enterpriseId !== undefined) { pool = this.clients[authorizeResult.enterpriseId]; if (pool === undefined) { - // eslint-disable-next-line no-multi-assign pool = this.clients[authorizeResult.enterpriseId] = new WebClientPool(); } } @@ -1163,16 +1161,15 @@ export default class App const rejectedListenerResults = settledListenerResults.filter(isRejected); if (rejectedListenerResults.length === 1) { throw rejectedListenerResults[0].reason; + // biome-ignore lint/style/noUselessElse: I think this is a biome issue actually... } else if (rejectedListenerResults.length > 1) { throw new MultipleListenerError(rejectedListenerResults.map((rlr) => rlr.reason)); } }, ); } catch (error) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // biome-ignore lint/suspicious/noExplicitAny: errors can be anything const e = error as any; - // disabling due to https://github.com/typescript-eslint/typescript-eslint/issues/1277 - // eslint-disable-next-line consistent-return return this.handleError({ context, error: e, @@ -1291,11 +1288,14 @@ export default class App throw new AppInitializationError( `${tokenUsage} \n\nSince you have not provided a token or authorize, you might be missing one or more required oauth installer options. See https://slack.dev/bolt-js/concepts#authenticating-oauth for these required fields.\n`, ); + // biome-ignore lint/style/noUselessElse: I think this is a biome issue actually... } else if (authorize !== undefined && usingOauth) { throw new AppInitializationError(`You cannot provide both authorize and oauth installer options. ${tokenUsage}`); + // biome-ignore lint/style/noUselessElse: I think this is a biome issue actually... } else if (authorize === undefined && usingOauth) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + // biome-ignore lint/style/noNonNullAssertion: we know installer is truthy here return httpReceiver.installer!.authorize; + // biome-ignore lint/style/noUselessElse: I think this is a biome issue actually... } else if (authorize !== undefined && !usingOauth) { return authorize as Authorize; } @@ -1587,9 +1587,9 @@ function escapeHtml(input: string | undefined | null): string { } function extractFunctionContext(body: StringIndexed) { - let functionExecutionId; - let functionBotAccessToken; - let functionInputs; + let functionExecutionId: string | undefined = undefined; + let functionBotAccessToken: string | undefined = undefined; + let functionInputs: FunctionInputs | undefined = undefined; // function_executed event if (body.event && body.event.type === 'function_executed' && body.event.function_execution_id) { diff --git a/src/types/options/index.ts b/src/types/options/index.ts index 633828267..8dab090b8 100644 --- a/src/types/options/index.ts +++ b/src/types/options/index.ts @@ -15,7 +15,7 @@ export interface SlackOptionsMiddlewareArgs { type?: A['type']; block_id?: A extends SlackOptions ? string | RegExp : never; diff --git a/src/types/utilities.ts b/src/types/utilities.ts index da019c804..477764416 100644 --- a/src/types/utilities.ts +++ b/src/types/utilities.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ import type { ChatPostMessageArguments, ChatPostMessageResponse } from '@slack/web-api'; /** Type predicate for use with `Promise.allSettled` for filtering for resolved results. */ @@ -7,6 +6,7 @@ export const isFulfilled = (p: PromiseSettledResult): p is PromiseFulfille export const isRejected = (p: PromiseSettledResult): p is PromiseRejectedResult => p.status === 'rejected'; /** Using type parameter T (generic), can distribute the Omit over a union set. */ +// biome-ignore lint/suspicious/noExplicitAny: any is the opposite of never type DistributiveOmit = T extends any ? Omit : never; // The say() utility function binds the message to the same channel as the incoming message that triggered the @@ -25,6 +25,7 @@ export type RespondArguments = DistributiveOmit Promise; export type AckFn = (response?: Response) => Promise; diff --git a/test/unit/App-routes.spec.ts b/test/unit/App-routes.spec.ts index de3d3a1ba..42f74da69 100644 --- a/test/unit/App-routes.spec.ts +++ b/test/unit/App-routes.spec.ts @@ -470,7 +470,6 @@ describe('App event routing', () => { it('should acknowledge any of possible events', async () => { // Arrange - const optionsFn = sinon.fake.resolves({}); overrides = buildOverrides([withNoopWebClient()]); const MockApp = await importApp(overrides); const dummyReceiverEvents = createReceiverEvents(); @@ -483,38 +482,10 @@ describe('App event routing', () => { authorize: sinon.fake.resolves(dummyAuthorizationResult), }); - app.options('external_select_action_id', async () => { - await optionsFn(); - }); - app.options( - { - type: 'block_suggestion', - action_id: 'external_select_action_id', - }, - async () => { - await optionsFn(); - }, - ); - app.options({ callback_id: 'dialog_suggestion_callback_id' }, async () => { - await optionsFn(); - }); - app.options( - { - type: 'dialog_suggestion', - callback_id: 'dialog_suggestion_callback_id', - }, - async () => { - await optionsFn(); - }, - ); - app.message('hello', noop); app.command('/echo', noop); app.command(/\/e.*/, noop); await Promise.all(dummyReceiverEvents.map((event) => fakeReceiver.sendEvent(event))); - - // Assert - assert.equal(optionsFn.callCount, 4); }); // This test confirms authorize is being used for org events diff --git a/test/unit/App/routing-options.spec.ts b/test/unit/App/routing-options.spec.ts new file mode 100644 index 000000000..b23e0b23d --- /dev/null +++ b/test/unit/App/routing-options.spec.ts @@ -0,0 +1,76 @@ +import sinon, { type SinonSpy } from 'sinon'; +import { + FakeReceiver, + type Override, + createFakeLogger, + createDummyBlockSuggestionsMiddlewareArgs, + importApp, + mergeOverrides, + noopMiddleware, + withConversationContext, + withMemoryStore, + withNoopAppMetadata, + withNoopWebClient, +} from '../helpers'; +import type App from '../../../src/App'; + +function buildOverrides(secondOverrides: Override[]): Override { + return mergeOverrides( + withNoopAppMetadata(), + withNoopWebClient(), + ...secondOverrides, + withMemoryStore(sinon.fake()), + withConversationContext(sinon.fake.returns(noopMiddleware)), + ); +} + +describe('App options() routing', () => { + let fakeReceiver: FakeReceiver; + let fakeHandler: SinonSpy; + const fakeLogger = createFakeLogger(); + let dummyAuthorizationResult: { botToken: string; botId: string }; + let MockApp: Awaited>; + let app: App; + + beforeEach(async () => { + fakeLogger.error.reset(); + fakeReceiver = new FakeReceiver(); + fakeHandler = sinon.fake(); + dummyAuthorizationResult = { botToken: '', botId: '' }; + MockApp = await importApp(buildOverrides([])); + app = new MockApp({ + logger: fakeLogger, + receiver: fakeReceiver, + authorize: sinon.fake.resolves(dummyAuthorizationResult), + }); + }); + + it('should route a block suggestion event to a handler registered with `options(string)` that matches the action ID', async () => { + app.options('my_id', fakeHandler); + await fakeReceiver.sendEvent({ + ...createDummyBlockSuggestionsMiddlewareArgs({ action_id: 'my_id' }), + }); + sinon.assert.called(fakeHandler); + }); + it('should route a block suggestion event to a handler registered with `options(RegExp)` that matches the action ID', async () => { + app.options(/my_action/, fakeHandler); + await fakeReceiver.sendEvent({ + ...createDummyBlockSuggestionsMiddlewareArgs({ action_id: 'my_action' }), + }); + sinon.assert.called(fakeHandler); + }); + it('should route a block suggestion event to a handler registered with `options({block_id})` that matches the block ID', async () => { + app.options({ block_id: 'my_id' }, fakeHandler); + await fakeReceiver.sendEvent({ + ...createDummyBlockSuggestionsMiddlewareArgs({ block_id: 'my_id' }), + }); + sinon.assert.called(fakeHandler); + }); + it('should route a block suggestion event to a handler registered with `options({type:block_suggestion})`', async () => { + app.options({ type: 'block_suggestion' }, fakeHandler); + await fakeReceiver.sendEvent({ + ...createDummyBlockSuggestionsMiddlewareArgs(), + }); + sinon.assert.called(fakeHandler); + }); +}); diff --git a/test/unit/helpers/events.ts b/test/unit/helpers/events.ts index a09311fff..fa09b478f 100644 --- a/test/unit/helpers/events.ts +++ b/test/unit/helpers/events.ts @@ -12,11 +12,13 @@ import type { SayFn, SlackActionMiddlewareArgs, SlackEventMiddlewareArgs, + SlackOptionsMiddlewareArgs, SlackShortcutMiddlewareArgs, SlackViewMiddlewareArgs, ViewClosedAction, ViewSubmitAction, ViewOutput, + BlockSuggestion, } from '../../../src/types'; const ts = '1234.56'; @@ -72,6 +74,7 @@ export function createDummyAppMentionEventMiddlewareArgs( say, }; } + interface DummyBlockActionOverride { action_id?: string; block_id?: string; @@ -111,6 +114,34 @@ export function createDummyBlockActionEventMiddlewareArgs( }; } +interface DummyBlockSuggestionOverride { + action_id?: string; + block_id?: string; + options?: BlockSuggestion; +} +export function createDummyBlockSuggestionsMiddlewareArgs( + optionsOverrides?: DummyBlockSuggestionOverride, +): SlackOptionsMiddlewareArgs { + const options: BlockSuggestion = optionsOverrides?.options || { + type: 'block_suggestion', + action_id: optionsOverrides?.action_id || 'action_id', + block_id: optionsOverrides?.block_id || 'block_id', + value: 'value', + action_ts: ts, + api_app_id: app_id, + team: { id: team, domain: 'slack.com' }, + user: { id: user, name: 'filmaj' }, + token, + container: {}, + }; + return { + payload: options, + body: options, + options, + ack: () => Promise.resolve(), + }; +} + function createDummyViewOutput(viewOverrides?: Partial): ViewOutput { return { type: 'view',