Merge branch '330602-fix-needs-is-undefined-in-pipeline-graph' into 'master'

Fix pipeline throwing error when needs is undefined

See merge request gitlab-org/gitlab!62027

app/assets/javascripts/pipelines/components/parsing_utils.js

@@ -55,21 +55,25 @@ export const createNodeDict = (nodes) => {
 export const makeLinksFromNodes = (nodes, nodeDict) => {
   const constantLinkValue = 10; // all links are the same weight
   return nodes
-    .map((group) => {
-      return group.jobs.map((job) => {
-        if (!job.needs) {
-          return [];
-        }
-
-        return job.needs.map((needed) => {
-          return {
-            source: nodeDict[needed]?.name,
-            target: group.name,
-            value: constantLinkValue,
-          };
-        });
-      });
-    })
+    .map(({ jobs, name: groupName }) =>
+      jobs.map(({ needs = [] }) =>
+        needs.reduce((acc, needed) => {
+          // It's possible that we have an optional job, which
+          // is being needed by another job. In that scenario,
+          // the needed job doesn't exist, so we don't want to
+          // create link for it.
+          if (nodeDict[needed]?.name) {
+            acc.push({
+              source: nodeDict[needed].name,
+              target: groupName,
+              value: constantLinkValue,
+            });
+          }
+
+          return acc;
+        }, []),
+      ),
+    )
     .flat(2);
 };
 

app/assets/javascripts/pipelines/utils.js

@@ -39,7 +39,13 @@ export const generateJobNeedsDict = (jobs = {}) => {
       }
 
       return jobs[jobName].needs
-        .map((job) => {
+        .reduce((needsAcc, job) => {
+          // It's possible that a needs refer to an optional job
+          // that is not defined in which case we don't add that entry
+          if (!jobs[job]) {
+            return needsAcc;
+          }
+
           // If we already have the needs of a job in the accumulator,
           // then we use the memoized data instead of the recursive call
           // to save some performance.
@@ -50,11 +56,11 @@ export const generateJobNeedsDict = (jobs = {}) => {
           // to the list of `needs` to ensure we can properly reference it.
           const group = jobs[job];
           if (group.size > 1) {
-            return [job, group.name, newNeeds];
+            return [...needsAcc, job, group.name, newNeeds];
           }
 
-          return [job, newNeeds];
-        })
+          return [...needsAcc, job, newNeeds];
+        }, [])
         .flat(Infinity);
     };
 

changelogs/unreleased/329895-fix-needs-is-undefined-in-pipeline-graph.yml

+---
+title: Fix pipeline graph undefined needs error
+merge_request: 62027
+author:
+type: fixed

spec/frontend/pipelines/components/dag/mock_data.js

@@ -398,6 +398,8 @@ export const multiNote = {
   },
 };
 
+export const missingJob = 'missing_job';
+
 /*
   It is important that the base include parallel jobs
   as well as non-parallel jobs with spaces in the name to prevent
@@ -657,4 +659,16 @@ export const mockParsedGraphQLNodes = [
     ],
     __typename: 'CiGroup',
   },
+  {
+    category: 'production',
+    name: 'production_e',
+    size: 1,
+    jobs: [
+      {
+        name: 'production_e',
+        needs: [missingJob],
+      },
+    ],
+    __typename: 'CiGroup',
+  },
 ];

spec/frontend/pipelines/parsing_utils_spec.js

@@ -10,7 +10,7 @@ import {
   getMaxNodes,
 } from '~/pipelines/components/parsing_utils';
 
-import { mockParsedGraphQLNodes } from './components/dag/mock_data';
+import { mockParsedGraphQLNodes, missingJob } from './components/dag/mock_data';
 import { generateResponse, mockPipelineResponse } from './graph/mock_data';
 
 describe('DAG visualization parsing utilities', () => {
@@ -24,6 +24,12 @@ describe('DAG visualization parsing utilities', () => {
       expect(unfilteredLinks[0]).toHaveProperty('target', 'test_a');
       expect(unfilteredLinks[0]).toHaveProperty('value', 10);
     });
+
+    it('does not generate a link for non-existing jobs', () => {
+      const sources = unfilteredLinks.map(({ source }) => source);
+
+      expect(sources.includes(missingJob)).toBe(false);
+    });
   });
 
   describe('filterByAncestors', () => {
@@ -88,7 +94,7 @@ describe('DAG visualization parsing utilities', () => {
         These lengths are determined by the mock data.
         If the data changes, the numbers may also change.
       */
-      expect(parsed.nodes).toHaveLength(21);
+      expect(parsed.nodes).toHaveLength(mockParsedGraphQLNodes.length);
       expect(cleanedNodes).toHaveLength(12);
     });
   });

spec/frontend/pipelines/pipeline_graph/utils_spec.js

@@ -111,6 +111,28 @@ describe('utils functions', () => {
       });
     });
 
+    it('removes needs which are not in the data', () => {
+      const inexistantJobName = 'job5';
+      const jobsWithNeeds = {
+        [jobName1]: job1,
+        [jobName2]: job2,
+        [jobName3]: job3,
+        [jobName4]: {
+          name: jobName4,
+          script: 'echo deploy',
+          stage: 'deploy',
+          needs: [inexistantJobName],
+        },
+      };
+
+      expect(generateJobNeedsDict(jobsWithNeeds)).toEqual({
+        [jobName1]: [],
+        [jobName2]: [],
+        [jobName3]: [jobName1, jobName2],
+        [jobName4]: [],
+      });
+    });
+
     it('handles parallel jobs by adding the group name as a need', () => {
       const size = 3;
       const jobOptimize1 = 'optimize_1';