From 44888193675f29a83e04faf4002fa8c0b537b1e4 Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Wed, 15 Mar 2023 17:02:41 -0400 Subject: [PATCH] Only consider the latest review for a user (#216) --- dist/index.js | 9 ++++++++- src/approve.test.ts | 22 ++++++++++++++++++++++ src/approve.ts | 13 ++++++++++--- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/dist/index.js b/dist/index.js index 2d9fa1f..d0ba857 100644 --- a/dist/index.js +++ b/dist/index.js @@ -10093,7 +10093,14 @@ function approve(token, context, prNumber, reviewMessage) { core.info(`Current user is ${login}`); const prHead = pr.head.sha; core.info(`Commit SHA is ${prHead}`); - const alreadyReviewed = reviews.some(({ user, state }) => (user === null || user === void 0 ? void 0 : user.login) === login && state === "APPROVED"); + // Only the most recent review for a user counts towards the review state + const latestReviewForUser = [...reviews] + .reverse() + .find(({ user }) => (user === null || user === void 0 ? void 0 : user.login) === login); + const alreadyReviewed = (latestReviewForUser === null || latestReviewForUser === void 0 ? void 0 : latestReviewForUser.state) === "APPROVED"; + // If there's an approved review from a user, but there's an outstanding review request, + // we need to create a new review. Review requests mean that existing "APPROVED" reviews + // don't count towards the mergeability of the PR. const outstandingReviewRequest = (_b = pr.requested_reviewers) === null || _b === void 0 ? void 0 : _b.some((reviewer) => reviewer.login == login); if (alreadyReviewed && !outstandingReviewRequest) { core.info(`Current user already approved pull request #${prNumber}, nothing to do`); diff --git a/src/approve.test.ts b/src/approve.test.ts index 9673641..48daaed 100644 --- a/src/approve.test.ts +++ b/src/approve.test.ts @@ -145,6 +145,28 @@ test("when a review is dismissed", async () => { expect(createReview.isDone()).toBe(true); }); +test("when a review is dismissed, but an earlier review is approved", async () => { + apiMocks.getUser(); + apiMocks.getPull(); + apiMocks.getReviews(200, [ + { + user: { login: "hmarr" }, + commit_id: "6a9ec7556f0a7fa5b49527a1eea4878b8a22d2e0", + state: "APPROVED", + }, + { + user: { login: "hmarr" }, + commit_id: "24c5451bbf1fb09caa3ac8024df4788aff4d4974", + state: "DISMISSED", + }, + ]); + const createReview = apiMocks.createReview(); + + await approve("gh-tok", new Context(), 101); + + expect(createReview.isDone()).toBe(true); +}); + test("when a review is not approved", async () => { apiMocks.getUser(); apiMocks.getPull(); diff --git a/src/approve.ts b/src/approve.ts index 97b641c..c1cabd8 100644 --- a/src/approve.ts +++ b/src/approve.ts @@ -39,12 +39,19 @@ export async function approve( const prHead = pr.head.sha; core.info(`Commit SHA is ${prHead}`); - const alreadyReviewed = reviews.some( - ({ user, state }) => user?.login === login && state === "APPROVED" - ); + // Only the most recent review for a user counts towards the review state + const latestReviewForUser = [...reviews] + .reverse() + .find(({ user }) => user?.login === login); + const alreadyReviewed = latestReviewForUser?.state === "APPROVED"; + + // If there's an approved review from a user, but there's an outstanding review request, + // we need to create a new review. Review requests mean that existing "APPROVED" reviews + // don't count towards the mergeability of the PR. const outstandingReviewRequest = pr.requested_reviewers?.some( (reviewer) => reviewer.login == login ); + if (alreadyReviewed && !outstandingReviewRequest) { core.info( `Current user already approved pull request #${prNumber}, nothing to do`