From 6cca5fb3440f36aa34a462b6b0990312176a4ab8 Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Mon, 9 Sep 2024 11:43:28 +0100
Subject: [PATCH] Recalculate "graph.metrics" and "graph.dimensions" if empty
(#47692)
* Add repro
* Update `isValid` check for metrics and dimensions
* Fix unit test
---
.../reproductions-3.cy.spec.js | 96 +++++++++++++++++++
.../AddSeriesModal.unit.spec.tsx | 17 +++-
.../visualizations/lib/settings/graph.js | 22 ++++-
3 files changed, 130 insertions(+), 5 deletions(-)
diff --git a/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js b/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js
index d0a0b63eade..f180dfca818 100644
--- a/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js
+++ b/e2e/test/scenarios/question-reproductions/reproductions-3.cy.spec.js
@@ -2126,6 +2126,102 @@ describe("issue 41612", () => {
});
});
+describe("issue 36027", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsAdmin();
+
+ const CONCRETE_CREATED_AT_FIELD_REF = [
+ "field",
+ ORDERS.CREATED_AT,
+ { "base-type": "type/DateTime", "temporal-unit": "month" },
+ ];
+
+ const CREATED_AT_FIELD_REF = [
+ "field",
+ "CREATED_AT",
+ { "base-type": "type/DateTime", "temporal-unit": "month" },
+ ];
+
+ const BASE_QUERY = {
+ aggregation: [["count"]],
+ breakout: [CONCRETE_CREATED_AT_FIELD_REF],
+ "source-table": ORDERS_ID,
+ };
+
+ createQuestion({ query: BASE_QUERY }, { wrapId: true }).then(
+ baseQuestionId => {
+ createQuestion(
+ {
+ display: "waterfall",
+ query: {
+ aggregation: [
+ ["sum", ["field", "count", { "base-type": "type/Integer" }]],
+ ],
+ breakout: [CREATED_AT_FIELD_REF],
+ joins: [
+ {
+ alias: "Q1",
+ strategy: "left-join",
+ "source-table": `card__${baseQuestionId}`,
+ condition: [
+ "<=",
+ CREATED_AT_FIELD_REF,
+ CONCRETE_CREATED_AT_FIELD_REF,
+ ],
+ },
+ ],
+ "source-query": BASE_QUERY,
+ },
+ visualization_settings: {
+ "graph.dimensions": ["CREATED_AT"],
+ "graph.metrics": ["sum"],
+ },
+ },
+ { visitQuestion: true },
+ );
+ },
+ );
+ });
+
+ it("should use default metrics/dimensions if they're missing after removing some query clauses (metabase#36027)", () => {
+ openNotebook();
+ getNotebookStep("summarize", { stage: 1 })
+ .findByLabelText("Remove step")
+ .click({ force: true });
+ getNotebookStep("join", { stage: 1 })
+ .findByLabelText("Remove step")
+ .click({ force: true });
+ visualize();
+
+ echartsContainer().within(() => {
+ cy.findByText("Created At").should("be.visible"); // x-axis
+ cy.findByText("Count").should("be.visible"); // y-axis
+
+ // x-axis values
+ ["January 2023", "January 2024", "January 2025", "January 2026"].forEach(
+ state => {
+ cy.findByText(state).should("be.visible");
+ },
+ );
+
+ // y-axis values
+ [
+ "0",
+ "3,000",
+ "6,000",
+ "9,000",
+ "12,000",
+ "15,000",
+ "18,000",
+ "21,000",
+ ].forEach(state => {
+ cy.findByText(state).should("be.visible");
+ });
+ });
+ });
+});
+
function expectNoScrollbarContainer(element) {
const hasScrollbarContainer =
element.scrollHeight <= element.clientHeight &&
diff --git a/frontend/src/metabase/dashboard/components/AddSeriesModal/AddSeriesModal.unit.spec.tsx b/frontend/src/metabase/dashboard/components/AddSeriesModal/AddSeriesModal.unit.spec.tsx
index 761f0159f9c..6a7ab251549 100644
--- a/frontend/src/metabase/dashboard/components/AddSeriesModal/AddSeriesModal.unit.spec.tsx
+++ b/frontend/src/metabase/dashboard/components/AddSeriesModal/AddSeriesModal.unit.spec.tsx
@@ -105,7 +105,22 @@ describe("AddSeriesModal", () => {
dashcard: incompleteDashcard,
dashcardData: {
[incompleteDashcard.id]: {
- [incompleteCard.id]: dataset,
+ [incompleteCard.id]: createMockDataset({
+ data: createMockDatasetData({
+ rows: [
+ ["1958-04-01T00:00:00+07:00"],
+ ["1958-05-01T00:00:00+07:00"],
+ ["1958-06-01T00:00:00+07:00"],
+ ["1958-07-01T00:00:00+07:00"],
+ ],
+ cols: [
+ createMockColumn({
+ base_type: "type/Date",
+ display_name: displayColumnName,
+ }),
+ ],
+ }),
+ }),
},
},
});
diff --git a/frontend/src/metabase/visualizations/lib/settings/graph.js b/frontend/src/metabase/visualizations/lib/settings/graph.js
index 4f937d4c715..883a776c943 100644
--- a/frontend/src/metabase/visualizations/lib/settings/graph.js
+++ b/frontend/src/metabase/visualizations/lib/settings/graph.js
@@ -81,8 +81,15 @@ export const GRAPH_DATA_SETTINGS = {
series.length <= MAX_SERIES
? "0.5rem"
: "1rem",
- isValid: (series, vizSettings) =>
- getAreDimensionsAndMetricsValid(series, vizSettings),
+ isValid: (series, vizSettings) => {
+ const dimensions = vizSettings["graph.dimensions"] ?? [];
+ if (dimensions.length === 0) {
+ const defaultDimensions = getDefaultDimensions(series, vizSettings);
+ return defaultDimensions.length === 0;
+ } else {
+ return getAreDimensionsAndMetricsValid(series, vizSettings);
+ }
+ },
getDefault: (series, vizSettings) =>
getDefaultDimensions(series, vizSettings),
persistDefault: true,
@@ -142,8 +149,15 @@ export const GRAPH_DATA_SETTINGS = {
section: t`Data`,
title: t`Y-axis`,
widget: "fields",
- isValid: (series, vizSettings) =>
- getAreDimensionsAndMetricsValid(series, vizSettings),
+ isValid: (series, vizSettings) => {
+ const metrics = vizSettings["graph.metrics"] ?? [];
+ if (metrics.length === 0) {
+ const defaultMetrics = getDefaultMetrics(series, vizSettings);
+ return defaultMetrics.length === 0;
+ } else {
+ return getAreDimensionsAndMetricsValid(series, vizSettings);
+ }
+ },
getDefault: (series, vizSettings) => getDefaultMetrics(series, vizSettings),
persistDefault: true,
getProps: ([{ card, data }], vizSettings, _onChange, extra) => {
--
GitLab