Repository: Yoctol/bottender
Pull request #802 (2020-06-15 11:15:54)

Fix slack session when channel id is null
# issue
When user clicked a button on home tab, session key can't generate correctly.

# spec
If the slack event is not related to any channel, then the session key set to the user id.

codecov-commenter: # [Codecov](https://codecov.io/gh/Yoctol/bottender/pull/802?src=pr&el=h1) Report
> Merging [#802](https://codecov.io/gh/Yoctol/bottender/pull/802?src=pr&el=desc) into [release/1.5](https://codecov.io/gh/Yoctol/bottender/commit/61d623444b642b9c652bd4708bf481d67024d7a8&el=desc) will **increase** coverage by `2.56%`.
> The diff coverage is `86.92%`.

[![Impacted file tree graph](https://codecov.io/gh/Yoctol/bottender/pull/802/graphs/tree.svg?width=650&height=150&src=pr&token=JX3EGtxYH3)](https://codecov.io/gh/Yoctol/bottender/pull/802?src=pr&el=tree)

```diff
@@               Coverage Diff               @@
##           release/1.5     #802      +/-   ##
===============================================
+ Coverage        81.08%   83.65%   +2.56%     
===============================================
  Files              108      115       +7     
  Lines             4747     6068    +1321     
  Branches          1197     1491     +294     
===============================================
+ Hits              3849     5076    +1227     
- Misses             862      988     +126     
+ Partials            36        4      -32     
```


| [Impacted Files](https://codecov.io/gh/Yoctol/bottender/pull/802?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...s/bottender/src/cli/providers/messenger/profile.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy9jbGkvcHJvdmlkZXJzL21lc3Nlbmdlci9wcm9maWxlLnRz) | `83.05% <ø> (-3.56%)` | :arrow_down: |
| [packages/bottender/src/console/ConsoleConnector.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy9jb25zb2xlL0NvbnNvbGVDb25uZWN0b3IudHM=) | `100.00% <ø> (ø)` | |
| [packages/bottender/src/console/ConsoleContext.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy9jb25zb2xlL0NvbnNvbGVDb250ZXh0LnRz) | `100.00% <ø> (ø)` | |
| [packages/bottender/src/line/LineEvent.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy9saW5lL0xpbmVFdmVudC50cw==) | `96.64% <ø> (-1.11%)` | :arrow_down: |
| [...ckages/bottender/src/messenger/MessengerContext.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy9tZXNzZW5nZXIvTWVzc2VuZ2VyQ29udGV4dC50cw==) | `83.59% <ø> (-1.48%)` | :arrow_down: |
| [packages/bottender/src/slack/SlackContext.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy9zbGFjay9TbGFja0NvbnRleHQudHM=) | `92.20% <ø> (+6.00%)` | :arrow_up: |
| [...ckages/bottender/src/telegram/TelegramConnector.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy90ZWxlZ3JhbS9UZWxlZ3JhbUNvbm5lY3Rvci50cw==) | `96.93% <ø> (+0.38%)` | :arrow_up: |
| [...kages/bottender/src/test-utils/SimulatedContext.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy90ZXN0LXV0aWxzL1NpbXVsYXRlZENvbnRleHQudHM=) | `87.50% <ø> (-12.50%)` | :arrow_down: |
| [packages/bottender/src/types.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy90eXBlcy50cw==) | `100.00% <ø> (ø)` | |
| [packages/bottender/src/viber/ViberConnector.ts](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree#diff-cGFja2FnZXMvYm90dGVuZGVyL3NyYy92aWJlci9WaWJlckNvbm5lY3Rvci50cw==) | `91.56% <ø> (+5.60%)` | :arrow_up: |
| ... and [117 more](https://codecov.io/gh/Yoctol/bottender/pull/802/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/Yoctol/bottender/pull/802?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/Yoctol/bottender/pull/802?src=pr&el=footer). Last update [9b251e4...658916c](https://codecov.io/gh/Yoctol/bottender/pull/802?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packages/bottender/src/slack/SlackConnector.ts b/packages/bottender/src/slack/SlackConnector.ts
index 08d18e7c..234c8cfa 100644
--- a/packages/bottender/src/slack/SlackConnector.ts
+++ b/packages/bottender/src/slack/SlackConnector.ts
@@ -151,10 +151,8 @@ export default class SlackConnector
     return this._client;
   }
 
-  getUniqueSessionKey(body: SlackRequestBody): string {
-    // FIXME: define types for every slack events
-    const rawEvent = this._getRawEventFromRequest(body) as any;
-
+  // FIXME: define types for every slack events
+  _getChannelId(rawEvent: any): string | null {
     // For interactive_message format
     if (
       rawEvent.channel &&
@@ -188,43 +186,52 @@ export default class SlackConnector
     );
   }
 
-  async updateSession(session: Session, body: SlackRequestBody): Promise<void> {
-    if (this._isBotEventRequest(body)) {
-      return;
-    }
-
-    const rawEvent = this._getRawEventFromRequest(body);
-    let userFromBody;
+  _getUserId(rawEvent: SlackRawEvent): string | null {
     if (
       rawEvent.type === 'interactive_message' ||
       rawEvent.type === 'block_actions' ||
       rawEvent.type === 'view_submission' ||
       rawEvent.type === 'view_closed'
     ) {
-      userFromBody = (rawEvent as UIEvent).user.id;
-    } else {
-      userFromBody =
-        (rawEvent as Message).user || (rawEvent as CommandEvent).userId;
+      return (rawEvent as UIEvent).user.id;
+    }
+    return (
+      (rawEvent as Message).user ||
+      (rawEvent as CommandEvent).userId ||
+      (rawEvent as UIEvent).user?.id
+    );
+  }
+
+  getUniqueSessionKey(body: SlackRequestBody): string | null {
+    const rawEvent = this._getRawEventFromRequest(body);
+    return this._getChannelId(rawEvent) || this._getUserId(rawEvent);
+  }
+
+  async updateSession(session: Session, body: SlackRequestBody): Promise<void> {
+    if (this._isBotEventRequest(body)) {
+      return;
     }
 
+    const rawEvent = this._getRawEventFromRequest(body);
+    const userId = this._getUserId(rawEvent);
+    const channelId = this._getChannelId(rawEvent);
+
     if (
       typeof session.user === 'object' &&
       session.user &&
       session.user.id &&
-      session.user.id === userFromBody
+      session.user.id === userId
     ) {
       return;
     }
-    const channelId = this.getUniqueSessionKey(body);
-    const senderId = userFromBody;
 
-    if (!senderId) {
+    if (!userId) {
       return;
     }
 
     if (this._skipLegacyProfile) {
       session.user = {
-        id: senderId,
+        id: userId,
         _updatedAt: new Date().toISOString(),
       };
 
@@ -253,7 +260,7 @@ export default class SlackConnector
     }
 
     const promises: Record<string, any> = {
-      sender: this._client.getUserInfo(senderId),
+      sender: this._client.getUserInfo(userId),
     };
 
     // TODO: check join or leave events?
@@ -261,12 +268,14 @@ export default class SlackConnector
       !session.channel ||
       (session.channel.members &&
         Array.isArray(session.channel.members) &&
-        session.channel.members.indexOf(senderId) < 0)
+        session.channel.members.indexOf(userId) < 0)
     ) {
-      promises.channel = this._client.getConversationInfo(channelId);
-      promises.channelMembers = this._client.getAllConversationMembers(
-        channelId
-      );
+      if (channelId) {
+        promises.channel = this._client.getConversationInfo(channelId);
+        promises.channelMembers = this._client.getAllConversationMembers(
+          channelId
+        );
+      }
     }
 
     // TODO: how to know if user leave team?
@@ -275,7 +284,7 @@ export default class SlackConnector
       !session.team ||
       (session.team.members &&
         Array.isArray(session.team.members) &&
-        session.team.members.indexOf(senderId) < 0)
+        session.team.members.indexOf(userId) < 0)
     ) {
       promises.allUsers = this._client.getAllUserList();
     }
@@ -284,7 +293,7 @@ export default class SlackConnector
 
     // FIXME: refine user
     session.user = {
-      id: senderId,
+      id: userId,
       _updatedAt: new Date().toISOString(),
       ...results.sender,
     };


Test patch:
diff --git a/packages/bottender/src/slack/__tests__/SlackConnector.spec.ts b/packages/bottender/src/slack/__tests__/SlackConnector.spec.ts
index 5f379297..d9cca603 100644
--- a/packages/bottender/src/slack/__tests__/SlackConnector.spec.ts
+++ b/packages/bottender/src/slack/__tests__/SlackConnector.spec.ts
@@ -138,6 +138,127 @@ const viewSubmissionRequest = {
   },
 };
 
+const appHomeOpenedOnMessagesTabRequest = {
+  type: 'app_home_opened',
+  user: 'U0HD00000',
+  channel: 'DQMT00000',
+  tab: 'messages',
+  eventTs: '1592278860.498134',
+};
+
+const appHomeOpenedOnHomeTabRequest = {
+  type: 'app_home_opened',
+  user: 'U0HD00000',
+  channel: 'DQMT00000',
+  tab: 'home',
+  view: {
+    id: 'V0151K00000',
+    teamId: 'T0HD00000',
+    type: 'home',
+    blocks: [
+      {
+        type: 'actions',
+        blockId: 'Rm7lr',
+        elements: [
+          {
+            type: 'button',
+            actionId: 'zrZ',
+            text: {
+              type: 'plain_text',
+              text: 'value',
+              emoji: true,
+            },
+            value: 'value',
+          },
+        ],
+      },
+    ],
+    privateMetadata: '',
+    callbackId: '',
+    state: { values: {} },
+    hash: '1592278862.48a704aa',
+    title: { type: 'plain_text', text: 'View Title', emoji: true },
+    clearOnClose: false,
+    notifyOnClose: false,
+    close: null,
+    submit: null,
+    previousViewId: null,
+    rootViewId: 'V0151K00000',
+    appId: 'AQ8600000',
+    externalId: '',
+    appInstalledTeamId: 'T0HD00000',
+    botId: 'BQMT00000',
+  },
+  eventTs: '1592278910.074390',
+};
+
+const blockActionsOnHomeTabRequest = {
+  type: 'block_actions',
+  user: {
+    id: 'U0HD00000',
+    username: 'username',
+    name: 'name',
+    teamId: 'T0HD00000',
+  },
+  apiAppId: 'AQ8600000',
+  token: 'xxxxxxxxxxxxxxxxxxxxxxxx',
+  container: { type: 'view', viewId: 'V0151K00000' },
+  triggerId: '1192207164308.17443231012.b30e1f98a3ac48b9171d3b859a0124a5',
+  team: { id: 'T0HD00000', domain: 'domain' },
+  view: {
+    id: 'V0151K00000',
+    teamId: 'T0HD00000',
+    type: 'home',
+    blocks: [
+      {
+        type: 'actions',
+        blockId: 'Rm7lr',
+        elements: [
+          {
+            type: 'button',
+            actionId: 'zrZ',
+            text: {
+              type: 'plain_text',
+              text: 'value',
+              emoji: true,
+            },
+            value: 'value',
+          },
+        ],
+      },
+    ],
+    privateMetadata: '',
+    callbackId: '',
+    state: { values: {} },
+    hash: '1592278912.3f2f9db2',
+    title: { type: 'plain_text', text: 'View Title', emoji: true },
+    clearOnClose: false,
+    notifyOnClose: false,
+    close: null,
+    submit: null,
+    previousViewId: null,
+    rootViewId: 'V0151K00000',
+    appId: 'AQ8600000',
+    externalId: '',
+    appInstalledTeamId: 'T0HD00000',
+    botId: 'BQMT00000',
+  },
+  actions: [
+    {
+      actionId: 'zrZ',
+      blockId: 'Rm7lr',
+      text: {
+        type: 'plain_text',
+        text: 'value',
+        emoji: true,
+      },
+      value: 'value',
+      type: 'button',
+      actionTs: '1592279552.931549',
+    },
+  ],
+};
+
 const RtmMessage = {
   type: 'message',
   channel: 'G7W5WAAAA',
@@ -210,54 +331,75 @@ describe('#client', () => {
 });
 
 describe('#getUniqueSessionKey', () => {
-  it('extract correct channel id', () => {
+  it('extract correct session key', () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(request.body);
-    expect(channelId).toBe('C6A900000');
+    const sessionKey = connector.getUniqueSessionKey(request.body);
+    expect(sessionKey).toBe('C6A900000');
   });
 
-  it('extract correct channel id from interactive message request', () => {
+  it('extract correct session key from interactive message request', () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(
+    const sessionKey = connector.getUniqueSessionKey(
       interactiveMessageRequest.body
     );
-    expect(channelId).toBe('D7WTL9ECE');
+    expect(sessionKey).toBe('D7WTL9ECE');
+  });
+
+  it('extract correct session key from RTM WebSocket message', () => {
+    const { connector } = setup();
+    const sessionKey = connector.getUniqueSessionKey(RtmMessage);
+    expect(sessionKey).toBe('G7W5WAAAA');
   });
 
-  it('extract correct channel id from RTM WebSocket message', () => {
+  it('extract correct session key from reaction_added event', () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(RtmMessage);
-    expect(channelId).toBe('G7W5WAAAA');
+    const sessionKey = connector.getUniqueSessionKey(ReactionAddedRequest.body);
+    expect(sessionKey).toBe('C0G9QF9GZ');
   });
 
-  it('extract correct channel id from reaction_added event', () => {
+  it('extract correct session key from pin_added event', () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(ReactionAddedRequest.body);
-    expect(channelId).toBe('C0G9QF9GZ');
+    const sessionKey = connector.getUniqueSessionKey(PinAddedRequest.body);
+    expect(sessionKey).toBe('C02ELGNBH');
   });
 
-  it('extract correct channel id from pin_added event', () => {
+  it('extract correct session key from slash command', () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(PinAddedRequest.body);
-    expect(channelId).toBe('C02ELGNBH');
+    const sessionKey = connector.getUniqueSessionKey(slashCommandMessage);
+    expect(sessionKey).toBe('G7W5W0000');
   });
 
-  it('extract correct channel id from slash command', () => {
+  it("extract correct session key from view event's private_metadata", () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(slashCommandMessage);
-    expect(channelId).toBe('G7W5W0000');
+    const sessionKey = connector.getUniqueSessionKey(
+      viewSubmissionRequest.body
+    );
+    expect(sessionKey).toBe('C02ELGNBH');
   });
 
-  it('extract correct channel id from slash command', () => {
+  // home tab
+  it('extract correct session key from appHomeOpenedOnMessagesTabRequest', () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(slashCommandMessage);
-    expect(channelId).toBe('G7W5W0000');
+    const sessionKey = connector.getUniqueSessionKey(
+      appHomeOpenedOnMessagesTabRequest
+    );
+    expect(sessionKey).toBe('DQMT00000');
   });
 
-  it("extract correct channel id from view event's private_metadata", () => {
+  it('extract correct session key from appHomeOpenedOnHomeTabRequest', () => {
     const { connector } = setup();
-    const channelId = connector.getUniqueSessionKey(viewSubmissionRequest.body);
-    expect(channelId).toBe('C02ELGNBH');
+    const sessionKey = connector.getUniqueSessionKey(
+      appHomeOpenedOnHomeTabRequest
+    );
+    expect(sessionKey).toBe('DQMT00000');
+  });
+
+  it('extract correct session key from blockActionsOnHomeTabRequest', () => {
+    const { connector } = setup();
+    const sessionKey = connector.getUniqueSessionKey(
+      blockActionsOnHomeTabRequest
+    );
+    expect(sessionKey).toBe('U0HD00000');
   });
 });