Merge pull request #709 from aoberoi/fix-loosely-indexed-event-types

Overriding patch coverage status check. Overall project coverage has increased with this change. Planned and in progress changes that will soon land will cover the areas of this diff that are currently not covered.

src/helpers.ts

@@ -29,15 +29,34 @@ export enum IncomingEventType {
  */
 export function getTypeAndConversation(body: any): { type?: IncomingEventType; conversationId?: string } {
   if (body.event !== undefined) {
-    const eventBody = body as SlackEventMiddlewareArgs<string>['body'];
+    const { event } = body as SlackEventMiddlewareArgs<string>['body'];
+
+    // Find conversationId
+    const conversationId: string | undefined = (() => {
+      let foundConversationId: string;
+      if ('channel' in event) {
+        if (typeof event.channel === 'string') {
+          foundConversationId = event.channel;
+        } else if ('id' in event.channel) {
+          foundConversationId = event.channel.id;
+        }
+      }
+      if ('channel_id' in event) {
+        foundConversationId = event.channel_id;
+      }
+      if ('item' in event && 'channel' in event.item) {
+        // no channel for reaction_added, reaction_removed, star_added, or star_removed with file or file_comment items
+        foundConversationId = event.item.channel;
+      }
+      // Using non-null assertion (!) because the alternative is to use `foundConversation: (string | undefined)`, which
+      // impedes the very useful type checker help above that ensures the value is only defined to strings, not
+      // undefined. This is safe when used in combination with the || operator with a default value.
+      return foundConversationId! || undefined;
+    })();
+
     return {
+      conversationId,
       type: IncomingEventType.Event,
-      conversationId:
-        eventBody.event.channel !== undefined
-          ? eventBody.event.channel
-          : eventBody.event.item !== undefined
-          ? eventBody.event.item.channel
-          : undefined,
     };
   }
   if (body.command !== undefined) {

src/middleware/builtin.spec.ts

@@ -5,10 +5,18 @@ import sinon from 'sinon';
 import { ErrorCode, ContextMissingPropertyError } from '../errors';
 import { Override, createFakeLogger } from '../test-helpers';
 import rewiremock from 'rewiremock';
-import { SlackEventMiddlewareArgs, NextFn, Context, MessageEvent, SlackCommandMiddlewareArgs } from '../types';
+import {
+  SlackEventMiddlewareArgs,
+  NextFn,
+  Context,
+  SlackEvent,
+  MessageEvent,
+  SlackCommandMiddlewareArgs,
+} from '../types';
 import { onlyCommands, onlyEvents, matchCommandName, matchEventType, subtype } from './builtin';
 import { SlashCommand } from '../types/command';
-import { SlackEvent, AppMentionEvent, BotMessageEvent } from '../types/events';
+import { AppMentionEvent } from '../types/events';
+import { GenericMessageEvent } from '../types/events/message-events';
 import { WebClient } from '@slack/web-api';
 import { Logger } from '@slack/logger';
 
@@ -688,8 +696,8 @@ async function importBuiltin(overrides: Override = {}): Promise<typeof import('.
   return rewiremock.module(() => import('./builtin'), overrides);
 }
 
-function createFakeMessageEvent(content: string | MessageEvent['blocks'] = ''): MessageEvent {
-  const event: Partial<MessageEvent> = {
+function createFakeMessageEvent(content: string | GenericMessageEvent['blocks'] = ''): MessageEvent {
+  const event: Partial<GenericMessageEvent> = {
     type: 'message',
     channel: 'CHANNEL_ID',
     user: 'USER_ID',
@@ -739,10 +747,9 @@ const appMentionEvent: AppMentionEvent = {
   event_ts: '123.123',
 };
 
-const botMessageEvent: BotMessageEvent & MessageEvent = {
+const botMessageEvent: MessageEvent = {
   type: 'message',
   subtype: 'bot_message',
-  channel: 'C1234567',
   user: 'U1234567',
   ts: '123.123',
   text: 'this is my message',

src/middleware/builtin.ts

@@ -215,7 +215,7 @@ export function matchMessage(
   return async ({ event, context, next }) => {
     let tempMatches: RegExpMatchArray | null;
 
-    if (event.text === undefined) {
+    if (!('text' in event) || event.text === undefined) {
       return;
     }
 
@@ -301,7 +301,7 @@ export function ignoreSelf(): Middleware<AnyMiddlewareArgs> {
       const eventsWhichShouldBeKept = ['member_joined_channel', 'member_left_channel'];
       const isEventShouldBeKept = eventsWhichShouldBeKept.includes(args.event.type);
 
-      if (botUserId !== undefined && args.event.user === botUserId && !isEventShouldBeKept) {
+      if (botUserId !== undefined && 'user' in args.event && args.event.user === botUserId && !isEventShouldBeKept) {
         return;
       }
     }
@@ -334,7 +334,7 @@ export function directMention(): Middleware<SlackEventMiddlewareArgs<'message'>>
       );
     }
 
-    if (message.text === undefined) {
+    if (!('text' in message) || message.text === undefined) {
       return;
     }