diff --git a/.github/actions/require-multiple-reviewers-v1/dist/index.js b/.github/actions/require-multiple-reviewers-v1/dist/index.js index 2bbe5c9..6b38a63 100644 --- a/.github/actions/require-multiple-reviewers-v1/dist/index.js +++ b/.github/actions/require-multiple-reviewers-v1/dist/index.js @@ -29910,8 +29910,8 @@ async function run(core, github) { repo, pull_number: github.context.payload.pull_request.number }); - const approvals = reviews.filter(review => review.state === 'APPROVED'); - conclusion = approvals.length < numberOfReviewers ? 'failure' : 'success'; + const approvals = (0, utils_1.getApproversCount)(reviews); + conclusion = approvals < numberOfReviewers ? 'failure' : 'success'; } await octokit.request('POST /repos/{owner}/{repo}/check-runs', { owner, @@ -29947,6 +29947,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) { Object.defineProperty(exports, "__esModule", ({ value: true })); exports.getAnnotations = getAnnotations; exports.getImportantFilesChanged = getImportantFilesChanged; +exports.getApproversCount = getApproversCount; const fs_1 = __importDefault(__nccwpck_require__(7147)); const ignore_1 = __importDefault(__nccwpck_require__(5989)); function getAnnotations(importantFilesChanged, reviewersNumber) { @@ -29959,10 +29960,30 @@ function getAnnotations(importantFilesChanged, reviewersNumber) { })); } function getImportantFilesChanged(IMPORTANT_FILES_PATH, changedFiles) { - const i = (0, ignore_1.default)() - .add(fs_1.default.readFileSync(IMPORTANT_FILES_PATH, 'utf-8').toString()); + const i = (0, ignore_1.default)().add(fs_1.default.readFileSync(IMPORTANT_FILES_PATH, 'utf-8').toString()); return changedFiles.filter(file => i.ignores(file)); } +function getApproversCount(reviews) { + 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()); + const approvals = Array.from(latestReviews.values()).filter(review => review.state === 'APPROVED'); + return approvals.length; +} /***/ }), diff --git a/.github/actions/require-multiple-reviewers-v1/dist/types.d.ts b/.github/actions/require-multiple-reviewers-v1/dist/types.d.ts index 07a72e8..429e5b7 100644 --- a/.github/actions/require-multiple-reviewers-v1/dist/types.d.ts +++ b/.github/actions/require-multiple-reviewers-v1/dist/types.d.ts @@ -6,4 +6,5 @@ export type GitHub = Pick; type Output = NonNullable; export type Annotation = NonNullable[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]; export {}; diff --git a/.github/actions/require-multiple-reviewers-v1/dist/utils.d.ts b/.github/actions/require-multiple-reviewers-v1/dist/utils.d.ts index 2f05488..e84065a 100644 --- a/.github/actions/require-multiple-reviewers-v1/dist/utils.d.ts +++ b/.github/actions/require-multiple-reviewers-v1/dist/utils.d.ts @@ -1,3 +1,4 @@ -import { Annotation } from './types'; +import { Annotation, Review } from './types'; export declare function getAnnotations(importantFilesChanged: Array, reviewersNumber: number): Array; export declare function getImportantFilesChanged(IMPORTANT_FILES_PATH: string, changedFiles: string[]): string[]; +export declare function getApproversCount(reviews: Array): number; diff --git a/.github/actions/require-multiple-reviewers-v1/src/run.test.ts b/.github/actions/require-multiple-reviewers-v1/src/run.test.ts index 3602dab..a82d278 100644 --- a/.github/actions/require-multiple-reviewers-v1/src/run.test.ts +++ b/.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 () => { diff --git a/.github/actions/require-multiple-reviewers-v1/src/run.ts b/.github/actions/require-multiple-reviewers-v1/src/run.ts index d2be3d6..29332e9 100644 --- a/.github/actions/require-multiple-reviewers-v1/src/run.ts +++ b/.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 { @@ -48,8 +52,8 @@ export default async function run(core: Core, github: GitHub): Promise { 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', { diff --git a/.github/actions/require-multiple-reviewers-v1/src/types.ts b/.github/actions/require-multiple-reviewers-v1/src/types.ts index dddddb2..1432dd8 100644 --- a/.github/actions/require-multiple-reviewers-v1/src/types.ts +++ b/.github/actions/require-multiple-reviewers-v1/src/types.ts @@ -14,3 +14,5 @@ type Output = NonNullable< export type Annotation = NonNullable[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] diff --git a/.github/actions/require-multiple-reviewers-v1/src/utils.test.ts b/.github/actions/require-multiple-reviewers-v1/src/utils.test.ts index eab8f4f..963b158 100644 --- a/.github/actions/require-multiple-reviewers-v1/src/utils.test.ts +++ b/.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) + + 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) + + 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) + + expect(result).to.equal(0) + }) + + it('should handle an empty array of reviews', () => { + const reviews: Array = [] + + 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) + + expect(result).to.equal(1) + }) + }) }) diff --git a/.github/actions/require-multiple-reviewers-v1/src/utils.ts b/.github/actions/require-multiple-reviewers-v1/src/utils.ts index 466f03c..1325a50 100644 --- a/.github/actions/require-multiple-reviewers-v1/src/utils.ts +++ b/.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): 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()) + // Filter only approvals + const approvals = Array.from(latestReviews.values()).filter( + review => review.state === 'APPROVED' + ) + return approvals.length +}