Fixing bug with multiple reviews by same user

.github/actions/require-multiple-reviewers-v1/src/run.test.ts

@@ -96,6 +96,8 @@ describe('run()', () => {
       .stub(utils, 'getImportantFilesChanged')
       .returns(['file1', 'file2'])
 
+    const getApproversCount = sinon.stub(utils, 'getApproversCount').returns(2)
+
     await run(core as unknown as Core, github as unknown as GitHub)
 
     assert.isTrue(core.setFailed.notCalled)
@@ -120,6 +122,7 @@ describe('run()', () => {
     )
 
     getImportantFilesChanged.restore()
+    getApproversCount.restore()
   })
 
   it('should fail if not enough reviewers', async () => {
@@ -180,6 +183,8 @@ describe('run()', () => {
       .stub(utils, 'getImportantFilesChanged')
       .returns(['file1', 'file2'])
 
+    const getApproversCount = sinon.stub(utils, 'getApproversCount').returns(0)
+
     await run(core as unknown as Core, github as unknown as GitHub)
 
     assert.isTrue(core.setFailed.notCalled)
@@ -211,6 +216,7 @@ describe('run()', () => {
     )
 
     getImportantFilesChanged.restore()
+    getApproversCount.restore()
   })
 
   it('should fail if number-of-reviewers is not a number', async () => {

.github/actions/require-multiple-reviewers-v1/src/run.ts

@@ -1,4 +1,8 @@
-import { getAnnotations, getImportantFilesChanged } from './utils'
+import {
+  getAnnotations,
+  getImportantFilesChanged,
+  getApproversCount
+} from './utils'
 import type { Conclusion, Core, GitHub } from './types'
 
 export default async function run(core: Core, github: GitHub): Promise<void> {
@@ -48,8 +52,8 @@ export default async function run(core: Core, github: GitHub): Promise<void> {
         pull_number: github.context.payload.pull_request!.number
       })
 
-      const approvals = reviews.filter(review => review.state === 'APPROVED')
-      conclusion = approvals.length < numberOfReviewers ? 'failure' : 'success'
+      const approvals = getApproversCount(reviews)
+      conclusion = approvals < numberOfReviewers ? 'failure' : 'success'
     }
 
     await octokit.request('POST /repos/{owner}/{repo}/check-runs', {

.github/actions/require-multiple-reviewers-v1/src/types.ts

@@ -14,3 +14,5 @@ type Output = NonNullable<
 export type Annotation = NonNullable<Output['annotations']>[number]
 export type Conclusion =
   Endpoints['POST /repos/{owner}/{repo}/check-runs']['parameters']['conclusion']
+export type Review =
+  Endpoints['GET /repos/{owner}/{repo}/pulls/{pull_number}/reviews']['response']['data'][0]

.github/actions/require-multiple-reviewers-v1/src/utils.test.ts

@@ -1,8 +1,12 @@
 import { expect } from 'chai'
 import fs from 'fs'
 import sinon from 'sinon'
-import { Annotation } from './types'
-import { getAnnotations, getImportantFilesChanged } from './utils'
+import { Annotation, Review } from './types'
+import {
+  getAnnotations,
+  getImportantFilesChanged,
+  getApproversCount
+} from './utils'
 
 describe('utils', () => {
   describe('getAnnotations', () => {
@@ -177,4 +181,101 @@ describe('utils', () => {
       expect(result).to.deep.equal([])
     })
   })
+
+  describe('getApproversCount', () => {
+    it('should return the correct number of approvers', () => {
+      const reviews = [
+        {
+          user: { login: 'user1' },
+          state: 'APPROVED',
+          submitted_at: '2023-01-01T00:00:00Z'
+        },
+        {
+          user: { login: 'user2' },
+          state: 'APPROVED',
+          submitted_at: '2023-01-02T00:00:00Z'
+        },
+        {
+          user: { login: 'user3' },
+          state: 'CHANGES_REQUESTED',
+          submitted_at: '2023-01-03T00:00:00Z'
+        }
+      ]
+
+      const result = getApproversCount(reviews as Array<Review>)
+
+      expect(result).to.equal(2)
+    })
+
+    it('should handle multiple reviews from the same user and count only the latest approval', () => {
+      const reviews = [
+        {
+          user: { login: 'user1' },
+          state: 'APPROVED',
+          submitted_at: '2023-01-01T00:00:00Z'
+        },
+        {
+          user: { login: 'user1' },
+          state: 'CHANGES_REQUESTED',
+          submitted_at: '2023-01-02T00:00:00Z'
+        },
+        {
+          user: { login: 'user2' },
+          state: 'APPROVED',
+          submitted_at: '2023-01-03T00:00:00Z'
+        },
+        {
+          user: { login: 'user2' },
+          state: 'APPROVED',
+          submitted_at: '2023-01-04T00:00:00Z'
+        }
+      ]
+
+      const result = getApproversCount(reviews as Array<Review>)
+
+      expect(result).to.equal(1)
+    })
+
+    it('should return 0 if there are no approvals', () => {
+      const reviews = [
+        {
+          user: { login: 'user1' },
+          state: 'CHANGES_REQUESTED',
+          submitted_at: '2023-01-01T00:00:00Z'
+        },
+        {
+          user: { login: 'user2' },
+          state: 'COMMENTED',
+          submitted_at: '2023-01-02T00:00:00Z'
+        }
+      ]
+
+      const result = getApproversCount(reviews as Array<Review>)
+
+      expect(result).to.equal(0)
+    })
+
+    it('should handle an empty array of reviews', () => {
+      const reviews: Array<Review> = []
+
+      const result = getApproversCount(reviews)
+
+      expect(result).to.equal(0)
+    })
+
+    it('should handle reviews with missing user information', () => {
+      const reviews = [
+        { user: null, state: 'APPROVED', submitted_at: '2023-01-01T00:00:00Z' },
+        {
+          user: { login: 'user2' },
+          state: 'APPROVED',
+          submitted_at: '2023-01-02T00:00:00Z'
+        }
+      ]
+
+      const result = getApproversCount(reviews as Array<Review>)
+
+      expect(result).to.equal(1)
+    })
+  })
 })

.github/actions/require-multiple-reviewers-v1/src/utils.ts

@@ -1,6 +1,6 @@
 import fs from 'fs'
 import ignore from 'ignore'
-import { Annotation } from './types'
+import { Annotation, Review } from './types'
 
 /**
  * Returns an array of annotations for the important files that require multiple reviewers.
@@ -38,3 +38,35 @@ export function getImportantFilesChanged(
   // Return the files that have changed and are important, since they will be "ignored" if this was a gitignore file.
   return changedFiles.filter(file => i.ignores(file))
 }
+
+/**
+ * Returns the number of approvals after filtering to the latest review of each reviewer.
+ * @param reviews
+ * @returns number
+ */
+export function getApproversCount(reviews: Array<Review>): number {
+  // Get the latest review from each reviewer
+  const latestReviews = reviews.reduce((acc, review) => {
+    if (!review.user) {
+      return acc
+    }
+    if (!acc.has(review.user.login)) {
+      acc.set(review.user.login, review)
+    } else {
+      const existingReview = acc.get(review.user.login)
+      if (
+        review.submitted_at &&
+        existingReview!.submitted_at &&
+        new Date(review.submitted_at) > new Date(existingReview!.submitted_at)
+      ) {
+        acc.set(review.user.login, review)
+      }
+    }
+    return acc
+  }, new Map<string, Review>())
+  // Filter only approvals
+  const approvals = Array.from(latestReviews.values()).filter(
+    review => review.state === 'APPROVED'
+  )
+  return approvals.length
+}