Merge pull request #99 from DavideViolante/test

Fix not sending ms teams notification when user is not in the mapping
DavideViolante · Feb 16, 2023 · 18f0fec · 18f0fec
2 parents 738a8b8 + b098cad
commit 18f0fec
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 22 deletions.
diff --git a/README.md b/README.md
@@ -46,7 +46,7 @@ jobs:
   pr-reviews-reminder:
     runs-on: ubuntu-latest
     steps:
-    - uses: davideviolante/pr-reviews-reminder-action@v2.4.0
+    - uses: davideviolante/pr-reviews-reminder-action@v2.6.0
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
       with:

diff --git a/dist/index.js b/dist/index.js
@@ -132,14 +132,17 @@ function getTeamsMentions(github2provider, pr2user) {
   // Add mentions array only if the map is provided, or no notification is sent
   if (Object.keys(github2provider).length > 0) {
     for (const user of pr2user) {
-      mentions.push({
-        type: `mention`,
-        text: `<at>${user.login}</at>`,
-        mentioned: {
-          id: github2provider[user.login],
-          name: user.login,
-        },
-      });
+      // mentioed property needs id and name, or no notification is sent
+      if (github2provider[user.login]) {
+        mentions.push({
+          type: `mention`,
+          text: `<at>${user.login}</at>`,
+          mentioned: {
+            id: github2provider[user.login],
+            name: user.login,
+          },
+        });
+      }
     }
   }
   return mentions;
@@ -10774,8 +10777,16 @@ async function main() {
           break;
         }
       }
-      await sendNotification(webhookUrl, messageObject);
+      const resNotification = await sendNotification(webhookUrl, messageObject);
+      // https://github.com/MicrosoftDocs/msteams-docs/issues/402
+      // If MS Teams fails, it might return still 200 OK, but data is not 1:
+      if (provider === 'msteams' && resNotification.data !== 1) {
+        core.info('Error: MS Teams notification failed.');
+        core.info(`Debugging: request body sent:\n${resNotification.config?.data}`);
+        return core.setFailed(resNotification.data);
+      }
       core.info(`Notification sent successfully!`);
+      core.info(`Debugging: request body sent:\n${resNotification.config?.data}`);
     }
   } catch (error) {
     core.setFailed(error.message);

diff --git a/functions.js b/functions.js
@@ -126,14 +126,17 @@ function getTeamsMentions(github2provider, pr2user) {
   // Add mentions array only if the map is provided, or no notification is sent
   if (Object.keys(github2provider).length > 0) {
     for (const user of pr2user) {
-      mentions.push({
-        type: `mention`,
-        text: `<at>${user.login}</at>`,
-        mentioned: {
-          id: github2provider[user.login],
-          name: user.login,
-        },
-      });
+      // mentioed property needs id and name, or no notification is sent
+      if (github2provider[user.login]) {
+        mentions.push({
+          type: `mention`,
+          text: `<at>${user.login}</at>`,
+          mentioned: {
+            id: github2provider[user.login],
+            name: user.login,
+          },
+        });
+      }
     }
   }
   return mentions;

diff --git a/index.js b/index.js
@@ -82,8 +82,16 @@ async function main() {
           break;
         }
       }
-      await sendNotification(webhookUrl, messageObject);
+      const resNotification = await sendNotification(webhookUrl, messageObject);
+      // https://github.com/MicrosoftDocs/msteams-docs/issues/402
+      // If MS Teams fails, it might return still 200 OK, but data is not 1:
+      if (provider === 'msteams' && resNotification.data !== 1) {
+        core.info('Error: MS Teams notification failed.');
+        core.info(`Debugging: request body sent:\n${resNotification.config?.data}`);
+        return core.setFailed(resNotification.data);
+      }
       core.info(`Notification sent successfully!`);
+      core.info(`Debugging: request body sent:\n${resNotification.config?.data}`);
     }
   } catch (error) {
     core.setFailed(error.message);

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "pr-reviews-reminder-action",
-  "version": "2.5.0",
+  "version": "2.6.0",
   "description": "Reminder for Pull Request pending reviews",
   "scripts": {
     "build": "ncc build index.js",

diff --git a/test/test.js b/test/test.js
@@ -111,6 +111,28 @@ const mockPr2User = [
     login: 'User2',
   },
 ];
+const mockPr2UserWrongId = [
+  {
+    url: 'https://example.com/1',
+    title: 'Title1',
+    login: 'User1',
+  },
+  {
+    url: 'https://example.com/1',
+    title: 'Title1',
+    login: 'User22',
+  },
+  {
+    url: 'https://example.com/3',
+    title: 'Title3',
+    login: 'User3',
+  },
+  {
+    url: 'https://example.com/5',
+    title: 'Title5',
+    login: 'User22',
+  },
+];
 const mockStringToConvert = 'name1:ID1,name2:ID2,name3:ID3';
 const mockStringToConvertOneUser = 'name1:ID1';
 const mockStringToConvertMultiline = `name1:ID1,
@@ -162,6 +184,24 @@ const mockTeamsMentions = [
     },
   },
 ];
+const mockTeamsMentionsWrongIds = [
+  {
+    type: `mention`,
+    text: `<at>User1</at>`,
+    mentioned: {
+      id: 'ID123',
+      name: 'User1',
+    },
+  },
+  {
+    type: `mention`,
+    text: `<at>User3</at>`,
+    mentioned: {
+      id: 'ID789',
+      name: 'User3',
+    },
+  },
+];
 const mockTeamsMessageRequest = {
   type: `message`,
   attachments: [
@@ -374,6 +414,15 @@ describe('Pull Request Reviews Reminder Action tests', () => {
     assert.strictEqual(dRow, 'Hey <at>User2</at>, the PR "Title5" is waiting for your review: [https://example.com/5](https://example.com/5)');
   });
 
+  it('Should print the pretty message, one reviewer per row, Teams (correct map, wrong ids)', () => {
+    const message = prettyMessage(mockPr2UserWrongId, mockGithub2provider, 'msteams');
+    const [aRow, bRow, cRow, dRow] = message.split('  \n');
+    assert.strictEqual(aRow, 'Hey <at>User1</at>, the PR "Title1" is waiting for your review: [https://example.com/1](https://example.com/1)');
+    assert.strictEqual(bRow, 'Hey @User22, the PR "Title1" is waiting for your review: [https://example.com/1](https://example.com/1)');
+    assert.strictEqual(cRow, 'Hey <at>User3</at>, the PR "Title3" is waiting for your review: [https://example.com/3](https://example.com/3)');
+    assert.strictEqual(dRow, 'Hey @User22, the PR "Title5" is waiting for your review: [https://example.com/5](https://example.com/5)');
+  });
+
   it('Should print the pretty message, one reviewer per row, Teams (malformed map)', () => {
     const message = prettyMessage(mockPr2User, mockGithub2providerMalformed, 'msteams');
     const [aRow, bRow, cRow, dRow] = message.split('  \n');
@@ -397,6 +446,11 @@ describe('Pull Request Reviews Reminder Action tests', () => {
     assert.deepEqual(mentions, mockTeamsMentions);
   });
 
+  it('Should create mentions array, wrong IDs, Teams', () => {
+    const mentions = getTeamsMentions(mockGithub2provider, mockPr2UserWrongId);
+    assert.deepEqual(mentions, mockTeamsMentionsWrongIds);
+  });
+
   it('Should create empty mentions array, Teams', () => {
     const mentions = getTeamsMentions({}, mockPr2User);
     assert.deepEqual(mentions, []);