diff --git a/lib/config/definitions.js b/lib/config/definitions.js index cdbcdfbcfff32ef18da1c7c10826d2b2679f3081..6b7a7a306d9e17b5d27d09d64e760de10cc18eca 100644 --- a/lib/config/definitions.js +++ b/lib/config/definitions.js @@ -883,7 +883,7 @@ const options = [ name: 'prCreation', description: 'When to create the PR for a branch.', type: 'string', - allowedValues: ['immediate', 'not-pending', 'status-success'], + allowedValues: ['immediate', 'not-pending', 'status-success', 'approval'], default: 'immediate', }, { diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js index fa813001225cb6d0e4ac1ffe5992a74ab29b58d6..1c308e430cb34bf7dcdba3289c8e3129eba6bbf4 100644 --- a/lib/workers/branch/index.js +++ b/lib/workers/branch/index.js @@ -363,6 +363,9 @@ async function processBranch(branchConfig, prHourlyLimitReached, packageFiles) { ); const pr = await prWorker.ensurePr(config); // TODO: ensurePr should check for automerge itself + if (pr === 'needs-pr-approval') { + return 'needs-pr-approval'; + } if (pr) { const topic = ':warning: Artifact update problem'; if (config.artifactErrors && config.artifactErrors.length) { diff --git a/lib/workers/pr/index.js b/lib/workers/pr/index.js index f642736d12906ad610243b9fb474704d0ebd1882..199a99e1cd5f007ec741c644590ab6506b168529 100644 --- a/lib/workers/pr/index.js +++ b/lib/workers/pr/index.js @@ -12,6 +12,7 @@ async function ensurePr(prConfig) { logger.trace({ config }, 'ensurePr'); // If there is a group, it will use the config of the first upgrade in the array const { branchName, prTitle, upgrades } = config; + const masterIssueCheck = (config.masterIssueChecks || {})[config.branchName]; // Check if existing PR exists const existingPr = await platform.getBranchPr(branchName); if (existingPr) { @@ -76,6 +77,12 @@ async function ensurePr(prConfig) { return null; } logger.debug('Branch status success'); + } else if ( + config.prCreation === 'approval' && + !existingPr && + masterIssueCheck !== 'approvePr' + ) { + return 'needs-pr-approval'; } else if ( config.prCreation === 'not-pending' && !existingPr && diff --git a/lib/workers/repository/master-issue.js b/lib/workers/repository/master-issue.js index 6a036b66354a59d998930474089e251b46079c68..93806459f890770422c8f28208c0dff81937e4f0 100644 --- a/lib/workers/repository/master-issue.js +++ b/lib/workers/repository/master-issue.js @@ -23,7 +23,12 @@ function getListItem(branch, type, pr) { async function ensureMasterIssue(config, branches) { if ( - !(config.masterIssue || branches.some(branch => branch.masterIssueApproval)) + !( + config.masterIssue || + branches.some( + branch => branch.masterIssueApproval || branch.masterIssuePrApproval + ) + ) ) { return; } @@ -61,7 +66,7 @@ async function ensureMasterIssue(config, branches) { ); if (pendingApprovals.length) { issueBody += '## Pending Approval\n\n'; - issueBody += `These PRs will be created by ${appName} only once you click their checkbox below.\n\n`; + issueBody += `These branches will be created by ${appName} only once you click their checkbox below.\n\n`; for (const branch of pendingApprovals) { issueBody += getListItem(branch, 'approve'); } @@ -103,6 +108,18 @@ async function ensureMasterIssue(config, branches) { } issueBody += '\n'; } + const awaitingPr = branches.filter( + branch => branch.res === 'needs-pr-approval' + ); + if (awaitingPr.length) { + issueBody += '## PR Creation Approval Required\n\n'; + issueBody += + "These branches exist but PRs won't be created until you tick the checkbox.\n\n"; + for (const branch of awaitingPr) { + issueBody += getListItem(branch, 'approvePr'); + } + issueBody += '\n'; + } const prEdited = branches.filter(branch => branch.res === 'pr-edited'); if (prEdited.length) { issueBody += '## Edited/Blocked\n\n'; @@ -115,6 +132,7 @@ async function ensureMasterIssue(config, branches) { } const otherRes = [ 'needs-approval', + 'needs-pr-approval', 'not-scheduled', 'pr-hourly-limit-reached', 'already-existed', diff --git a/lib/workers/repository/process/index.js b/lib/workers/repository/process/index.js index e7a2ba4600856d24212e4d30be6fd272d65d0f59..86d49b7c79b3e67120ee12e0c14bc371a5394201 100644 --- a/lib/workers/repository/process/index.js +++ b/lib/workers/repository/process/index.js @@ -14,14 +14,17 @@ async function processRepo(config) { if ( config.masterIssue || config.masterIssueApproval || + config.prCreation === 'approval' || (config.packageRules && - config.packageRules.some(rule => rule.masterIssueApproval)) + config.packageRules.some( + rule => rule.masterIssueApproval || rule.prCreation === 'approval' + )) ) { config.masterIssueTitle = config.masterIssueTitle || `Update Dependencies (${appName} Bot)`; const issue = await platform.findIssue(config.masterIssueTitle); if (issue) { - const checkMatch = ' - \\[x\\] <!-- ([a-z]+)-branch=([^\\s]+) -->'; + const checkMatch = ' - \\[x\\] <!-- ([a-zA-Z]+)-branch=([^\\s]+) -->'; const checked = issue.body.match(new RegExp(checkMatch, 'g')); if (checked && checked.length) { checked.forEach(check => { diff --git a/lib/workers/repository/updates/generate.js b/lib/workers/repository/updates/generate.js index 26707056deec6f1cf8e2e8072bec8991fba9d60c..d92b4d1caf5dfb9e6ab7a5056663e86f3e5cbdea 100644 --- a/lib/workers/repository/updates/generate.js +++ b/lib/workers/repository/updates/generate.js @@ -265,6 +265,9 @@ function generateBranchConfig(branchUpgrades) { config.masterIssueApproval = config.upgrades.some( upgrade => upgrade.masterIssueApproval ); + config.masterIssuePrApproval = config.upgrades.some( + upgrade => upgrade.prCreation === 'approval' + ); config.automerge = config.upgrades.every(upgrade => upgrade.automerge); config.blockedByPin = config.upgrades.every(upgrade => upgrade.blockedByPin); const tableRows = config.upgrades diff --git a/renovate-schema.json b/renovate-schema.json index 3060486369a83cdb4a0050e3f1adfa4c47da9c44..88d7094a7e8277ef41355b71dcdf8d756f428d32 100644 --- a/renovate-schema.json +++ b/renovate-schema.json @@ -579,7 +579,7 @@ "prCreation": { "description": "When to create the PR for a branch.", "type": "string", - "enum": ["immediate", "not-pending", "status-success"], + "enum": ["immediate", "not-pending", "status-success", "approval"], "default": "immediate" }, "prNotPendingHours": { diff --git a/test/workers/branch/index.spec.js b/test/workers/branch/index.spec.js index 7a202d3eb702349d651af95cedfd9c26bb0b8a1e..e7f847cafadf8696a6d6c3c0669734bfb576f5ef 100644 --- a/test/workers/branch/index.spec.js +++ b/test/workers/branch/index.spec.js @@ -188,6 +188,21 @@ describe('workers/branch', () => { expect(automerge.tryBranchAutomerge).toHaveBeenCalledTimes(1); expect(prWorker.ensurePr).toHaveBeenCalledTimes(0); }); + it('returns if branch exists and prCreation set to approval', async () => { + getUpdated.getUpdatedPackageFiles.mockReturnValueOnce({ + updatedPackageFiles: [{}], + }); + npmPostExtract.getAdditionalFiles.mockReturnValueOnce({ + artifactErrors: [], + updatedArtifacts: [{}], + }); + platform.branchExists.mockReturnValueOnce(true); + automerge.tryBranchAutomerge.mockReturnValueOnce('failed'); + prWorker.ensurePr.mockReturnValueOnce('needs-pr-approval'); + expect(await branchWorker.processBranch(config)).toEqual( + 'needs-pr-approval' + ); + }); it('ensures PR and tries automerge', async () => { getUpdated.getUpdatedPackageFiles.mockReturnValueOnce({ updatedPackageFiles: [{}], diff --git a/test/workers/pr/index.spec.js b/test/workers/pr/index.spec.js index 82bc7a62eed52d8232e8cfb60e1272cddb33b3c9..1be7083c53c56d3210af4e6fd904b8d28ac0bad4 100644 --- a/test/workers/pr/index.spec.js +++ b/test/workers/pr/index.spec.js @@ -146,6 +146,12 @@ describe('workers/pr', () => { const pr = await prWorker.ensurePr(config); expect(pr).toBeNull(); }); + it('should return needs-approval if prCreation set to approval', async () => { + platform.getBranchStatus.mockReturnValueOnce('success'); + config.prCreation = 'approval'; + const pr = await prWorker.ensurePr(config); + expect(pr).toBe('needs-pr-approval'); + }); it('should create PR if success', async () => { platform.getBranchStatus.mockReturnValueOnce('success'); config.prCreation = 'status-success'; diff --git a/test/workers/repository/_fixtures/master-issue_with_3_PR_in_approval.txt b/test/workers/repository/_fixtures/master-issue_with_3_PR_in_approval.txt new file mode 100644 index 0000000000000000000000000000000000000000..87196750c5e0986bf43eaf035fb63e2cc3eb99a3 --- /dev/null +++ b/test/workers/repository/_fixtures/master-issue_with_3_PR_in_approval.txt @@ -0,0 +1,10 @@ +This [master issue](https://renovatebot.com/blog/master-issue) contains a list of Renovate updates and their statuses. + +## PR Creation Approval Required + +These branches exist but PRs won't be created until you tick the checkbox. + + - [ ] <!-- approvePr-branch=branchName1 -->pr1 + - [ ] <!-- approvePr-branch=branchName2 -->pr2 (`dep2`, `dep3`) + - [ ] <!-- approvePr-branch=branchName3 -->pr3 + diff --git a/test/workers/repository/_fixtures/master-issue_with_8_PR.txt b/test/workers/repository/_fixtures/master-issue_with_8_PR.txt index e7906b8b0812c5b707498fdd0cbe48729418217c..8cbdbcc67fe6d049a4b3c3a982f1f1c229bff836 100644 --- a/test/workers/repository/_fixtures/master-issue_with_8_PR.txt +++ b/test/workers/repository/_fixtures/master-issue_with_8_PR.txt @@ -2,7 +2,7 @@ This [master issue](https://renovatebot.com/blog/master-issue) contains a list o ## Pending Approval -These PRs will be created by Renovate only once you click their checkbox below. +These branches will be created by Renovate only once you click their checkbox below. - [ ] <!-- approve-branch=branchName1 -->pr1 - [ ] <!-- approve-branch=branchName2 -->pr2 diff --git a/test/workers/repository/master-issue.spec.js b/test/workers/repository/master-issue.spec.js index a2ab057fedb53aee0bc99166630b17f8d256e559..896cee0141b7f16b9a2565c2c85243545b3fbc09 100644 --- a/test/workers/repository/master-issue.spec.js +++ b/test/workers/repository/master-issue.spec.js @@ -326,5 +326,46 @@ describe('workers/repository/master-issue', () => { // same with dry run await dryRun(branches, platform, 0, 0, 0, 2); }); + + it('checks an issue with 3 PR in approval', async () => { + const branches = [ + { + prTitle: 'pr1', + upgrades: [{ depName: 'dep1' }], + res: 'needs-pr-approval', + branchName: 'branchName1', + }, + { + prTitle: 'pr2', + upgrades: [{ depName: 'dep2' }, { depName: 'dep3' }], + res: 'needs-pr-approval', + branchName: 'branchName2', + }, + { + prTitle: 'pr3', + upgrades: [{ depName: 'dep3' }], + res: 'needs-pr-approval', + branchName: 'branchName3', + }, + ]; + config.masterIssue = true; + config.masterIssuePrApproval = true; + await masterIssue.ensureMasterIssue(config, branches); + expect(platform.ensureIssueClosing).toHaveBeenCalledTimes(0); + expect(platform.ensureIssue).toHaveBeenCalledTimes(1); + expect(platform.ensureIssue.mock.calls[0][0]).toBe( + config.masterIssueTitle + ); + expect(platform.ensureIssue.mock.calls[0][1]).toBe( + fs.readFileSync( + 'test/workers/repository/_fixtures/master-issue_with_3_PR_in_approval.txt', + 'utf8' + ) + ); + expect(platform.findPr).toHaveBeenCalledTimes(0); + + // same with dry run + await dryRun(branches, platform); + }); }); }); diff --git a/test/workers/repository/updates/__snapshots__/generate.spec.js.snap b/test/workers/repository/updates/__snapshots__/generate.spec.js.snap index 45f9fda6f1af9f7d96b030567f668ec5bc6f83dd..5df4b751ccc70c0bde4b33a63eed6c0238898666 100644 --- a/test/workers/repository/updates/__snapshots__/generate.spec.js.snap +++ b/test/workers/repository/updates/__snapshots__/generate.spec.js.snap @@ -18,6 +18,7 @@ Object { "displayFrom": "", "displayTo": "0.6.0", "masterIssueApproval": false, + "masterIssuePrApproval": false, "newValue": "0.6.0", "prTitle": "some-title", "prettyDepType": "dependency",