diff --git a/src/Slapp.ts b/src/Slapp.ts index d27c84c1f..d83b3495e 100644 --- a/src/Slapp.ts +++ b/src/Slapp.ts @@ -68,7 +68,7 @@ export interface AuthorizeResult { botToken?: string; // used by `say` (preferred over userToken) userToken?: string; // used by `say` (overridden by botToken) botId?: string; // required for `ignoreSelf` global middleware - botUserId?: string; // optional + botUserId?: string; // optional but helps `ignoreSelf` global middleware be more complete [ key: string ]: any; } diff --git a/src/middleware/builtin.ts b/src/middleware/builtin.ts index 154871151..cc66b670c 100644 --- a/src/middleware/builtin.ts +++ b/src/middleware/builtin.ts @@ -199,22 +199,47 @@ export function matchEventType(type: string): Middleware { - return ({ next }) => { - // TODO - // emit error if the botId is not defined - // if (msg.isBot() && msg.isMessage() && msg.body.event.subtype === 'bot_message') { - // let bothFalsy = !msg.meta.app_bot_id && !msg.meta.bot_id - // let bothEqual = msg.meta.app_bot_id === msg.meta.bot_id - // if (!bothFalsy && bothEqual) { - // return - // } - // } - next(); + return (args) => { + // TODO: we might be able to query for the botId and/or botUserId and cache it to avoid errors/warnings. + // if so, the botId check would emit a warning instead of an error. + + // When context does not have a botId in it, then this middleware cannot perform its job. Bail immediately. + if (args.context.botId === undefined) { + // TODO: coded error + args.next(new Error('Cannot ignore events from the app\'s own bot user without a bot ID. ' + + 'Use authorize option to return a botId.')); + return; + } + + const botId = args.context.botId as string; + const botUserId = args.context.botUserId !== undefined ? args.context.botUserId as string : undefined; + + if (isEventArgs(args)) { + // Once we've narrowed the type down to SlackEventMiddlewareArgs, there's no way to further narrow it + // down to 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 + if (message.subtype === 'bot_message' && message.bot_id === botId) { + return; + } + + } + } + + // Look for any events (not just Events API) that are from the same userId as this app, and return to skip + // NOTE: this goes further than Slapp's previous ignoreSelf middleware. That middleware only applied this filter + // when the event was of type message. Is this okay? + if (botUserId !== undefined && args.payload.user === botUserId) { + return; + } + + // If all the previous checks didn't skip this message, then its okay to resume to next + args.next(); }; } @@ -222,12 +247,24 @@ export function ignoreSelfMiddleware(): Middleware { * Middleware that ignores messages from any bot user */ export function ignoreBotsMiddleware(): Middleware { - return ({ next }) => { - // TODO - // if (msg.isBot() && msg.isMessage() && msg.body.event.subtype === 'bot_message') { - // return - // } - next(); + return (args) => { + if (isEventArgs(args)) { + // Once we've narrowed the type down to SlackEventMiddlewareArgs, there's no way to further narrow it + // down to 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 + if (message.subtype === 'bot_message') { + return; + } + } + } + + // If all the previous checks didn't skip this message, then its okay to resume to next + args.next(); }; } @@ -247,6 +284,12 @@ function isCallbackIdentifiedAction(action: SlackAction): action is Call return (action as CallbackIdentifiedAction).callback_id !== undefined; } +function isEventArgs( + args: AnyMiddlewareArgs, +): args is SlackEventMiddlewareArgs { + return (args as SlackEventMiddlewareArgs).event !== undefined; +} + /** * Helper function that determines if a payload is an options payload * TODO: Get rid of use of KeyValueMapping