From 4413ab2c1629680df33aca1a87f4ca8482617c42 Mon Sep 17 00:00:00 2001
From: Jake Awe <50372925+AyodeAwe@users.noreply.github.com>
Date: Tue, 12 Mar 2024 09:17:26 -0500
Subject: [PATCH] Fix bad committer user information (#179)

---
 src/plugins/AutoMerger/auto_merger.ts     | 36 ++++++++++++++---------
 test/auto_merger.test.ts                  |  6 ++++
 test/fixtures/responses/list_commits.json |  2 +-
 test/fixtures/responses/list_reviews.json |  6 ++++
 4 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/src/plugins/AutoMerger/auto_merger.ts b/src/plugins/AutoMerger/auto_merger.ts
index daf51387..c357d0ee 100644
--- a/src/plugins/AutoMerger/auto_merger.ts
+++ b/src/plugins/AutoMerger/auto_merger.ts
@@ -199,12 +199,18 @@ export class AutoMerger extends OpsBotPlugin {
       uniqueAuthors.push(commitAuthor);
     }
 
-    return Promise.all(
-      uniqueAuthors.map(
-        async (author) =>
-          (await octokit.users.getByUsername({ username: author })).data
+    return (
+      await Promise.all(
+        uniqueAuthors.map(async (author) => {
+          try {
+            return (await octokit.users.getByUsername({ username: author }))
+              .data;
+          } catch (error) {
+            return null;
+          }
+        })
       )
-    );
+    ).filter((x): x is UsersGetByUsernameResponseData => Boolean(x));
   }
 
   /**
@@ -232,15 +238,17 @@ export class AutoMerger extends OpsBotPlugin {
       uniqueApprovers.push(approvalAuthor);
     }
 
-    return Promise.all(
-      uniqueApprovers.map(
-        async (approver) =>
-          (
-            await octokit.users.getByUsername({
-              username: approver,
-            })
-          ).data
+    return (
+      await Promise.all(
+        uniqueApprovers.map(async (approver) => {
+          try {
+            return (await octokit.users.getByUsername({ username: approver }))
+              .data;
+          } catch (error) {
+            return null;
+          }
+        })
       )
-    );
+    ).filter((x): x is UsersGetByUsernameResponseData => Boolean(x));
   }
 }
diff --git a/test/auto_merger.test.ts b/test/auto_merger.test.ts
index 5be895d4..e52a60ff 100644
--- a/test/auto_merger.test.ts
+++ b/test/auto_merger.test.ts
@@ -64,13 +64,16 @@ describe("Auto Merger", () => {
     mockGetUserPermissionLevel.mockResolvedValueOnce(user_permission);
     mockPaginate.mockResolvedValueOnce(list_commits); // listCommits in getAuthors
     mockGetByUsername.mockResolvedValueOnce(userNoName);
+    mockGetByUsername.mockRejectedValueOnce(null);
     mockPaginate.mockResolvedValueOnce(list_reviews); // listReviews in getApprovers
     mockGetByUsername.mockResolvedValueOnce(user);
+    mockGetByUsername.mockRejectedValueOnce(null);
 
     await new AutoMerger(statusContext.successStatus).maybeMergePR();
 
     expect(mockPullsGet).toBeCalledTimes(1);
     expect(mockPullsGet.mock.calls[0][0].pull_number).toBe(1234);
+    expect(mockGetByUsername).toBeCalledTimes(4);
 
     expect(mockMerge.mock.calls[0][0]).toMatchObject({
       owner: "rapidsai",
@@ -115,13 +118,16 @@ URL: https://github.com/rapidsai/cudf/pull/6775`,
     mockGetUserPermissionLevel.mockResolvedValueOnce(user_permission);
     mockPaginate.mockResolvedValueOnce(list_commits); // listCommits in getAuthors
     mockGetByUsername.mockResolvedValueOnce(userNoName);
+    mockGetByUsername.mockRejectedValueOnce(null);
     mockPaginate.mockResolvedValueOnce(list_reviews); // listReviews in getApprovers
     mockGetByUsername.mockResolvedValueOnce(user);
+    mockGetByUsername.mockRejectedValueOnce(null);
 
     await new AutoMerger(statusContext.successStatus).maybeMergePR();
 
     expect(mockPullsGet).toBeCalledTimes(1);
     expect(mockPullsGet.mock.calls[0][0].pull_number).toBe(1234);
+    expect(mockGetByUsername).toBeCalledTimes(4);
 
     expect(mockMerge.mock.calls[0][0]).toMatchObject({
       owner: "rapidsai",
diff --git a/test/fixtures/responses/list_commits.json b/test/fixtures/responses/list_commits.json
index acf36ebd..e1ec440c 100644
--- a/test/fixtures/responses/list_commits.json
+++ b/test/fixtures/responses/list_commits.json
@@ -27,7 +27,7 @@
     {
       "sha": "0ddcf220b748a343f78941ecb30ddcfb92f76441",
       "author": {
-        "login": "VibhuJawa"
+        "login": "another_user"
       }
     },
     {
diff --git a/test/fixtures/responses/list_reviews.json b/test/fixtures/responses/list_reviews.json
index 5f669c97..7008f97f 100644
--- a/test/fixtures/responses/list_reviews.json
+++ b/test/fixtures/responses/list_reviews.json
@@ -16,5 +16,11 @@
       "login": "VibhuJawa"
     },
     "state": "APPROVED"
+  },
+  {
+    "user": {
+      "login": "another_user"
+    },
+    "state": "APPROVED"
   }
 ]