From fef2e75d3cc8b5bea901d745ebc6b5ee71d2ab36 Mon Sep 17 00:00:00 2001
From: adam-james <21064735+adam-james-v@users.noreply.github.com>
Date: Mon, 18 Mar 2024 13:24:59 -0700
Subject: [PATCH] Lock :display when using 'zoom' in Dashboard (#38496)
* Use proper :display when using 'zoom' in Dashboard
Fixes: #38307
WIP, I want to change at a different call site so that defaults are still used in case there is no :display set.
* Lock Display so that drill through doesn't overwrite it with defaults
* Lock display only when drill action coming from a dashboard
* Use isDashboard properly
* Add test to confirm Display is locked from Dashboards
* minor changes to the test
* Lock Display specifically for zoom-in-binning and timeseries
* Fix wrong string quotes
* Try a simple change to see if the e2e test will pass in CI
* Fix test
* Minor refactor
* Wait for dashcard queries in e2e test
* Wait for loading spinner to be gone
* Fix type errors
* Fix incorrect dashboard ID used in test
---------
Co-authored-by: Anton Kulyk <kuliks.anton@gmail.com>
---
.../dashboard-drill.cy.spec.js | 36 +++++++++++++++++++
.../zoom-in-binning-drill.ts | 9 ++++-
.../zoom-in-timeseries-drill.ts | 13 ++++---
3 files changed, 53 insertions(+), 5 deletions(-)
diff --git a/e2e/test/scenarios/dashboard-cards/dashboard-drill.cy.spec.js b/e2e/test/scenarios/dashboard-cards/dashboard-drill.cy.spec.js
index 3a1bb938bdc..8081c248be8 100644
--- a/e2e/test/scenarios/dashboard-cards/dashboard-drill.cy.spec.js
+++ b/e2e/test/scenarios/dashboard-cards/dashboard-drill.cy.spec.js
@@ -644,6 +644,42 @@ describe("scenarios > dashboard > dashboard drill", () => {
});
});
+ it("should keep card's display when doing zoom drill-through from dashboard (metabase#38307)", () => {
+ cy.intercept("POST", "/api/dataset").as("dataset");
+ cy.intercept("/api/dashboard/*/dashcard/*/card/*/query").as(
+ "dashcardQuery",
+ );
+
+ const questionDetails = {
+ name: "38307",
+ query: {
+ "source-table": REVIEWS_ID,
+ aggregation: [["count"]],
+ breakout: [["field", REVIEWS.CREATED_AT, { "temporal-unit": "month" }]],
+ },
+ display: "bar",
+ };
+
+ const dashboardDetails = {
+ name: "38307",
+ };
+
+ cy.createQuestionAndDashboard({ questionDetails, dashboardDetails }).then(
+ ({ body: { dashboard_id } }) => {
+ visitDashboard(dashboard_id);
+
+ // click the first bar on the card's graph and do a zoom drill-through
+ cy.get(".bar").eq(0).click({ force: true });
+ cy.findByText("See this month by week").click();
+
+ cy.wait("@dataset");
+
+ // check that the display is still a bar chart by checking that a .bar element exists
+ cy.get(".bar").should("exist");
+ },
+ );
+ });
+
it("should not hide custom formatting when click behavior is enabled (metabase#14597)", () => {
const columnKey = JSON.stringify(["name", "MY_NUMBER"]);
const questionSettings = {
diff --git a/frontend/src/metabase/querying/utils/drills/zoom-in-binning-drill/zoom-in-binning-drill.ts b/frontend/src/metabase/querying/utils/drills/zoom-in-binning-drill/zoom-in-binning-drill.ts
index da753280b6b..8764246631c 100644
--- a/frontend/src/metabase/querying/utils/drills/zoom-in-binning-drill/zoom-in-binning-drill.ts
+++ b/frontend/src/metabase/querying/utils/drills/zoom-in-binning-drill/zoom-in-binning-drill.ts
@@ -5,8 +5,10 @@ import type * as Lib from "metabase-lib";
export const zoomInBinningDrill: Drill<Lib.ZoomDrillThruInfo> = ({
drill,
+ clicked,
applyDrill,
}) => {
+ const isDashboard = clicked?.extraData?.dashboard != null;
return [
{
name: "zoom-in.binning",
@@ -14,7 +16,12 @@ export const zoomInBinningDrill: Drill<Lib.ZoomDrillThruInfo> = ({
section: "zoom",
icon: "zoom_in",
buttonType: "horizontal",
- question: () => applyDrill(drill).setDefaultDisplay(),
+ question: () => {
+ const question = applyDrill(drill);
+ return isDashboard
+ ? question.lockDisplay()
+ : question.setDefaultDisplay();
+ },
},
];
};
diff --git a/frontend/src/metabase/querying/utils/drills/zoom-in-timeseries-drill/zoom-in-timeseries-drill.ts b/frontend/src/metabase/querying/utils/drills/zoom-in-timeseries-drill/zoom-in-timeseries-drill.ts
index 6734cf69b1f..1b673f59f99 100644
--- a/frontend/src/metabase/querying/utils/drills/zoom-in-timeseries-drill/zoom-in-timeseries-drill.ts
+++ b/frontend/src/metabase/querying/utils/drills/zoom-in-timeseries-drill/zoom-in-timeseries-drill.ts
@@ -4,18 +4,23 @@ import type * as Lib from "metabase-lib";
export const zoomInTimeseriesDrill: Drill<Lib.ZoomTimeseriesDrillThruInfo> = ({
drill,
drillInfo,
+ clicked,
applyDrill,
}) => {
- const { displayName } = drillInfo;
-
+ const isDashboard = clicked?.extraData?.dashboard != null;
return [
{
name: "zoom-in.timeseries",
- title: displayName,
+ title: drillInfo.displayName,
section: "zoom",
icon: "zoom_in",
buttonType: "horizontal",
- question: () => applyDrill(drill).setDefaultDisplay(),
+ question: () => {
+ const question = applyDrill(drill);
+ return isDashboard
+ ? question.lockDisplay()
+ : question.setDefaultDisplay();
+ },
},
];
};
--
GitLab