Remove time-series grouping widget for custom date breakout (#33810)

* Remove unsupported timeseries grouping on expression dimension

* Add unit tests

* Update frontend/src/metabase/query_builder/containers/QueryBuilder.unit.spec.tsx

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>

* Update frontend/src/metabase/query_builder/containers/QueryBuilder.unit.spec.tsx

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>

* Rename a test ID to make it more consistent with the component name

* Fix the code that broke other E2E tests

* Fix failed test from merging master

* Another attempt to fix this

* Fix type error

* Make the code easier to read by eliminating double negation

---------

Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com>

e2e/test/scenarios/question/reproductions/28599-model-conversion-time-footer.cy.spec.js

@@ -36,7 +36,7 @@ describe("issue 28599", () => {
   });
 
   it("should not show time granularity footer after question conversion to a model (metabase#28599)", () => {
-    cy.findByTestId("timeseries-mode-bar").within(() => {
+    cy.findByTestId("time-series-mode-footer").within(() => {
       cy.findByText(`View`).should("be.visible");
       cy.findByText(`All Time`).should("be.visible");
       cy.findByText(`by`).should("be.visible");
@@ -49,6 +49,6 @@ describe("issue 28599", () => {
 
     cy.wait("@updateCard");
 
-    cy.findByTestId("timeseries-mode-bar").should("not.exist");
+    cy.findByTestId("time-series-mode-bar").should("not.exist");
   });
 });

e2e/test/scenarios/visualizations/drillthroughs/chart_drill.cy.spec.js

@@ -677,7 +677,7 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
       cy.findByText(`≠`).should("be.visible");
     });
 
-    cy.findByTestId("timeseries-mode-bar").within(() => {
+    cy.findByTestId("time-series-mode-footer").within(() => {
       cy.findByText(`View`).should("be.visible");
       cy.findByText(`All Time`).should("be.visible");
       cy.findByText(`by`).should("be.visible");
@@ -728,7 +728,7 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
       cy.findByText(`≠`).should("be.visible");
     });
 
-    cy.findByTestId("timeseries-mode-bar").within(() => {
+    cy.findByTestId("time-series-mode-footer").within(() => {
       cy.findByText(`View`).should("be.visible");
       cy.findByText(`All Time`).should("be.visible");
       cy.findByText(`by`).should("be.visible");

e2e/test/scenarios/visualizations/drillthroughs/table_drills.cy.spec.js

@@ -176,7 +176,7 @@ describe("scenarios > visualizations > drillthroughs > table_drills", function (
       cy.findByText(`≠`).should("be.visible");
     });
 
-    cy.findByTestId("timeseries-mode-bar").within(() => {
+    cy.findByTestId("time-series-mode-footer").within(() => {
       cy.findByText(`View`).should("be.visible");
       cy.findByText(`All Time`).should("be.visible");
       cy.findByText(`by`).should("be.visible");

frontend/src/metabase-types/api/query.ts

@@ -355,7 +355,8 @@ export type Expression =
       ExpressionOperand,
       ExpressionOperand,
       ExpressionOperand,
-    ];
+    ]
+  | ConcreteFieldReference;
 
 type ExpressionOperator = string;
 type ExpressionOperand =

frontend/src/metabase/query_builder/containers/QueryBuilder.unit.spec.tsx

@@ -51,6 +51,37 @@ const TEST_CARD = createMockCard({
   dataset: true,
 });
 
+const TEST_TIME_SERIES_WITH_DATE_BREAKOUT_CARD = createMockCard({
+  ...TEST_CARD,
+  dataset: false,
+  dataset_query: {
+    database: SAMPLE_DB_ID,
+    type: "query",
+    query: {
+      "source-table": ORDERS_ID,
+      aggregation: [["count"]],
+      breakout: [["field", ORDERS.CREATED_AT, null]],
+    },
+  },
+});
+
+const TEST_TIME_SERIES_WITH_CUSTOM_DATE_BREAKOUT_CARD = createMockCard({
+  ...TEST_CARD,
+  dataset: false,
+  dataset_query: {
+    database: SAMPLE_DB_ID,
+    type: "query",
+    query: {
+      "source-table": ORDERS_ID,
+      aggregation: [["count"]],
+      breakout: [["expression", "Custom Created At"]],
+      expressions: {
+        "Custom Created At": ["field", ORDERS.CREATED_AT, null],
+      },
+    },
+  },
+});
+
 const TEST_CARD_VISUALIZATION = createMockCard({
   ...TEST_CARD,
   dataset_query: {
@@ -229,6 +260,43 @@ describe("QueryBuilder", () => {
 
         expect(screen.getByDisplayValue(TEST_CARD.name)).toBeInTheDocument();
       });
+
+      it("renders time series grouping widget for date field breakout", async () => {
+        await setup({
+          card: TEST_TIME_SERIES_WITH_DATE_BREAKOUT_CARD,
+        });
+        const timeSeriesModeFooter = await screen.findByTestId(
+          "time-series-mode-footer",
+        );
+        expect(timeSeriesModeFooter).toBeInTheDocument();
+        expect(
+          within(timeSeriesModeFooter).getByText("by"),
+        ).toBeInTheDocument();
+        expect(
+          within(timeSeriesModeFooter).getByTestId(
+            "time-series-grouping-select-button",
+          ),
+        ).toBeInTheDocument();
+      });
+
+      it("doesn't render time series grouping widget for custom date field breakout (metabase#33504)", async () => {
+        await setup({
+          card: TEST_TIME_SERIES_WITH_CUSTOM_DATE_BREAKOUT_CARD,
+        });
+
+        const timeSeriesModeFooter = await screen.findByTestId(
+          "time-series-mode-footer",
+        );
+        expect(timeSeriesModeFooter).toBeInTheDocument();
+        expect(
+          within(timeSeriesModeFooter).queryByText("by"),
+        ).not.toBeInTheDocument();
+        expect(
+          within(timeSeriesModeFooter).queryByTestId(
+            "time-series-grouping-select-button",
+          ),
+        ).not.toBeInTheDocument();
+      });
     });
 
     describe("renders the row count regardless of visualization type", () => {

frontend/src/metabase/visualizations/click-actions/components/TimeseriesGroupingWidget.jsx

@@ -30,7 +30,9 @@ export class TimeseriesGroupingWidget extends Component {
       return (
         <PopoverWithTrigger
           triggerElement={
-            <SelectButton hasValue>{dimension.subDisplayName()}</SelectButton>
+            <SelectButton hasValue dataTestId="time-series-grouping">
+              {dimension.subDisplayName()}
+            </SelectButton>
           }
           triggerClasses="my2"
           ref={ref => (this._popover = ref)}

frontend/src/metabase/visualizations/click-actions/components/TimeseriesModeFooter.tsx

 import { t } from "ttag";
 import type { ModeFooterComponentProps } from "metabase/visualizations/types";
 import type Question from "metabase-lib/Question";
+import StructuredQuery from "metabase-lib/queries/StructuredQuery";
 import { TimeseriesFilterWidget } from "./TimeseriesFilterWidget";
 import { TimeseriesGroupingWidget } from "./TimeseriesGroupingWidget";
 
 type Props = ModeFooterComponentProps;
 
-export const TimeseriesModeFooter = (props: Props): JSX.Element => {
+export const TimeseriesModeFooter = (props: Props): JSX.Element | null => {
   const onChange = (question: Question) => {
     const { updateQuestion } = props;
     updateQuestion(question, { run: true });
   };
 
+  // We could encounter stale `mode` e.g. when converting a question from GUI to native,
+  // the `mode` would remain `timeseries` when it should have been `native` instead.
+  // So we shouldn't assume we'll always get time series question here.
+  if (!(props.query instanceof StructuredQuery)) {
+    return null;
+  }
+  const [breakout] = props.query.breakouts();
+  if (!breakout) {
+    return null;
+  }
+  const hasTimeSeriesGroupingWidget = !breakout.dimension().isExpression();
+
   return (
-    <div className="flex layout-centered" data-testid="timeseries-mode-bar">
+    <div className="flex layout-centered" data-testid="time-series-mode-footer">
       <span className="mr1">{t`View`}</span>
       <TimeseriesFilterWidget {...props} card={props.lastRunCard} />
-      <span className="mx1">{t`by`}</span>
-      <TimeseriesGroupingWidget
-        {...props}
-        onChange={onChange}
-        card={props.lastRunCard}
-      />
+      {hasTimeSeriesGroupingWidget && (
+        <>
+          <span className="mx1">{t`by`}</span>
+          <TimeseriesGroupingWidget
+            {...props}
+            onChange={onChange}
+            card={props.lastRunCard}
+          />
+        </>
+      )}
     </div>
   );
 };

frontend/src/metabase/visualizations/types/click-actions.ts

@@ -167,5 +167,5 @@ export interface QueryClickActionsMode {
 
   clickActions: LegacyDrill[];
   fallback?: LegacyDrill;
-  ModeFooter?: (props: ModeFooterComponentProps) => JSX.Element;
+  ModeFooter?: (props: ModeFooterComponentProps) => JSX.Element | null;
 }