From dfe47e94cc06a5cd3efea7c74fced5f36c62af05 Mon Sep 17 00:00:00 2001 From: Olabode Lawal-Shittabey Date: Thu, 15 Aug 2024 19:34:39 +0100 Subject: [PATCH] fix(edge-case): fetching `associatedPRs` on 100+ `context.commits` in `success` lifecycle (#892) * refactor: stop export of `buildAssociatedPRsQuery` * feat: add `pageInfo` to `getAssociatedPRs` query * feat: add `loadSingleCommitAssociatedPRs` graphql query * feat: implement logic to fetch more `associatedPRs` * test: fix unit tests in `success` * test: fix `intergration` * feat: implement logic to handle associatedPRs fetch on 100+ commits * test: add test case `dd comment and labels to PRs associated with release commits and issues (multipaged associatedPRs)` --- lib/success.js | 77 ++++++++- test/integration.test.js | 15 ++ test/success.test.js | 356 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 440 insertions(+), 8 deletions(-) diff --git a/lib/success.js b/lib/success.js index 8c2d4583..59ab8d0f 100644 --- a/lib/success.js +++ b/lib/success.js @@ -67,13 +67,44 @@ export default async function success(pluginConfig, context, { Octokit }) { const releaseInfos = releases.filter((release) => Boolean(release.name)); const shas = commits.map(({ hash }) => hash); - const { repository } = await octokit.graphql( - buildAssociatedPRsQuery(shas), - { owner, repo }, - ); - const associatedPRs = Object.values(repository).map( - (item) => item.associatedPullRequests.nodes, - ); + const associatedPRs = []; + + // Split commit shas into chunks of 100 shas + const chunkSize = 100; + const shasChunks = []; + for (let i = 0; i < shas.length; i += chunkSize) { + const chunk = shas.slice(i, i + chunkSize); + shasChunks.push(chunk); + } + for (const chunk of shasChunks) { + const { repository } = await octokit.graphql( + buildAssociatedPRsQuery(chunk), + { owner, repo }, + ); + const responseAssociatedPRs = Object.values(repository).map( + (item) => item.associatedPullRequests, + ); + for (const { nodes, pageInfo } of responseAssociatedPRs) { + associatedPRs.push(nodes); + if (pageInfo.hasNextPage) { + let cursor = pageInfo.endCursor; + let hasNextPage = true; + while (hasNextPage) { + const { repository } = await octokit.graphql( + loadSingleCommitAssociatedPRs, + { owner, repo, sha: response.commit.oid, cursor }, + ); + const { associatedPullRequests } = repository.commit; + associatedPRs.push(associatedPullRequests.nodes); + if (associatedPullRequests.pageInfo.hasNextPage) { + cursor = associatedPullRequests.pageInfo.endCursor; + } else { + hasNextPage = false; + } + } + } + } + } const uniqueAssociatedPRs = uniqBy(flatten(associatedPRs), "number"); @@ -252,7 +283,7 @@ export default async function success(pluginConfig, context, { Octokit }) { * @param {Array} shas * @returns {string} */ -export function buildAssociatedPRsQuery(shas) { +function buildAssociatedPRsQuery(shas) { return `#graphql query getAssociatedPRs($owner: String!, $repo: String!) { repository(owner: $owner, name: $repo) { @@ -260,7 +291,12 @@ export function buildAssociatedPRsQuery(shas) { .map((sha) => { return `commit${sha.slice(0, 6)}: object(oid: "${sha}") { ...on Commit { + oid associatedPullRequests(first: 100) { + pageInfo { + endCursor + hasNextPage + } nodes { url number @@ -275,3 +311,28 @@ export function buildAssociatedPRsQuery(shas) { } `; } + +/** + * GraphQL Query to fetch additional associatedPR for commits that has more than 100 associatedPRs + */ +const loadSingleCommitAssociatedPRs = `#graphql + query getCommitAssociatedPRs($owner: String!, $repo: String!, $sha: String!, $cursor: String) { + repository(owner: $owner, name: $repo) { + commit: object(oid: $sha) { + ...on Commit { + associatedPullRequests(after: $cursor, first: 100) { + pageInfo { + endCursor + hasNextPage + } + nodes { + url + number + body + } + } + } + } + } + } +`; diff --git a/test/integration.test.js b/test/integration.test.js index 030a7a54..68afd4a9 100644 --- a/test/integration.test.js +++ b/test/integration.test.js @@ -454,7 +454,12 @@ test("Comment and add labels on PR included in the releases", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -674,7 +679,12 @@ test("Verify, release and notify success", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -828,7 +838,12 @@ test("Verify, update release and notify success", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, diff --git a/test/success.test.js b/test/success.test.js index 6bdc20b5..97fcb787 100644 --- a/test/success.test.js +++ b/test/success.test.js @@ -63,12 +63,22 @@ test("Add comment and labels to PRs associated with release commits and issues s data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, commit456: { + oid: "456", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[1]], }, }, @@ -193,6 +203,197 @@ test("Add comment and labels to PRs associated with release commits and issues s t.true(fetch.done()); }); +test("Add comment and labels to PRs associated with release commits and issues (multipaged associatedPRs)", async (t) => { + const owner = "test_user"; + const repo = "test_repo"; + const env = { GITHUB_TOKEN: "github_token" }; + const failTitle = "The automated release is failing 🚨"; + const pluginConfig = { failTitle }; + const prs = [ + { number: 1, pull_request: {}, state: "closed" }, + { number: 2, pull_request: {}, body: "Fixes #3", state: "closed" }, + { number: 5, pull_request: {}, state: "closed" }, + { number: 6, pull_request: {}, state: "closed" }, + ]; + const options = { + branch: "master", + repositoryUrl: `https://github.com/${owner}/${repo}.git`, + }; + const commits = [ + { + hash: "123", + message: "Commit 1 message\n\n Fix #1", + tree: { long: "aaa" }, + }, + { hash: "456", message: "Commit 2 message", tree: { long: "ccc" } }, + { + hash: "789", + message: `Commit 3 message Closes https://github.com/${owner}/${repo}/issues/4`, + tree: { long: "ccc" }, + }, + ]; + const nextRelease = { version: "1.0.0" }; + const releases = [ + { name: "GitHub release", url: "https://github.com/release" }, + ]; + + const fetch = fetchMock + .sandbox() + .getOnce(`https://api.github.local/repos/${owner}/${repo}`, { + full_name: `${owner}/${repo}`, + }) + .postOnce("https://api.github.local/graphql", { + data: { + repository: { + commit123: { + oid: "123", + associatedPullRequests: { + pageInfo: { + endCursor: "YE", + hasNextPage: true, + }, + nodes: [prs[0]], + }, + }, + commit456: { + oid: "456", + associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, + nodes: [prs[1]], + }, + }, + commit789: { + oid: "789", + associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, + nodes: [prs[2]], + }, + }, + }, + }, + }) + .postOnce( + "https://api.github.local/graphql", + { + data: { + repository: { + commit: { + associatedPullRequests: { + pageInfo: { + endCursor: "NE", + hasNextPage: false, + }, + nodes: [prs[3]], + }, + }, + }, + }, + }, + { + overwriteRoutes: true, + }, + ) + .getOnce( + `https://api.github.local/repos/${owner}/${repo}/pulls/6/commits`, + [{ sha: commits[0].hash }], + ) + .postOnce( + `https://api.github.local/repos/${owner}/${repo}/issues/1/comments`, + { + html_url: "https://github.com/successcomment-1", + }, + ) + .postOnce( + `https://api.github.local/repos/${owner}/${repo}/issues/1/labels`, + {}, + { body: ["released"] }, + ) + .postOnce( + `https://api.github.local/repos/${owner}/${repo}/issues/4/comments`, + { html_url: "https://github.com/successcomment-4" }, + ) + .postOnce( + `https://api.github.local/repos/${owner}/${repo}/issues/4/labels`, + {}, + { body: ["released"] }, + ) + .postOnce( + `https://api.github.local/repos/${owner}/${repo}/issues/6/comments`, + { html_url: "https://github.com/successcomment-6" }, + ) + .postOnce( + `https://api.github.local/repos/${owner}/${repo}/issues/6/labels`, + {}, + { body: ["released"] }, + ) + .getOnce( + `https://api.github.local/search/issues?q=${encodeURIComponent( + "in:title", + )}+${encodeURIComponent( + `repo:${owner}/${repo}`, + )}+${encodeURIComponent("type:issue")}+${encodeURIComponent( + "state:open", + )}+${encodeURIComponent(failTitle)}`, + { items: [] }, + ); + + await success( + pluginConfig, + { + env, + options, + commits, + nextRelease, + releases, + logger: t.context.logger, + }, + { + Octokit: TestOctokit.defaults((options) => ({ + ...options, + request: { ...options.request, fetch }, + })), + }, + ); + + t.true( + t.context.log.calledWith( + "Added comment to issue #%d: %s", + 1, + "https://github.com/successcomment-1", + ), + ); + t.true( + t.context.log.calledWith("Added labels %O to issue #%d", ["released"], 1), + ); + t.true( + t.context.log.calledWith( + "Added comment to issue #%d: %s", + 4, + "https://github.com/successcomment-4", + ), + ); + t.true( + t.context.log.calledWith("Added labels %O to issue #%d", ["released"], 4), + ); + t.true( + t.context.log.calledWith( + "Added comment to issue #%d: %s", + 6, + "https://github.com/successcomment-6", + ), + ); + t.true( + t.context.log.calledWith("Added labels %O to issue #%d", ["released"], 6), + ); + t.true(fetch.done()); +}); + test("Add comment and labels to PRs associated with release commits and issues closed by PR/commits comments with custom URL", async (t) => { const owner = "test_user"; const repo = "test_repo"; @@ -233,12 +434,22 @@ test("Add comment and labels to PRs associated with release commits and issues c data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, commit456: { + oid: "456", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[1]], }, }, @@ -425,32 +636,62 @@ test("Make multiple search queries if necessary", async (t) => { data: { repository: { commitaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: { + oid: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, commitbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb: { + oid: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[1]], }, }, commitcccccccccccccccccccccccccccccccccccccccccc: { + oid: "cccccccccccccccccccccccccccccccccccccccccc", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[2]], }, }, commiteeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee: { + oid: "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[3]], }, }, commitffffffffffffffffffffffffffffffffffffffffff: { + oid: "ffffffffffffffffffffffffffffffffffffffffff", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[4]], }, }, commitgggggggggggggggggggggggggggggggggggggggggg: { + oid: "gggggggggggggggggggggggggggggggggggggggggg", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[5]], }, }, @@ -670,12 +911,22 @@ test("Do not add comment and labels for unrelated PR returned by search (compare data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, commit456: { + oid: "456", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[1]], }, }, @@ -773,7 +1024,12 @@ test("Do not add comment and labels if no PR is associated with release commits" data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [], }, }, @@ -896,17 +1152,32 @@ test("Do not add comment and labels to PR/issues from other repo", async (t) => data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [], }, }, commit456: { + oid: "456", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [], }, }, commit789: { + oid: "789", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [], }, }, @@ -997,17 +1268,32 @@ test("Ignore missing and forbidden issues/PRs", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, commit456: { + oid: "456", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[1]], }, }, commit789: { + oid: "789", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[2]], }, }, @@ -1167,7 +1453,12 @@ test("Add custom comment and labels", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1263,7 +1554,12 @@ test("Add custom label", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1354,7 +1650,12 @@ test("Comment on issue/PR without ading a label", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1448,7 +1749,12 @@ test("Editing the release to include all release links at the bottom", async (t) data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1553,7 +1859,12 @@ test("Editing the release to include all release links at the top", async (t) => data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1655,7 +1966,12 @@ test("Editing the release to include all release links with no additional releas data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1746,7 +2062,12 @@ test("Editing the release to include all release links with no additional releas data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1830,7 +2151,12 @@ test("Editing the release to include all release links with no releases", async data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -1916,7 +2242,12 @@ test("Editing the release with no ID in the release", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, @@ -2007,12 +2338,22 @@ test("Ignore errors when adding comments and closing issues", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[0]], }, }, commit456: { + oid: "456", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [prs[1]], }, }, @@ -2141,7 +2482,12 @@ test("Close open issues when a release is successful", async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [], }, }, @@ -2294,7 +2640,12 @@ test('Skip closing issues if "failComment" is "false"', async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [], }, }, @@ -2346,7 +2697,12 @@ test('Skip closing issues if "failTitle" is "false"', async (t) => { data: { repository: { commit123: { + oid: "123", associatedPullRequests: { + pageInfo: { + endCursor: "NI", + hasNextPage: false, + }, nodes: [], }, },