diff --git a/lib/api/github.js b/lib/api/github.js index 5f901a568dd026ccba51222eef6b353301406b2b..0bfc19de0866c847c0262a6f8f442a1814245a4d 100644 --- a/lib/api/github.js +++ b/lib/api/github.js @@ -29,6 +29,10 @@ module.exports = { addAssignees, addReviewers, addLabels, + getComments, + addComment, + editComment, + ensureComment, // PR getPrList, findPr, @@ -471,6 +475,52 @@ async function addLabels(issueNo, labels) { }); } +async function getComments(issueNo) { + // GET /repos/:owner/:repo/issues/:number/comments + logger.debug(`Getting comments for #${issueNo}`); + const url = `repos/${config.repoName}/issues/${issueNo}/comments?per_page=100`; + const comments = (await get(url, { paginate: true })).body; + logger.debug(`Found ${comments.length} comments`); + return comments; +} + +async function addComment(issueNo, body) { + // POST /repos/:owner/:repo/issues/:number/comments + await get.post(`repos/${config.repoName}/issues/${issueNo}/comments`, { + body: { body }, + }); +} + +async function editComment(commentId, body) { + // PATCH /repos/:owner/:repo/issues/comments/:id + await get.patch(`repos/${config.repoName}/issues/comments/${commentId}`, { + body: { body }, + }); +} + +async function ensureComment(issueNo, topic, content) { + logger.debug(`Ensuring comment "${topic}" in #${issueNo}`); + const body = `### ${topic}\n\n${content}`; + const comments = await getComments(issueNo); + let commentId; + let commentNeedsUpdating; + comments.forEach(comment => { + if (comment.body.startsWith(`### ${topic}\n\n`)) { + commentId = comment.id; + commentNeedsUpdating = comment.body !== body; + } + }); + if (!commentId) { + await addComment(issueNo, body); + logger.info({ repository: config.repoName, issueNo }, 'Added comment'); + } else if (commentNeedsUpdating) { + await editComment(commentId, body); + logger.info({ repository: config.repoName, issueNo }, 'Updated comment'); + } else { + logger.debug('Comment is already update-to-date'); + } +} + // Pull Request async function getPrList() { diff --git a/lib/api/gitlab.js b/lib/api/gitlab.js index 4ff6a291b9128154962f4041ad6a5d710516910c..f30a6c17120560e15f695c7a220ede83d049674c 100644 --- a/lib/api/gitlab.js +++ b/lib/api/gitlab.js @@ -23,6 +23,7 @@ module.exports = { addAssignees, addReviewers, addLabels, + ensureComment, // PR findPr, createPr, @@ -337,6 +338,10 @@ async function addLabels(prNo, labels) { await get.put(url); } +async function ensureComment() { + // Todo: implement. See GitHub API for example +} + async function findPr(branchName, prTitle, state = 'all') { logger.debug(`findPr(${branchName}, ${prTitle}, ${state})`); const urlString = `projects/${config.repoName}/merge_requests?state=${state}&per_page=100`; diff --git a/lib/workers/branch/check-existing.js b/lib/workers/branch/check-existing.js index 8557831f53d1085049584884d73731f7c67a48e7..f07064ea80c6c80c83934cfea75edddfc590e769 100644 --- a/lib/workers/branch/check-existing.js +++ b/lib/workers/branch/check-existing.js @@ -9,16 +9,12 @@ async function prAlreadyExisted(config) { logger.trace({ config }, 'prAlreadyExisted'); if (config.recreateClosed) { logger.debug('recreateClosed is true'); - return false; + return null; } logger.debug('recreateClosed is false'); // Return if same PR already existed // Check for current PR title format - const pr = await config.api.findPr( - config.branchName, - config.prTitle, - 'closed' - ); + let pr = await config.api.findPr(config.branchName, config.prTitle, 'closed'); if (pr) { logger.debug('Found closed PR with current title'); // this code exists to ignore mistakenly closed PRs which occurred due to a bug @@ -31,19 +27,20 @@ async function prAlreadyExisted(config) { { closedAt, problemStart, problemStopped }, 'Ignoring mistakenly closed PR' ); - return false; + return null; } - return true; + return pr; } // Check for legacy PR title format // TODO: remove this in v10 const legacyPrTitle = config.prTitle .replace(/to v(\d+)$/, 'to version $1.x') // Major .replace(/to v(\d+)/, 'to version $1'); // Non-major - if (await config.api.findPr(config.branchName, legacyPrTitle, 'closed')) { + pr = await config.api.findPr(config.branchName, legacyPrTitle, 'closed'); + if (pr) { logger.debug('Found closed PR with legacy title'); - return true; + return pr; } logger.debug('prAlreadyExisted=false'); - return false; + return null; } diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js index e39457929d3464c8474768cca54d3b1ac50683d9..9a2ab3ac37b60dc686912bb0982d4f38644a352f 100644 --- a/lib/workers/branch/index.js +++ b/lib/workers/branch/index.js @@ -45,8 +45,18 @@ async function processBranch(branchConfig) { logger.info(`Branch has ${dependencies.length} upgrade(s)`); - if (await prAlreadyExisted(config)) { + const pr = await prAlreadyExisted(config); + if (pr) { logger.info('Closed PR already exists. Skipping branch.'); + const subject = 'Renovate Ignore Notification'; + let content = + 'Renovate will now ignore this PR, as it has been closed. If this was a mistake or you changed your mind, you can simply reopen or rename this PR to reactivate Renovate.'; + if (config.isMajor) { + content += `\n\nYou will otherwise not receive PRs for any releases of this major version (${config.newVersionMajor}.x) unless you upgrade it manually yourself. If Renovate later detects that it has been manuall upgraded, it will resume creating PRs for minor and patch upgrades and you do not need to reopen this PR.`; + } else { + content += `\n\n**Note**: The ignore applies only to this specific version (${config.newVersion}), so you will still receive a PR once a newer version is released. If you wish to permanently ignore this dependency, please add it to the \`ignoreDeps\` array of your renovate config.`; + } + await config.api.ensureComment(pr.number, subject, content); return 'already-existed'; } config.parentBranch = await getParentBranch(config); diff --git a/test/api/__snapshots__/github.spec.js.snap b/test/api/__snapshots__/github.spec.js.snap index f4108e4235c4ef773bfe9058bd0f8631d4738252..053adefe64c5e9876fb1f6b9862eb258f1c16e2f 100644 --- a/test/api/__snapshots__/github.spec.js.snap +++ b/test/api/__snapshots__/github.spec.js.snap @@ -432,6 +432,38 @@ Array [ ] `; +exports[`api/github ensureComment add comment if not found 1`] = ` +Array [ + Array [ + "repos/some/repo/issues/42/comments", + Object { + "body": Object { + "body": "### some-subject + +some +content", + }, + }, + ], +] +`; + +exports[`api/github ensureComment add updates comment if necessary 1`] = ` +Array [ + Array [ + "repos/some/repo/issues/comments/1234", + Object { + "body": Object { + "body": "### some-subject + +some +content", + }, + }, + ], +] +`; + exports[`api/github findFilePaths(fileName) should return the files matching the fileName 1`] = ` Array [ "package.json", diff --git a/test/api/github.spec.js b/test/api/github.spec.js index 92e48fc18a3e8b467d1789f5bf9b961ccd73e903..262abd3969e3cb7a20341d3190d72b268813f93e 100644 --- a/test/api/github.spec.js +++ b/test/api/github.spec.js @@ -968,6 +968,34 @@ describe('api/github', () => { expect(get.post.mock.calls).toMatchSnapshot(); }); }); + describe('ensureComment', () => { + it('add comment if not found', async () => { + await initRepo('some/repo', 'token'); + get.mockReturnValueOnce({ body: [] }); + await github.ensureComment(42, 'some-subject', 'some\ncontent'); + expect(get.post.mock.calls).toHaveLength(1); + expect(get.post.mock.calls).toMatchSnapshot(); + }); + it('add updates comment if necessary', async () => { + await initRepo('some/repo', 'token'); + get.mockReturnValueOnce({ + body: [{ id: 1234, body: '### some-subject\n\nblablabla' }], + }); + await github.ensureComment(42, 'some-subject', 'some\ncontent'); + expect(get.post.mock.calls).toHaveLength(0); + expect(get.patch.mock.calls).toHaveLength(1); + expect(get.patch.mock.calls).toMatchSnapshot(); + }); + it('skips comment', async () => { + await initRepo('some/repo', 'token'); + get.mockReturnValueOnce({ + body: [{ id: 1234, body: '### some-subject\n\nsome\ncontent' }], + }); + await github.ensureComment(42, 'some-subject', 'some\ncontent'); + expect(get.post.mock.calls).toHaveLength(0); + expect(get.patch.mock.calls).toHaveLength(0); + }); + }); describe('findPr(branchName, prTitle, state)', () => { it('returns true if no title and all state', async () => { get.mockReturnValueOnce({ diff --git a/test/api/gitlab.spec.js b/test/api/gitlab.spec.js index 78f6b5f133835ebd29c5343856b4dd50cb90d64d..72d71f7f99f9762617bea661df219511757de69b 100644 --- a/test/api/gitlab.spec.js +++ b/test/api/gitlab.spec.js @@ -488,6 +488,11 @@ describe('api/gitlab', () => { expect(get.put.mock.calls).toMatchSnapshot(); }); }); + describe('ensureComment', () => { + it('exists', async () => { + await gitlab.ensureComment(42, 'some-subject', 'some\ncontent'); + }); + }); describe('findPr(branchName, prTitle, state)', () => { it('returns null if no results', async () => { get.mockReturnValueOnce({ diff --git a/test/workers/branch/check-existing.spec.js b/test/workers/branch/check-existing.spec.js index 2bf417c0b5864e8aa701b2c026f1469de01079ab..2f8367a41717fdfebc8d202d1d7e70ce46512159 100644 --- a/test/workers/branch/check-existing.spec.js +++ b/test/workers/branch/check-existing.spec.js @@ -18,30 +18,30 @@ describe('workers/branch/check-existing', () => { }); it('returns false if recreating closed PRs', async () => { config.recreateClosed = true; - expect(await prAlreadyExisted(config)).toBe(false); + expect(await prAlreadyExisted(config)).toBe(null); expect(config.api.findPr.mock.calls.length).toBe(0); }); it('returns false if both checks miss', async () => { config.recreatedClosed = true; - expect(await prAlreadyExisted(config)).toBe(false); + expect(await prAlreadyExisted(config)).toBe(null); expect(config.api.findPr.mock.calls.length).toBe(2); }); it('returns true if first check hits', async () => { - config.api.findPr.mockReturnValueOnce({}); - expect(await prAlreadyExisted(config)).toBe(true); + config.api.findPr.mockReturnValueOnce({ number: 12 }); + expect(await prAlreadyExisted(config)).toEqual({ number: 12 }); expect(config.api.findPr.mock.calls.length).toBe(1); }); it('returns true if second check hits', async () => { config.api.findPr.mockReturnValueOnce(null); - config.api.findPr.mockReturnValueOnce({}); - expect(await prAlreadyExisted(config)).toBe(true); + config.api.findPr.mockReturnValueOnce({ number: 13 }); + expect(await prAlreadyExisted(config)).toEqual({ number: 13 }); expect(config.api.findPr.mock.calls.length).toBe(2); }); it('returns false if mistaken', async () => { config.api.findPr.mockReturnValueOnce({ closed_at: '2017-10-15T21:28:07.000Z', }); - expect(await prAlreadyExisted(config)).toBe(false); + expect(await prAlreadyExisted(config)).toBe(null); }); }); }); diff --git a/test/workers/branch/index.spec.js b/test/workers/branch/index.spec.js index 8f1a56eac522832a9246336aec4f426a9ca6eb5c..e6cee06da6659409f5eaa58620bb53dbccf14fbf 100644 --- a/test/workers/branch/index.spec.js +++ b/test/workers/branch/index.spec.js @@ -29,7 +29,7 @@ describe('workers/branch', () => { beforeEach(() => { config = { ...defaultConfig, - api: { branchExists: jest.fn() }, + api: { branchExists: jest.fn(), ensureComment: jest.fn() }, errors: [], warnings: [], logger, @@ -49,12 +49,22 @@ describe('workers/branch', () => { await branchWorker.processBranch(config); expect(checkExisting.prAlreadyExisted.mock.calls).toHaveLength(0); }); - it('skips branch if closed PR found', async () => { + it('skips branch if closed major PR found', async () => { schedule.isScheduledNow.mockReturnValueOnce(false); config.api.branchExists.mockReturnValueOnce(true); - checkExisting.prAlreadyExisted.mockReturnValueOnce(true); + config.isMajor = true; + checkExisting.prAlreadyExisted.mockReturnValueOnce({ number: 13 }); await branchWorker.processBranch(config); expect(parent.getParentBranch.mock.calls.length).toBe(0); + expect(config.logger.error.mock.calls).toHaveLength(0); + }); + it('skips branch if closed minor PR found', async () => { + schedule.isScheduledNow.mockReturnValueOnce(false); + config.api.branchExists.mockReturnValueOnce(true); + checkExisting.prAlreadyExisted.mockReturnValueOnce({ number: 13 }); + await branchWorker.processBranch(config); + expect(parent.getParentBranch.mock.calls.length).toBe(0); + expect(config.logger.error.mock.calls).toHaveLength(0); }); it('returns if no branch exists', async () => { packageFiles.getUpdatedPackageFiles.mockReturnValueOnce([]);