Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 284 regex events #763

Merged
merged 10 commits into from
Feb 17, 2021
5 changes: 5 additions & 0 deletions src/App.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,11 @@ describe('App', () => {
app.event('app_home_opened', async ({}) => {
/* noop */
});

app.event(/app_home_opened|app_mention/, async ({}) => {
/* noop */
});

app.message('hello', async ({}) => {
/* noop */
});
Expand Down
15 changes: 14 additions & 1 deletion src/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
SlackShortcutMiddlewareArgs,
SlackViewMiddlewareArgs,
SlackAction,
EventTypePattern,
SlackShortcut,
Context,
SayFn,
Expand Down Expand Up @@ -410,8 +411,20 @@ export default class App {
public event<EventType extends string = string>(
eventName: EventType,
...listeners: Middleware<SlackEventMiddlewareArgs<EventType>>[]
): void;
public event<EventType extends RegExp = RegExp>(
eventName: EventType,
...listeners: Middleware<SlackEventMiddlewareArgs<string>>[]
): void;
public event<EventType extends EventTypePattern = EventTypePattern>(
eventNameOrPattern: EventType,
...listeners: Middleware<SlackEventMiddlewareArgs<string>>[]
): void {
this.listeners.push([onlyEvents, matchEventType(eventName), ...listeners] as Middleware<AnyMiddlewareArgs>[]);
this.listeners.push([
onlyEvents,
matchEventType(eventNameOrPattern),
...listeners,
] as Middleware<AnyMiddlewareArgs>[]);
}

// TODO: just make a type alias for Middleware<SlackEventMiddlewareArgs<'message'>>
Expand Down
78 changes: 75 additions & 3 deletions src/middleware/builtin.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from '../types';
import { onlyCommands, onlyEvents, matchCommandName, matchEventType, subtype } from './builtin';
import { SlashCommand } from '../types/command';
import { AppMentionEvent } from '../types/events';
import { AppMentionEvent, AppHomeOpenedEvent } from '../types/events';
import { GenericMessageEvent } from '../types/events/message-events';
import { WebClient } from '@slack/web-api';
import { Logger } from '@slack/logger';
Expand Down Expand Up @@ -532,7 +532,7 @@ describe('onlyEvents', () => {
const args: SlackEventMiddlewareArgs<'app_mention'> & { event?: SlackEvent } = {
payload: appMentionEvent,
event: appMentionEvent,
message: null as never, // a bit hackey to sartisfy TS compiler
message: null as never, // a bit hackey to satisfy TS compiler as 'null' cannot be assigned to type 'never'
body: {
token: 'token-value',
team_id: 'T1234567',
Expand Down Expand Up @@ -582,7 +582,7 @@ describe('matchEventType', () => {
return {
payload: appMentionEvent,
event: appMentionEvent,
message: null as never, // a bit hackey to sartisfy TS compiler
message: null as never, // a bit hackey to satisfy TS compiler as 'null' cannot be assigned to type 'never'
body: {
token: 'token-value',
team_id: 'T1234567',
Expand All @@ -597,6 +597,27 @@ describe('matchEventType', () => {
};
}

function buildArgsAppHomeOpened(): SlackEventMiddlewareArgs<'app_home_opened'> & {
event?: SlackEvent;
} {
return {
payload: appHomeOpenedEvent,
event: appHomeOpenedEvent,
message: null as never, // a bit hackey to satisfy TS compiler as 'null' cannot be assigned to type 'never'
body: {
token: 'token-value',
team_id: 'T1234567',
api_app_id: 'A1234567',
event: appHomeOpenedEvent,
type: 'event_callback',
event_id: 'event-id-value',
event_time: 123,
authed_users: [],
},
say: sayNoop,
};
}

it('should detect valid requests', async () => {
const fakeNext = sinon.fake();
await matchEventType('app_mention')({
Expand All @@ -609,6 +630,30 @@ describe('matchEventType', () => {
assert.isTrue(fakeNext.called);
});

it('should detect valid RegExp requests with app_mention', async () => {
const fakeNext = sinon.fake();
await matchEventType(/app_mention|app_home_opened/)({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect! this is the intended usage and i'm glad you specifically have a test that illustrates it.

logger,
client,
next: fakeNext,
context: {},
...buildArgs(),
});
assert.isTrue(fakeNext.called);
});

it('should detect valid RegExp requests with app_home_opened', async () => {
const fakeNext = sinon.fake();
await matchEventType(/app_mention|app_home_opened/)({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the difference between this test case and the one above it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no big difference, I just wanted to cover tests for the regex(/app_mention|app_home_opened/) specific to app_home_opened. We can remove this test case if it’s unnecessary to have redundant tests.
.

logger,
client,
next: fakeNext,
context: {},
...buildArgsAppHomeOpened(),
});
assert.isTrue(fakeNext.called);
});

it('should skip other requests', async () => {
const fakeNext = sinon.fake();
await matchEventType('app_home_opened')({
Expand All @@ -620,6 +665,18 @@ describe('matchEventType', () => {
});
assert.isFalse(fakeNext.called);
});

it('should skip other requests for RegExp', async () => {
const fakeNext = sinon.fake();
await matchEventType(/foo/)({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call to have a test case for both matching and not matching.

logger,
client,
next: fakeNext,
context: {},
...buildArgs(),
});
assert.isFalse(fakeNext.called);
});
});

describe('subtype', () => {
Expand Down Expand Up @@ -749,6 +806,21 @@ const appMentionEvent: AppMentionEvent = {
thread_ts: '123.123',
};

const appHomeOpenedEvent: AppHomeOpenedEvent = {
type: 'app_home_opened',
user: 'USERNAME',
channel: 'U1234567',
tab: 'home',
view: {
type: 'home',
blocks: [],
clear_on_close: false,
notify_on_close: false,
external_id: '',
},
event_ts: '123.123',
};

const botMessageEvent: MessageEvent = {
type: 'message',
subtype: 'bot_message',
Expand Down
29 changes: 22 additions & 7 deletions src/middleware/builtin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
MessageShortcut,
BlockElementAction,
SlackViewAction,
EventTypePattern,
} from '../types';
import { ActionConstraints, ViewConstraints, ShortcutConstraints } from '../App';
import { ContextMissingPropertyError } from '../errors';
Expand Down Expand Up @@ -254,16 +255,31 @@ export function matchCommandName(name: string): Middleware<SlackCommandMiddlewar
};
}

/**
* Middleware that filters out any event that isn't of given type
/*
* Middleware that filters out events that don't match pattern
*/
export function matchEventType(type: string): Middleware<SlackEventMiddlewareArgs> {
return async ({ event, next }) => {
// Filter out any events that are not the correct type
if (type !== event.type) {
export function matchEventType(pattern: EventTypePattern): Middleware<SlackEventMiddlewareArgs> {
return async ({ event, context, next }) => {
let tempMatches: RegExpMatchArray | null;
if (!('type' in event) || event.type === undefined) {
return;
}

// Filter out events that don't contain the pattern
if (typeof pattern === 'string') {
if (event.type !== pattern) {
return;
}
} else {
tempMatches = event.type.match(pattern);

if (tempMatches !== null) {
context['matches'] = tempMatches;
} else {
return;
}
}

// TODO: remove the non-null assertion operator
await next!();
};
Expand All @@ -287,7 +303,6 @@ export function ignoreSelf(): Middleware<AnyMiddlewareArgs> {
// SlackEventMiddlewareArgs<'message'> without a cast, so the following couple lines do that.
if (args.message !== undefined) {
const message = args.message as SlackEventMiddlewareArgs<'message'>['message'];

// TODO: revisit this once we have all the message subtypes defined to see if we can do this better with
// type narrowing
// Look for an event that is identified as a bot message from the same bot ID as this app, and return to skip
Expand Down
2 changes: 2 additions & 0 deletions src/types/events/base-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export type SlackEvent =
| WorkflowStepDeletedEvent
| WorkflowStepExecuteEvent;

export type EventTypePattern = string | RegExp;

/**
* Any event in Slack's Events API
*
Expand Down