diff --git a/spec/api-review-state.spec.ts b/spec/api-review-state.spec.ts index 3505a77..a5efc1b 100644 --- a/spec/api-review-state.spec.ts +++ b/spec/api-review-state.spec.ts @@ -869,6 +869,7 @@ describe('api review', () => { return true; }) .reply(200); + nock('https://api.github.com') .get( `/repos/electron/electron/commits/${payload.pull_request.head.sha}/check-runs?per_page=100`, @@ -882,7 +883,88 @@ describe('api review', () => { }); }); - it('correctly update api review check and api review label when pr is unlabeled', async () => { + it('adds the API requested label if a PR is marked ready for review', async () => { + const payload = loadFixture('api-review-state/pull_request.ready_for_review.json'); + const { pull_request } = payload; + + nock('https://api.github.com') + .get(`/repos/electron/electron/issues/${payload.number}/labels?per_page=100&page=1`) + .reply(200, [ + { + id: 1034512799, + node_id: 'MDU6TGFiZWwxMDM0NTEyNzk5', + url: 'https://api.github.com/repos/electron/electron/labels/semver/minor', + name: 'semver/minor', + color: '6ac2dd', + default: false, + description: 'backwards-compatible bug fixes', + }, + ]); + + nock('https://api.github.com') + .post(`/repos/electron/electron/issues/${pull_request.number}/labels`, (body) => { + expect(body).toEqual([REVIEW_LABELS.REQUESTED]); + return true; + }) + .reply(200); + + await robot.receive({ + id: '123-456', + name: 'pull_request', + payload, + }); + }); + + it('removes the API requested label if a PR is put in draft mode', async () => { + const payload = loadFixture('api-review-state/pull_request.converted_to_draft.json'); + const { pull_request } = payload; + + nock('https://api.github.com') + .get(`/repos/electron/electron/issues/${payload.number}/labels?per_page=100&page=1`) + .reply(200, [ + { + id: 1034512799, + node_id: 'MDU6TGFiZWwxMDM0NTEyNzk5', + url: 'https://api.github.com/repos/electron/electron/labels/semver/minor', + name: 'semver/minor', + color: '6ac2dd', + default: false, + description: 'backwards-compatible bug fixes', + }, + { + id: 1603621692, + node_id: 'MDU6TGFiZWwxNjAzNjIxNjky', + url: 'https://api.github.com/repos/electron/electron/labels/api-review/requested%20%F0%9F%97%B3', + name: REVIEW_LABELS.REQUESTED, + color: 'c918ba', + default: false, + description: '', + }, + ]); + + const encoded = encodeURIComponent(REVIEW_LABELS.REQUESTED); + nock('https://api.github.com') + .delete(`/repos/electron/electron/issues/${pull_request.number}/labels/${encoded}`) + .reply(200, [ + { + id: 208045946, + node_id: 'MDU6TGFiZWwyMDgwNDU5NDY=', + url: `https://api.github.com/repos/electron/electron/labels/${encoded}`, + name: REVIEW_LABELS.REQUESTED, + description: '', + color: 'f29513', + default: true, + }, + ]); + + await robot.receive({ + id: '123-456', + name: 'pull_request', + payload, + }); + }); + + it('correctly updates API review check and API review label when pr is unlabeled', async () => { const payload = loadFixture('api-review-state/pull_request.unlabeled.json'); nock('https://api.github.com') diff --git a/spec/fixtures/api-review-state/pull_request.converted_to_draft.json b/spec/fixtures/api-review-state/pull_request.converted_to_draft.json new file mode 100644 index 0000000..fea71b2 --- /dev/null +++ b/spec/fixtures/api-review-state/pull_request.converted_to_draft.json @@ -0,0 +1,147 @@ +{ + "action": "converted_to_draft", + "number": 26876, + "pull_request": { + "url": "https://api.github.com/repos/electron/electron/pulls/26876", + "id": 534054584, + "node_id": "MDExOlB1bGxSZXF1ZXN0NTM0MDU0NTg0", + "html_url": "https://github.com/electron/electron/pull/26876", + "diff_url": "https://github.com/electron/electron/pull/26876.diff", + "patch_url": "https://github.com/electron/electron/pull/26876.patch", + "issue_url": "https://api.github.com/repos/electron/electron/issues/26876", + "number": 26876, + "state": "open", + "locked": false, + "draft": true, + "title": "chore: fix JS linting", + "user": { + "login": "MarshallOfSound", + "id": 6634592, + "node_id": "MDQ6VXNlcjY2MzQ1OTI=", + "avatar_url": "https://avatars3.githubusercontent.com/u/6634592?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/MarshallOfSound" + }, + "body": "* Ensure --fix output is actually written to disk\r\n* Cache bust on lint.js file changes\r\n* Ensure CI does not use the linting cache\r\n\r\nNotes: no-notes", + "created_at": "2020-12-08T01:24:55Z", + "updated_at": "2020-12-08T01:24:55Z", + "closed_at": null, + "merged_at": null, + "merge_commit_sha": null, + "assignee": null, + "labels": [ + { + "id": 1034512799, + "node_id": "MDU6TGFiZWwxMDM0NTEyNzk5", + "url": "https://api.github.com/repos/electron/electron/labels/semver/major", + "name": "semver/minor", + "color": "6ac2dd", + "default": false, + "description": "backwards-compatible bug fixes" + }, + { + "id": 1603621692, + "node_id": "MDU6TGFiZWwxNjAzNjIxNjky", + "url": "https://api.github.com/repos/electron/electron/labels/api-review/requested%20%F0%9F%97%B3", + "name": "api-review/requested 🗳", + "color": "c918ba", + "default": false, + "description": "" + } + ], + "head": { + "label": "electron:fix-lint-js", + "ref": "fix-lint-js", + "sha": "c6b1b7168ab850a47f856c4a30f7a441bede1117", + "user": { + "login": "electron", + "id": 13409222 + }, + "repo": { + "id": 9384267, + "node_id": "MDEwOlJlcG9zaXRvcnk5Mzg0MjY3", + "name": "electron", + "full_name": "electron/electron", + "private": false, + "owner": { + "login": "electron", + "id": 13409222 + } + } + }, + "base": { + "label": "electron:main", + "ref": "main", + "sha": "c41b8d536b2d886abbe739374c0a46f99242a894", + "user": { + "login": "electron", + "id": 13409222, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjEzNDA5MjIy", + "avatar_url": "https://avatars.githubusercontent.com/u/13409222?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/electron", + "html_url": "https://github.com/electron", + "followers_url": "https://api.github.com/users/electron/followers", + "following_url": "https://api.github.com/users/electron/following{/other_user}", + "gists_url": "https://api.github.com/users/electron/gists{/gist_id}", + "starred_url": "https://api.github.com/users/electron/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/electron/subscriptions", + "organizations_url": "https://api.github.com/users/electron/orgs", + "repos_url": "https://api.github.com/users/electron/repos", + "events_url": "https://api.github.com/users/electron/events{/privacy}", + "received_events_url": "https://api.github.com/users/electron/received_events", + "type": "Organization", + "site_admin": false + }, + "repo": { + "id": 9384267, + "node_id": "MDEwOlJlcG9zaXRvcnk5Mzg0MjY3", + "name": "electron", + "full_name": "electron/electron", + "default_branch": "main", + "private": false, + "owner": { + "login": "electron", + "id": 13409222, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjEzNDA5MjIy", + "avatar_url": "https://avatars.githubusercontent.com/u/13409222?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/electron", + "html_url": "https://github.com/electron", + "followers_url": "https://api.github.com/users/electron/followers", + "following_url": "https://api.github.com/users/electron/following{/other_user}", + "gists_url": "https://api.github.com/users/electron/gists{/gist_id}", + "starred_url": "https://api.github.com/users/electron/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/electron/subscriptions", + "organizations_url": "https://api.github.com/users/electron/orgs", + "repos_url": "https://api.github.com/users/electron/repos", + "events_url": "https://api.github.com/users/electron/events{/privacy}", + "received_events_url": "https://api.github.com/users/electron/received_events", + "type": "Organization", + "site_admin": false + } + } + } + }, + "repository": { + "id": 9384267, + "node_id": "MDEwOlJlcG9zaXRvcnk5Mzg0MjY3", + "name": "electron", + "full_name": "electron/electron", + "private": false, + "owner": { + "login": "electron", + "id": 13409222 + }, + "license": { + "key": "mit", + "name": "MIT License", + "spdx_id": "MIT", + "url": "https://api.github.com/licenses/mit", + "node_id": "MDc6TGljZW5zZTEz" + } + }, + "sender": { + "login": "electron" + } +} \ No newline at end of file diff --git a/spec/fixtures/api-review-state/pull_request.ready_for_review.json b/spec/fixtures/api-review-state/pull_request.ready_for_review.json new file mode 100644 index 0000000..b0279c4 --- /dev/null +++ b/spec/fixtures/api-review-state/pull_request.ready_for_review.json @@ -0,0 +1,138 @@ +{ + "action": "ready_for_review", + "number": 26876, + "pull_request": { + "url": "https://api.github.com/repos/electron/electron/pulls/26876", + "id": 534054584, + "node_id": "MDExOlB1bGxSZXF1ZXN0NTM0MDU0NTg0", + "html_url": "https://github.com/electron/electron/pull/26876", + "diff_url": "https://github.com/electron/electron/pull/26876.diff", + "patch_url": "https://github.com/electron/electron/pull/26876.patch", + "issue_url": "https://api.github.com/repos/electron/electron/issues/26876", + "number": 26876, + "state": "open", + "locked": false, + "draft": false, + "title": "chore: fix JS linting", + "user": { + "login": "MarshallOfSound", + "id": 6634592, + "node_id": "MDQ6VXNlcjY2MzQ1OTI=", + "avatar_url": "https://avatars3.githubusercontent.com/u/6634592?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/MarshallOfSound" + }, + "body": "* Ensure --fix output is actually written to disk\r\n* Cache bust on lint.js file changes\r\n* Ensure CI does not use the linting cache\r\n\r\nNotes: no-notes", + "created_at": "2020-12-08T01:24:55Z", + "updated_at": "2020-12-08T01:24:55Z", + "closed_at": null, + "merged_at": null, + "merge_commit_sha": null, + "assignee": null, + "labels": [ + { + "id": 1034512799, + "node_id": "MDU6TGFiZWwxMDM0NTEyNzk5", + "url": "https://api.github.com/repos/electron/electron/labels/semver/major", + "name": "semver/minor", + "color": "6ac2dd", + "default": false, + "description": "backwards-compatible bug fixes" + } + ], + "head": { + "label": "electron:fix-lint-js", + "ref": "fix-lint-js", + "sha": "c6b1b7168ab850a47f856c4a30f7a441bede1117", + "user": { + "login": "electron", + "id": 13409222 + }, + "repo": { + "id": 9384267, + "node_id": "MDEwOlJlcG9zaXRvcnk5Mzg0MjY3", + "name": "electron", + "full_name": "electron/electron", + "private": false, + "owner": { + "login": "electron", + "id": 13409222 + } + } + }, + "base": { + "label": "electron:main", + "ref": "main", + "sha": "c41b8d536b2d886abbe739374c0a46f99242a894", + "user": { + "login": "electron", + "id": 13409222, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjEzNDA5MjIy", + "avatar_url": "https://avatars.githubusercontent.com/u/13409222?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/electron", + "html_url": "https://github.com/electron", + "followers_url": "https://api.github.com/users/electron/followers", + "following_url": "https://api.github.com/users/electron/following{/other_user}", + "gists_url": "https://api.github.com/users/electron/gists{/gist_id}", + "starred_url": "https://api.github.com/users/electron/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/electron/subscriptions", + "organizations_url": "https://api.github.com/users/electron/orgs", + "repos_url": "https://api.github.com/users/electron/repos", + "events_url": "https://api.github.com/users/electron/events{/privacy}", + "received_events_url": "https://api.github.com/users/electron/received_events", + "type": "Organization", + "site_admin": false + }, + "repo": { + "id": 9384267, + "node_id": "MDEwOlJlcG9zaXRvcnk5Mzg0MjY3", + "name": "electron", + "full_name": "electron/electron", + "default_branch": "main", + "private": false, + "owner": { + "login": "electron", + "id": 13409222, + "node_id": "MDEyOk9yZ2FuaXphdGlvbjEzNDA5MjIy", + "avatar_url": "https://avatars.githubusercontent.com/u/13409222?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/electron", + "html_url": "https://github.com/electron", + "followers_url": "https://api.github.com/users/electron/followers", + "following_url": "https://api.github.com/users/electron/following{/other_user}", + "gists_url": "https://api.github.com/users/electron/gists{/gist_id}", + "starred_url": "https://api.github.com/users/electron/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/electron/subscriptions", + "organizations_url": "https://api.github.com/users/electron/orgs", + "repos_url": "https://api.github.com/users/electron/repos", + "events_url": "https://api.github.com/users/electron/events{/privacy}", + "received_events_url": "https://api.github.com/users/electron/received_events", + "type": "Organization", + "site_admin": false + } + } + } + }, + "repository": { + "id": 9384267, + "node_id": "MDEwOlJlcG9zaXRvcnk5Mzg0MjY3", + "name": "electron", + "full_name": "electron/electron", + "private": false, + "owner": { + "login": "electron", + "id": 13409222 + }, + "license": { + "key": "mit", + "name": "MIT License", + "spdx_id": "MIT", + "url": "https://api.github.com/licenses/mit", + "node_id": "MDc6TGljZW5zZTEz" + } + }, + "sender": { + "login": "electron" + } +} \ No newline at end of file diff --git a/src/api-review-state.ts b/src/api-review-state.ts index 74c164c..29c2d22 100644 --- a/src/api-review-state.ts +++ b/src/api-review-state.ts @@ -34,6 +34,10 @@ const isBot = (user: string) => user === getEnvVar('BOT_USER_NAME', 'bot'); export const isReviewLabel = (label: string) => Object.values(REVIEW_LABELS).includes(label); export const isSemverMajorMinorLabel = (label: string) => [SEMVER_LABELS.MINOR, SEMVER_LABELS.MAJOR].includes(label); +export const hasSemverMajorMinorLabel = (pr: PullRequest) => + pr.labels.some((l) => isSemverMajorMinorLabel(l.name)); +export const hasAPIReviewRequestedLabel = (pr: PullRequest) => + pr.labels.some((l) => l.name === REVIEW_LABELS.REQUESTED); /** * Determines the PR readiness date depending on its semver label. @@ -361,6 +365,10 @@ export async function checkPRReadyForMerge( } export function setupAPIReviewStateManagement(probot: Probot) { + /** + * If a PR is opened or synchronized, we want to ensure the + * API review check is up-to-date. + */ probot.on( ['pull_request.synchronize', 'pull_request.opened'], async (context: Context<'pull_request'>) => { @@ -368,6 +376,10 @@ export function setupAPIReviewStateManagement(probot: Probot) { }, ); + /** + * If a PR review is submitted, we want to ensure the API + * review check is up-to-date. + */ probot.on( 'pull_request_review.submitted', async (context: Context<'pull_request_review.submitted'>) => { @@ -377,6 +389,52 @@ export function setupAPIReviewStateManagement(probot: Probot) { }, ); + /** + * If a PR with API review requirements is marked ready for review, + * we want to add the 'api-review/requested 🗳' label. + */ + probot.on('pull_request.ready_for_review', async (context: Context<'pull_request'>) => { + const { pull_request: pr, repository } = context.payload; + + if (hasSemverMajorMinorLabel(pr)) { + probot.log( + 'semver-minor or semver-major PR:', + `${repository.full_name}#${pr.number}`, + 'was marked as ready for review', + "Adding the 'api-review/requested 🗳' label", + ); + + await addLabels(context.octokit, { + ...context.repo({}), + prNumber: pr.number, + labels: [REVIEW_LABELS.REQUESTED], + }); + } + }); + + /** + * If a PR with existing API review requirements is converted to draft status, + * we want to remove the 'api-review/requested 🗳' label. + */ + probot.on('pull_request.converted_to_draft', async (context: Context<'pull_request'>) => { + const { pull_request: pr, repository } = context.payload; + + if (hasSemverMajorMinorLabel(pr) && hasAPIReviewRequestedLabel(pr)) { + probot.log( + 'semver-minor or semver-major PR:', + `${repository.full_name}#${pr.number}`, + 'was converted to draft status', + "Removing the 'api-review/requested 🗳' label", + ); + + await removeLabel(context.octokit, { + ...context.repo({}), + prNumber: pr.number, + name: REVIEW_LABELS.REQUESTED, + }); + } + }); + /** * If a potential API PR is labeled, there are several decision trees we * can potentially take, outlined as follows: @@ -408,14 +466,16 @@ export function setupAPIReviewStateManagement(probot: Probot) { // Once a PR is merged, allow tampering. if (pr.merged) return; - // Exclude PRs from api review if they: + // Exclude PRs from API review if they: // 1) Have backport, backport-skip, or fast-track labels // 2) Are targeting a non-main branch // 3) Are semver-patch PRs + // 4) Are draft PRs const excludePR = pr.labels.some((l) => EXCLUDE_LABELS.includes(l.name)) || pr.base.ref !== pr.base.repo.default_branch || - label.name === SEMVER_LABELS.PATCH; + label.name === SEMVER_LABELS.PATCH || + pr.draft; // If a PR is semver-minor or semver-major and the PR does not have an // exclusion condition, automatically add the 'api-review/requested 🗳' label. @@ -446,7 +506,6 @@ export function setupAPIReviewStateManagement(probot: Probot) { name: label.name, }); } - // Remove the api-review/requested 🗳' label if it was added prior to an exclusion label - // for example if the backport label was added by trop after cation got to it. } else if (excludePR) {