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 0000000000000000000000000000000000000000..db93be94221e3cfe8317cee13acc4264658ac463 --- /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 b29f22b93915178a0263c4e9444183bc3082ae75..a975180a26806a5b30d89f1e8e151499592fc40d 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 1577e9a27177164227985096a20968eecc706a16..154d2615e72934b0dced4f41e292fdb42d6c9085 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 2f775ffcb50113b8286f67cb3c4aa1bac1be8e09..bf14bd145914d829ca3c8ed5166bdf27a8559e54 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 83b0bd52abe91d859f0dd18d1399d5b2d830f805..01c6866cf61f4fd288828767f691aaab7107c834 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 ec9c9e9343f3b0e2fb0cc332de3bbc61fdf0c9e5..002a53a70103860b99834261bbbba4fe8b71f3e5 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 7c980089a37500afb0777ec26c528c24e0909551..f0a4e37ab6a021e2e017f5ace5574d6d1a3eedbd 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 0ed8c2fc800a3baf61ed12d213594b0f4e09d8a7..1bc9a769acd112532a28818bae0883b11ab98156 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; }