Merge branch '215517-pull-out-drawing-fns' into 'master'

DAG MVC: Refactor the Graph!

See merge request gitlab-org/gitlab!33401

app/assets/javascripts/pipelines/components/dag/dag.vue

@@ -4,7 +4,7 @@ import axios from '~/lib/utils/axios_utils';
 import { __ } from '~/locale';
 import DagGraph from './dag_graph.vue';
 import { DEFAULT, PARSE_FAILURE, LOAD_FAILURE, UNSUPPORTED_DATA } from './constants';
-import { parseData } from './utils';
+import { parseData } from './parsing_utils';
 
 export default {
   // eslint-disable-next-line @gitlab/require-i18n-strings

app/assets/javascripts/pipelines/components/dag/dag_graph.vue

@@ -3,7 +3,8 @@ import * as d3 from 'd3';
 import { uniqueId } from 'lodash';
 import { PARSE_FAILURE } from './constants';
 
-import { createSankey, getMaxNodes, removeOrphanNodes } from './utils';
+import { getMaxNodes, removeOrphanNodes } from './parsing_utils';
+import { calculateClip, createLinkPath, createSankey, labelPosition } from './drawing_utils';
 
 export default {
   viewOptions: {
@@ -78,7 +79,7 @@ export default {
       return (
         link
           .append('path')
-          .attr('d', this.createLinkPath)
+          .attr('d', (d, i) => createLinkPath(d, i, this.$options.viewOptions.nodeWidth))
           .attr('stroke', ({ gradId }) => `url(#${gradId})`)
           .style('stroke-linejoin', 'round')
           // minus two to account for the rounded nodes
@@ -89,7 +90,10 @@ export default {
 
     appendLabelAsForeignObject(d, i, n) {
       const currentNode = n[i];
-      const { height, wrapperWidth, width, x, y, textAlign } = this.labelPosition(d);
+      const { height, wrapperWidth, width, x, y, textAlign } = labelPosition(d, {
+        ...this.$options.viewOptions,
+        width: this.width,
+      });
 
       const labelClasses = [
         'gl-display-flex',
@@ -128,44 +132,13 @@ export default {
     },
 
     createClip(link) {
-      /*
-        Because large link values can overrun their box, we create a clip path
-        to trim off the excess in charts that have few nodes per column and are
-        therefore tall.
-
-        The box is created by
-          M: moving to outside midpoint of the source node
-          V: drawing a vertical line to maximum of the bottom link edge or
-            the lowest edge of the node (can be d.y0 or d.y1 depending on the link's path)
-          H: drawing a horizontal line to the outside edge of the destination node
-          V: drawing a vertical line back up to the minimum of the top link edge or
-            the highest edge of the node (can be d.y0 or d.y1 depending on the link's path)
-          H: drawing a horizontal line back to the outside edge of the source node
-          Z: closing the path, back to the start point
-      */
-
-      const clip = ({ y0, y1, source, target, width }) => {
-        const bottomLinkEdge = Math.max(y1, y0) + width / 2;
-        const topLinkEdge = Math.min(y0, y1) - width / 2;
-
-        /* eslint-disable @gitlab/require-i18n-strings */
-        return `
-          M${source.x0}, ${y1}
-          V${Math.max(bottomLinkEdge, y0, y1)}
-          H${target.x1}
-          V${Math.min(topLinkEdge, y0, y1)}
-          H${source.x0}
-          Z`;
-        /* eslint-enable @gitlab/require-i18n-strings */
-      };
-
       return link
         .append('clipPath')
         .attr('id', d => {
           return this.createAndAssignId(d, 'clipId', 'dag-clip');
         })
         .append('path')
-        .attr('d', clip);
+        .attr('d', calculateClip);
     },
 
     createGradient(link) {
@@ -189,44 +162,6 @@ export default {
         .attr('stop-color', ({ target }) => this.color(target));
     },
 
-    createLinkPath({ y0, y1, source, target, width }, idx) {
-      const { nodeWidth } = this.$options.viewOptions;
-
-      /*
-        Creates a series of staggered midpoints for the link paths, so they
-        don't run along one channel and can be distinguished.
-
-        First, get a point staggered by index and link width, modulated by the link box
-        to find a point roughly between the nodes.
-
-        Then offset it by nodeWidth, so it doesn't run under any nodes at the left.
-
-        Determine where it would overlap at the right.
-
-        Finally, select the leftmost of these options:
-          - offset from the source node based on index + fudge;
-          - a fuzzy offset from the right node, using Math.random adds a little blur
-          - a hard offset from the end node, if random pushes it over
-
-        Then draw a line from the start node to the bottom-most point of the midline
-        up to the topmost point in that line and then to the middle of the end node
-      */
-
-      const xValRaw = source.x1 + (((idx + 1) * width) % (target.x1 - source.x0));
-      const xValMin = xValRaw + nodeWidth;
-      const overlapPoint = source.x1 + (target.x0 - source.x1);
-      const xValMax = overlapPoint - nodeWidth * 1.4;
-
-      const midPointX = Math.min(xValMin, target.x0 - nodeWidth * 4 * Math.random(), xValMax);
-
-      return d3.line()([
-        [(source.x0 + source.x1) / 2, y0],
-        [midPointX, y0],
-        [midPointX, y1],
-        [(target.x0 + target.x1) / 2, y1],
-      ]);
-    },
-
     createLinks(svg, linksData) {
       const link = this.generateLinks(svg, linksData);
       this.createGradient(link);
@@ -322,42 +257,6 @@ export default {
       return ({ name }) => colorFn(name);
     },
 
-    labelPosition({ x0, x1, y0, y1 }) {
-      const { paddingForLabels, labelMargin, nodePadding } = this.$options.viewOptions;
-
-      const firstCol = x0 <= paddingForLabels;
-      const lastCol = x1 >= this.width - paddingForLabels;
-
-      if (firstCol) {
-        return {
-          x: 0 + labelMargin,
-          y: y0,
-          height: `${y1 - y0}px`,
-          width: paddingForLabels - 2 * labelMargin,
-          textAlign: 'right',
-        };
-      }
-
-      if (lastCol) {
-        return {
-          x: this.width - paddingForLabels + labelMargin,
-          y: y0,
-          height: `${y1 - y0}px`,
-          width: paddingForLabels - 2 * labelMargin,
-          textAlign: 'left',
-        };
-      }
-
-      return {
-        x: (x1 + x0) / 2,
-        y: y0 - nodePadding,
-        height: `${nodePadding}px`,
-        width: 'max-content',
-        wrapperWidth: paddingForLabels - 2 * labelMargin,
-        textAlign: x0 < this.width / 2 ? 'left' : 'right',
-      };
-    },
-
     transformData(parsed) {
       const baseLayout = createSankey()(parsed);
       const cleanedNodes = removeOrphanNodes(baseLayout.nodes);

app/assets/javascripts/pipelines/components/dag/drawing_utils.js

+import * as d3 from 'd3';
+import { sankey, sankeyLeft } from 'd3-sankey';
+
+export const calculateClip = ({ y0, y1, source, target, width }) => {
+  /*
+    Because large link values can overrun their box, we create a clip path
+    to trim off the excess in charts that have few nodes per column and are
+    therefore tall.
+
+    The box is created by
+      M: moving to outside midpoint of the source node
+      V: drawing a vertical line to maximum of the bottom link edge or
+        the lowest edge of the node (can be d.y0 or d.y1 depending on the link's path)
+      H: drawing a horizontal line to the outside edge of the destination node
+      V: drawing a vertical line back up to the minimum of the top link edge or
+        the highest edge of the node (can be d.y0 or d.y1 depending on the link's path)
+      H: drawing a horizontal line back to the outside edge of the source node
+      Z: closing the path, back to the start point
+  */
+
+  const bottomLinkEdge = Math.max(y1, y0) + width / 2;
+  const topLinkEdge = Math.min(y0, y1) - width / 2;
+
+  /* eslint-disable @gitlab/require-i18n-strings */
+  return `
+    M${source.x0}, ${y1}
+    V${Math.max(bottomLinkEdge, y0, y1)}
+    H${target.x1}
+    V${Math.min(topLinkEdge, y0, y1)}
+    H${source.x0}
+    Z
+  `;
+  /* eslint-enable @gitlab/require-i18n-strings */
+};
+
+export const createLinkPath = ({ y0, y1, source, target, width }, idx, nodeWidth) => {
+  /*
+    Creates a series of staggered midpoints for the link paths, so they
+    don't run along one channel and can be distinguished.
+
+    First, get a point staggered by index and link width, modulated by the link box
+    to find a point roughly between the nodes.
+
+    Then offset it by nodeWidth, so it doesn't run under any nodes at the left.
+
+    Determine where it would overlap at the right.
+
+    Finally, select the leftmost of these options:
+      - offset from the source node based on index + fudge;
+      - a fuzzy offset from the right node, using Math.random adds a little blur
+      - a hard offset from the end node, if random pushes it over
+
+    Then draw a line from the start node to the bottom-most point of the midline
+    up to the topmost point in that line and then to the middle of the end node
+  */
+
+  const xValRaw = source.x1 + (((idx + 1) * width) % (target.x1 - source.x0));
+  const xValMin = xValRaw + nodeWidth;
+  const overlapPoint = source.x1 + (target.x0 - source.x1);
+  const xValMax = overlapPoint - nodeWidth * 1.4;
+
+  const midPointX = Math.min(xValMin, target.x0 - nodeWidth * 4 * Math.random(), xValMax);
+
+  return d3.line()([
+    [(source.x0 + source.x1) / 2, y0],
+    [midPointX, y0],
+    [midPointX, y1],
+    [(target.x0 + target.x1) / 2, y1],
+  ]);
+};
+
+/*
+    createSankey calls the d3 layout to generate the relationships and positioning
+    values for the nodes and links in the graph.
+  */
+
+export const createSankey = ({
+  width = 10,
+  height = 10,
+  nodeWidth = 10,
+  nodePadding = 10,
+  paddingForLabels = 1,
+} = {}) => {
+  const sankeyGenerator = sankey()
+    .nodeId(({ name }) => name)
+    .nodeAlign(sankeyLeft)
+    .nodeWidth(nodeWidth)
+    .nodePadding(nodePadding)
+    .extent([
+      [paddingForLabels, paddingForLabels],
+      [width - paddingForLabels, height - paddingForLabels],
+    ]);
+  return ({ nodes, links }) =>
+    sankeyGenerator({
+      nodes: nodes.map(d => ({ ...d })),
+      links: links.map(d => ({ ...d })),
+    });
+};
+
+export const labelPosition = ({ x0, x1, y0, y1 }, viewOptions) => {
+  const { paddingForLabels, labelMargin, nodePadding, width } = viewOptions;
+
+  const firstCol = x0 <= paddingForLabels;
+  const lastCol = x1 >= width - paddingForLabels;
+
+  if (firstCol) {
+    return {
+      x: 0 + labelMargin,
+      y: y0,
+      height: `${y1 - y0}px`,
+      width: paddingForLabels - 2 * labelMargin,
+      textAlign: 'right',
+    };
+  }
+
+  if (lastCol) {
+    return {
+      x: width - paddingForLabels + labelMargin,
+      y: y0,
+      height: `${y1 - y0}px`,
+      width: paddingForLabels - 2 * labelMargin,
+      textAlign: 'left',
+    };
+  }
+
+  return {
+    x: (x1 + x0) / 2,
+    y: y0 - nodePadding,
+    height: `${nodePadding}px`,
+    width: 'max-content',
+    wrapperWidth: paddingForLabels - 2 * labelMargin,
+    textAlign: x0 < width / 2 ? 'left' : 'right',
+  };
+};

app/assets/javascripts/pipelines/components/dag/utils.js → app/assets/javascripts/pipelines/components/dag/parsing_utils.js

-import { sankey, sankeyLeft } from 'd3-sankey';
 import { uniqWith, isEqual } from 'lodash';
 
 /*
@@ -136,34 +135,6 @@ export const parseData = data => {
   return { nodes, links };
 };
 
-/*
-    createSankey calls the d3 layout to generate the relationships and positioning
-    values for the nodes and links in the graph.
-  */
-
-export const createSankey = ({
-  width = 10,
-  height = 10,
-  nodeWidth = 10,
-  nodePadding = 10,
-  paddingForLabels = 1,
-} = {}) => {
-  const sankeyGenerator = sankey()
-    .nodeId(({ name }) => name)
-    .nodeAlign(sankeyLeft)
-    .nodeWidth(nodeWidth)
-    .nodePadding(nodePadding)
-    .extent([
-      [paddingForLabels, paddingForLabels],
-      [width - paddingForLabels, height - paddingForLabels],
-    ]);
-  return ({ nodes, links }) =>
-    sankeyGenerator({
-      nodes: nodes.map(d => ({ ...d })),
-      links: links.map(d => ({ ...d })),
-    });
-};
-
 /*
   The number of nodes in the most populous generation drives the height of the graph.
 */

spec/frontend/pipelines/components/dag/__snapshots__/dag_graph_spec.js.snap

@@ -15,7 +15,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H377.3333333333333
     V100
     H100
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M108,129L190,129L190,129L369.3333333333333,129\\" stroke=\\"url(#dag-grad53)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip63)\\"></path>
     </g>
@@ -31,7 +32,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H638.6666666666666
     V100
     H361.3333333333333
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M369.3333333333333,129L509.3333333333333,129L509.3333333333333,129.0000000000002L630.6666666666666,129.0000000000002\\" stroke=\\"url(#dag-grad54)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip64)\\"></path>
     </g>
@@ -47,7 +49,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H638.6666666666666
     V158.0000000000002
     H100
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M108,212.00000000000003L306,212.00000000000003L306,187.0000000000002L630.6666666666666,187.0000000000002\\" stroke=\\"url(#dag-grad55)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip65)\\"></path>
     </g>
@@ -63,7 +66,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H377.3333333333333
     V240.99999999999977
     H100
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M108,295L338.93333333333334,295L338.93333333333334,269.9999999999998L369.3333333333333,269.9999999999998\\" stroke=\\"url(#dag-grad56)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip66)\\"></path>
     </g>
@@ -79,7 +83,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H377.3333333333333
     V323.99999999999994
     H100
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M108,378.00000000000006L144.66666666666669,378.00000000000006L144.66666666666669,352.99999999999994L369.3333333333333,352.99999999999994\\" stroke=\\"url(#dag-grad57)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip67)\\"></path>
     </g>
@@ -95,7 +100,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H638.6666666666666
     V240.99999999999977
     H361.3333333333333
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M369.3333333333333,269.9999999999998L464,269.9999999999998L464,270.0000000000001L630.6666666666666,270.0000000000001\\" stroke=\\"url(#dag-grad58)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip68)\\"></path>
     </g>
@@ -111,7 +117,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H638.6666666666666
     V299.0000000000001
     H361.3333333333333
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M369.3333333333333,352.99999999999994L522,352.99999999999994L522,328.0000000000001L630.6666666666666,328.0000000000001\\" stroke=\\"url(#dag-grad59)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip69)\\"></path>
     </g>
@@ -127,7 +134,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H638.6666666666666
     V381.99999999999994
     H361.3333333333333
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M369.3333333333333,410.99999999999994L580,410.99999999999994L580,411L630.6666666666666,411\\" stroke=\\"url(#dag-grad60)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip70)\\"></path>
     </g>
@@ -143,7 +151,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H900
     V241.0000000000001
     H622.6666666666666
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M630.6666666666666,270.0000000000001L861.6,270.0000000000001L861.6,270.1890725105691L892,270.1890725105691\\" stroke=\\"url(#dag-grad61)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip71)\\"></path>
     </g>
@@ -159,7 +168,8 @@ exports[`The DAG graph in the basic case renders the graph svg 1`] = `
     H900
     V382
     H622.6666666666666
-          Z\\"></path>
+    Z
+  \\"></path>
       </clipPath>
       <path d=\\"M630.6666666666666,411L679.9999999999999,411L679.9999999999999,411L892,411\\" stroke=\\"url(#dag-grad62)\\" style=\\"stroke-linejoin: round;\\" stroke-width=\\"56\\" clip-path=\\"url(#dag-clip72)\\"></path>
     </g>

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

 import { mount } from '@vue/test-utils';
 import DagGraph from '~/pipelines/components/dag/dag_graph.vue';
-import { createSankey, removeOrphanNodes } from '~/pipelines/components/dag/utils';
+import { createSankey } from '~/pipelines/components/dag/drawing_utils';
+import { removeOrphanNodes } from '~/pipelines/components/dag/parsing_utils';
 import { parsedData } from './mock_data';
 
 describe('The DAG graph', () => {

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

+import { createSankey } from '~/pipelines/components/dag/drawing_utils';
+import { parseData } from '~/pipelines/components/dag/parsing_utils';
+import { mockBaseData } from './mock_data';
+
+describe('DAG visualization drawing utilities', () => {
+  const parsed = parseData(mockBaseData.stages);
+
+  const layoutSettings = {
+    width: 200,
+    height: 200,
+    nodeWidth: 10,
+    nodePadding: 20,
+    paddingForLabels: 100,
+  };
+
+  const sankeyLayout = createSankey(layoutSettings)(parsed);
+
+  describe('createSankey', () => {
+    it('returns a nodes data structure with expected d3-added properties', () => {
+      const exampleNode = sankeyLayout.nodes[0];
+      expect(exampleNode).toHaveProperty('sourceLinks');
+      expect(exampleNode).toHaveProperty('targetLinks');
+      expect(exampleNode).toHaveProperty('depth');
+      expect(exampleNode).toHaveProperty('layer');
+      expect(exampleNode).toHaveProperty('x0');
+      expect(exampleNode).toHaveProperty('x1');
+      expect(exampleNode).toHaveProperty('y0');
+      expect(exampleNode).toHaveProperty('y1');
+    });
+
+    it('returns a links data structure with expected d3-added properties', () => {
+      const exampleLink = sankeyLayout.links[0];
+      expect(exampleLink).toHaveProperty('source');
+      expect(exampleLink).toHaveProperty('target');
+      expect(exampleLink).toHaveProperty('width');
+      expect(exampleLink).toHaveProperty('y0');
+      expect(exampleLink).toHaveProperty('y1');
+    });
+
+    describe('data structure integrity', () => {
+      const newObject = { name: 'bad-actor' };
+
+      beforeEach(() => {
+        sankeyLayout.nodes.unshift(newObject);
+      });
+
+      it('sankey does not propagate changes back to the original', () => {
+        expect(sankeyLayout.nodes[0]).toBe(newObject);
+        expect(parsed.nodes[0]).not.toBe(newObject);
+      });
+
+      afterEach(() => {
+        sankeyLayout.nodes.shift();
+      });
+    });
+  });
+});

spec/frontend/pipelines/components/dag/utils_spec.js → spec/frontend/pipelines/components/dag/parsing_utils_spec.js

@@ -3,11 +3,11 @@ import {
   makeLinksFromNodes,
   filterByAncestors,
   parseData,
-  createSankey,
   removeOrphanNodes,
   getMaxNodes,
-} from '~/pipelines/components/dag/utils';
+} from '~/pipelines/components/dag/parsing_utils';
 
+import { createSankey } from '~/pipelines/components/dag/drawing_utils';
 import { mockBaseData } from './mock_data';
 
 describe('DAG visualization parsing utilities', () => {
@@ -105,44 +105,6 @@ describe('DAG visualization parsing utilities', () => {
     });
   });
 
-  describe('createSankey', () => {
-    it('returns a nodes data structure with expected d3-added properties', () => {
-      expect(sankeyLayout.nodes[0]).toHaveProperty('sourceLinks');
-      expect(sankeyLayout.nodes[0]).toHaveProperty('targetLinks');
-      expect(sankeyLayout.nodes[0]).toHaveProperty('depth');
-      expect(sankeyLayout.nodes[0]).toHaveProperty('layer');
-      expect(sankeyLayout.nodes[0]).toHaveProperty('x0');
-      expect(sankeyLayout.nodes[0]).toHaveProperty('x1');
-      expect(sankeyLayout.nodes[0]).toHaveProperty('y0');
-      expect(sankeyLayout.nodes[0]).toHaveProperty('y1');
-    });
-
-    it('returns a links data structure with expected d3-added properties', () => {
-      expect(sankeyLayout.links[0]).toHaveProperty('source');
-      expect(sankeyLayout.links[0]).toHaveProperty('target');
-      expect(sankeyLayout.links[0]).toHaveProperty('width');
-      expect(sankeyLayout.links[0]).toHaveProperty('y0');
-      expect(sankeyLayout.links[0]).toHaveProperty('y1');
-    });
-
-    describe('data structure integrity', () => {
-      const newObject = { name: 'bad-actor' };
-
-      beforeEach(() => {
-        sankeyLayout.nodes.unshift(newObject);
-      });
-
-      it('sankey does not propagate changes back to the original', () => {
-        expect(sankeyLayout.nodes[0]).toBe(newObject);
-        expect(parsed.nodes[0]).not.toBe(newObject);
-      });
-
-      afterEach(() => {
-        sankeyLayout.nodes.shift();
-      });
-    });
-  });
-
   describe('removeOrphanNodes', () => {
     it('removes sankey nodes that have no needs and are not needed', () => {
       const cleanedNodes = removeOrphanNodes(sankeyLayout.nodes);