Skip to content

Commit dfd59ba

Browse files
Retrieve all PR reviews when checking approvals (#190)
The results from octokit.pulls.listReviews are paginated; if there were more than 30 "reviews" (note that this includes standalone comments on the PR) before the requested number of approvals, the PR would be considered unapproved and wouldn't get merged. By using the `octokit.paginate` helper we make sure to retrieve all reviews.
1 parent 8b306ad commit dfd59ba

File tree

3 files changed

+14
-17
lines changed

3 files changed

+14
-17
lines changed

dist/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ async function fetchApprovalReviewCount(context, pullRequest) {
468468
}
469469

470470
logger.debug("Getting reviews for", number, "...");
471-
let { data: reviews } = await octokit.pulls.listReviews({
471+
const reviews = await octokit.paginate(octokit.pulls.listReviews, {
472472
owner: pullRequest.base.repo.owner.login,
473473
repo: pullRequest.base.repo.name,
474474
pull_number: number
@@ -22095,7 +22095,7 @@ module.exports = JSON.parse('[[[0,44],"disallowed_STD3_valid"],[[45,46],"valid"]
2209522095
/***/ ((module) => {
2209622096

2209722097
"use strict";
22098-
module.exports = JSON.parse('{"name":"automerge-action","version":"0.15.1","description":"GitHub action to automatically merge pull requests","main":"lib/api.js","author":"Pascal","license":"MIT","private":true,"bin":{"automerge-action":"./bin/automerge.js"},"scripts":{"test":"jest","it":"node it/it.js","lint":"prettier -l lib/** test/** && eslint .","compile":"ncc build bin/automerge.js --license LICENSE -o dist","prepublish":"yarn lint && yarn test && yarn compile"},"dependencies":{"@actions/core":"^1.6.0","@octokit/rest":"^18.12.0","argparse":"^2.0.1","fs-extra":"^10.0.1","object-resolve-path":"^1.1.1","tmp":"^0.2.1"},"devDependencies":{"@vercel/ncc":"^0.33.3","dotenv":"^16.0.0","eslint":"^8.11.0","eslint-plugin-jest":"^26.1.3","jest":"^27.5.1","prettier":"^2.6.0"},"prettier":{"trailingComma":"none","arrowParens":"avoid"}}');
22098+
module.exports = JSON.parse('{"name":"automerge-action","version":"0.15.2","description":"GitHub action to automatically merge pull requests","main":"lib/api.js","author":"Pascal","license":"MIT","private":true,"bin":{"automerge-action":"./bin/automerge.js"},"scripts":{"test":"jest","it":"node it/it.js","lint":"prettier -l lib/** test/** && eslint .","compile":"ncc build bin/automerge.js --license LICENSE -o dist","prepublish":"yarn lint && yarn test && yarn compile"},"dependencies":{"@actions/core":"^1.6.0","@octokit/rest":"^18.12.0","argparse":"^2.0.1","fs-extra":"^10.0.1","object-resolve-path":"^1.1.1","tmp":"^0.2.1"},"devDependencies":{"@vercel/ncc":"^0.33.3","dotenv":"^16.0.0","eslint":"^8.11.0","eslint-plugin-jest":"^26.1.3","jest":"^27.5.1","prettier":"^2.6.0"},"prettier":{"trailingComma":"none","arrowParens":"avoid"}}');
2209922099

2210022100
/***/ })
2210122101

lib/api.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ async function fetchApprovalReviewCount(context, pullRequest) {
461461
}
462462

463463
logger.debug("Getting reviews for", number, "...");
464-
let { data: reviews } = await octokit.pulls.listReviews({
464+
const reviews = await octokit.paginate(octokit.pulls.listReviews, {
465465
owner: pullRequest.base.repo.owner.login,
466466
repo: pullRequest.base.repo.name,
467467
pull_number: number

test/api.test.js

+11-14
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ test("only merge PRs with required approvals", async () => {
4848
pulls: {
4949
list: jest.fn(() => ({ data: [pr] })),
5050
merge: jest.fn(() => (merged = true)),
51-
listReviews: jest.fn(() => ({ data: [] }))
52-
}
51+
listReviews: Symbol("listReviews")
52+
},
53+
paginate: jest.fn(() => [])
5354
};
5455

5556
const event = {
@@ -63,24 +64,20 @@ test("only merge PRs with required approvals", async () => {
6364
expect(merged).toEqual(false); // if there's no approval, it should fail
6465

6566
merged = false;
66-
octokit.pulls.listReviews.mockReturnValueOnce({
67-
data: [
68-
{ state: "APPROVED", user: { login: "approval_user" } },
69-
{ state: "APPROVED", user: { login: "approval_user2" } }
70-
]
71-
});
67+
octokit.paginate.mockReturnValueOnce([
68+
{ state: "APPROVED", user: { login: "approval_user" } },
69+
{ state: "APPROVED", user: { login: "approval_user2" } }
70+
]);
7271

7372
// WHEN
7473
await api.executeGitHubAction({ config, octokit }, "check_suite", event);
7574
expect(merged).toEqual(true); // if there are two approvals, it should succeed
7675

7776
merged = false;
78-
octokit.pulls.listReviews.mockReturnValueOnce({
79-
data: [
80-
{ state: "APPROVED", user: { login: "approval_user" } },
81-
{ state: "APPROVED", user: { login: "approval_user" } }
82-
]
83-
});
77+
octokit.paginate.mockReturnValueOnce([
78+
{ state: "APPROVED", user: { login: "approval_user" } },
79+
{ state: "APPROVED", user: { login: "approval_user" } }
80+
]);
8481

8582
// WHEN a user has given
8683
await api.executeGitHubAction({ config, octokit }, "check_suite", event);

0 commit comments

Comments
 (0)