From 914bce893593e430a1ce6e2dddab340d41d34e5a Mon Sep 17 00:00:00 2001
From: Uladzimir Havenchyk <125459446+uladzimirdev@users.noreply.github.com>
Date: Mon, 9 Oct 2023 14:08:18 +0300
Subject: [PATCH] [Dashboard] Do not lose filters state on navigating back from
Question (#34399)
---
...ack-navigation-preserve-filters.cy.spec.js | 120 ++++++++++++++++++
.../dashboard/actions/data-fetching.js | 2 +-
.../components/Dashboard/Dashboard.jsx | 2 +-
.../containers/DashboardApp/DashboardApp.jsx | 4 +-
.../metabase/dashboard/hoc/DashboardData.jsx | 4 +-
frontend/src/metabase/dashboard/reducers.js | 7 +-
.../metabase/dashboard/reducers.unit.spec.js | 44 ++++++-
.../metabase/dashboard/selectors/selectors.js | 2 +-
.../FilterApplyButton/FilterApplyButton.tsx | 3 +-
.../components/FilterApplyButton/index.ts | 3 +-
.../components/EmbedFrame/EmbedFrame.tsx | 2 +-
11 files changed, 177 insertions(+), 16 deletions(-)
create mode 100644 e2e/test/scenarios/dashboard/reproductions/34382-dashboard-back-navigation-preserve-filters.cy.spec.js
diff --git a/e2e/test/scenarios/dashboard/reproductions/34382-dashboard-back-navigation-preserve-filters.cy.spec.js b/e2e/test/scenarios/dashboard/reproductions/34382-dashboard-back-navigation-preserve-filters.cy.spec.js
new file mode 100644
index 00000000000..595e62e0f83
--- /dev/null
+++ b/e2e/test/scenarios/dashboard/reproductions/34382-dashboard-back-navigation-preserve-filters.cy.spec.js
@@ -0,0 +1,120 @@
+import {
+ dashboardParametersContainer,
+ getDashboardCard,
+ restore,
+ visitDashboard,
+ filterWidget,
+ queryBuilderHeader,
+ popover,
+} from "e2e/support/helpers";
+import { PRODUCTS, PRODUCTS_ID } from "metabase-types/api/mocks/presets";
+
+describe("issue 34382", () => {
+ beforeEach(() => {
+ restore();
+ cy.signInAsAdmin();
+
+ cy.intercept("POST", "/api/dashboard/*/dashcard/*/card/*/query").as(
+ "dashcardQuery",
+ );
+ });
+
+ it("should preserve filter value when navigating between the dashboard and the query builder with auto-apply disabled (metabase#34382)", () => {
+ createDashboardWithCards();
+ cy.get("@dashboardId").then(visitDashboard);
+
+ addFilterValue("Gizmo");
+ applyFilter();
+
+ cy.log("Navigate to Products question");
+ getDashboardCard().findByText("Products").click();
+
+ cy.log("Navigate back to dashboard");
+ queryBuilderHeader()
+ .findByLabelText("Back to Products in a dashboard")
+ .click();
+
+ cy.location("search").should("eq", "?category=Gizmo");
+ filterWidget().contains("Gizmo");
+
+ getDashboardCard().within(() => {
+ // only products with category "Gizmo" are filtered
+ cy.findAllByTestId("table-row").should("have.length", 8);
+ cy.findAllByText("Gizmo").should("have.length", 8);
+ });
+ });
+});
+
+const createDashboardWithCards = () => {
+ const getParameterMapping = ({ card_id }, parameters) => ({
+ parameter_mappings: parameters.map(parameter => ({
+ card_id,
+ parameter_id: parameter.id,
+ target: ["dimension", ["field", PRODUCTS.CATEGORY, null]],
+ })),
+ });
+
+ const questionDetails = {
+ name: "Products",
+ query: { "source-table": PRODUCTS_ID },
+ };
+
+ const questionDashcardDetails = {
+ row: 0,
+ col: 0,
+ size_x: 8,
+ size_y: 8,
+ visualization_settings: {},
+ };
+ const filterDetails = {
+ name: "Product Category",
+ slug: "category",
+ id: "96917421",
+ type: "category",
+ };
+
+ const dashboardDetails = {
+ name: "Products in a dashboard",
+ auto_apply_filters: false,
+ parameters: [filterDetails],
+ };
+
+ cy.createDashboard(dashboardDetails).then(
+ ({ body: { id: dashboard_id } }) => {
+ cy.createQuestion(questionDetails).then(
+ ({ body: { id: question_id } }) => {
+ cy.request("PUT", `/api/dashboard/${dashboard_id}/cards`, {
+ cards: [
+ {
+ id: -1,
+ card_id: question_id,
+ ...questionDashcardDetails,
+ ...getParameterMapping({ card_id: question_id }, [
+ filterDetails,
+ ]),
+ },
+ ],
+ });
+ },
+ );
+
+ cy.wrap(dashboard_id).as("dashboardId");
+ },
+ );
+};
+
+function addFilterValue(value) {
+ filterWidget().click();
+ popover().within(() => {
+ cy.findByText(value).click();
+ cy.findByRole("button", { name: "Add filter" }).click();
+ });
+}
+
+function applyFilter() {
+ dashboardParametersContainer()
+ .findByRole("button", { name: "Apply" })
+ .click();
+
+ cy.wait("@dashcardQuery");
+}
diff --git a/frontend/src/metabase/dashboard/actions/data-fetching.js b/frontend/src/metabase/dashboard/actions/data-fetching.js
index 1c761e7b880..7b8377d0418 100644
--- a/frontend/src/metabase/dashboard/actions/data-fetching.js
+++ b/frontend/src/metabase/dashboard/actions/data-fetching.js
@@ -268,7 +268,7 @@ export const fetchCardData = createThunkAction(
},
});
- // If the dataset_query was filtered then we don't have permisison to view this card, so
+ // If the dataset_query was filtered then we don't have permission to view this card, so
// shortcircuit and return a fake 403
if (!card.dataset_query) {
return {
diff --git a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx
index b3383a35aff..671f71f2a9e 100644
--- a/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx
+++ b/frontend/src/metabase/dashboard/components/Dashboard/Dashboard.jsx
@@ -7,7 +7,7 @@ import { getMainElement } from "metabase/lib/dom";
import DashboardHeader from "metabase/dashboard/containers/DashboardHeader";
import SyncedParametersList from "metabase/parameters/components/SyncedParametersList/SyncedParametersList";
-import FilterApplyButton from "metabase/parameters/components/FilterApplyButton";
+import { FilterApplyButton } from "metabase/parameters/components/FilterApplyButton";
import { getVisibleParameters } from "metabase/parameters/utils/ui";
import DashboardControls from "metabase/dashboard/hoc/DashboardControls";
import { getValuePopulatedParameters } from "metabase-lib/parameters/utils/parameter-values";
diff --git a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx
index 474a2d778ee..59b64e8deda 100644
--- a/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx
+++ b/frontend/src/metabase/dashboard/containers/DashboardApp/DashboardApp.jsx
@@ -61,7 +61,7 @@ import {
getSlowCards,
getIsAutoApplyFilters,
getSelectedTabId,
- getisNavigatingBackToDashboard,
+ getIsNavigatingBackToDashboard,
} from "../../selectors";
import { DASHBOARD_SLOW_TIMEOUT } from "../../constants";
@@ -106,7 +106,7 @@ const mapStateToProps = state => {
embedOptions: getEmbedOptions(state),
selectedTabId: getSelectedTabId(state),
isAutoApplyFilters: getIsAutoApplyFilters(state),
- isNavigatingBackToDashboard: getisNavigatingBackToDashboard(state),
+ isNavigatingBackToDashboard: getIsNavigatingBackToDashboard(state),
};
};
diff --git a/frontend/src/metabase/dashboard/hoc/DashboardData.jsx b/frontend/src/metabase/dashboard/hoc/DashboardData.jsx
index 728ce44a12b..d4a77416842 100644
--- a/frontend/src/metabase/dashboard/hoc/DashboardData.jsx
+++ b/frontend/src/metabase/dashboard/hoc/DashboardData.jsx
@@ -12,7 +12,7 @@ import {
getSlowCards,
getParameters,
getParameterValues,
- getisNavigatingBackToDashboard,
+ getIsNavigatingBackToDashboard,
getSelectedTabId,
} from "metabase/dashboard/selectors";
@@ -26,7 +26,7 @@ const mapStateToProps = (state, props) => {
slowCards: getSlowCards(state, props),
parameters: getParameters(state, props),
parameterValues: getParameterValues(state, props),
- isNavigatingBackToDashboard: getisNavigatingBackToDashboard(state),
+ isNavigatingBackToDashboard: getIsNavigatingBackToDashboard(state),
};
};
diff --git a/frontend/src/metabase/dashboard/reducers.js b/frontend/src/metabase/dashboard/reducers.js
index 0ebbc23a856..693cd2e32c4 100644
--- a/frontend/src/metabase/dashboard/reducers.js
+++ b/frontend/src/metabase/dashboard/reducers.js
@@ -338,7 +338,11 @@ const parameterValues = handleActions(
const draftParameterValues = handleActions(
{
- [INITIALIZE]: { next: () => ({}) },
+ [INITIALIZE]: {
+ next: (state, { payload: { clearCache = true } = {} }) => {
+ return clearCache ? {} : state;
+ },
+ },
[FETCH_DASHBOARD]: {
next: (
state,
@@ -358,7 +362,6 @@ const draftParameterValues = handleActions(
[REMOVE_PARAMETER]: {
next: (state, { payload: { id } }) => dissoc(state, id),
},
- [RESET]: { next: () => ({}) },
},
{},
);
diff --git a/frontend/src/metabase/dashboard/reducers.unit.spec.js b/frontend/src/metabase/dashboard/reducers.unit.spec.js
index e1ebbc35cef..5d72de849d2 100644
--- a/frontend/src/metabase/dashboard/reducers.unit.spec.js
+++ b/frontend/src/metabase/dashboard/reducers.unit.spec.js
@@ -102,10 +102,50 @@ describe("dashboard reducers", () => {
),
).toEqual({ ...initState, isEditing: null });
});
+
+ it("should return unchanged state if `clearCache: false` passed", () => {
+ expect(
+ reducer(
+ {
+ ...initState,
+ draftParameterValues: {
+ "60bca071": ["Gadget", "Doohickey", "Gizmo"],
+ },
+ },
+ {
+ type: INITIALIZE,
+ payload: {
+ clearCache: false,
+ },
+ },
+ ),
+ ).toEqual({
+ ...initState,
+ draftParameterValues: {
+ "60bca071": ["Gadget", "Doohickey", "Gizmo"],
+ },
+ });
+ });
+
+ it("should reset state if `clearCache`: false` is not passed", () => {
+ expect(
+ reducer(
+ {
+ ...initState,
+ draftParameterValues: {
+ "60bca071": ["Gadget", "Doohickey", "Gizmo"],
+ },
+ },
+ {
+ type: INITIALIZE,
+ },
+ ),
+ ).toEqual(initState);
+ });
});
describe("SET_EDITING_DASHBOARD", () => {
- it("should clear sideabr state when entering edit mode", () => {
+ it("should clear sidebar state when entering edit mode", () => {
const state = {
...initState,
sidebar: { name: "foo", props: { abc: 123 } },
@@ -118,7 +158,7 @@ describe("dashboard reducers", () => {
).toEqual({ ...state, isEditing: true, sidebar: { props: {} } });
});
- it("should clear sideabr state when leaving edit mode", () => {
+ it("should clear sidebar state when leaving edit mode", () => {
const state = {
...initState,
sidebar: { name: "foo", props: { abc: 123 } },
diff --git a/frontend/src/metabase/dashboard/selectors/selectors.js b/frontend/src/metabase/dashboard/selectors/selectors.js
index 25a55586b4f..944071ec100 100644
--- a/frontend/src/metabase/dashboard/selectors/selectors.js
+++ b/frontend/src/metabase/dashboard/selectors/selectors.js
@@ -201,7 +201,7 @@ export const getCanShowAutoApplyFiltersToast = createSelector(
export const getDocumentTitle = state =>
state.dashboard.loadingControls.documentTitle;
-export const getisNavigatingBackToDashboard = state =>
+export const getIsNavigatingBackToDashboard = state =>
state.dashboard.isNavigatingBackToDashboard;
export const getIsBookmarked = (state, props) =>
diff --git a/frontend/src/metabase/parameters/components/FilterApplyButton/FilterApplyButton.tsx b/frontend/src/metabase/parameters/components/FilterApplyButton/FilterApplyButton.tsx
index b0ee948b42e..0214fd1cfd1 100644
--- a/frontend/src/metabase/parameters/components/FilterApplyButton/FilterApplyButton.tsx
+++ b/frontend/src/metabase/parameters/components/FilterApplyButton/FilterApplyButton.tsx
@@ -8,8 +8,7 @@ import {
import { applyDraftParameterValues } from "metabase/dashboard/actions";
import { ApplyButton } from "./FilterApplyButton.styled";
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default function FilterApplyButton() {
+export function FilterApplyButton() {
const isAutoApplyFilters = useSelector(getIsAutoApplyFilters);
const hasUnappliedParameterValues = useSelector(
getHasUnappliedParameterValues,
diff --git a/frontend/src/metabase/parameters/components/FilterApplyButton/index.ts b/frontend/src/metabase/parameters/components/FilterApplyButton/index.ts
index a8fcc752f24..a00b70a88d9 100644
--- a/frontend/src/metabase/parameters/components/FilterApplyButton/index.ts
+++ b/frontend/src/metabase/parameters/components/FilterApplyButton/index.ts
@@ -1,2 +1 @@
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./FilterApplyButton";
+export { FilterApplyButton } from "./FilterApplyButton";
diff --git a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
index 37cb9e3e437..08816a17c65 100644
--- a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
+++ b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
@@ -14,7 +14,7 @@ import { isWithinIframe, initializeIframeResizer } from "metabase/lib/dom";
import { parseHashOptions } from "metabase/lib/browser";
import SyncedParametersList from "metabase/parameters/components/SyncedParametersList/SyncedParametersList";
-import FilterApplyButton from "metabase/parameters/components/FilterApplyButton";
+import { FilterApplyButton } from "metabase/parameters/components/FilterApplyButton";
import type {
Dashboard,
--
GitLab