diff --git a/e2e/test/scenarios/filters/time-series-chrome.cy.spec.ts b/e2e/test/scenarios/filters/time-series-chrome.cy.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..e92f5d3b3a3184edb0e4e472f8722fb3f25fa5fb --- /dev/null +++ b/e2e/test/scenarios/filters/time-series-chrome.cy.spec.ts @@ -0,0 +1,247 @@ +import { SAMPLE_DB_ID } from "e2e/support/cypress_data"; +import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database"; +import { restore, visitQuestionAdhoc } from "e2e/support/helpers"; + +const { PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE; + +describe("time-series chrome filter widget", () => { + beforeEach(() => { + restore(); + cy.signInAsNormalUser(); + }); + + describe("smoke tests", () => { + beforeEach(() => { + visitQuestionAdhoc({ + dataset_query: { + database: SAMPLE_DB_ID, + query: { + "source-table": PRODUCTS_ID, + aggregation: [["count"]], + breakout: [ + ["field", PRODUCTS.CREATED_AT, { "temporal-unit": "month" }], + ], + }, + type: "query", + }, + }); + }); + + it("should properly display the component and all its operators", () => { + const operators = [ + "Previous", + "Next", + "Current", + "Before", + "After", + "On", + "Between", + ]; + + cy.log( + "should display 'All time' as the initialy selected operator (metabase#22247)", + ); + cy.findByTestId("timeseries-filter-button") + .should("have.text", "All time") + .click(); + + cy.findByTestId("datetime-filter-picker") + .findByDisplayValue("All time") + .click(); + + cy.findByRole("listbox").within(() => { + cy.findByRole("option", { name: "All time" }).should( + "have.attr", + "aria-selected", + "true", + ); + + cy.log("Make sure we display all the operators"); + operators.forEach(operator => { + cy.findByRole("option", { name: operator }).should("be.visible"); + }); + }); + + cy.findByTestId("datetime-filter-picker").within(() => { + cy.log("Include 'current' interval switch should not be displayed"); + cy.findByLabelText(/^Include/).should("not.exist"); + cy.button("Apply").should("not.be.disabled"); + }); + }); + + it("should stay in sync with the relative date filter", () => { + cy.findByTestId("timeseries-filter-button").click(); + updateOperator("All time", "Previous"); + + cy.log("Check the state of the time-series chrome"); + cy.findByTestId("datetime-filter-picker").within(() => { + // Top row + cy.findByDisplayValue("Previous").should("be.visible"); + cy.findByDisplayValue("30").should("be.visible"); + cy.findByDisplayValue("days").should("be.visible"); + + cy.log("Toggle should be always off initially"); + // This is targeting the input checkbox that is hidden + cy.findByLabelText("Include today").should( + "have.attr", + "aria-checked", + "false", + ); + + // This is clicking on an actual label in the UI + cy.findByText("Include today").click(); + + cy.findByLabelText("Include today").should( + "have.attr", + "aria-checked", + "true", + ); + cy.button("Apply").click(); + cy.wait("@dataset"); + }); + + cy.findByTestId("filter-pill") + .should("have.text", "Created At is in the previous 30 days") + .click(); + + cy.log( + "Make sure the relative date picker reflects the state of the time-series chrome", + ); + cy.findByTestId("datetime-filter-picker").within(() => { + cy.findByRole("tab", { name: "Previous" }).should( + "have.attr", + "aria-selected", + "true", + ); + + cy.findByLabelText("Include today").should( + "have.attr", + "aria-checked", + "true", + ); + + cy.log( + "Switch should preserve its state after we change the direction", + ); + cy.findByRole("tab", { name: "Next" }).click(); + cy.findByLabelText("Include today").should( + "have.attr", + "aria-checked", + "true", + ); + }); + }); + }); + + describe("'Include this' switch", () => { + beforeEach(() => { + visitQuestionAdhoc({ + dataset_query: { + database: SAMPLE_DB_ID, + query: { + "source-table": PRODUCTS_ID, + aggregation: [["count"]], + breakout: [ + ["field", PRODUCTS.CREATED_AT, { "temporal-unit": "month" }], + ], + filter: [ + "time-interval", + [ + "field", + PRODUCTS.CREATED_AT, + { + "base-type": "type/DateTime", + }, + ], + 30, + "year", + { + "include-current": true, + }, + ], + }, + type: "query", + }, + }); + + cy.findByTestId("filter-pill").should( + "have.text", + "Created At is in the next 30 years", + ); + cy.findByTestId("timeseries-filter-button") + .should("have.text", "Next 30 Years") + .click(); + }); + + it("should preserve the state of 'Include current' switch when changing direction or the interval", () => { + cy.findByTestId("datetime-filter-picker").within(() => { + cy.findByLabelText("Include this year").should( + "have.attr", + "aria-checked", + "true", + ); + }); + + cy.log("Change the interval"); + cy.findByTestId("datetime-filter-picker") + .findByDisplayValue("years") + .click(); + cy.findByRole("listbox") + .findByRole("option", { name: "quarters" }) + .click(); + + cy.findByTestId("datetime-filter-picker").within(() => { + cy.findByDisplayValue("quarters").should("be.visible"); + cy.findByLabelText("Include this quarter").should( + "have.attr", + "aria-checked", + "true", + ); + + // Toggle off + cy.findByText("Include this quarter").click(); + }); + + cy.log("Change the direction"); + updateOperator("Next", "Previous"); + cy.findByTestId("datetime-filter-picker").within(() => { + cy.findByDisplayValue("Previous").should("be.visible"); + cy.findByLabelText("Include this quarter").should( + "have.attr", + "aria-checked", + "false", + ); + }); + }); + + it("should reset the 'Include current' switch state when navigating away from the relative interval date filter", () => { + cy.findByTestId("datetime-filter-picker").within(() => { + cy.findByLabelText("Include this year").should( + "have.attr", + "aria-checked", + "true", + ); + }); + + updateOperator("Next", "Current"); + cy.findByTestId("datetime-filter-picker") + .findByLabelText("Include today") + .should("not.exist"); + + updateOperator("Current", "Previous"); + cy.findByTestId("datetime-filter-picker").within(() => { + cy.findByDisplayValue("Previous").should("be.visible"); + cy.findByLabelText("Include this year").should( + "have.attr", + "aria-checked", + "false", + ); + }); + }); + }); +}); + +function updateOperator(from: string, to: string) { + cy.findByTestId("datetime-filter-picker").findByDisplayValue(from).click(); + cy.findByRole("listbox").findByRole("option", { name: to }).click(); +} diff --git a/e2e/test/scenarios/question-reproductions/reproductions-2.cy.spec.js b/e2e/test/scenarios/question-reproductions/reproductions-2.cy.spec.js index 79a11db6afc780c27f0c63d07899539a37a339ec..34f1a5adc15a605b7e75cb4a41a4ab5b5102c9ef 100644 --- a/e2e/test/scenarios/question-reproductions/reproductions-2.cy.spec.js +++ b/e2e/test/scenarios/question-reproductions/reproductions-2.cy.spec.js @@ -7,15 +7,12 @@ import { openOrdersTable, popover, modal, - summarize, openNativeEditor, startNewQuestion, entityPickerModal, entityPickerModalTab, visitQuestion, - openProductsTable, visitQuestionAdhoc, - sidebar, openNotebook, selectFilterOperator, chartPathWithFillColor, @@ -30,105 +27,6 @@ import { const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID, PEOPLE } = SAMPLE_DATABASE; -describe("time-series filter widget", () => { - beforeEach(() => { - restore(); - cy.signInAsAdmin(); - - openProductsTable(); - }); - - it("should properly display All time as the initial filtering (metabase#22247)", () => { - summarize(); - - sidebar().contains("Created At").click(); - cy.wait("@dataset"); - - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("All time").click(); - - popover().within(() => { - // Implicit assertion: there is only one select button - cy.findByDisplayValue("All time").should("be.visible"); - - cy.button("Apply").should("not.be.disabled"); - }); - }); - - it("should allow switching from All time filter", () => { - cy.findAllByText("Summarize").first().click(); - cy.findAllByText("Created At").last().click(); - cy.wait("@dataset"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Done").click(); - - // switch to previous 30 quarters - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("All time").click(); - popover().within(() => { - cy.findByDisplayValue("All time").click(); - }); - cy.findByTextEnsureVisible("Previous").click(); - cy.findByDisplayValue("days").click(); - cy.findByTextEnsureVisible("quarters").click(); - cy.button("Apply").click(); - cy.wait("@dataset"); - - cy.findByTestId("qb-filters-panel") - .findByText("Created At is in the previous 30 quarters") - .should("be.visible"); - }); - - it("should stay in-sync with the actual filter", () => { - cy.findAllByText("Filter").first().click(); - cy.findByTestId("filter-column-Created At").within(() => { - cy.findByLabelText("More options").click(); - }); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Last 3 months").click(); - cy.button("Apply filters").click(); - cy.wait("@dataset"); - - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Created At is in the previous 3 months").click(); - cy.findByDisplayValue("months").click(); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("years").click(); - cy.button("Update filter").click(); - cy.wait("@dataset"); - - cy.findAllByText("Summarize").first().click(); - cy.findAllByText("Created At").last().click(); - cy.wait("@dataset"); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Done").click(); - - cy.findByTestId("qb-filters-panel") - .findByText("Created At is in the previous 3 years") - .should("be.visible"); - - cy.findByTestId("timeseries-filter-button").click(); - popover().within(() => { - cy.findByDisplayValue("Previous").should("be.visible"); - cy.findByDisplayValue("All time").should("not.exist"); - cy.findByDisplayValue("Next").should("not.exist"); - }); - - // switch to All time filter - popover().within(() => { - cy.findByDisplayValue("Previous").click(); - }); - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("All time").click(); - cy.button("Apply").click(); - cy.wait("@dataset"); - - // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage - cy.findByText("Created At is in the previous 3 years").should("not.exist"); - cy.findByTextEnsureVisible("All time"); - }); -}); - describe("issue 23023", () => { const questionDetails = { display: "table", diff --git a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx index 14cda7afa31b40fc3560b50b9409558dc6bddee0..b160427ec797c45af3d8d4e5b558326ef762533f 100644 --- a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx +++ b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.tsx @@ -3,6 +3,8 @@ import { useMemo } from "react"; import { Group, Stack } from "metabase/ui"; import { SimpleRelativeDatePicker } from "../RelativeDatePicker"; +import { IncludeCurrentSwitch } from "../RelativeDatePicker/IncludeCurrentSwitch"; +import { isIntervalValue, isRelativeValue } from "../RelativeDatePicker/utils"; import { SimpleSpecificDatePicker } from "../SpecificDatePicker"; import type { DatePickerOperator, DatePickerValue } from "../types"; @@ -39,10 +41,13 @@ export function DateOperatorPicker({ <Stack> <Group> <FlexSelect data={options} value={optionType} onChange={handleChange} /> - {value?.type === "relative" && ( + {isRelativeValue(value) && ( <SimpleRelativeDatePicker value={value} onChange={onChange} /> )} </Group> + {isRelativeValue(value) && isIntervalValue(value) && ( + <IncludeCurrentSwitch value={value} onChange={onChange} /> + )} {value?.type === "specific" && ( <SimpleSpecificDatePicker value={value} onChange={onChange} /> )} diff --git a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx index 14682d4068befc189ae834121561dff75a50f2a3..f0b99d7fe52ad7c9e1a81e96b5f95db08419bc26 100644 --- a/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx +++ b/frontend/src/metabase/querying/components/DatePicker/DateOperatorPicker/DateOperatorPicker.unit.spec.tsx @@ -78,4 +78,87 @@ describe("DateOperatorPicker", () => { unit: "month", }); }); + + describe("Include current switch", () => { + it("should not show 'Include current' switch by default", async () => { + setup(); + + await userEvent.click(screen.getByDisplayValue("All time")); + expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument(); + }); + + it("should not show 'Include current' switch by for the specific date value", async () => { + setup(); + + await userEvent.click(screen.getByDisplayValue("All time")); + await userEvent.click(screen.getByText("Between")); + expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument(); + }); + + it("should not show 'Include current' switch by for the `Current` relative date", async () => { + setup(); + + await userEvent.click(screen.getByDisplayValue("All time")); + await userEvent.click(screen.getByText("Current")); + expect(screen.queryByLabelText(/^Include/)).not.toBeInTheDocument(); + }); + + it("should show 'Include current' switch and work for the relative `Previous` or `Next`", async () => { + setup({ + value: { + type: "relative", + value: 1, + unit: "month", + }, + }); + + expect(screen.getByDisplayValue("Next")).toBeInTheDocument(); + expect(screen.getByLabelText("Include this month")).toBeInTheDocument(); + }); + + it("should turn the 'Include current' switch on and register that change", async () => { + const { onChange } = setup({ + value: { + type: "relative", + value: 1, + unit: "month", + }, + }); + + await userEvent.click(screen.getByLabelText("Include this month")); + + expect(onChange).toHaveBeenCalledWith({ + options: { + "include-current": true, + }, + type: "relative", + value: 1, + unit: "month", + }); + }); + + it("should turn the 'Include current' switch off and register that change", async () => { + const { onChange } = setup({ + value: { + options: { + "include-current": true, + }, + type: "relative", + value: 1, + unit: "month", + }, + }); + + await userEvent.click(screen.getByLabelText("Include this month")); + + expect(onChange).toHaveBeenCalledWith({ + options: { + "include-current": false, + }, + type: "relative", + value: 1, + unit: "month", + }); + }); + }); }); diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/DateIntervalPicker.tsx b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/DateIntervalPicker.tsx index edd1bfec548602189b33c7fcbc348168c7267df7..84449650c896f16556390ba48dc455d7e58717ab 100644 --- a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/DateIntervalPicker.tsx +++ b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/DateIntervalPicker/DateIntervalPicker.tsx @@ -10,10 +10,10 @@ import { NumberInput, Select, Text, - Switch, Tooltip, } from "metabase/ui"; +import { IncludeCurrentSwitch } from "../IncludeCurrentSwitch"; import type { DateIntervalValue } from "../types"; import { formatDateRange, @@ -22,13 +22,7 @@ import { getUnitOptions, } from "../utils"; -import { - getIncludeCurrentLabel, - getIncludeCurrent, - setIncludeCurrent, - setUnit, - setDefaultOffset, -} from "./utils"; +import { setUnit, setDefaultOffset } from "./utils"; interface DateIntervalPickerProps { value: DateIntervalValue; @@ -47,7 +41,6 @@ export function DateIntervalPicker({ }: DateIntervalPickerProps) { const interval = getInterval(value); const unitOptions = getUnitOptions(value); - const includeCurrent = getIncludeCurrent(value); const dateRangeText = formatDateRange(value); const handleIntervalChange = (inputValue: number | "") => { @@ -67,10 +60,6 @@ export function DateIntervalPicker({ onChange(setDefaultOffset(value)); }; - const handleIncludeCurrentSwitch = () => { - onChange(setIncludeCurrent(value, !includeCurrent)); - }; - const handleSubmit = (event: FormEvent) => { event.preventDefault(); onSubmit(); @@ -105,15 +94,7 @@ export function DateIntervalPicker({ )} </Flex> <Flex p="md" pt={0}> - <Switch - aria-checked={includeCurrent} - checked={includeCurrent} - data-testid="include-current-interval-option" - label={t`Include ${getIncludeCurrentLabel(value.unit)}`} - labelPosition="right" - onChange={handleIncludeCurrentSwitch} - size="sm" - /> + <IncludeCurrentSwitch value={value} onChange={onChange} /> </Flex> <Divider /> <Group px="md" py="sm" position="apart"> diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/IncludeCurrentSwitch/IncludeCurrentSwitch.tsx b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/IncludeCurrentSwitch/IncludeCurrentSwitch.tsx new file mode 100644 index 0000000000000000000000000000000000000000..b0e4017f47510badd1d5b5dd750f781be292a918 --- /dev/null +++ b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/IncludeCurrentSwitch/IncludeCurrentSwitch.tsx @@ -0,0 +1,38 @@ +import { t } from "ttag"; + +import { Switch } from "metabase/ui"; + +import { + getIncludeCurrentLabel, + getIncludeCurrent, + setIncludeCurrent, +} from "../DateIntervalPicker/utils"; +import type { DateIntervalValue } from "../types"; + +interface IncludeCurrentSwitchProps { + value: DateIntervalValue; + onChange: (value: DateIntervalValue) => void; +} + +export const IncludeCurrentSwitch = ({ + value, + onChange, +}: IncludeCurrentSwitchProps) => { + const includeCurrent = getIncludeCurrent(value); + + const handleIncludeCurrentSwitch = () => { + onChange(setIncludeCurrent(value, !includeCurrent)); + }; + + return ( + <Switch + aria-checked={includeCurrent} + checked={includeCurrent} + data-testid="include-current-interval-option" + label={t`Include ${getIncludeCurrentLabel(value.unit)}`} + labelPosition="right" + onChange={handleIncludeCurrentSwitch} + size="sm" + /> + ); +}; diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/IncludeCurrentSwitch/index.ts b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/IncludeCurrentSwitch/index.ts new file mode 100644 index 0000000000000000000000000000000000000000..8f0b3250d25145a6937e6442b9474c767108a1fa --- /dev/null +++ b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/IncludeCurrentSwitch/index.ts @@ -0,0 +1 @@ +export * from "./IncludeCurrentSwitch"; diff --git a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/utils.ts b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/utils.ts index 3c1915ef3d669fc8430d348e5e3f4bedad169ad7..7478209631dcab5d201c27a1951197a80b7da33f 100644 --- a/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/utils.ts +++ b/frontend/src/metabase/querying/components/DatePicker/RelativeDatePicker/utils.ts @@ -5,11 +5,18 @@ import type { RelativeIntervalDirection, RelativeDatePickerValue, DatePickerTruncationUnit, + DatePickerValue, } from "../types"; import { DEFAULT_VALUE } from "./constants"; import type { DateIntervalValue, DateOffsetIntervalValue } from "./types"; +export function isRelativeValue( + value?: DatePickerValue, +): value is RelativeDatePickerValue { + return value?.type === "relative"; +} + export function isIntervalValue( value: RelativeDatePickerValue, ): value is DateIntervalValue {