Skip to content

Commit 4ff5cdd

Browse files
gabrieldutraarikfr
gabrieldutra
authored and
arikfr
committed
Fix Sankey issue to render 1 and 5 stages (#4965)
* Sankey: Make sure last stage has "Exit" * Sankey: Use 2 as min stage width size to render * Use null instead of "Exit" * Add comment about corresponding exit node * Add multiple stages on Cypress test
1 parent ef412d5 commit 4ff5cdd

3 files changed

Lines changed: 131 additions & 60 deletions

File tree

‎client/cypress/integration/visualizations/sankey_sunburst_spec.js‎

Lines changed: 126 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,79 +1,147 @@
11
/* global cy */
22

3-
import { createQuery } from "../../support/redash-api";
3+
import { createQuery, createDashboard, createVisualization, addWidget } from "../../support/redash-api";
4+
import { getWidgetTestId } from "../../support/dashboard";
45

56
const SQL = `
6-
SELECT 'a' AS stage1, 'a1' AS stage2, 11 AS value UNION ALL
7-
SELECT 'a' AS stage1, 'a2' AS stage2, 12 AS value UNION ALL
8-
SELECT 'a' AS stage1, 'a3' AS stage2, 45 AS value UNION ALL
9-
SELECT 'a' AS stage1, 'a4' AS stage2, 54 AS value UNION ALL
10-
SELECT 'b' AS stage1, 'b1' AS stage2, 33 AS value UNION ALL
11-
SELECT 'b' AS stage1, 'b2' AS stage2, 73 AS value UNION ALL
12-
SELECT 'b' AS stage1, 'b3' AS stage2, 90 AS value UNION ALL
13-
SELECT 'c' AS stage1, 'c1' AS stage2, 19 AS value UNION ALL
14-
SELECT 'c' AS stage1, 'c2' AS stage2, 92 AS value UNION ALL
15-
SELECT 'c' AS stage1, 'c3' AS stage2, 63 AS value UNION ALL
16-
SELECT 'c' AS stage1, 'c4' AS stage2, 44 AS v
7+
SELECT 'a' AS s1, 'a1' AS s2, 'a2' AS s3, null AS s4, null AS s5, 11 AS value UNION ALL
8+
SELECT 'a' AS s1, 'a2' AS s2, null AS s3, null AS s4, null AS s5, 12 AS value UNION ALL
9+
SELECT 'a' AS s1, 'a3' AS s2, null AS s3, null AS s4, null AS s5, 45 AS value UNION ALL
10+
SELECT 'a' AS s1, 'a4' AS s2, null AS s3, null AS s4, null AS s5, 54 AS value UNION ALL
11+
SELECT 'b' AS s1, 'b1' AS s2, 'a2' AS s3, 'c1' AS s4, null AS s5, 33 AS value UNION ALL
12+
SELECT 'b' AS s1, 'b2' AS s2, 'a4' AS s3, 'c3' AS s4, null AS s5, 73 AS value UNION ALL
13+
SELECT 'b' AS s1, 'b3' AS s2, null AS s3, null AS s4, null AS s5, 90 AS value UNION ALL
14+
SELECT 'c' AS s1, 'c1' AS s2, null AS s3, null AS s4, null AS s5, 19 AS value UNION ALL
15+
SELECT 'c' AS s1, 'c2' AS s2, 'b2' AS s3, 'a2' AS s4, 'a3' AS s5, 92 AS value UNION ALL
16+
SELECT 'c' AS s1, 'c3' AS s2, 'c4' AS s3, null AS s4, null AS s5, 63 AS value UNION ALL
17+
SELECT 'c' AS s1, 'c4' AS s2, null AS s3, null AS s4, null AS s5, 44 AS value
1718
`;
1819

1920
describe("Sankey and Sunburst", () => {
20-
const viewportWidth = Cypress.config("viewportWidth");
21-
2221
beforeEach(() => {
2322
cy.login();
24-
createQuery({ query: SQL }).then(({ id }) => {
25-
cy.visit(`queries/${id}/source`);
26-
cy.getByTestId("ExecuteButton").click();
27-
});
2823
});
2924

30-
it("creates Sunburst", () => {
31-
const visualizationName = "Sunburst";
25+
describe("Creation through UI", () => {
26+
beforeEach(() => {
27+
createQuery({ query: SQL }).then(({ id }) => {
28+
cy.visit(`queries/${id}/source`);
29+
cy.getByTestId("ExecuteButton").click();
30+
});
31+
});
32+
33+
it("creates Sunburst", () => {
34+
const visualizationName = "Sunburst";
35+
36+
cy.getByTestId("NewVisualization").click();
37+
cy.getByTestId("VisualizationType").click();
38+
cy.getByTestId("VisualizationType.SUNBURST_SEQUENCE").click();
39+
cy.getByTestId("VisualizationName")
40+
.clear()
41+
.type(visualizationName);
42+
cy.getByTestId("VisualizationPreview")
43+
.find("svg")
44+
.should("exist");
45+
46+
cy.getByTestId("EditVisualizationDialog")
47+
.contains("button", "Save")
48+
.click();
49+
cy.getByTestId("QueryPageVisualizationTabs")
50+
.contains("span", visualizationName)
51+
.should("exist");
52+
});
3253

33-
cy.getByTestId("NewVisualization").click();
34-
cy.getByTestId("VisualizationType").click();
35-
cy.getByTestId("VisualizationType.SUNBURST_SEQUENCE").click();
36-
cy.getByTestId("VisualizationName")
37-
.clear()
38-
.type(visualizationName);
39-
cy.getByTestId("VisualizationPreview")
40-
.find("svg")
41-
.should("exist");
54+
it("creates Sankey", () => {
55+
const visualizationName = "Sankey";
4256

43-
// wait a bit before taking snapshot
44-
cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
45-
cy.percySnapshot("Visualizations - Sunburst", { widths: [viewportWidth] });
57+
cy.getByTestId("NewVisualization").click();
58+
cy.getByTestId("VisualizationType").click();
59+
cy.getByTestId("VisualizationType.SANKEY").click();
60+
cy.getByTestId("VisualizationName")
61+
.clear()
62+
.type(visualizationName);
63+
cy.getByTestId("VisualizationPreview")
64+
.find("svg")
65+
.should("exist");
4666

47-
cy.getByTestId("EditVisualizationDialog")
48-
.contains("button", "Save")
49-
.click();
50-
cy.getByTestId("QueryPageVisualizationTabs")
51-
.contains("span", visualizationName)
52-
.should("exist");
67+
cy.getByTestId("EditVisualizationDialog")
68+
.contains("button", "Save")
69+
.click();
70+
cy.getByTestId("QueryPageVisualizationTabs")
71+
.contains("span", visualizationName)
72+
.should("exist");
73+
});
5374
});
5475

55-
it("creates Sankey", () => {
56-
const visualizationName = "Sankey";
76+
const STAGES_WIDGETS = [
77+
{ name: "1 stage", query: `SELECT s1,value FROM (${SQL}) q`, position: { autoHeight: false, sizeY: 10, sizeX: 2 } },
78+
{
79+
name: "2 stages",
80+
query: `SELECT s1,s2,value FROM (${SQL}) q`,
81+
position: { autoHeight: false, col: 2, sizeY: 10, sizeX: 2 },
82+
},
83+
{
84+
name: "3 stages",
85+
query: `SELECT s1,s2,s3,value FROM (${SQL}) q`,
86+
position: { autoHeight: false, col: 4, sizeY: 10, sizeX: 2 },
87+
},
88+
{
89+
name: "4 stages",
90+
query: `SELECT s1,s2,s3,s4,value FROM (${SQL}) q`,
91+
position: { autoHeight: false, row: 9, sizeY: 10, sizeX: 2 },
92+
},
93+
{
94+
name: "5 stages",
95+
query: `SELECT s1,s2,s3,s4,s5,value FROM (${SQL}) q`,
96+
position: { autoHeight: false, row: 9, col: 2, sizeY: 10, sizeX: 2 },
97+
},
98+
];
99+
100+
it("takes a snapshot with Sunburst (1 - 5 stages)", function() {
101+
createDashboard("Sunburst Visualization").then(dashboard => {
102+
this.dashboardUrl = `/dashboard/${dashboard.slug}`;
103+
return cy
104+
.all(
105+
STAGES_WIDGETS.map(sunburst => () =>
106+
createQuery({ name: `Sunburst with ${sunburst.name}`, query: sunburst.query })
107+
.then(queryData => createVisualization(queryData.id, "SUNBURST_SEQUENCE", "Sunburst", {}))
108+
.then(visualization => addWidget(dashboard.id, visualization.id, { position: sunburst.position }))
109+
)
110+
)
111+
.then(widgets => {
112+
cy.visit(this.dashboardUrl);
113+
widgets.forEach(widget => {
114+
cy.getByTestId(getWidgetTestId(widget)).within(() => cy.get("svg").should("exist"));
115+
});
57116

58-
cy.getByTestId("NewVisualization").click();
59-
cy.getByTestId("VisualizationType").click();
60-
cy.getByTestId("VisualizationType.SANKEY").click();
61-
cy.getByTestId("VisualizationName")
62-
.clear()
63-
.type(visualizationName);
64-
cy.getByTestId("VisualizationPreview")
65-
.find("svg")
66-
.should("exist");
117+
// wait a bit before taking snapshot
118+
cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
119+
cy.percySnapshot("Visualizations - Sunburst");
120+
});
121+
});
122+
});
67123

68-
// wait a bit before taking snapshot
69-
cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
70-
cy.percySnapshot("Visualizations - Sankey", { widths: [viewportWidth] });
124+
it("takes a snapshot with Sankey (1 - 5 stages)", function() {
125+
createDashboard("Sankey Visualization").then(dashboard => {
126+
this.dashboardUrl = `/dashboard/${dashboard.slug}`;
127+
return cy
128+
.all(
129+
STAGES_WIDGETS.map(sankey => () =>
130+
createQuery({ name: `Sankey with ${sankey.name}`, query: sankey.query })
131+
.then(queryData => createVisualization(queryData.id, "SANKEY", "Sankey", {}))
132+
.then(visualization => addWidget(dashboard.id, visualization.id, { position: sankey.position }))
133+
)
134+
)
135+
.then(widgets => {
136+
cy.visit(this.dashboardUrl);
137+
widgets.forEach(widget => {
138+
cy.getByTestId(getWidgetTestId(widget)).within(() => cy.get("svg").should("exist"));
139+
});
71140

72-
cy.getByTestId("EditVisualizationDialog")
73-
.contains("button", "Save")
74-
.click();
75-
cy.getByTestId("QueryPageVisualizationTabs")
76-
.contains("span", visualizationName)
77-
.should("exist");
141+
// wait a bit before taking snapshot
142+
cy.wait(500); // eslint-disable-line cypress/no-unnecessary-waiting
143+
cy.percySnapshot("Visualizations - Sankey");
144+
});
145+
});
78146
});
79147
});

‎viz-lib/src/visualizations/sankey/d3sankey.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,11 @@ function Sankey() {
8282
x += 1;
8383
}
8484

85-
//
8685
moveSinksRight(x);
87-
x = d3.max(nodes, n => n.x); // get new maximum x value
86+
x = Math.max(
87+
d3.max(nodes, n => n.x),
88+
2
89+
); // get new maximum x value (min 2)
8890
scaleNodeBreadths((size[0] - nodeWidth) / (x - 1));
8991
}
9092

‎viz-lib/src/visualizations/sankey/initSankey.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ function graph(data) {
6969
addLink(row[dataKeys[1]], row[dataKeys[2]], row.value || 0, 2);
7070
addLink(row[dataKeys[2]], row[dataKeys[3]], row.value || 0, 3);
7171
addLink(row[dataKeys[3]], row[dataKeys[4]], row.value || 0, 4);
72+
addLink(row[dataKeys[4]], null, row.value || 0, 5); // this line ensures that the last stage has a corresponding exit node
7273
});
7374

7475
const color = d3.scale.category20();

0 commit comments

Comments
 (0)