From ce5c603be1d54b98e222774d98e8c5a3e4884139 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Sat, 22 Aug 2020 02:51:31 +0530 Subject: [PATCH 01/17] feat(1415): Add logic to handle webhook events from external repos --- plugins/webhooks/index.js | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index e22434770..f06718ec6 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -1,3 +1,6 @@ +/* eslint-disable max-lines-per-function */ +/* eslint-disable max-params */ + 'use strict'; const joi = require('joi'); @@ -278,9 +281,12 @@ async function triggeredPipelines( const scmBranch = `${splitUri[0]}:${splitUri[1]}:${splitUri[2]}`; const scmRepoId = `${splitUri[0]}:${splitUri[1]}`; const listConfig = { search: { field: 'scmUri', keyword: `${scmRepoId}:%` } }; + const externalRepoSearchConfig = { params: { contains: { subscribedScmUrls: scmUri } } }; const pipelines = await pipelineFactory.list(listConfig); + const pipelinesWithSubscribedExtRepos = await pipelineFactory.list(externalRepoSearchConfig); + let pipelinesOnCommitBranch = []; let pipelinesOnOtherBranch = []; @@ -314,7 +320,9 @@ async function triggeredPipelines( ); }); - return pipelinesOnCommitBranch.concat(pipelinesOnOtherBranch); + const currentRepoPipelines = pipelinesOnCommitBranch.concat(pipelinesOnOtherBranch); + + return currentRepoPipelines.concat(pipelinesWithSubscribedExtRepos); } /** @@ -376,6 +384,7 @@ async function createPREvents(options, request) { logger.info(`skip create event for branch: ${b}`); } } + const { skipMessage, resolvedChainPR } = getSkipMessageAndChainPR({ // Workaround for pipelines which has NULL value in `pipeline.annotations` pipeline: !p.annotations ? { annotations: {}, ...p } : p, @@ -384,6 +393,15 @@ async function createPREvents(options, request) { chainPR }); + let subscribedEvent = false; + let subscribedScmConfig = {}; + + // Check is the webhook event is from a subscribed repo + if (scmConfig.scmUri !== p.scmUri) { + subscribedEvent = true; + subscribedScmConfig = scmConfig; + } + const eventConfig = { pipelineId: p.id, type: 'pr', @@ -401,7 +419,9 @@ async function createPREvents(options, request) { prTitle, prInfo: await eventFactory.scm.getPrInfo(scmConfig), prSource, - baseBranch: branch + baseBranch: branch, + subscribedEvent, + subscribedScmConfig }; if (skipMessage) { From e37d7c7c6785c7e476076fec9e66ed222bc5407d Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Sat, 22 Aug 2020 02:53:12 +0530 Subject: [PATCH 02/17] feat(1415): Change addWebhook method to addMultipleWebhooks and few fixes --- plugins/pipelines/create.js | 7 +++---- plugins/pipelines/syncWebhooks.js | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/plugins/pipelines/create.js b/plugins/pipelines/create.js index f2e5220fe..ebe95c8e1 100644 --- a/plugins/pipelines/create.js +++ b/plugins/pipelines/create.js @@ -113,10 +113,9 @@ module.exports = () => ({ }); } - const results = await Promise.all([ - pipeline.sync(), - pipeline.addWebhook(`${request.server.info.uri}/v4/webhooks`) - ]); + const results = await pipeline.sync(); + + await pipeline.addWebhooks(`${request.server.info.uri}/v4/webhooks`); const location = urlLib.format({ host: request.headers.host, diff --git a/plugins/pipelines/syncWebhooks.js b/plugins/pipelines/syncWebhooks.js index ed75e14ea..6000e50cd 100644 --- a/plugins/pipelines/syncWebhooks.js +++ b/plugins/pipelines/syncWebhooks.js @@ -51,7 +51,7 @@ module.exports = () => ({ } }) // user has good permissions, add or update webhooks - .then(() => pipeline.addWebhook(`${request.server.info.uri}/v4/webhooks`)) + .then(() => pipeline.addWebhooks(`${request.server.info.uri}/v4/webhooks`)) .then(() => h.response().code(204)) ); }) From 5653f848769322ff717355ba7444990de8dbcb77 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 27 Aug 2020 14:32:02 +0530 Subject: [PATCH 03/17] feat(1415): Add tests for other events and fix queries --- plugins/webhooks/index.js | 59 ++++++++++++----- test/plugins/webhooks.test.js | 117 ++++++++++++++++++++++++++++++++-- 2 files changed, 153 insertions(+), 23 deletions(-) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index f06718ec6..b2c6a66f5 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -253,6 +253,14 @@ function getSkipMessageAndChainPR({ pipeline, prSource, restrictPR, chainPR }) { return result; } +const uriTrimmer = uri => { + const uriToArray = uri.split(':'); + + while (uriToArray.length > 2) uriToArray.pop(); + + return uriToArray.join(':'); +}; + /** * Get all pipelines which has triggered job * @method triggeredPipelines @@ -281,11 +289,11 @@ async function triggeredPipelines( const scmBranch = `${splitUri[0]}:${splitUri[1]}:${splitUri[2]}`; const scmRepoId = `${splitUri[0]}:${splitUri[1]}`; const listConfig = { search: { field: 'scmUri', keyword: `${scmRepoId}:%` } }; - const externalRepoSearchConfig = { params: { contains: { subscribedScmUrls: scmUri } } }; + const externalRepoSearchConfig = { search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }; const pipelines = await pipelineFactory.list(listConfig); - const pipelinesWithSubscribedExtRepos = await pipelineFactory.list(externalRepoSearchConfig); + const pipelinesWithSubscribedRepos = await pipelineFactory.list(externalRepoSearchConfig); let pipelinesOnCommitBranch = []; let pipelinesOnOtherBranch = []; @@ -322,7 +330,7 @@ async function triggeredPipelines( const currentRepoPipelines = pipelinesOnCommitBranch.concat(pipelinesOnOtherBranch); - return currentRepoPipelines.concat(pipelinesWithSubscribedExtRepos); + return currentRepoPipelines.concat(pipelinesWithSubscribedRepos); } /** @@ -393,15 +401,6 @@ async function createPREvents(options, request) { chainPR }); - let subscribedEvent = false; - let subscribedScmConfig = {}; - - // Check is the webhook event is from a subscribed repo - if (scmConfig.scmUri !== p.scmUri) { - subscribedEvent = true; - subscribedScmConfig = scmConfig; - } - const eventConfig = { pipelineId: p.id, type: 'pr', @@ -419,11 +418,15 @@ async function createPREvents(options, request) { prTitle, prInfo: await eventFactory.scm.getPrInfo(scmConfig), prSource, - baseBranch: branch, - subscribedEvent, - subscribedScmConfig + baseBranch: branch }; + // Check is the webhook event is from a subscribed repo + if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) { + eventConfig.subscribedEvent = true; + eventConfig.subscribedScmConfig = scmConfig; + } + if (skipMessage) { eventConfig.skipMessage = skipMessage; } @@ -748,7 +751,15 @@ function createMeta(parsed) { * @param {String} [skipMessage] Message to skip starting builds * @returns {Promise} Promise that resolves into events */ -async function createEvents(eventFactory, userFactory, pipelineFactory, pipelines, parsed, skipMessage) { +async function createEvents( + eventFactory, + userFactory, + pipelineFactory, + pipelines, + parsed, + skipMessage, + scmConfigFromHook +) { const { action, branch, sha, username, scmContext, changedFiles, type, releaseName, ref } = parsed; const events = []; const meta = createMeta(parsed); @@ -840,6 +851,12 @@ async function createEvents(eventFactory, userFactory, pipelineFactory, pipeline ref }; + // Check is the webhook event is from a subscribed repo + if (uriTrimmer(scmConfigFromHook.scmUri) !== uriTrimmer(pTuple.pipeline.scmUri)) { + eventConfig.subscribedEvent = true; + eventConfig.subscribedScmConfig = scmConfigFromHook; + } + if (skipMessage) { eventConfig.skipMessage = skipMessage; } @@ -911,7 +928,15 @@ async function pushEvent(request, h, parsed, skipMessage, token) { if (!pipelines || pipelines.length === 0) { request.log(['webhook', hookId], `Skipping since Pipeline ${fullCheckoutUrl} does not exist`); } else { - events = await createEvents(eventFactory, userFactory, pipelineFactory, pipelines, parsed, skipMessage); + events = await createEvents( + eventFactory, + userFactory, + pipelineFactory, + pipelines, + parsed, + skipMessage, + scmConfig + ); } const hasBuildEvents = events.filter(e => e.builds !== null); diff --git a/test/plugins/webhooks.test.js b/test/plugins/webhooks.test.js index f6a7a3a6e..39b95087a 100644 --- a/test/plugins/webhooks.test.js +++ b/test/plugins/webhooks.test.js @@ -390,6 +390,7 @@ describe('webhooks plugin test', () => { const checkoutUrl = 'git@github.com:baxterthehacker/public-repo.git'; const fullCheckoutUrl = 'git@github.com:baxterthehacker/public-repo.git#master'; const scmUri = 'github.com:123456:master'; + const scmRepoId = `github.com:123456`; const pipelineId = 'pipelineHash'; const jobId = 2; const buildId = 'buildHash'; @@ -579,6 +580,9 @@ describe('webhooks plugin test', () => { pipelineMock.workflowGraph = workflowGraph; pipelineMock.jobs = Promise.resolve([mainJobMock, jobMock]); pipelineFactoryMock.scm.parseHook.withArgs(reqHeaders, payload).resolves(parsed); + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .resolves([pipelineMock]); pipelineFactoryMock.list.resolves([pipelineMock]); }); @@ -1139,6 +1143,9 @@ describe('webhooks plugin test', () => { }); pipelineFactoryMock.list.resolves([pipelineMock, pMock1, pMock2, pMock3]); + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .resolves([]); return server.inject(options).then(reply => { assert.equal(reply.statusCode, 201); @@ -1236,6 +1243,9 @@ describe('webhooks plugin test', () => { pipelineFactoryMock.scm.getChangedFiles.resolves(['lib/test.js']); pipelineFactoryMock.list.resolves([pipelineMock, pMock1, pMock2]); + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .resolves([]); return server.inject(options).then(reply => { assert.equal(reply.statusCode, 201); @@ -1328,6 +1338,43 @@ describe('webhooks plugin test', () => { }); }); + it('returns 201 when the hook source triggers subscribed event', () => { + pipelineFactoryMock.scm.parseUrl + .withArgs({ checkoutUrl: fullCheckoutUrl, token, scmContext }) + .resolves('github.com:789123:master'); + pipelineFactoryMock.list.resolves([]); + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: '%github.com:789123:%' } }) + .resolves([pipelineMock]); + const scmConfigSubscribe = { + scmUri: 'github.com:789123:master', + token, + scmContext + }; + + return server.inject(options).then(reply => { + assert.equal(reply.statusCode, 201); + assert.calledWith(eventFactoryMock.create, { + pipelineId, + type: 'pipeline', + webhooks: true, + username, + scmContext, + sha, + configPipelineSha: latestSha, + startFrom: '~commit', + baseBranch: 'master', + causeMessage: `Merged by ${username}`, + changedFiles, + releaseName: undefined, + ref: undefined, + meta: {}, + subscribedEvent: true, + subscribedScmConfig: scmConfigSubscribe + }); + }); + }); + it('returns 204 when no pipeline', () => { pipelineFactoryMock.get.resolves(null); pipelineFactoryMock.list.resolves([]); @@ -1812,6 +1859,47 @@ describe('webhooks plugin test', () => { }); }); + it('returns 201 when the hook source triggers subscribed event', () => { + pipelineFactoryMock.scm.parseUrl + .withArgs({ checkoutUrl: fullCheckoutUrl, token, scmContext }) + .resolves('github.com:789123:master'); + pipelineFactoryMock.list.resolves([]); + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: '%github.com:789123:%' } }) + .resolves([pipelineMock]); + const scmConfigSubscribe = { + scmUri: 'github.com:789123:master', + token, + scmContext, + prNum: 2 + }; + + return server.inject(options).then(reply => { + assert.equal(reply.statusCode, 201); + assert.calledWith(eventFactoryMock.create, { + prInfo, + pipelineId, + type: 'pr', + webhooks: true, + username, + scmContext, + sha, + configPipelineSha: latestSha, + startFrom: '~pr', + prNum: 2, + prTitle: 'Update the README with new information', + prRef, + prSource: 'branch', + changedFiles, + causeMessage: `Opened by ${scmDisplayName}:${username}`, + chainPR: false, + baseBranch: 'master', + subscribedEvent: true, + subscribedScmConfig: scmConfigSubscribe + }); + }); + }); + it('returns 201 when getCommitSha() is rejected', () => { pipelineFactoryMock.scm.getCommitSha.rejects(new Error('some error')); @@ -2217,6 +2305,10 @@ describe('webhooks plugin test', () => { it('has the workflow for stopping builds before starting a new one', () => { const abortMsg = 'Aborted because new commit was pushed to PR#1'; + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .resolves([]); + return server.inject(options).then(reply => { assert.calledOnce(model1.update); assert.calledOnce(model2.update); @@ -2448,13 +2540,18 @@ describe('webhooks plugin test', () => { pipelineFactoryMock.scm.parseHook.withArgs(reqHeaders, options.payload).resolves(parsed); }); - it('returns 200 on success', () => - server.inject(options).then(reply => { + it('returns 200 on success', () => { + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .resolves([]); + + return server.inject(options).then(reply => { assert.equal(reply.statusCode, 200); assert.calledOnce(jobMock.update); assert.strictEqual(jobMock.state, 'ENABLED'); assert.isTrue(jobMock.archived); - })); + }); + }); it('returns 204 when pipeline to be closed does not exist', () => { pipelineFactoryMock.list.resolves([]); @@ -2464,18 +2561,26 @@ describe('webhooks plugin test', () => { }); }); - it('stops running builds', () => - server.inject(options).then(() => { + it('stops running builds', () => { + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .resolves([]); + + return server.inject(options).then(() => { assert.calledOnce(model1.update); assert.calledOnce(model2.update); assert.strictEqual(model1.status, 'ABORTED'); assert.strictEqual(model1.statusMessage, 'Aborted because PR#1 was closed'); assert.strictEqual(model2.status, 'ABORTED'); assert.strictEqual(model2.statusMessage, 'Aborted because PR#1 was closed'); - })); + }); + }); it('returns 500 when failed', () => { jobMock.update.rejects(new Error('Failed to update')); + pipelineFactoryMock.list + .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .resolves([]); return server.inject(options).then(reply => { assert.equal(reply.statusCode, 500); From dad09bd04e3feeecb81e882eecc694a4bbef8b61 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Fri, 28 Aug 2020 16:50:00 +0530 Subject: [PATCH 04/17] feat(1415): Configure subscribed jobs to be triggered wrt to startFrom --- plugins/webhooks/index.js | 18 +++++++++++------- test/plugins/webhooks.test.js | 4 ++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index b2c6a66f5..eb618a51b 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -421,12 +421,6 @@ async function createPREvents(options, request) { baseBranch: branch }; - // Check is the webhook event is from a subscribed repo - if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) { - eventConfig.subscribedEvent = true; - eventConfig.subscribedScmConfig = scmConfig; - } - if (skipMessage) { eventConfig.skipMessage = skipMessage; } @@ -435,6 +429,14 @@ async function createPREvents(options, request) { eventConfig.startFrom = '~pr'; } + // Check is the webhook event is from a subscribed repo and + // set the jobs entrypoint from ~startfrom + if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) { + eventConfig.subscribedEvent = true; + eventConfig.subscribedScmConfig = scmConfig; + eventConfig.startFrom = '~subscribe'; + } + return eventConfig; }) ); @@ -851,10 +853,12 @@ async function createEvents( ref }; - // Check is the webhook event is from a subscribed repo + // Check is the webhook event is from a subscribed repo and + // set the jobs entry point to ~subscribe if (uriTrimmer(scmConfigFromHook.scmUri) !== uriTrimmer(pTuple.pipeline.scmUri)) { eventConfig.subscribedEvent = true; eventConfig.subscribedScmConfig = scmConfigFromHook; + eventConfig.startFrom = '~subscribe'; } if (skipMessage) { diff --git a/test/plugins/webhooks.test.js b/test/plugins/webhooks.test.js index 39b95087a..2fc895a3f 100644 --- a/test/plugins/webhooks.test.js +++ b/test/plugins/webhooks.test.js @@ -1362,7 +1362,7 @@ describe('webhooks plugin test', () => { scmContext, sha, configPipelineSha: latestSha, - startFrom: '~commit', + startFrom: '~subscribe', baseBranch: 'master', causeMessage: `Merged by ${username}`, changedFiles, @@ -1885,7 +1885,7 @@ describe('webhooks plugin test', () => { scmContext, sha, configPipelineSha: latestSha, - startFrom: '~pr', + startFrom: '~subscribe', prNum: 2, prTitle: 'Update the README with new information', prRef, From 648763f47982b4984dbbaef25ae053a59c953f88 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Sat, 29 Aug 2020 12:41:41 +0530 Subject: [PATCH 05/17] fix(1415): Fix update-sync sequence in update pipeline --- plugins/pipelines/update.js | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/plugins/pipelines/update.js b/plugins/pipelines/update.js index f1f00f650..68af06167 100644 --- a/plugins/pipelines/update.js +++ b/plugins/pipelines/update.js @@ -45,7 +45,14 @@ module.exports = () => ({ security: [{ token: [] }] } }, +<<<<<<< HEAD handler: async (request, h) => { +======= + // eslint-disable-next-line max-statements + // eslint-disable-next-line max-lines-per-function + // eslint-disable-next-line max-statements + handler: (request, reply) => { +>>>>>>> 85389c2... fix(1415): Fix update-sync sequence in update pipeline const checkoutUrl = helper.formatCheckoutUrl(request.payload.checkoutUrl); const rootDir = helper.sanitizeRootDir(request.payload.rootDir); const { id } = request.params; @@ -64,6 +71,7 @@ module.exports = () => ({ return ( Promise.all([pipelineFactory.get({ id }), userFactory.get({ username, scmContext })]) // get the pipeline given its ID and the user + // eslint-disable-next-line max-lines-per-function .then(([oldPipeline, user]) => { // if the pipeline ID is invalid, reject if (!oldPipeline) { @@ -135,17 +143,15 @@ module.exports = () => ({ oldPipeline.name = scmRepo.name; // update pipeline with new scmRepo and branch - return oldPipeline - .update() - .then(updatedPipeline => - Promise.all([ - updatedPipeline.sync(), - updatedPipeline.addWebhook( - `${request.server.info.uri}/v4/webhooks` - ) - ]) - ) - .then(results => h.response(results[0].toJson()).code(200)); + return oldPipeline.update().then(async updatedPipeline => { + await updatedPipeline.addWebhooks( + `${request.server.info.uri}/v4/webhooks` + ); + + const result = await updatedPipeline.sync(); + + return reply(result.toJson()).code(200); + }); }) ) ); From 5ebd3d1a182ffd5a4ec8d6a98001e712c260092d Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Sat, 29 Aug 2020 12:42:50 +0530 Subject: [PATCH 06/17] fix(1415): fix tests for the pipeline plugin --- test/plugins/pipelines.test.js | 42 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/plugins/pipelines.test.js b/test/plugins/pipelines.test.js index 9d09a2b15..dab931ec9 100644 --- a/test/plugins/pipelines.test.js +++ b/test/plugins/pipelines.test.js @@ -79,7 +79,7 @@ const decoratePipelineMock = pipeline => { const mock = hoek.clone(pipeline); mock.sync = sinon.stub(); - mock.addWebhook = sinon.stub(); + mock.addWebhooks = sinon.stub(); mock.syncPRs = sinon.stub(); mock.update = sinon.stub(); mock.toJson = sinon.stub().returns(pipeline); @@ -1304,7 +1304,7 @@ describe('pipeline plugin test', () => { }); it('returns 500 when model returns an error', () => { - pipelineMock.addWebhook.rejects(new Error('icantdothatdave')); + pipelineMock.addWebhooks.rejects(new Error('icantdothatdave')); return server.inject(options).then(reply => { assert.equal(reply.statusCode, 500); @@ -1436,7 +1436,7 @@ describe('pipeline plugin test', () => { pipelineMock = getPipelineMocks(testPipeline); pipelineMock.sync.resolves(pipelineMock); - pipelineMock.addWebhook.resolves(null); + pipelineMock.addWebhooks.resolves(null); pipelineFactoryMock.get.resolves(null); pipelineFactoryMock.create.resolves(pipelineMock); @@ -1653,7 +1653,7 @@ describe('pipeline plugin test', () => { }) .resolves([getCollectionMock(testDefaultCollection)]); - pipelineMock.addWebhook.rejects(testError); + pipelineMock.addWebhooks.rejects(testError); return server.inject(options).then(reply => { assert.equal(reply.statusCode, 500); @@ -1704,7 +1704,7 @@ describe('pipeline plugin test', () => { pipelineMock = getPipelineMocks(testPipeline); updatedPipelineMock = hoek.clone(pipelineMock); - updatedPipelineMock.addWebhook.resolves(null); + updatedPipelineMock.addWebhooks.resolves(null); pipelineFactoryMock.get.withArgs({ id }).resolves(pipelineMock); pipelineFactoryMock.get.withArgs({ scmUri }).resolves(null); @@ -1719,7 +1719,7 @@ describe('pipeline plugin test', () => { it('returns 200 and correct pipeline data', () => server.inject(options).then(reply => { assert.calledOnce(pipelineMock.update); - assert.calledOnce(updatedPipelineMock.addWebhook); + assert.calledOnce(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 200); })); @@ -1733,7 +1733,7 @@ describe('pipeline plugin test', () => { return server.inject(options).then(reply => { assert.calledOnce(pipelineMock.update); - assert.calledOnce(updatedPipelineMock.addWebhook); + assert.calledOnce(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 200); }); }); @@ -1790,7 +1790,7 @@ describe('pipeline plugin test', () => { pipelineFactoryMock.get.withArgs({ id }).resolves(null); return server.inject(options).then(reply => { - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 404); }); }); @@ -1799,7 +1799,7 @@ describe('pipeline plugin test', () => { pipelineMock.configPipelineId = 123; return server.inject(options).then(reply => { - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 403); }); }); @@ -1808,7 +1808,7 @@ describe('pipeline plugin test', () => { userMock.getPermissions.withArgs(scmUri).resolves({ admin: false }); return server.inject(options).then(reply => { - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 403); }); }); @@ -1817,20 +1817,20 @@ describe('pipeline plugin test', () => { userMock.getPermissions.withArgs(oldScmUri).resolves({ admin: false }); return server.inject(options).then(reply => { - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 403); }); }); it('returns 200 when the user is admin of old repo with deprecated scmContext', () => { pipelineMock.admins = { [username]: true }; - pipelineMock.scmContext = 'depreacated'; + pipelineMock.scmContext = 'deprecated'; return server.inject(options).then(reply => { // Only call once to get permissions on the new repo assert.calledOnce(userMock.getPermissions); assert.calledWith(userMock.getPermissions, scmUri); - assert.calledOnce(updatedPipelineMock.addWebhook); + assert.calledOnce(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 200); }); }); @@ -1843,7 +1843,7 @@ describe('pipeline plugin test', () => { // Only call once to get permissions on the new repo assert.calledOnce(userMock.getPermissions); assert.calledWith(userMock.getPermissions, scmUri); - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 403); }); }); @@ -1857,7 +1857,7 @@ describe('pipeline plugin test', () => { }; return server.inject(options).then(reply => { - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 401); }); }); @@ -1867,7 +1867,7 @@ describe('pipeline plugin test', () => { return server.inject(options).then(reply => { assert.equal(reply.statusCode, 409); - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.strictEqual(reply.result.message, `Pipeline already exists with the ID: ${pipelineMock.id}`); }); }); @@ -1878,7 +1878,7 @@ describe('pipeline plugin test', () => { pipelineFactoryMock.get.withArgs({ id }).rejects(testError); return server.inject(options).then(reply => { - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 500); }); }); @@ -1889,7 +1889,7 @@ describe('pipeline plugin test', () => { pipelineMock.update.rejects(testError); return server.inject(options).then(reply => { - assert.notCalled(updatedPipelineMock.addWebhook); + assert.notCalled(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 500); }); }); @@ -1900,7 +1900,7 @@ describe('pipeline plugin test', () => { pipelineMock.sync.rejects(testError); return server.inject(options).then(reply => { - assert.calledOnce(updatedPipelineMock.addWebhook); + assert.calledOnce(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 500); }); }); @@ -1908,10 +1908,10 @@ describe('pipeline plugin test', () => { it('returns 500 when the pipeline model fails to add webhooks during create', () => { const testError = new Error('pipelineModelAddWebhookError'); - updatedPipelineMock.addWebhook.rejects(testError); + updatedPipelineMock.addWebhooks.rejects(testError); return server.inject(options).then(reply => { - assert.calledOnce(updatedPipelineMock.addWebhook); + assert.calledOnce(updatedPipelineMock.addWebhooks); assert.equal(reply.statusCode, 500); }); }); From 5b8f9c8e2e5c284f22507828eb238df066919510 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Wed, 2 Sep 2020 22:40:18 +0530 Subject: [PATCH 07/17] feat(1415): Add improved subscribed pr handling --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 64450431c..eb89fec63 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "main": "index.js", "scripts": { "pretest": "eslint --max-warnings 550 . --quiet", - "test": "nyc --report-dir ./artifacts/coverage --reporter=lcov mocha --recursive --timeout 10000 --retries 1 --exit --allow-uncaught true --color true", + "test": "nyc --report-dir ./artifacts/coverage --reporter=lcov mocha ./test/plugins/webhooks.test.js --timeout 10000 --retries 1 --exit --allow-uncaught true --color true", "start": "./bin/server", "debug": "node --nolazy ./bin/server", "profile": "node --prof ./bin/server", From 7b86e0dd29e1504413841fd5d85a58e249d4df18 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Wed, 2 Sep 2020 22:42:44 +0530 Subject: [PATCH 08/17] fix(1415): Fix subscribed PR webhook events and tests --- plugins/webhooks/index.js | 191 ++++++++++++++++++++++++---------- test/plugins/webhooks.test.js | 44 ++++---- 2 files changed, 155 insertions(+), 80 deletions(-) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index eb618a51b..b084de830 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -356,7 +356,6 @@ async function createPREvents(options, request) { const { username, scmConfig, - sha, prRef, prNum, pipelines, @@ -366,14 +365,19 @@ async function createPREvents(options, request) { action, prSource, restrictPR, - chainPR + chainPR, + ref, + releaseName, + meta } = options; const { scm } = request.server.app.pipelineFactory; const { eventFactory } = request.server.app; const { pipelineFactory } = request.server.app; + const { userFactory } = request.server.app; const scmDisplayName = scm.getDisplayName({ scmContext: scmConfig.scmContext }); const userDisplayName = `${scmDisplayName}:${username}`; const events = []; + let { sha } = options; scmConfig.prNum = prNum; @@ -382,14 +386,38 @@ async function createPREvents(options, request) { const b = await p.branch; // obtain pipeline's latest commit sha for branch specific job let configPipelineSha = ''; + let subscribedConfigSha = ''; + let eventConfig = {}; - try { - configPipelineSha = await pipelineFactory.scm.getCommitSha(scmConfig); - } catch (err) { - if (err.status >= 500) { - throw err; - } else { - logger.info(`skip create event for branch: ${b}`); + // Check is the webhook event is from a subscribed repo and + // and fetch the source repo commit sha and save the subscribed sha + if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) { + subscribedConfigSha = sha; + + try { + sha = await pipelineFactory.scm.getCommitSha({ + scmUri: p.scmUri, + scmContext: scmConfig.scmContext, + token: scmConfig.token + }); + } catch (err) { + if (err.status >= 500) { + throw err; + } else { + logger.info(`skip create event for branch: ${b}`); + } + } + + configPipelineSha = sha; + } else { + try { + configPipelineSha = await pipelineFactory.scm.getCommitSha(scmConfig); + } catch (err) { + if (err.status >= 500) { + throw err; + } else { + logger.info(`skip create event for branch: ${b}`); + } } } @@ -401,7 +429,9 @@ async function createPREvents(options, request) { chainPR }); - const eventConfig = { + const prInfo = await eventFactory.scm.getPrInfo(scmConfig); + + eventConfig = { pipelineId: p.id, type: 'pr', webhooks: true, @@ -416,25 +446,45 @@ async function createPREvents(options, request) { prRef, prNum, prTitle, - prInfo: await eventFactory.scm.getPrInfo(scmConfig), + prInfo, prSource, baseBranch: branch }; - if (skipMessage) { - eventConfig.skipMessage = skipMessage; - } - if (b === branch) { eventConfig.startFrom = '~pr'; } // Check is the webhook event is from a subscribed repo and // set the jobs entrypoint from ~startfrom + // For subscribed PR event, it should be mimiced as a commit + // in order to function properly if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) { - eventConfig.subscribedEvent = true; - eventConfig.subscribedScmConfig = scmConfig; - eventConfig.startFrom = '~subscribe'; + eventConfig = { + pipelineId: p.id, + type: 'pipeline', + webhooks: true, + username, + scmContext: scmConfig.scmContext, + startFrom: '~subscribe', + sha, + configPipelineSha, + changedFiles, + baseBranch: branch, + causeMessage: `Merged by ${username}`, + meta, + releaseName, + ref, + subscribedEvent: true, + subscribedConfigSha, + subscribedSourceUrl: prInfo.url + }; + + await updateAdmins(userFactory, username, scmConfig.scmContext, p.id); + } + + if (skipMessage) { + eventConfig.skipMessage = skipMessage; } return eventConfig; @@ -603,6 +653,41 @@ async function obtainScmToken(pluginOptions, userFactory, username, scmContext) return user.unsealToken(); } +/** + * Create metadata by the parsed event + * @param {Object} parsed It has information to create metadata + * @returns {Object} Metadata + */ +function createMeta(parsed) { + const { action, ref, releaseId, releaseName, releaseAuthor } = parsed; + + if (action === 'release') { + return { + sd: { + release: { + id: releaseId, + name: releaseName, + author: releaseAuthor + }, + tag: { + name: ref + } + } + }; + } + if (action === 'tag') { + return { + sd: { + tag: { + name: ref + } + } + }; + } + + return {}; +} + /** * Act on a Pull Request change (create, sync, close) * - Opening a PR should sync the pipeline (creating the job) and start the new PR job @@ -645,6 +730,7 @@ function pullRequestEvent(pluginOptions, request, h, parsed, token) { scmContext }; const { restrictPR, chainPR } = pluginOptions; + const meta = createMeta(parsed); request.log(['webhook', hookId], `PR #${prNum} ${action} for ${fullCheckoutUrl}`); @@ -684,7 +770,10 @@ function pullRequestEvent(pluginOptions, request, h, parsed, token) { fullCheckoutUrl, restrictPR, chainPR, - pipelines + pipelines, + ref, + releaseName, + meta }; await batchUpdateAdmins({ userFactory, pipelines, username, scmContext }); @@ -707,41 +796,6 @@ function pullRequestEvent(pluginOptions, request, h, parsed, token) { }); } -/** - * Create metadata by the parsed event - * @param {Object} parsed It has information to create metadata - * @returns {Object} Metadata - */ -function createMeta(parsed) { - const { action, ref, releaseId, releaseName, releaseAuthor } = parsed; - - if (action === 'release') { - return { - sd: { - release: { - id: releaseId, - name: releaseName, - author: releaseAuthor - }, - tag: { - name: ref - } - } - }; - } - if (action === 'tag') { - return { - sd: { - tag: { - name: ref - } - } - }; - } - - return {}; -} - /** * Create events for each pipeline * @async createEvents @@ -857,8 +911,35 @@ async function createEvents( // set the jobs entry point to ~subscribe if (uriTrimmer(scmConfigFromHook.scmUri) !== uriTrimmer(pTuple.pipeline.scmUri)) { eventConfig.subscribedEvent = true; - eventConfig.subscribedScmConfig = scmConfigFromHook; eventConfig.startFrom = '~subscribe'; + eventConfig.subscribedConfigSha = eventConfig.sha; + + try { + eventConfig.sha = await pipelineFactory.scm.getCommitSha(scmConfig); + } catch (err) { + if (err.status >= 500) { + throw err; + } else { + logger.info(`skip create event for this subscribed trigger`); + } + } + + try { + const commitInfo = await pipelineFactory.scm.decorateCommit({ + scmUri: scmConfigFromHook.scmUri, + scmContext, + sha: eventConfig.subscribedConfigSha, + token + }); + + eventConfig.subscribedSourceUrl = commitInfo.url; + } catch (err) { + if (err.status >= 500) { + throw err; + } else { + logger.info(`skip create event for this subscribed trigger`); + } + } } if (skipMessage) { diff --git a/test/plugins/webhooks.test.js b/test/plugins/webhooks.test.js index 2fc895a3f..b2aee981a 100644 --- a/test/plugins/webhooks.test.js +++ b/test/plugins/webhooks.test.js @@ -1346,11 +1346,6 @@ describe('webhooks plugin test', () => { pipelineFactoryMock.list .withArgs({ search: { field: 'subscribedScmUrls', keyword: '%github.com:789123:%' } }) .resolves([pipelineMock]); - const scmConfigSubscribe = { - scmUri: 'github.com:789123:master', - token, - scmContext - }; return server.inject(options).then(reply => { assert.equal(reply.statusCode, 201); @@ -1360,8 +1355,9 @@ describe('webhooks plugin test', () => { webhooks: true, username, scmContext, - sha, + sha: latestSha, configPipelineSha: latestSha, + subscribedConfigSha: sha, startFrom: '~subscribe', baseBranch: 'master', causeMessage: `Merged by ${username}`, @@ -1369,8 +1365,7 @@ describe('webhooks plugin test', () => { releaseName: undefined, ref: undefined, meta: {}, - subscribedEvent: true, - subscribedScmConfig: scmConfigSubscribe + subscribedEvent: true }); }); }); @@ -1864,38 +1859,37 @@ describe('webhooks plugin test', () => { .withArgs({ checkoutUrl: fullCheckoutUrl, token, scmContext }) .resolves('github.com:789123:master'); pipelineFactoryMock.list.resolves([]); + pipelineMock.baxterthehacker = 'master'; + pipelineMock.admins = { + baxterthehacker: true + }; pipelineFactoryMock.list .withArgs({ search: { field: 'subscribedScmUrls', keyword: '%github.com:789123:%' } }) .resolves([pipelineMock]); - const scmConfigSubscribe = { - scmUri: 'github.com:789123:master', - token, - scmContext, - prNum: 2 - }; + eventFactoryMock.scm.getPrInfo.resolves({ + url: 'foo' + }); return server.inject(options).then(reply => { assert.equal(reply.statusCode, 201); assert.calledWith(eventFactoryMock.create, { - prInfo, pipelineId, - type: 'pr', + type: 'pipeline', webhooks: true, username, scmContext, - sha, + sha: latestSha, configPipelineSha: latestSha, + subscribedConfigSha: sha, startFrom: '~subscribe', - prNum: 2, - prTitle: 'Update the README with new information', - prRef, - prSource: 'branch', - changedFiles, - causeMessage: `Opened by ${scmDisplayName}:${username}`, - chainPR: false, baseBranch: 'master', + causeMessage: `Merged by ${username}`, + changedFiles, + releaseName: undefined, + ref: undefined, + meta: {}, subscribedEvent: true, - subscribedScmConfig: scmConfigSubscribe + subscribedSourceUrl: 'foo' }); }); }); From 9d107caf14235650181f72d6de3fd343f1249ac1 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 3 Sep 2020 01:01:11 +0530 Subject: [PATCH 09/17] cleanup(1415): remove leftout commit texts --- plugins/pipelines/update.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/plugins/pipelines/update.js b/plugins/pipelines/update.js index 68af06167..7ab3aafbb 100644 --- a/plugins/pipelines/update.js +++ b/plugins/pipelines/update.js @@ -45,14 +45,7 @@ module.exports = () => ({ security: [{ token: [] }] } }, -<<<<<<< HEAD handler: async (request, h) => { -======= - // eslint-disable-next-line max-statements - // eslint-disable-next-line max-lines-per-function - // eslint-disable-next-line max-statements - handler: (request, reply) => { ->>>>>>> 85389c2... fix(1415): Fix update-sync sequence in update pipeline const checkoutUrl = helper.formatCheckoutUrl(request.payload.checkoutUrl); const rootDir = helper.sanitizeRootDir(request.payload.rootDir); const { id } = request.params; @@ -150,7 +143,7 @@ module.exports = () => ({ const result = await updatedPipeline.sync(); - return reply(result.toJson()).code(200); + return h.response(result.toJson()).code(200); }); }) ) From 253027b8e15e9b3f3fdf5a759eb23620e5b11917 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 3 Sep 2020 01:11:00 +0530 Subject: [PATCH 10/17] fix(1415): bump back faulty package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index eb89fec63..64450431c 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "main": "index.js", "scripts": { "pretest": "eslint --max-warnings 550 . --quiet", - "test": "nyc --report-dir ./artifacts/coverage --reporter=lcov mocha ./test/plugins/webhooks.test.js --timeout 10000 --retries 1 --exit --allow-uncaught true --color true", + "test": "nyc --report-dir ./artifacts/coverage --reporter=lcov mocha --recursive --timeout 10000 --retries 1 --exit --allow-uncaught true --color true", "start": "./bin/server", "debug": "node --nolazy ./bin/server", "profile": "node --prof ./bin/server", From e635c8deab94fc90f672ead7ee6d8e49fe2fc50a Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 3 Sep 2020 03:36:02 +0530 Subject: [PATCH 11/17] fix(1415): fix return object in create pipeline --- plugins/pipelines/create.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/pipelines/create.js b/plugins/pipelines/create.js index ebe95c8e1..0f09ea132 100644 --- a/plugins/pipelines/create.js +++ b/plugins/pipelines/create.js @@ -123,7 +123,7 @@ module.exports = () => ({ protocol: request.server.info.protocol, pathname: `${request.path}/${pipeline.id}` }); - const data = await results[0].toJson(); + const data = await results.toJson(); return h .response(data) From a6bf0f624ab1836b10a1a82957f41c74e60d5223 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Fri, 4 Sep 2020 17:34:08 +0530 Subject: [PATCH 12/17] fix(1415): Remove stray eslint comments and add docstrings --- plugins/webhooks/index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index b084de830..a905a06fe 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -1,6 +1,3 @@ -/* eslint-disable max-lines-per-function */ -/* eslint-disable max-params */ - 'use strict'; const joi = require('joi'); @@ -253,6 +250,11 @@ function getSkipMessageAndChainPR({ pipeline, prSource, restrictPR, chainPR }) { return result; } +/** + * Returns the uri keeping only the host and the repo ID + * @param {String} uri The uri to be trimmed + * @return {String} + */ const uriTrimmer = uri => { const uriToArray = uri.split(':'); @@ -389,7 +391,7 @@ async function createPREvents(options, request) { let subscribedConfigSha = ''; let eventConfig = {}; - // Check is the webhook event is from a subscribed repo and + // Check if the webhook event is from a subscribed repo and // and fetch the source repo commit sha and save the subscribed sha if (uriTrimmer(scmConfig.scmUri) !== uriTrimmer(p.scmUri)) { subscribedConfigSha = sha; @@ -455,7 +457,7 @@ async function createPREvents(options, request) { eventConfig.startFrom = '~pr'; } - // Check is the webhook event is from a subscribed repo and + // Check if the webhook event is from a subscribed repo and // set the jobs entrypoint from ~startfrom // For subscribed PR event, it should be mimiced as a commit // in order to function properly From 7e36fc0e00c74b1764e322f3ec79e4d1a0dd754e Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Fri, 4 Sep 2020 21:55:54 +0530 Subject: [PATCH 13/17] fix(1415): Add method name to docstring comment --- plugins/webhooks/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index a905a06fe..1772ce28e 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -252,6 +252,7 @@ function getSkipMessageAndChainPR({ pipeline, prSource, restrictPR, chainPR }) { /** * Returns the uri keeping only the host and the repo ID + * @method uriTrimmer * @param {String} uri The uri to be trimmed * @return {String} */ From 6ac1ab41ae76c4384166f0c3cd195b5f219d7849 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Sun, 6 Sep 2020 20:57:58 +0530 Subject: [PATCH 14/17] fix(1415): Refactor subscribed search query with subscribedScmUrlsWithActions key --- plugins/webhooks/index.js | 2 +- test/plugins/webhooks.test.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index 1772ce28e..d2d97ec33 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -292,7 +292,7 @@ async function triggeredPipelines( const scmBranch = `${splitUri[0]}:${splitUri[1]}:${splitUri[2]}`; const scmRepoId = `${splitUri[0]}:${splitUri[1]}`; const listConfig = { search: { field: 'scmUri', keyword: `${scmRepoId}:%` } }; - const externalRepoSearchConfig = { search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }; + const externalRepoSearchConfig = { search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }; const pipelines = await pipelineFactory.list(listConfig); diff --git a/test/plugins/webhooks.test.js b/test/plugins/webhooks.test.js index b2aee981a..552d8d550 100644 --- a/test/plugins/webhooks.test.js +++ b/test/plugins/webhooks.test.js @@ -1864,7 +1864,9 @@ describe('webhooks plugin test', () => { baxterthehacker: true }; pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: '%github.com:789123:%' } }) + .withArgs({ + search: { field: 'subscribedScmUrlsWithActions', keyword: '%github.com:789123:%' } + }) .resolves([pipelineMock]); eventFactoryMock.scm.getPrInfo.resolves({ url: 'foo' @@ -2300,7 +2302,7 @@ describe('webhooks plugin test', () => { const abortMsg = 'Aborted because new commit was pushed to PR#1'; pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }) .resolves([]); return server.inject(options).then(reply => { From 3d784f851cd204c1f30a560a8bee42ca7c75e32f Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 24 Sep 2020 03:53:55 +0530 Subject: [PATCH 15/17] fix(1415): Fix tests for webhooks --- test/plugins/webhooks.test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/plugins/webhooks.test.js b/test/plugins/webhooks.test.js index 552d8d550..cfd30461f 100644 --- a/test/plugins/webhooks.test.js +++ b/test/plugins/webhooks.test.js @@ -581,7 +581,7 @@ describe('webhooks plugin test', () => { pipelineMock.jobs = Promise.resolve([mainJobMock, jobMock]); pipelineFactoryMock.scm.parseHook.withArgs(reqHeaders, payload).resolves(parsed); pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }) .resolves([pipelineMock]); pipelineFactoryMock.list.resolves([pipelineMock]); }); @@ -1144,7 +1144,7 @@ describe('webhooks plugin test', () => { pipelineFactoryMock.list.resolves([pipelineMock, pMock1, pMock2, pMock3]); pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }) .resolves([]); return server.inject(options).then(reply => { @@ -1244,7 +1244,7 @@ describe('webhooks plugin test', () => { pipelineFactoryMock.scm.getChangedFiles.resolves(['lib/test.js']); pipelineFactoryMock.list.resolves([pipelineMock, pMock1, pMock2]); pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }) .resolves([]); return server.inject(options).then(reply => { @@ -1344,7 +1344,7 @@ describe('webhooks plugin test', () => { .resolves('github.com:789123:master'); pipelineFactoryMock.list.resolves([]); pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: '%github.com:789123:%' } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: '%github.com:789123:%' } }) .resolves([pipelineMock]); return server.inject(options).then(reply => { @@ -2538,7 +2538,7 @@ describe('webhooks plugin test', () => { it('returns 200 on success', () => { pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }) .resolves([]); return server.inject(options).then(reply => { @@ -2559,7 +2559,7 @@ describe('webhooks plugin test', () => { it('stops running builds', () => { pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }) .resolves([]); return server.inject(options).then(() => { @@ -2575,7 +2575,7 @@ describe('webhooks plugin test', () => { it('returns 500 when failed', () => { jobMock.update.rejects(new Error('Failed to update')); pipelineFactoryMock.list - .withArgs({ search: { field: 'subscribedScmUrls', keyword: `%${scmRepoId}:%` } }) + .withArgs({ search: { field: 'subscribedScmUrlsWithActions', keyword: `%${scmRepoId}:%` } }) .resolves([]); return server.inject(options).then(reply => { From 5e6d33482a30c6c09d5d25ecc29c6edd505c6c3f Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 1 Oct 2020 23:30:07 +0530 Subject: [PATCH 16/17] fix(1415): Minor code formatting --- plugins/webhooks/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index d2d97ec33..8397b4934 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -980,9 +980,7 @@ async function createEvents( * @param {String} [skipMessage] Message to skip starting builds */ async function pushEvent(request, h, parsed, skipMessage, token) { - const { eventFactory } = request.server.app; - const { pipelineFactory } = request.server.app; - const { userFactory } = request.server.app; + const { eventFactory, pipelineFactory, userFactory } = request.server.app; const { hookId, checkoutUrl, branch, scmContext, type, action, changedFiles, releaseName, ref } = parsed; const fullCheckoutUrl = `${checkoutUrl}#${branch}`; const scmConfig = { From b68ace6b0e96fbec666d09ccb043e4cd02480b29 Mon Sep 17 00:00:00 2001 From: Supratik Das Date: Thu, 8 Oct 2020 03:22:24 +0530 Subject: [PATCH 17/17] cleanup(1415): Minor code cleanup --- plugins/pipelines/update.js | 1 - plugins/webhooks/index.js | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/plugins/pipelines/update.js b/plugins/pipelines/update.js index 7ab3aafbb..b181ed089 100644 --- a/plugins/pipelines/update.js +++ b/plugins/pipelines/update.js @@ -64,7 +64,6 @@ module.exports = () => ({ return ( Promise.all([pipelineFactory.get({ id }), userFactory.get({ username, scmContext })]) // get the pipeline given its ID and the user - // eslint-disable-next-line max-lines-per-function .then(([oldPipeline, user]) => { // if the pipeline ID is invalid, reject if (!oldPipeline) { diff --git a/plugins/webhooks/index.js b/plugins/webhooks/index.js index 8397b4934..84f19cbf4 100644 --- a/plugins/webhooks/index.js +++ b/plugins/webhooks/index.js @@ -374,9 +374,7 @@ async function createPREvents(options, request) { meta } = options; const { scm } = request.server.app.pipelineFactory; - const { eventFactory } = request.server.app; - const { pipelineFactory } = request.server.app; - const { userFactory } = request.server.app; + const { eventFactory, pipelineFactory, userFactory } = request.server.app; const scmDisplayName = scm.getDisplayName({ scmContext: scmConfig.scmContext }); const userDisplayName = `${scmDisplayName}:${username}`; const events = [];