fix(approvals-satisfied): remove @ from usernames (#627)

* fix(approvals-satisfied): remove @ from usernames * fix(manage-merge-queue): properly handle user list from CODEOWNERS * format * space separate * update test case --------- Co-authored-by: Dan Adajian <[email protected]>
ExpediaGroup · Jul 9, 2024 · cd995d2 · cd995d2
1 parent 4a3a2d4
commit cd995d2
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 7 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -7,3 +7,4 @@
 /file/path/1         @ExpediaGroup/test-owners-1
 /file/path/2         @ExpediaGroup/test-owners-2
 /file/path/shared    @ExpediaGroup/test-shared-owners-1 @ExpediaGroup/test-shared-owners-2
+/file/path/users     @user1 @user2
diff --git a/dist/431.index.js b/dist/431.index.js
diff --git a/dist/431.index.js.map b/dist/431.index.js.map
diff --git a/dist/676.index.js b/dist/676.index.js
diff --git a/dist/676.index.js.map b/dist/676.index.js.map
diff --git a/src/helpers/approvals-satisfied.ts b/src/helpers/approvals-satisfied.ts
@@ -71,7 +71,7 @@ export const approvalsSatisfied = async ({
       if (isTeam(teamOrUsers)) {
         return await fetchTeamLogins(teamOrUsers);
       } else {
-        return teamOrUsers.split(',');
+        return teamOrUsers.replaceAll('@', '').split(',');
       }
     });
     const codeOwnerLogins = uniq(loginsLists.flat());

diff --git a/test/helpers/approvals-satisfied.test.ts b/test/helpers/approvals-satisfied.test.ts
@@ -525,7 +525,7 @@ describe('approvalsSatisfied', () => {
       ]
     });
     const result = await approvalsSatisfied({
-      users: 'user1,user2',
+      users: '@user1,@user2',
       pull_number: '12345'
     });
     expect(octokit.pulls.listReviews).toHaveBeenCalledWith({ pull_number: 12345, repo: 'repo', owner: 'owner', page: 1, per_page: 100 });
@@ -543,11 +543,35 @@ describe('approvalsSatisfied', () => {
       ]
     });
     const result = await approvalsSatisfied({
-      users: 'user1,user2',
+      users: '@user1,@user2',
       pull_number: '12345'
     });
     expect(octokit.pulls.listReviews).toHaveBeenCalledWith({ pull_number: 12345, repo: 'repo', owner: 'owner', page: 1, per_page: 100 });
     expect(getRequiredCodeOwnersEntries).not.toHaveBeenCalled();
     expect(result).toBe(false);
   });
+
+  it('should return true when approvals are satisfied and users are explicitly defined in CODEOWNERS', async () => {
+    (getRequiredCodeOwnersEntries as jest.Mock).mockResolvedValue([{ owners: ['@user1', '@user2'] }]);
+    mockPagination({
+      data: [
+        {
+          state: 'APPROVED',
+          user: { login: 'user1' }
+        },
+        {
+          state: 'APPROVED',
+          user: { login: 'user2' }
+        },
+        {
+          state: 'APPROVED',
+          user: { login: 'user3' }
+        }
+      ]
+    });
+    const result = await approvalsSatisfied({
+      number_of_reviewers: '2'
+    });
+    expect(result).toBe(true);
+  });
 });
diff --git a/test/utils/get-core-member-logins.test.ts b/test/utils/get-core-member-logins.test.ts
@@ -109,6 +109,9 @@ describe('getCoreMemberLogins', () => {
                   },
                   {
                     filename: sharedFile
+                  },
+                  {
+                    filename: 'file/path/users/file.ts'
                   }
                 ]
               : []
@@ -126,6 +129,10 @@ describe('getCoreMemberLogins', () => {
           {
             pattern: '/file/path/shared',
             owners: ['@ExpediaGroup/test-shared-owners-1', '@ExpediaGroup/test-shared-owners-2']
+          },
+          {
+            pattern: '/file/path/users',
+            owners: ['@user1', '@user2']
           }
         ]);
       });