Do NOT call TeamsInfo.getMember for the bot (#2237)

* Do NOT call TeamsInfo.getMember for the bot * fix failing TeamsInfo tests
microsoft · May 14, 2020 · 9dbb82e · 9dbb82e
1 parent 0656bdf
commit 9dbb82e
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 6 deletions.
diff --git a/libraries/botbuilder/src/teamsActivityHandler.ts b/libraries/botbuilder/src/teamsActivityHandler.ts
@@ -393,13 +393,14 @@ export class TeamsActivityHandler extends ActivityHandler {
             for (let i=0; i<context.activity.membersAdded.length; i++) {
                 const channelAccount = context.activity.membersAdded[i];
 
-                // check whether we have a TeamChannelAccount
+                // check whether we have a TeamChannelAccount, or the member is the bot
                 if ('givenName' in channelAccount ||
                     'surname' in channelAccount ||
                     'email' in channelAccount ||
-                    'userPrincipalName' in channelAccount) {
+                    'userPrincipalName' in channelAccount ||
+                    context.activity.recipient.id === channelAccount.id) {
 
-                    // we must have a TeamsChannelAccount so skip to the next one
+                    // we must have a TeamsChannelAccount, or a bot so skip to the next one
                     continue;
                 }
 

diff --git a/libraries/botbuilder/tests/teamsActivityHandler.test.js b/libraries/botbuilder/tests/teamsActivityHandler.test.js
@@ -1,5 +1,5 @@
 const assert = require('assert');
-const { TeamsActivityHandler } = require('../');
+const { TeamsActivityHandler, TeamsInfo } = require('../');
 const { ActivityTypes, TestAdapter } = require('botbuilder-core');
 
 function createInvokeActivity(name, value = {}, channelData = {}) {
@@ -513,7 +513,8 @@ describe('TeamsActivityHandler', () => {
         function createConvUpdateActivity(channelData) {
             const activity = {
                 type: ActivityTypes.ConversationUpdate,
-                channelData
+                channelData,
+                channelId: 'msteams'
             };
             return activity;
         }
@@ -543,7 +544,8 @@ describe('TeamsActivityHandler', () => {
         it('onTeamsMembersAdded routed activity', () => {
             const bot = new TeamsActivityHandler();
             let onTeamsMemberAddedEvent = false;
-
+            let getMemberCalledCount = 0;
+
             const membersAddedMock = [{ id: 'test'} , { id: 'id' }];
             const team = { id: 'team' };
             const activity = createConvUpdateActivity({ team, eventType: 'teamMemberAdded' });
@@ -578,11 +580,75 @@ describe('TeamsActivityHandler', () => {
                 await bot.run(context);
             });
 
+            const wasGetMember = TeamsInfo.getMember;
+            TeamsInfo.getMember = function(context, userId) {
+                getMemberCalledCount ++;
+                return membersAddedMock.filter(obj=>obj.id === userId)[0];
+            };
+
+            adapter.send(activity)
+                .then(() => {
+                    assert(onTeamsMemberAddedEvent, 'onTeamsMemberAddedEvent handler not called');
+                    assert(onConversationUpdateCalled, 'handleTeamsFileConsentAccept handler not called');
+                    assert(onDialogCalled, 'onDialog handler not called');
+                    assert(getMemberCalledCount === 2, 'TeamsInfo.getMember not called twice');
+                    TeamsInfo.getMember = wasGetMember;
+                });
+        });
+
+        it('onTeamsMembersAdded for bot routed activity does NOT call TeamsInfo.getMember', () => {
+            const bot = new TeamsActivityHandler();
+            let onTeamsMemberAddedEvent = false;
+            let getMemberCalled = false;
+
+            const membersAddedMock = [{ id: 'botid' }];
+            const team = { id: 'team' };
+            const activity = createConvUpdateActivity({ team, eventType: 'teamMemberAdded' });
+            activity.membersAdded = membersAddedMock;
+            activity.recipient = { id: membersAddedMock[0].id };
+
+            bot.onConversationUpdate(async (context, next) => {
+                assert(context, 'context not found');
+                assert(next, 'next not found');
+                onConversationUpdateCalled = true;
+                await next();
+            });
+
+            bot.onTeamsMembersAddedEvent(async (membersAdded, teamInfo, context, next) => {
+                assert(membersAdded, 'membersAdded not found');
+                assert(teamInfo, 'teamsInfo not found');
+                assert(context, 'context not found');
+                assert(next, 'next not found');
+                assert.strictEqual(teamInfo, team);
+                assert.strictEqual(membersAdded, membersAddedMock);
+                onTeamsMemberAddedEvent = true;
+                await next();
+            });
+
+            bot.onDialog(async (context, next) => {
+                assert(context, 'context not found');
+                assert(next, 'next not found');
+                onDialogCalled = true;
+                await next();
+            });
+
+            const adapter = new TestAdapter(async context => {
+                await bot.run(context);
+            });
+
+            const wasGetMember = TeamsInfo.getMember;
+            TeamsInfo.getMember = function(context, userId) {
+                getMemberCalled = true;
+                return membersAddedMock.filter(obj=>obj.id === userId)[0];
+            };
+
             adapter.send(activity)
                 .then(() => {
                     assert(onTeamsMemberAddedEvent, 'onTeamsMemberAddedEvent handler not called');
                     assert(onConversationUpdateCalled, 'handleTeamsFileConsentAccept handler not called');
                     assert(onDialogCalled, 'onDialog handler not called');
+                    assert(getMemberCalled === false, 'TeamsInfo.getMember was called, but should not have been');
+                    TeamsInfo.getMember = wasGetMember;
                 });
         });