From c2eb976553a62098bc26ef2012af88b046e86743 Mon Sep 17 00:00:00 2001 From: Filip Maj Date: Thu, 19 Sep 2024 11:50:45 -0700 Subject: [PATCH] some todos, adding action routing tests. --- src/App.ts | 16 +++++++----- src/types/actions/block-action.ts | 5 ++-- test/unit/App-routes.spec.ts | 10 ------- test/unit/App/middleware.spec.ts | 22 +++++++++++++--- test/unit/App/routing-action.spec.ts | 39 ++++++++++++---------------- test/unit/helpers/events.ts | 16 +++++++----- 6 files changed, 54 insertions(+), 54 deletions(-) diff --git a/src/App.ts b/src/App.ts index b47cac25f..4839c9a49 100644 --- a/src/App.ts +++ b/src/App.ts @@ -158,8 +158,7 @@ export interface AuthorizeResult { userId?: string; teamId?: string; enterpriseId?: string; - // TODO: for better type safety, we may want to revisit this - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // biome-ignore lint/suspicious/noExplicitAny: TODO: for better type safety, we may want to revisit this [key: string]: any; } @@ -167,7 +166,8 @@ export interface ActionConstraints { type?: A['type']; block_id?: A extends BlockAction ? string | RegExp : never; action_id?: A extends BlockAction ? string | RegExp : never; - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // TODO: callback ID doesn't apply to block actions, so the SlackAction generic above is too wide to apply here. + // biome-ignore lint/suspicious/noExplicitAny: TODO: for better type safety, we may want to revisit this callback_id?: Extract extends any ? string | RegExp : never; } @@ -176,7 +176,7 @@ export interface OptionsConstraints { type?: A['type']; block_id?: A extends SlackOptions ? string | RegExp : never; action_id?: A extends SlackOptions ? string | RegExp : never; - // eslint-disable-next-line @typescript-eslint/no-explicit-any + // biome-ignore lint/suspicious/noExplicitAny: TODO: for better type safety, we may want to revisit this callback_id?: Extract extends any ? string | RegExp : never; } @@ -769,13 +769,15 @@ export default class App (k) => k !== 'action_id' && k !== 'block_id' && k !== 'callback_id' && k !== 'type', ); if (unknownConstraintKeys.length > 0) { + // TODO:event() will throw an error if you provide an invalid event name; we should align this behaviour. this.logger.error( `Action listener cannot be attached using unknown constraint keys: ${unknownConstraintKeys.join(', ')}`, ); return; } - const _listeners = listeners as any; // FIXME: workaround for TypeScript 4.7 breaking changes + // biome-ignore lint/suspicious/noExplicitAny: FIXME: workaround for TypeScript 4.7 breaking changes + const _listeners = listeners as any; this.listeners.push([onlyActions, matchConstraints(constraints), ..._listeners] as Middleware[]); } @@ -783,8 +785,8 @@ export default class App commandName: string | RegExp, ...listeners: Middleware[] ): void { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const _listeners = listeners as any; // FIXME: workaround for TypeScript 4.7 breaking changes + // biome-ignore lint/suspicious/noExplicitAny: FIXME: workaround for TypeScript 4.7 breaking changes + const _listeners = listeners as any; this.listeners.push([ onlyCommands, matchCommandName(commandName), diff --git a/src/types/actions/block-action.ts b/src/types/actions/block-action.ts index a94340208..9c49a0534 100644 --- a/src/types/actions/block-action.ts +++ b/src/types/actions/block-action.ts @@ -253,7 +253,7 @@ export interface BlockAction { it('should acknowledge any of possible events', async () => { // Arrange - const actionFn = sinon.fake.resolves({}); const viewFn = sinon.fake.resolves({}); const optionsFn = sinon.fake.resolves({}); overrides = buildOverrides([withNoopWebClient()]); @@ -485,15 +484,6 @@ describe('App event routing', () => { authorize: sinon.fake.resolves(dummyAuthorizationResult), }); - app.action('block_action_id', async () => { - await actionFn(); - }); - app.action({ callback_id: 'interactive_message_callback_id' }, async () => { - await actionFn(); - }); - app.action({ callback_id: 'dialog_submission_callback_id' }, async () => { - await actionFn(); - }); app.view('view_callback_id', async () => { await viewFn(); }); diff --git a/test/unit/App/middleware.spec.ts b/test/unit/App/middleware.spec.ts index 96911fe8a..3da1338ca 100644 --- a/test/unit/App/middleware.spec.ts +++ b/test/unit/App/middleware.spec.ts @@ -498,12 +498,18 @@ describe('App middleware processing', () => { app.error(fakeErrorHandler); await fakeReceiver.sendEvent( createDummyBlockActionEventMiddlewareArgs( - action_id, - 'bid', + { + action: { + type: 'button', + action_id, + block_id: 'bid', + action_ts: '1', + text: { type: 'plain_text', text: 'hi' }, + }, + }, { response_url, }, - { type: 'button', action_id, block_id: 'bid', action_ts: '1', text: { type: 'plain_text', text: 'hi' } }, ), ); @@ -528,10 +534,18 @@ describe('App middleware processing', () => { app.error(fakeErrorHandler); await fakeReceiver.sendEvent( createDummyBlockActionEventMiddlewareArgs( + { + action: { + type: 'button', + action_id, + block_id: 'bid', + action_ts: '1', + text: { type: 'plain_text', text: 'hi' }, + }, + }, { response_url, }, - { type: 'button', action_id, block_id: 'bid', action_ts: '1', text: { type: 'plain_text', text: 'hi' } }, ), ); diff --git a/test/unit/App/routing-action.spec.ts b/test/unit/App/routing-action.spec.ts index 02b84aaf1..8009b8703 100644 --- a/test/unit/App/routing-action.spec.ts +++ b/test/unit/App/routing-action.spec.ts @@ -24,7 +24,7 @@ function buildOverrides(secondOverrides: Override[]): Override { ); } -describe('App shortcut() routing', () => { +describe('App action() routing', () => { let fakeReceiver: FakeReceiver; let fakeHandler: SinonSpy; const fakeLogger = createFakeLogger(); @@ -45,44 +45,37 @@ describe('App shortcut() routing', () => { }); }); - it('should route a block action event to a handler registered with `action(string)` that matches the callback ID', async () => { - app.action('my_callback_id', fakeHandler); + it('should route a block action event to a handler registered with `action(string)` that matches the action ID', async () => { + app.action('my_id', fakeHandler); await fakeReceiver.sendEvent({ - ...createDummyMessageShortcutMiddlewareArgs('my_callback_id'), + ...createDummyBlockActionEventMiddlewareArgs({ action_id: 'my_id' }), }); sinon.assert.called(fakeHandler); }); - it('should route a Slack shortcut event to a handler registered with `shortcut(RegExp)` that matches the callback ID', async () => { - app.shortcut(/my_call/, fakeHandler); + it('should route a block action event to a handler registered with `action(RegExp)` that matches the action ID', async () => { + app.action(/my_action/, fakeHandler); await fakeReceiver.sendEvent({ - ...createDummyMessageShortcutMiddlewareArgs('my_callback_id'), + ...createDummyBlockActionEventMiddlewareArgs({ action_id: 'my_action' }), }); sinon.assert.called(fakeHandler); }); - it('should route a Slack shortcut event to a handler registered with `shortcut({callback_id})` that matches the callback ID', async () => { - app.shortcut({ callback_id: 'my_callback_id' }, fakeHandler); + it('should route a block action event to a handler registered with `action({block_id})` that matches the block ID', async () => { + app.action({ block_id: 'my_id' }, fakeHandler); await fakeReceiver.sendEvent({ - ...createDummyMessageShortcutMiddlewareArgs('my_callback_id'), + ...createDummyBlockActionEventMiddlewareArgs({ block_id: 'my_id' }), }); sinon.assert.called(fakeHandler); }); - it('should route a Slack shortcut event to a handler registered with `shortcut({type})` that matches the type', async () => { - app.shortcut({ type: 'message_action' }, fakeHandler); + it('should route a block action event to a handler registered with `action({type:block_actions})`', async () => { + app.action({ type: 'block_actions' }, fakeHandler); await fakeReceiver.sendEvent({ - ...createDummyMessageShortcutMiddlewareArgs(), + ...createDummyBlockActionEventMiddlewareArgs(), }); sinon.assert.called(fakeHandler); }); - it('should route a Slack shortcut event to a handler registered with `shortcut({type, callback_id})` that matches both the type and the callback_id', async () => { - app.shortcut({ type: 'message_action', callback_id: 'my_id' }, fakeHandler); - await fakeReceiver.sendEvent({ - ...createDummyMessageShortcutMiddlewareArgs('my_id'), - }); - sinon.assert.called(fakeHandler); - }); - it('should throw if provided a constraint with unknown shortcut constraint keys', async () => { - // @ts-ignore providing known invalid shortcut constraint parameter - app.shortcut({ id: 'boom' }, fakeHandler); + it('should throw if provided a constraint with unknown action constraint keys', async () => { + // @ts-ignore providing known invalid action constraint parameter + app.action({ id: 'boom' }, fakeHandler); sinon.assert.calledWithMatch(fakeLogger.error, 'unknown constraint keys'); }); }); diff --git a/test/unit/helpers/events.ts b/test/unit/helpers/events.ts index e730fc9c5..f2c1fe194 100644 --- a/test/unit/helpers/events.ts +++ b/test/unit/helpers/events.ts @@ -71,18 +71,20 @@ export function createDummyAppMentionEventMiddlewareArgs( say, }; } -// TODO: support overloads to support multiple ways of overriding the action? expose action_id, block_id, and the full action too +interface DummyBlockActionOverride { + action_id?: string; + block_id?: string; + action?: BlockElementAction; +} export function createDummyBlockActionEventMiddlewareArgs( - action_id = 'action_id', - block_id = 'block_id', + actionOverrides?: DummyBlockActionOverride, // biome-ignore lint/suspicious/noExplicitAny: allow mocking tools to provide any override bodyOverrides?: Record, - action?: BlockElementAction, ): SlackActionMiddlewareArgs { - const act: BlockElementAction = action || { + const act: BlockElementAction = actionOverrides?.action || { type: 'button', - action_id, - block_id, + action_id: actionOverrides?.action_id || 'action_id', + block_id: actionOverrides?.block_id || 'block_id', action_ts: ts, text: { type: 'plain_text', text: 'hi' }, };