nodejs · aduh95 · Mar 18, 2023 · Mar 8, 2023
diff --git a/lib/pr_checker.js b/lib/pr_checker.js
@@ -30,6 +30,15 @@ const FAST_TRACK_RE = /^Fast-track has been requested by @(.+?)\. Please 👍 to
 const FAST_TRACK_MIN_APPROVALS = 2;
 const GIT_CONFIG_GUIDE_URL = 'https://github.com/nodejs/node/blob/99b1ada/doc/guides/contributing/pull-requests.md#step-1-fork';
 
+// eslint-disable-next-line no-extend-native
+Array.prototype.findLastIndex ??= function findLastIndex(fn) {
+  const reversedIndex = Reflect.apply(
+    Array.prototype.findIndex,
+    this.slice().reverse(),
+    arguments);
+  return reversedIndex === -1 ? -1 : this.length - reversedIndex - 1;
+};
+
 export default class PRChecker {
   /**
    * @param {{}} cli
@@ -520,11 +529,14 @@ export default class PRChecker {
     } = this;
     const { maxCommits } = argv;
 
-    if (reviews.length < 1) {
+    const reviewIndex = reviews.findLastIndex(
+      review => review.authorCanPushToRepository && review.state === 'APPROVED'
+    );
+
+    if (reviewIndex === -1) {
       return false;
     }
 
-    const reviewIndex = reviews.length - 1;
     const reviewDate = reviews[reviewIndex].publishedAt;
 
     const afterCommits = [];
@@ -537,7 +549,7 @@ export default class PRChecker {
 
     const totalCommits = afterCommits.length;
     if (totalCommits > 0) {
-      cli.warn('Commits were pushed since the last review:');
+      cli.warn('Commits were pushed since the last approving review:');
       const sliceLength = maxCommits === 0 ? totalCommits : -maxCommits;
       afterCommits.slice(sliceLength)
         .forEach(commit => {

diff --git a/lib/queries/Reviews.gql b/lib/queries/Reviews.gql
@@ -13,6 +13,7 @@ query Reviews($prid: Int!, $owner: String!, $repo: String!, $after: String) {
           author {
             login
           }
+          authorCanPushToRepository
           url
           publishedAt
         }

diff --git a/test/fixtures/more_than_three_commits_after_review_reviews.json b/test/fixtures/more_than_three_commits_after_review_reviews.json
@@ -4,6 +4,7 @@
   "author": {
     "login": "foo"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-89923489",
   "publishedAt": "2017-07-23T11:19:25Z"
 }]
diff --git a/test/fixtures/multiple_commits_after_review_reviews.json b/test/fixtures/multiple_commits_after_review_reviews.json
@@ -4,6 +4,7 @@
   "author": {
     "login": "foo"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-89923489",
   "publishedAt": "2017-09-23T11:19:25Z"
 }]
diff --git a/test/fixtures/reviews_approved.json b/test/fixtures/reviews_approved.json
@@ -4,6 +4,7 @@
   "author": {
     "login": "foo"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480624",
   "publishedAt": "2017-10-24T11:19:00Z"
 },
@@ -13,6 +14,7 @@
   "author": {
     "login": "Baz"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71488392",
   "publishedAt": "2017-10-24T11:50:52Z"
 },
@@ -22,6 +24,7 @@
   "author": {
     "login": "Baz"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-714882992",
   "publishedAt": "2017-10-24T12:30:52Z"
 },
@@ -31,6 +34,7 @@
   "author": {
     "login": "Quux"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71817236",
   "publishedAt": "2017-10-24T14:49:01Z"
 },
@@ -40,6 +44,7 @@
   "author": {
     "login": "Baz"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71488236",
   "publishedAt": "2017-10-24T14:49:02Z"
 },
@@ -49,6 +54,7 @@
   "author": {
     "login": "Quo"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71817236",
   "publishedAt": "2017-10-24T19:09:52Z"
 },
@@ -58,6 +64,7 @@
   "author": {
     "login": "bot"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71839232",
   "publishedAt": "2017-10-28T19:21:52Z"
 }]
diff --git a/test/fixtures/reviews_requesting_changes.json b/test/fixtures/reviews_requesting_changes.json
@@ -5,6 +5,7 @@
   "author": {
     "login": "foo"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480624",
   "publishedAt": "2017-10-24T11:19:25Z"
 },
@@ -14,6 +15,7 @@
   "author": {
     "login": "Baz"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71488392",
   "publishedAt": "2017-10-24T11:50:52Z"
 },
@@ -23,6 +25,7 @@
   "author": {
     "login": "Baz"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-714882992",
   "publishedAt": "2017-10-24T12:30:52Z"
 },
@@ -32,6 +35,7 @@
   "author": {
     "login": "Quo"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71817236",
   "publishedAt": "2017-10-24T19:09:52Z"
 },
@@ -41,6 +45,7 @@
   "author": {
     "login": "bot"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71839232",
   "publishedAt": "2017-10-24T19:21:52Z"
 },
@@ -50,6 +55,7 @@
   "author": {
     "login": "bar"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71482624",
   "publishedAt": "2017-10-24T11:27:02Z"
 }]
diff --git a/test/fixtures/single_commit_after_review_reviews.json b/test/fixtures/single_commit_after_review_reviews.json
@@ -4,6 +4,25 @@
   "author": {
     "login": "foo"
   },
+  "authorCanPushToRepository": true,
   "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480624",
   "publishedAt": "2017-10-24T11:19:25Z"
+},{
+  "bodyText": "Not sure about that last change",
+  "state": "COMMENTED",
+  "author": {
+    "login": "foo"
+  },
+  "authorCanPushToRepository": true,
+  "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480625",
+  "publishedAt": "2017-10-26T11:19:25Z"
+},{
+  "bodyText": "Good idea",
+  "state": "APPROVED",
+  "author": {
+    "login": "bar"
+  },
+  "authorCanPushToRepository": false,
+  "url": "https://github.com/nodejs/node/pull/16438#pullrequestreview-71480626",
+  "publishedAt": "2017-10-26T11:19:25Z"
 }]
diff --git a/test/unit/pr_checker.test.js b/test/unit/pr_checker.test.js
@@ -2059,7 +2059,7 @@ describe('PRChecker', () => {
 
       const expectedLogs = {
         warn: [
-          ['Commits were pushed since the last review:'],
+          ['Commits were pushed since the last approving review:'],
           ['- src: fix issue with es-modules']
         ],
         info: [],
@@ -2091,7 +2091,7 @@ describe('PRChecker', () => {
 
       const expectedLogs = {
         warn: [
-          ['Commits were pushed since the last review:'],
+          ['Commits were pushed since the last approving review:'],
           ['- src: add requested feature'],
           ['- nit: edit mistakes']
         ],
@@ -2122,7 +2122,7 @@ describe('PRChecker', () => {
       const { commits, reviews } = moreThanThreeCommitsAfterReview;
       const expectedLogs = {
         warn: [
-          ['Commits were pushed since the last review:'],
+          ['Commits were pushed since the last approving review:'],
           ['- src: add requested feature'],
           ['- nit: edit mistakes'],
           ['- final: we should be good to go'],
@@ -2180,20 +2180,18 @@ describe('PRChecker', () => {
         commits: simpleCommits,
         collaborators,
         authorIsNew: () => true,
-        getThread() {
-          return PRData.prototype.getThread.call(this);
-        }
+        getThread: PRData.prototype.getThread
       }, {}, argv);
 
       const status = checker.checkCommitsAfterReview();
-      assert.deepStrictEqual(status, true);
+      assert.strictEqual(status, true);
     });
 
     it('should log as expected if passed 1 as flag', () => {
       const { commits, reviews } = moreThanThreeCommitsAfterReview;
       const expectedLogs = {
         warn: [
-          ['Commits were pushed since the last review:'],
+          ['Commits were pushed since the last approving review:'],
           ['- final: we should be good to go'],
           ['...(use `--max-commits 4` to see the full list of commits)']
         ],
@@ -2224,7 +2222,7 @@ describe('PRChecker', () => {
       const { commits, reviews } = moreThanThreeCommitsAfterReview;
       const expectedLogs = {
         warn: [
-          ['Commits were pushed since the last review:'],
+          ['Commits were pushed since the last approving review:'],
           ['...(use `--max-commits 4` to see the full list of commits)']
         ],
         info: [],