From 067d3d612eaa03ae883d4fd3ce03c84979b6fa7a Mon Sep 17 00:00:00 2001 From: kkawahar Date: Tue, 13 Oct 2020 15:10:36 +0900 Subject: [PATCH 1/4] fix: pipeline.addWebhook to pipeline.addWebhooks --- 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 f2e5220fe..4e1e2529f 100644 --- a/plugins/pipelines/create.js +++ b/plugins/pipelines/create.js @@ -115,7 +115,7 @@ module.exports = () => ({ const results = await Promise.all([ pipeline.sync(), - pipeline.addWebhook(`${request.server.info.uri}/v4/webhooks`) + pipeline.addWebhooks(`${request.server.info.uri}/v4/webhooks`) ]); const location = urlLib.format({ From 0d9bd55be21a5a2767e55a3159230acbae202a74 Mon Sep 17 00:00:00 2001 From: kkawahar Date: Tue, 13 Oct 2020 15:14:00 +0900 Subject: [PATCH 2/4] fix: test --- test/plugins/pipelines.test.js | 40 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/test/plugins/pipelines.test.js b/test/plugins/pipelines.test.js index 9d09a2b15..46c0c3d96 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,7 +1817,7 @@ 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); }); }); @@ -1830,7 +1830,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.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 2c3816811e52f3bdc09945f17d19ce23d193f089 Mon Sep 17 00:00:00 2001 From: kkawahar Date: Tue, 13 Oct 2020 15:16:27 +0900 Subject: [PATCH 3/4] fix: models version 28.3.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4566177dd..698f092cc 100644 --- a/package.json +++ b/package.json @@ -111,7 +111,7 @@ "screwdriver-executor-queue": "^3.0.0", "screwdriver-executor-router": "^2.0.0", "screwdriver-logger": "^1.0.1", - "screwdriver-models": "^28.0.0", + "screwdriver-models": "^28.3.0", "screwdriver-notifications-email": "^2.0.0", "screwdriver-notifications-slack": "^3.0.0", "screwdriver-scm-base": "^7.0.0", From 3ed89c12efed185e036177b04e3bbf83aa476ef2 Mon Sep 17 00:00:00 2001 From: Kenta Kawaharada Date: Tue, 13 Oct 2020 15:41:40 +0900 Subject: [PATCH 4/4] fix: review --- plugins/pipelines/syncWebhooks.js | 2 +- plugins/pipelines/update.js | 2 +- test/plugins/caches.test.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) 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)) ); }) diff --git a/plugins/pipelines/update.js b/plugins/pipelines/update.js index f1f00f650..cf4106fc5 100644 --- a/plugins/pipelines/update.js +++ b/plugins/pipelines/update.js @@ -140,7 +140,7 @@ module.exports = () => ({ .then(updatedPipeline => Promise.all([ updatedPipeline.sync(), - updatedPipeline.addWebhook( + updatedPipeline.addWebhooks( `${request.server.info.uri}/v4/webhooks` ) ]) diff --git a/test/plugins/caches.test.js b/test/plugins/caches.test.js index ba1bac951..99e4dd8a7 100644 --- a/test/plugins/caches.test.js +++ b/test/plugins/caches.test.js @@ -15,7 +15,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);