fix: store the template id properly when PR changes the job template (#…

…634) * fix: use configurated template id for pr-job * fix: tests
screwdriver-cd · Dec 12, 2024 · 582a8fa · 582a8fa
1 parent 49e6390
commit 582a8fa
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 15 deletions.
diff --git a/lib/pipeline.js b/lib/pipeline.js
@@ -466,8 +466,12 @@ class PipelineModel extends BaseModel {
             const jobName = j.parsePRJobName('job') || 'main';
 
             if (parsedConfig) {
-                j.permutations = parsedConfig.jobs[jobName];
-                j.templateId = parsedConfig.jobs[jobName][0].templateId || null;
+                const parsedJob = parsedConfig.jobs[jobName];
+                const jobConfig = parsedJob[0] || {};
+
+                j.permutations = parsedJob;
+                j.templateId = jobConfig.templateId || null;
+                j.description = jobConfig.description || null;
             }
             j.archived = archived;
             if (archived) logger.info(`pipelineId:${this.id}: Archiving ${j.name} job.`);
@@ -682,17 +686,22 @@ class PipelineModel extends BaseModel {
         for (const jobNames of getJobChunks(jobsToCreate)) {
             await Promise.all(
                 jobNames.map(jobName => {
+                    const parsedJob = parsedConfig.jobs[jobName];
                     const jobModel = {
-                        permutations: parsedConfig.jobs[jobName],
+                        permutations: parsedJob,
                         pipelineId: this.id,
                         name: `PR-${prNum}:${jobName}`
                     };
 
+                    const jobConfig = parsedJob[0] || {};
+
+                    // Use current config values
+                    jobModel.templateId = jobConfig.templateId || null;
+                    jobModel.description = jobConfig.description || null;
+
                     // If there is a pr parent
-                    if (prParentJobIdMap[jobName]) {
+                    if (prParentJobIdMap[jobName] && !prParentJobIdMap[jobName].archived) {
                         jobModel.prParentJobId = prParentJobIdMap[jobName].id;
-                        jobModel.templateId = prParentJobIdMap[jobName].templateId;
-                        jobModel.description = prParentJobIdMap[jobName].description;
                     }
 
                     // Create jobs

diff --git a/test/data/parserWithWorkflowGraphPR.json b/test/data/parserWithWorkflowGraphPR.json
@@ -65,7 +65,9 @@
                         "command": "npm test"
                     }
                 ],
-                "requires": ["~pr"]
+                "requires": ["~pr"],
+                "templateId": 5,
+                "description": "test job"
             }
         ],
         "publish": [

diff --git a/test/lib/pipeline.test.js b/test/lib/pipeline.test.js
@@ -195,8 +195,8 @@ describe('Pipeline Model', () => {
         testJob = getJobMocks({
             id: 100,
             name: 'test',
-            description: 'test job',
-            templateId: 5,
+            description: 'original test job',
+            templateId: 1,
             archived: false
         });
 
@@ -1684,7 +1684,9 @@ describe('Pipeline Model', () => {
                         {
                             commands: [{ command: 'npm test', name: 'test' }],
                             image: 'node:10',
-                            requires: ['~pr']
+                            requires: ['~pr'],
+                            templateId: 5,
+                            description: 'test job'
                         }
                     ],
                     pipelineId: testId,
@@ -1800,7 +1802,9 @@ describe('Pipeline Model', () => {
                         {
                             commands: [{ command: 'npm test', name: 'test' }],
                             image: 'node:10',
-                            requires: ['~pr']
+                            requires: ['~pr'],
+                            templateId: 5,
+                            description: 'test job'
                         }
                     ],
                     pipelineId: testId,
@@ -1975,10 +1979,14 @@ describe('Pipeline Model', () => {
                         {
                             commands: [{ command: 'npm test', name: 'test' }],
                             image: 'node:10',
-                            requires: ['~pr', '~pr:testBranch']
+                            requires: ['~pr', '~pr:testBranch'],
+                            templateId: 5,
+                            description: 'test job'
                         }
                     ],
-                    pipelineId: testId
+                    pipelineId: testId,
+                    templateId: 5,
+                    description: 'test job'
                 });
             });
         });
@@ -2047,8 +2055,8 @@ describe('Pipeline Model', () => {
                     pipelineId: testId,
                     name: 'PR-2:main',
                     prParentJobId: 99998,
-                    templateId: undefined,
-                    description: undefined
+                    templateId: null,
+                    description: null
                 });
             });
         });