From 8f8ab297744d7188101aef15b758f678af36aa6f Mon Sep 17 00:00:00 2001 From: Denis Berezin <denis.berezin@metabase.com> Date: Thu, 19 Oct 2023 14:54:50 +0200 Subject: [PATCH] Add dashboard parameter mapping for date range (#34544) * Add dashboard parameter mapping for date range * Fix unit tests * Add e2e tests * Update logic to use date range only for specific units --- .../17879-date-parameter-mapping.cy.spec.js | 131 ++++++++++++++++++ .../parameters/utils/click-behavior.js | 20 ++- .../queries/drills/dashboard-click-drill.js | 11 +- .../ClickBehaviorSidebar.tsx | 2 +- .../ClickBehaviorSidebarHeader.tsx | 7 +- .../ClickBehaviorSidebarHeader/index.ts | 3 +- frontend/src/metabase/lib/formatting/date.tsx | 32 +++-- frontend/src/metabase/lib/time.ts | 4 +- 8 files changed, 185 insertions(+), 25 deletions(-) create mode 100644 e2e/test/scenarios/dashboard/reproductions/17879-date-parameter-mapping.cy.spec.js diff --git a/e2e/test/scenarios/dashboard/reproductions/17879-date-parameter-mapping.cy.spec.js b/e2e/test/scenarios/dashboard/reproductions/17879-date-parameter-mapping.cy.spec.js new file mode 100644 index 00000000000..db93be94221 --- /dev/null +++ b/e2e/test/scenarios/dashboard/reproductions/17879-date-parameter-mapping.cy.spec.js @@ -0,0 +1,131 @@ +import { + editDashboard, + popover, + restore, + saveDashboard, + showDashboardCardActions, + visitDashboard, +} from "e2e/support/helpers"; +import { ORDERS, ORDERS_ID } from "metabase-types/api/mocks/presets"; + +describe("issue 17879", () => { + beforeEach(() => { + restore(); + cy.signInAsAdmin(); + + cy.intercept("POST", "/api/dashboard/*/dashcard/*/card/*/query").as( + "dashcardQuery", + ); + }); + + it("should map dashcard date parameter to correct date range filter in target question - month -> day (metabase#17879)", () => { + setupDashcardAndDrillToQuestion({ + sourceDateUnit: "month", + expectedFilterText: "Created At is April 1–30, 2022", + }); + }); + + it("should map dashcard date parameter to correct date range filter in target question - week -> day (metabase#17879)", () => { + setupDashcardAndDrillToQuestion({ + sourceDateUnit: "week", + expectedFilterText: "Created At is April 24–30, 2022", + }); + }); + + it("should map dashcard date parameter to correct date range filter in target question - year -> day (metabase#17879)", () => { + setupDashcardAndDrillToQuestion({ + sourceDateUnit: "year", + expectedFilterText: "Created At is January 1 – December 31, 2022", + }); + }); + + it("should map dashcard date parameter to correct date range filter in target question - year -> month (metabase#17879)", () => { + setupDashcardAndDrillToQuestion({ + sourceDateUnit: "year", + expectedFilterText: "Created At is January 1 – December 31, 2022", + targetDateUnit: "month", + }); + }); +}); + +function setupDashcardAndDrillToQuestion({ + sourceDateUnit, + expectedFilterText, + targetDateUnit = "default", +}) { + if (targetDateUnit === "default") { + cy.createQuestion({ + name: "Q1 - 17879", + query: { + "source-table": ORDERS_ID, + limit: 3, + }, + }); + } else { + cy.createQuestion({ + name: "Q1 - 17879", + query: { + "source-table": ORDERS_ID, + aggregation: [["count"]], + breakout: [ + ["field", ORDERS.CREATED_AT, { "temporal-unit": targetDateUnit }], + ], + limit: 3, + }, + }); + } + + cy.createDashboardWithQuestions({ + dashboardName: "Dashboard with aggregated Q2", + questions: [ + { + name: "Q2", + display: "line", + query: { + "source-table": ORDERS_ID, + aggregation: [["count"]], + breakout: [ + ["field", ORDERS.CREATED_AT, { "temporal-unit": sourceDateUnit }], + ], + limit: 3, + }, + }, + ], + }).then(({ dashboard }) => { + cy.intercept( + "POST", + `/api/dashboard/${dashboard.id}/dashcard/*/card/*/query`, + ).as("getCardQuery"); + + visitDashboard(dashboard.id); + editDashboard(dashboard.id); + + showDashboardCardActions(); + cy.findByTestId("dashboardcard-actions-panel").within(() => { + cy.icon("click").click(); + }); + + cy.findByText("Go to a custom destination").click(); + cy.findByText("Saved question").click(); + cy.findByText("Q1 - 17879").click(); + cy.findByText("Orders → Created At").click(); + + popover().within(() => { + cy.findByText("Created At").click(); + }); + + cy.findByText("Done").click(); + + saveDashboard(); + + cy.wait("@getCardQuery"); + + cy.findByTestId("visualization-root").within(() => { + cy.get("circle").first().click({ force: true }); + }); + + cy.url().should("include", `/question`); + + cy.findByTestId("qb-filters-panel").should("have.text", expectedFilterText); + }); +} diff --git a/frontend/src/metabase-lib/parameters/utils/click-behavior.js b/frontend/src/metabase-lib/parameters/utils/click-behavior.js index b29f22b9391..a975180a268 100644 --- a/frontend/src/metabase-lib/parameters/utils/click-behavior.js +++ b/frontend/src/metabase-lib/parameters/utils/click-behavior.js @@ -2,7 +2,10 @@ import _ from "underscore"; import { getIn } from "icepick"; import { parseTimestamp } from "metabase/lib/time"; -import { formatDateTimeForParameter } from "metabase/lib/formatting/date"; +import { + formatDateTimeForParameter, + formatDateToRangeForParameter, +} from "metabase/lib/formatting/date"; import { dimensionFilterForParameter, variableFilterForParameter, @@ -251,6 +254,8 @@ export function formatSourceForTarget( ) { const datum = data[source.type][source.id.toLowerCase()] || []; if (datum.column && isDate(datum.column)) { + const sourceDateUnit = datum.column.unit; + if (target.type === "parameter") { // we should serialize differently based on the target parameter type const parameter = getParameter(target, { extraData, clickBehavior }); @@ -258,15 +263,20 @@ export function formatSourceForTarget( return formatDateForParameterType( datum.value, parameter.type, - datum.column.unit, + sourceDateUnit, ); } } else { - // If the target is a dimension or variable,, we serialize as a date to remove the timestamp. - // TODO: provide better serialization for field filter widget types + // If the target is a dimension or variable, we serialize as a date to remove the timestamp + + if (["week", "month", "quarter", "year"].includes(datum.column.unit)) { + return formatDateToRangeForParameter(datum.value, datum.column.unit); + } + return formatDateForParameterType(datum.value, "date/single"); } } + return datum.value; } @@ -275,6 +285,7 @@ function formatDateForParameterType(value, parameterType, unit) { if (!m.isValid()) { return String(value); } + if (parameterType === "date/month-year") { return m.format("YYYY-MM"); } else if (parameterType === "date/quarter-year") { @@ -284,6 +295,7 @@ function formatDateForParameterType(value, parameterType, unit) { } else if (parameterType === "date/all-options") { return formatDateTimeForParameter(value, unit); } + return value; } diff --git a/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js b/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js index 1577e9a2717..154d2615e72 100644 --- a/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js +++ b/frontend/src/metabase-lib/queries/drills/dashboard-click-drill.js @@ -10,6 +10,7 @@ import { } from "metabase-lib/parameters/utils/click-behavior"; import Question from "metabase-lib/Question"; import * as ML_Urls from "metabase-lib/urls"; +import { isDate } from "metabase-lib/types/utils/isa"; export function getDashboardDrillType(clicked) { const clickBehavior = getClickBehavior(clicked); @@ -100,7 +101,7 @@ export function getDashboardDrillQuestionUrl(question, clicked) { target: target.dimension, id, slug: id, - type: getTypeForSource(source, extraData), + type: getTypeForSource(source, data, extraData), })) .value(); @@ -166,12 +167,18 @@ function getParameterValuesBySlug( .value(); } -function getTypeForSource(source, extraData) { +function getTypeForSource(source, data, extraData) { if (source.type === "parameter") { const parameters = getIn(extraData, ["dashboard", "parameters"]) || []; const { type = "text" } = parameters.find(p => p.id === source.id) || {}; return type; } + + const datum = data[source.type][source.id.toLowerCase()] || []; + if (datum.column && isDate(datum.column)) { + return "date"; + } + return "text"; } diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx index 2f775ffcb50..bf14bd14591 100644 --- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx +++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebar.tsx @@ -21,7 +21,7 @@ import { clickBehaviorIsValid } from "metabase-lib/parameters/utils/click-behavi import { getColumnKey } from "metabase-lib/queries/utils/get-column-key"; import { getClickBehaviorForColumn } from "./utils"; import ClickBehaviorSidebarContent from "./ClickBehaviorSidebarContent"; -import ClickBehaviorSidebarHeader from "./ClickBehaviorSidebarHeader"; +import { ClickBehaviorSidebarHeader } from "./ClickBehaviorSidebarHeader"; function shouldShowTypeSelector(clickBehavior?: ClickBehavior) { return !clickBehavior || clickBehavior.type == null; diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx index 83b0bd52abe..01c6866cf61 100644 --- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx +++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/ClickBehaviorSidebarHeader.tsx @@ -44,13 +44,10 @@ function HeaderContent({ dashcard, selectedColumn, onUnsetColumn }: Props) { return <DefaultHeader>{dashcard.card.name}</DefaultHeader>; } -function ClickBehaviorSidebarHeader(props: Props) { +export const ClickBehaviorSidebarHeader = (props: Props) => { return ( <SidebarHeader> <HeaderContent {...props} /> </SidebarHeader> ); -} - -// eslint-disable-next-line import/no-default-export -- deprecated usage -export default ClickBehaviorSidebarHeader; +}; diff --git a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/index.ts b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/index.ts index ec9c9e9343f..002a53a7010 100644 --- a/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/index.ts +++ b/frontend/src/metabase/dashboard/components/ClickBehaviorSidebar/ClickBehaviorSidebarHeader/index.ts @@ -1,2 +1 @@ -// eslint-disable-next-line import/no-default-export -- deprecated usage -export { default } from "./ClickBehaviorSidebarHeader"; +export { ClickBehaviorSidebarHeader } from "./ClickBehaviorSidebarHeader"; diff --git a/frontend/src/metabase/lib/formatting/date.tsx b/frontend/src/metabase/lib/formatting/date.tsx index 7c980089a37..f0a4e37ab6a 100644 --- a/frontend/src/metabase/lib/formatting/date.tsx +++ b/frontend/src/metabase/lib/formatting/date.tsx @@ -541,19 +541,33 @@ export function formatDateTimeForParameter(value: string, unit: DatetimeUnit) { } else if (unit === "day") { return m.format("YYYY-MM-DD"); } else if (unit) { - const start = m.clone().startOf(unit); - const end = m.clone().endOf(unit); + return formatDateToRangeForParameter(value, unit); + } - if (!start.isValid() || !end.isValid()) { - return String(value); - } + return String(value); +} - const isSameDay = start.isSame(end, "day"); +export function formatDateToRangeForParameter( + value: string, + unit: DatetimeUnit, +) { + const m = parseTimestamp(value, unit); + if (!m.isValid()) { + return String(value); + } + + const start = m.clone().startOf(unit); + const end = m.clone().endOf(unit); - return isSameDay - ? start.format("YYYY-MM-DD") - : `${start.format("YYYY-MM-DD")}~${end.format("YYYY-MM-DD")}`; + if (!start.isValid() || !end.isValid()) { + return String(value); } + + const isSameDay = start.isSame(end, "day"); + + return isSameDay + ? start.format("YYYY-MM-DD") + : `${start.format("YYYY-MM-DD")}~${end.format("YYYY-MM-DD")}`; } export function normalizeDateTimeRangeWithUnit( diff --git a/frontend/src/metabase/lib/time.ts b/frontend/src/metabase/lib/time.ts index 0ed8c2fc800..1bc9a769acd 100644 --- a/frontend/src/metabase/lib/time.ts +++ b/frontend/src/metabase/lib/time.ts @@ -190,7 +190,7 @@ type NUMERIC_UNIT_FORMATS_KEY_TYPE = export function parseTimestamp( value: MomentInput, unit: DatetimeUnit | null = null, - local: unknown = false, + isLocal = false, ) { let m: any; if (moment.isMoment(value)) { @@ -206,5 +206,5 @@ export function parseTimestamp( } else { m = moment.utc(value); } - return local ? m.local() : m; + return isLocal ? m.local() : m; } -- GitLab