Repository: screwdriver-cd/screwdriver

Issue:
PR jobs are not archived when a PR is closed
**What happened**:
For PR jobs triggered by branch filtering, there are cases where the job remains active and does not become `archived` even after the PR that triggered it is closed.

**What you expected to happen**:
When the PR is closed, all related PR jobs are archived.

**How to reproduce it**:
1. There is a pipeline that has pr jobs with branch filtering trigger.
2. Create a PR to the branch that is the target of branch filtering, not to a pipeline branch.
3. PR jobs are executed.
4. Close PR.
5. Triggered PR jobs are not archived.

Hints from issue comments:

Solution patch:
diff --git a/plugins/webhooks/helper.js b/plugins/webhooks/helper.js
index 4ffaba6..edccae6 100644
--- a/plugins/webhooks/helper.js
+++ b/plugins/webhooks/helper.js
@@ -715,6 +715,53 @@ async function batchStopJobs({ pipelines, prNum, action, name }) {
     await Promise.all(flatPRJobs.map(j => stopJob({ job: j, prNum, action })));
 }
 
+/**
+ * Archive PR jobs when PR closed
+ * @method archivePRJobs
+ * @param  {Object}         scmConfig       Response toolkit
+ * @param  {Hapi.request}   request         Request from user
+ * @param  {String}         prNum           Pull request number
+ * @param  {PipelineFactory}pipelineFactory PipelineFactory object
+ * @param  {String}         hookId          Unique ID for this scm event
+ * @return {Promise}
+ */
+async function archivePRJobs(scmConfig, request, prNum, pipelineFactory, hookId) {
+    const { scmUri } = scmConfig;
+    const splitUri = scmUri.split(':');
+    const scmRepoId = `${splitUri[0]}:${splitUri[1]}`;
+    const listConfig = {
+        search: { field: 'scmUri', keyword: `${scmRepoId}:%` },
+        params: {
+            state: 'ACTIVE'
+        }
+    };
+    const pipelines = await pipelineFactory.list(listConfig);
+    const prJobName = `PR-${prNum}`;
+    const updatePRJobs = job =>
+        stopJob({ job, prNum, action: 'Closed' })
+            .then(() => request.log(['webhook', hookId, job.id], `${job.name} stopped`))
+            .then(() => {
+                job.archived = true;
+
+                return job.update();
+            })
+            .then(() => request.log(['webhook', hookId, job.id], `${job.name} disabled and archived`));
+
+    await Promise.all(
+        pipelines.map(p =>
+            p.getJobs({ type: 'pr' }).then(jobs => {
+                const prJobs = jobs.filter(j => j.name.includes(prJobName));
+
+                return Promise.all(prJobs.map(j => updatePRJobs(j)));
+            })
+        )
+    ).catch(err => {
+        logger.error(`Failed to pullRequestClosed: [${hookId}]: ${err}`);
+
+        throw err;
+    });
+}
+
 /**
  * Create a new job and start the build for an opened pull-request
  * @async  pullRequestOpened
@@ -862,30 +909,10 @@ async function createPrClosedEvent(options, request) {
  * @param  {Hapi.reply}   reply                     Reply to user
  */
 async function pullRequestClosed(options, request, h) {
-    const { pipelines, hookId, name, prNum, action } = options;
-    const updatePRJobs = job =>
-        stopJob({ job, prNum, action })
-            .then(() => request.log(['webhook', hookId, job.id], `${job.name} stopped`))
-            .then(() => {
-                job.archived = true;
-
-                return job.update();
-            })
-            .then(() => request.log(['webhook', hookId, job.id], `${job.name} disabled and archived`));
+    const { pipelines, hookId } = options;
+    const prClosedJobs = [];
 
     try {
-        await Promise.all(
-            pipelines.map(p =>
-                p.getJobs({ type: 'pr' }).then(jobs => {
-                    const prJobs = jobs.filter(j => j.name.includes(name));
-
-                    return Promise.all(prJobs.map(j => updatePRJobs(j)));
-                })
-            )
-        );
-
-        const prClosedJobs = [];
-
         for (const p of pipelines) {
             const jobs = await p.getJobs({ type: 'pipeline' });
             const filteredJobs = jobs.filter(
@@ -1056,6 +1083,9 @@ function pullRequestEvent(pluginOptions, request, h, parsed, token) {
             return triggeredPipelines(pipelineFactory, scmConfig, branch, type, action, changedFiles, releaseName, ref);
         })
         .then(async pipelines => {
+            if (action === 'closed') {
+                await archivePRJobs(scmConfig, request, prNum, pipelineFactory, hookId);
+            }
             if (!pipelines || pipelines.length === 0) {
                 const message = `Skipping since Pipeline triggered by PRs against ${fullCheckoutUrl} does not exist`;
 



Test patch:
diff --git a/test/plugins/webhooks.helper.test.js b/test/plugins/webhooks.helper.test.js
index e7ab061..56428a1 100644
--- a/test/plugins/webhooks.helper.test.js
+++ b/test/plugins/webhooks.helper.test.js
@@ -3155,6 +3155,68 @@ describe('startHookEvent test', () => {
                 });
             });
 
+            it('archives prJobs in multiple pipelines with same repo when PR is closed', () => {
+                const job1 = {
+                    id: 1,
+                    name: 'PR-1:main',
+                    getRunningBuilds: sinon.stub().resolves([]),
+                    update: sinon.stub().resolves(null),
+                    archived: false
+                };
+                const job2 = {
+                    id: 2,
+                    name: 'PR-1:main2',
+                    getRunningBuilds: sinon.stub().resolves([]),
+                    update: sinon.stub().resolves(null),
+                    archived: false
+                };
+                const job3 = {
+                    id: 3,
+                    name: 'PR-2:main',
+                    getRunningBuilds: sinon.stub().resolves([]),
+                    update: sinon.stub().resolves(null),
+                    archived: false
+                };
+                const pMock1 = getPipelineMocks({
+                    id: 'pipelineHash1',
+                    scmUri: 'github.com:123456:branch1',
+                    annotations: {},
+                    admins: {
+                        baxterthehacker: false
+                    },
+                    getJobs: sinon.stub().resolves([job1])
+                });
+                const pMock2 = getPipelineMocks({
+                    id: 'pipelineHash2',
+                    scmUri: 'github.com:123456:branch2',
+                    annotations: {},
+                    admins: {
+                        baxterthehacker: false
+                    },
+                    getJobs: sinon.stub().resolves([job2])
+                });
+
+                pipelineFactoryMock.list
+                    .withArgs({
+                        search: { field: 'scmUri', keyword: `github.com:123456:%` },
+                        params: { state: 'ACTIVE' }
+                    })
+                    .resolves([pipelineMock, pMock1, pMock2]);
+
+                pipelineMock.getJobs.resolves([job1]);
+                pMock1.getJobs.resolves([job2]);
+                pMock2.getJobs.resolves([job3]);
+
+                return startHookEvent(request, responseHandler, parsed).then(reply => {
+                    assert.equal(reply.statusCode, 200);
+                    assert.calledOnce(job1.update);
+                    assert.calledOnce(job2.update);
+                    assert.isTrue(job1.archived);
+                    assert.isTrue(job2.archived);
+                    assert.isFalse(job3.archived);
+                });
+            });
+
             it('throws error when failed', () => {
                 jobMock.update.rejects(new Error('Failed to update'));
                 pipelineFactoryMock.list
@@ -3264,7 +3326,7 @@ describe('startHookEvent test', () => {
                         assert.calledWith(eventFactoryMock.create, expected);
                         assert.equal(reply.statusCode, 201);
                         assert.calledOnce(pipelineMock.update);
-                        assert.calledTwice(pipelineFactoryMock.list);
+                        assert.calledThrice(pipelineFactoryMock.list);
                         assert.calledWith(pipelineFactoryMock.list.firstCall, {
                             params: { state: 'ACTIVE' },
                             search: { field: 'scmUri', keyword: 'github.com:123456:%' }