Allow filtering on all stages in the QB (#47818)

e2e/test/scenarios/filters/filter-bulk.cy.spec.js

@@ -15,7 +15,7 @@ import {
 } from "e2e/support/helpers";
 import { createSegment } from "e2e/support/helpers/e2e-table-metadata-helpers";
 
-const { ORDERS_ID, ORDERS, PEOPLE_ID, PRODUCTS_ID } = SAMPLE_DATABASE;
+const { ORDERS_ID, ORDERS, PEOPLE_ID, PRODUCTS_ID, PRODUCTS } = SAMPLE_DATABASE;
 
 const rawQuestionDetails = {
   dataset_query: {
@@ -74,6 +74,27 @@ const aggregatedQuestionDetails = {
   },
 };
 
+const multiStageQuestionDetails = {
+  name: "Test question",
+  dataset_query: {
+    database: SAMPLE_DB_ID,
+    type: "query",
+    query: {
+      "source-query": {
+        "source-query": {
+          "source-table": PRODUCTS_ID,
+          aggregation: [["count"]],
+          breakout: [["field", PRODUCTS.CATEGORY, null]],
+        },
+        aggregation: [["count"]],
+        breakout: [["field", PRODUCTS.CATEGORY, null]],
+      },
+      aggregation: [["count"]],
+      breakout: [["field", PRODUCTS.CATEGORY, null]],
+    },
+  },
+};
+
 describe("scenarios > filters > bulk filtering", () => {
   beforeEach(() => {
     restore();
@@ -203,6 +224,68 @@ describe("scenarios > filters > bulk filtering", () => {
     cy.findByText("Showing 138 rows").should("be.visible");
   });
 
+  it("should be able to add and remove filters for all query stages", () => {
+    visitQuestionAdhoc(multiStageQuestionDetails);
+
+    cy.log("add filters for all stages in the filter modal");
+    filter();
+    modal().within(() => {
+      cy.log("stage 0");
+      cy.findByText("Products").click();
+      cy.findByLabelText("Gadget").click();
+
+      cy.log("stage 1");
+      cy.findByText("Summaries").click();
+      cy.findByLabelText("Widget").click();
+
+      cy.log("stage 2");
+      cy.findByText("Summaries (2)").click();
+      cy.findByLabelText("Gizmo").click();
+
+      cy.log("stage 3");
+      cy.findByText("Summaries (3)").click();
+      cy.findByLabelText("Doohickey").click();
+    });
+    applyFilters();
+
+    cy.log("check filters from all stages to be present in the filter panel");
+    cy.findByTestId("qb-filters-panel").within(() => {
+      cy.findByText("Category is Gadget").should("be.visible");
+      cy.findByText("Category is Widget").should("be.visible");
+      cy.findByText("Category is Gizmo").should("be.visible");
+      cy.findByText("Category is Doohickey").should("be.visible");
+    });
+
+    cy.log("check filters from all stages to be present in the filter modal");
+    filter();
+    modal().within(() => {
+      cy.log("stage 0");
+      cy.findByText("Products").click();
+      cy.findByLabelText("Gadget").should("be.checked");
+      cy.findByLabelText("Widget").should("not.be.checked");
+
+      cy.log("stage 1");
+      cy.findByText("Summaries").click();
+      cy.findByLabelText("Widget").should("be.checked");
+      cy.findByLabelText("Gizmo").should("not.be.checked");
+
+      cy.log("stage 2");
+      cy.findByText("Summaries (2)").click();
+      cy.findByLabelText("Gizmo").should("be.checked");
+      cy.findByLabelText("Doohickey").should("not.be.checked");
+
+      cy.log("stage 3");
+      cy.findByText("Summaries (3)").click();
+      cy.findByLabelText("Doohickey").should("be.checked");
+      cy.findByLabelText("Gadget").should("not.be.checked");
+    });
+
+    cy.log("clear all filters");
+    modal().button("Clear all filters").click();
+    applyFilters();
+    cy.findByTestId("qb-filters-panel").should("not.exist");
+  });
+
   describe("segment filters", () => {
     const SEGMENT_1_NAME = "Orders < 100";
     const SEGMENT_2_NAME = "Discounted Orders";
@@ -630,9 +713,6 @@ describe("scenarios > filters > bulk filtering", () => {
 });
 
 const applyFilters = () => {
-  modal().within(() => {
-    cy.findByTestId("apply-filters").click();
-  });
-
+  modal().findByTestId("apply-filters").click();
   cy.wait("@dataset");
 };

frontend/src/metabase-lib/query.ts

@@ -54,6 +54,13 @@ export function stageCount(query: Query): number {
   return ML.stage_count(query);
 }
 
+export function stageIndexes(query: Query): number[] {
+  return Array.from(
+    { length: stageCount(query) },
+    (_, stageIndex) => stageIndex,
+  );
+}
+
 export const hasClauses = (query: Query, stageIndex: number): boolean => {
   return ML.has_clauses(query, stageIndex);
 };

frontend/src/metabase-lib/query.unit.spec.ts

@@ -41,3 +41,15 @@ describe("suggestedName", () => {
     expect(Lib.suggestedName(query)).toBe("Orders");
   });
 });
+
+describe("stageIndexes", () => {
+  it("should return stage indexes for a single-stage query", () => {
+    const query = createQuery();
+    expect(Lib.stageIndexes(query)).toEqual([0]);
+  });
+
+  it("should return stage indexes for a multi-stage query", () => {
+    const query = Lib.appendStage(Lib.appendStage(createQuery()));
+    expect(Lib.stageIndexes(query)).toEqual([0, 1, 2]);
+  });
+});

frontend/src/metabase-lib/types.ts

@@ -207,7 +207,9 @@ export type DateTimeFingerprintDisplayInfo = {
   latest: string;
 };
 
-export type ColumnGroupDisplayInfo = TableDisplayInfo;
+export type ColumnGroupDisplayInfo = TableDisplayInfo & {
+  isMainGroup?: boolean;
+};
 
 export type SegmentDisplayInfo = {
   name: string;

frontend/src/metabase/common/utils/column-groups.ts

@@ -13,5 +13,8 @@ export function getColumnGroupIcon(
   if (groupInfo.isImplicitlyJoinable) {
     return "connections";
   }
-  return "sum";
+  if (groupInfo.isMainGroup) {
+    return "sum";
+  }
+  return "table";
 }

frontend/src/metabase/query_builder/components/view/ViewHeader/components/QuestionFiltersHeader/QuestionFiltersHeader.unit.spec.tsx

@@ -82,12 +82,12 @@ describe("QuestionFiltersHeader", () => {
     expect(screen.queryByTestId("TEST_CONTAINER")).toBeEmptyDOMElement();
   });
 
-  it("should render filters from the last two stages", () => {
+  it("should render filters from all stages", () => {
     setup();
-    expect(screen.getAllByTestId("filter-pill")).toHaveLength(2);
+    expect(screen.getAllByTestId("filter-pill")).toHaveLength(3);
+    expect(screen.getByText("Quantity is greater than 4")).toBeInTheDocument();
     expect(screen.getByText("Count is greater than 5")).toBeInTheDocument();
     expect(screen.getByText("User → Source is Organic")).toBeInTheDocument();
-    expect(screen.queryByText(/Quantity/i)).not.toBeInTheDocument();
   });
 
   it("should update a filter on the last stage", async () => {

frontend/src/metabase/querying/fields/components/FieldPanel/types.ts

@@ -12,5 +12,5 @@ export type ColumnGroupItem = {
   displayName: string;
   isSelected: boolean;
   isDisabled: boolean;
-  isSourceGroup: boolean;
+  isMainGroup: boolean;
 };

frontend/src/metabase/querying/fields/components/FieldPanel/utils.ts

@@ -35,7 +35,7 @@ function getGroupsWithColumns(
   columns: Lib.ColumnMetadata[],
 ): ColumnGroupItem[] {
   const groups = Lib.groupColumns(columns);
-  return groups.map((group, groupIndex) => {
+  return groups.map(group => {
     const groupInfo = Lib.displayInfo(query, stageIndex, group);
     const columnItems = getColumnItems(query, stageIndex, group);
 
@@ -44,7 +44,7 @@ function getGroupsWithColumns(
       displayName: groupInfo.displayName,
       isSelected: columnItems.every(({ isSelected }) => isSelected),
       isDisabled: columnItems.every(({ isDisabled }) => isDisabled),
-      isSourceGroup: groupIndex === 0,
+      isMainGroup: groupInfo.isMainGroup ?? false,
     };
   });
 }
@@ -53,7 +53,7 @@ function disableOnlySelectedQueryColumn(
   groupItems: ColumnGroupItem[],
 ): ColumnGroupItem[] {
   return groupItems.map(groupItem => {
-    if (!groupItem.isSourceGroup) {
+    if (!groupItem.isMainGroup) {
       return groupItem;
     }
 
@@ -139,10 +139,10 @@ export function toggleColumnGroupInQuery(
   groupItem: ColumnGroupItem,
 ) {
   if (groupItem.isSelected) {
-    // always leave 1 column in the first group selected to prevent creating queries without columns
+    // always leave 1 column in the main group selected to prevent creating queries without columns
     return groupItem.columnItems
       .filter(columnItem => columnItem.isSelected && !columnItem.isDisabled)
-      .filter((_, columnIndex) => !groupItem.isSourceGroup || columnIndex !== 0)
+      .filter((_, columnIndex) => !groupItem.isMainGroup || columnIndex !== 0)
       .reduce(
         (query, { column }) => Lib.removeField(query, stageIndex, column),
         query,

frontend/src/metabase/querying/filters/components/FilterPanel/utils.ts

@@ -3,8 +3,7 @@ import * as Lib from "metabase-lib";
 import type { FilterItem } from "./types";
 
 export function getFilterItems(query: Lib.Query): FilterItem[] {
-  const stageCount = Lib.stageCount(query);
-  const stageIndexes = stageCount > 1 ? [-2, -1] : [-1];
+  const stageIndexes = Lib.stageIndexes(query);
 
   return stageIndexes.flatMap(stageIndex => {
     const filters = Lib.filters(query, stageIndex);

frontend/src/metabase/querying/filters/components/FilterPanel/utils.unit.spec.ts

+import * as Lib from "metabase-lib";
+import { createQuery } from "metabase-lib/test-helpers";
+
+import { getFilterItems } from "./utils";
+
+const STAGE_COUNT = 4;
+
+function createFilteredQuery(query: Lib.Query) {
+  return Lib.filter(query, -1, Lib.expressionClause("=", [1, 1]));
+}
+
+function createMultiStageFilteredQuery() {
+  const stageIndexes = Array.from({ length: STAGE_COUNT }, (_, i) => i);
+  return stageIndexes.reduce(
+    query => Lib.appendStage(createFilteredQuery(query)),
+    createQuery(),
+  );
+}
+
+describe("getFilterItems", () => {
+  it("should get filters from all query stages", () => {
+    const query = createMultiStageFilteredQuery();
+    const items = getFilterItems(query);
+    expect(items.length).toEqual(STAGE_COUNT);
+  });
+});

frontend/src/metabase/querying/filters/hooks/use-filter-modal/utils/filters.ts

@@ -12,13 +12,17 @@ export function appendStageIfAggregated(query: Lib.Query) {
     : query;
 }
 
-function getStageIndexes(query: Lib.Query) {
-  const stageCount = Lib.stageCount(query);
-  return stageCount > 1 ? [-2, -1] : [-1];
+function getGroupName(
+  groupInfo: Lib.ColumnGroupDisplayInfo,
+  stageIndex: number,
+) {
+  return groupInfo.isMainGroup && stageIndex > 1
+    ? `${groupInfo.displayName} (${stageIndex})`
+    : groupInfo.displayName;
 }
 
 export function getGroupItems(query: Lib.Query): GroupItem[] {
-  const stageIndexes = getStageIndexes(query);
+  const stageIndexes = Lib.stageIndexes(query);
   return stageIndexes.flatMap(stageIndex => {
     const columns = Lib.filterableColumns(query, stageIndex);
     const groups = Lib.groupColumns(columns);
@@ -31,7 +35,7 @@ export function getGroupItems(query: Lib.Query): GroupItem[] {
 
       return {
         key: `${stageIndex}-${groupIndex}`,
-        displayName: groupInfo.displayName,
+        displayName: getGroupName(groupInfo, stageIndex),
         icon: getColumnGroupIcon(groupInfo),
         columnItems: availableColumns.map(column => {
           const columnInfo = Lib.displayInfo(query, stageIndex, column);
@@ -56,7 +60,7 @@ export function getGroupItems(query: Lib.Query): GroupItem[] {
 }
 
 export function hasFilters(query: Lib.Query) {
-  const stageIndexes = getStageIndexes(query);
+  const stageIndexes = Lib.stageIndexes(query);
   const filters = stageIndexes.flatMap(stageIndex =>
     Lib.filters(query, stageIndex),
   );
@@ -64,7 +68,7 @@ export function hasFilters(query: Lib.Query) {
 }
 
 export function removeFilters(query: Lib.Query) {
-  const stageIndexes = getStageIndexes(query);
+  const stageIndexes = Lib.stageIndexes(query);
   return stageIndexes.reduce(
     (newQuery, stageIndex) => Lib.removeFilters(newQuery, stageIndex),
     query,

frontend/src/metabase/querying/filters/hooks/use-filter-modal/utils/filters.unit.spec.ts

+import * as Lib from "metabase-lib";
+import { createQuery, createQueryWithClauses } from "metabase-lib/test-helpers";
+import { PRODUCTS_ID } from "metabase-types/api/mocks/presets";
+
+import { getGroupItems, hasFilters, removeFilters } from "./filters";
+
+const STAGE_COUNT = 4;
+
+function createFilteredQuery(query: Lib.Query) {
+  const joinTable = Lib.tableOrCardMetadata(query, PRODUCTS_ID);
+  const queryWithJoin = Lib.join(
+    query,
+    -1,
+    Lib.joinClause(
+      joinTable,
+      [
+        Lib.joinConditionClause(
+          query,
+          -1,
+          Lib.joinConditionOperators(query, -1)[0],
+          Lib.joinConditionLHSColumns(query, -1)[0],
+          Lib.joinConditionRHSColumns(query, -1, joinTable)[0],
+        ),
+      ],
+      Lib.availableJoinStrategies(query, -1)[0],
+    ),
+  );
+
+  const queryWithFilter = Lib.filter(
+    queryWithJoin,
+    -1,
+    Lib.expressionClause("=", [1, 1]),
+  );
+
+  return createQueryWithClauses({
+    query: queryWithFilter,
+    aggregations: [{ operatorName: "count" }],
+    breakouts: [{ tableName: "ORDERS", columnName: "TOTAL" }],
+  });
+}
+
+function createMultiStageFilteredQuery() {
+  const stageIndexes = Array.from({ length: STAGE_COUNT }, (_, i) => i);
+  return stageIndexes.reduce(
+    query => Lib.appendStage(createFilteredQuery(query)),
+    createQuery(),
+  );
+}
+
+describe("getGroupItems", () => {
+  it("should return groups for all stages", () => {
+    const query = createMultiStageFilteredQuery();
+    expect(getGroupItems(query).map(group => group.displayName)).toEqual([
+      "Orders",
+      "Products",
+      "User",
+      "Summaries",
+      "Products",
+      "Summaries (2)",
+      "Products",
+      "Summaries (3)",
+      "Products",
+      "Summaries (4)",
+    ]);
+  });
+});
+
+describe("hasFilters", () => {
+  it("should be true if there are filters on each stage", () => {
+    const query = createMultiStageFilteredQuery();
+    expect(hasFilters(query)).toBe(true);
+  });
+
+  it("should be true if there is a filter on a deeply nested stage", () => {
+    const stages = Array(STAGE_COUNT).fill(0);
+    const query = stages.reduce(
+      Lib.appendStage,
+      createFilteredQuery(createQuery()),
+    );
+    expect(hasFilters(query)).toBe(true);
+  });
+
+  it("should be false if there are no filters on any stage", () => {
+    const stages = Array(STAGE_COUNT).fill(0);
+    const query = stages.reduce(Lib.appendStage, createQuery());
+    expect(hasFilters(query)).toBe(false);
+  });
+});
+
+describe("removeFilters", () => {
+  it("should remove filters from all stages", () => {
+    const query = createMultiStageFilteredQuery();
+    const newQuery = removeFilters(query);
+    const stageIndexes = Lib.stageIndexes(newQuery);
+    expect(stageIndexes).toEqual([0, 1, 2, 3, 4]);
+    stageIndexes.forEach(stageIndex => {
+      expect(Lib.filters(newQuery, stageIndex)).toHaveLength(0);
+    });
+  });
+});

src/metabase/lib/column_group.cljc

@@ -10,6 +10,7 @@
    [metabase.lib.schema.id :as lib.schema.id]
    [metabase.lib.schema.metadata :as lib.schema.metadata]
    [metabase.lib.util :as lib.util]
+   [metabase.shared.util.i18n :as i18n]
    [metabase.util.malli :as mu]))
 
 (def ^:private GroupType
@@ -74,11 +75,12 @@
         (lib.metadata.calculation/display-info query stage-number card))
       ;; multi-stage queries (#30108)
       (when (next (:stages query))
-        {:display-name "Summaries"})
+        {:display-name (i18n/tru "Summaries")})
       ;; if this is a native query or something else that doesn't have a source Table or source Card then use the
       ;; stage display name.
       {:display-name (lib.metadata.calculation/display-name query stage-number stage)}))
-   {:is-from-join           false
+   {:is-main-group          true
+    :is-from-join           false
     :is-implicitly-joinable false}))
 
 (defmethod display-info-for-group-method :group-type/join.explicit
@@ -95,7 +97,8 @@
       (if-let [card (lib.metadata/card query card-id)]
         (lib.metadata.calculation/display-info query stage-number card)
         {:display-name (lib.card/fallback-display-name card-id)})))
-   {:is-from-join           true
+   {:is-main-group          false
+    :is-from-join           true
     :is-implicitly-joinable false}))
 
 (defmethod display-info-for-group-method :group-type/join.implicit
@@ -115,7 +118,8 @@
        ;; meaning (eg. ORDERS.customer_id vs. ORDERS.supplier_id both linking to a PEOPLE table).
        ;; See #30109 for more details.
        (update fk-info :display-name lib.util/strip-id)))
-   {:is-from-join           false
+   {:is-main-group          false
+    :is-from-join           false
     :is-implicitly-joinable true}))
 
 (defmethod lib.metadata.calculation/display-info-method :metadata/column-group

src/metabase/lib/metadata/calculation.cljc

@@ -303,6 +303,8 @@
    ;; if this is a Column, is it an implicitly joinable one? I.e. is it from a different table that we have not
    ;; already joined, but could implicitly join against?
    [:is-implicitly-joinable {:optional true} [:maybe :boolean]]
+   ;; if this is a ColumnGroup, is it the main one?
+   [:is-main-group {:optional true} [:maybe :boolean]]
    ;; For the `:table` field of a Column, is this the source table, or a joined table?
    [:is-source-table {:optional true} [:maybe :boolean]]
    ;; does this column occur in the breakout clause?

test/metabase/lib/column_group_test.cljc

@@ -81,8 +81,9 @@
                                              {:display-name "Sum of ID", :lib/source :source/previous-stage}]}]
             groups))
     (testing `lib/display-info
-      (is (=? [{:display-name "Summaries"
-                :is-from-join false
+      (is (=? [{:display-name           "Summaries"
+                :is-main-group          true
+                :is-from-join           false
                 :is-implicitly-joinable false}]
               (for [group groups]
                 (lib/display-info query group)))))
@@ -251,6 +252,7 @@
             groups))
     (testing `lib/display-info
       (is (=? [{:display-name           "Summaries"
+                :is-main-group          true
                 :is-from-join           false
                 :is-implicitly-joinable false}]
               (for [group groups]