Skip to content

Commit

Permalink
some todos, adding action routing tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
Filip Maj committed Sep 19, 2024
1 parent 0fc5cdd commit 1c8fa6c
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 54 deletions.
16 changes: 9 additions & 7 deletions src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,16 @@ 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;
}

export interface ActionConstraints<A extends SlackAction = SlackAction> {
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<A, { callback_id?: string }> extends any ? string | RegExp : never;
}

Expand All @@ -176,7 +176,7 @@ export interface OptionsConstraints<A extends SlackOptions = SlackOptions> {
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<A, { callback_id?: string }> extends any ? string | RegExp : never;
}

Expand Down Expand Up @@ -769,22 +769,24 @@ export default class App<AppCustomContext extends StringIndexed = StringIndexed>
(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<AnyMiddlewareArgs>[]);
}

public command<MiddlewareCustomContext extends StringIndexed = StringIndexed>(
commandName: string | RegExp,
...listeners: Middleware<SlackCommandMiddlewareArgs, AppCustomContext & MiddlewareCustomContext>[]
): 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),
Expand Down
5 changes: 2 additions & 3 deletions src/types/actions/block-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ export interface BlockAction<ElementAction extends BasicElementAction = BlockEle
user?: string; // undocumented that this is optional, it won't be there for bot messages
ts: string;
text?: string; // undocumented that this is optional, but how could it exist on block kit based messages?
// eslint-disable-next-line @typescript-eslint/no-explicit-any
// biome-ignore lint/suspicious/noExplicitAny: TODO: poorly typed message here
[key: string]: any;
};
view?: ViewOutput;
Expand All @@ -272,8 +272,7 @@ export interface BlockAction<ElementAction extends BasicElementAction = BlockEle
// TODO: we'll need to fill this out a little more carefully in the future, possibly using a generic parameter
container: StringIndexed;

// this appears in the block_suggestions schema, but we're not sure when its present or what its type would be
// eslint-disable-next-line @typescript-eslint/no-explicit-any
// biome-ignore lint/suspicious/noExplicitAny: TODO: this appears in the block_suggestions schema, but we're not sure when its present or what its type would be
app_unfurl?: any;

// exists for enterprise installs
Expand Down
10 changes: 0 additions & 10 deletions test/unit/App-routes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,6 @@ describe('App event routing', () => {

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()]);
Expand All @@ -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();
});
Expand Down
22 changes: 18 additions & 4 deletions test/unit/App/middleware.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' } },
),
);

Expand All @@ -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' } },
),
);

Expand Down
39 changes: 16 additions & 23 deletions test/unit/App/routing-action.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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');
});
});
16 changes: 9 additions & 7 deletions test/unit/helpers/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, any>,
action?: BlockElementAction,
): SlackActionMiddlewareArgs<BlockAction> {
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' },
};
Expand Down

0 comments on commit 1c8fa6c

Please sign in to comment.