From ed7e147b7cdace92b6f84e35523558927d1f5821 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Thu, 14 Mar 2024 18:06:59 +0200
Subject: [PATCH] Add UI to swap clauses in the notebook (#39205) (#40131)
---
.../helpers/e2e-ui-elements-helpers.js | 11 +-
.../visualization-options.cy.spec.js | 6 +-
.../scenarios/question/notebook.cy.spec.js | 63 ++++++++
.../scenarios/question/settings.cy.spec.js | 10 +-
.../bar_chart.cy.spec.js | 6 +-
.../visualizations-charts/funnel.cy.spec.js | 6 +-
.../28311-sorting-table-columns.cy.spec.js | 9 +-
.../visualizations-tabular/table.cy.spec.js | 9 +-
frontend/src/metabase-lib/query.ts | 9 ++
frontend/src/metabase-lib/types.ts | 2 +
.../steps/AggregateStep/AggregateStep.tsx | 18 ++-
.../steps/BreakoutStep/BreakoutStep.tsx | 14 ++
.../notebook/steps/ClauseStep/ClauseStep.tsx | 147 ++++++++++++++----
.../notebook/steps/ExpressionStep.tsx | 29 +++-
.../notebook/steps/FilterStep/FilterStep.tsx | 42 +++--
.../notebook/steps/SortStep/SortStep.tsx | 14 ++
.../components/FilterPicker/FilterPicker.tsx | 8 +-
17 files changed, 320 insertions(+), 83 deletions(-)
diff --git a/e2e/support/helpers/e2e-ui-elements-helpers.js b/e2e/support/helpers/e2e-ui-elements-helpers.js
index 5bf030939c9..5e64e90e2e7 100644
--- a/e2e/support/helpers/e2e-ui-elements-helpers.js
+++ b/e2e/support/helpers/e2e-ui-elements-helpers.js
@@ -168,8 +168,11 @@ export const moveColumnDown = (column, distance) => {
.trigger("mouseup", 0, distance * 50, { force: true });
};
-export const moveDnDKitColumnVertical = (column, distance) => {
- column
+export const moveDnDKitElement = (
+ element,
+ { horizontal = 0, vertical = 0 } = {},
+) => {
+ element
.trigger("pointerdown", 0, 0, {
force: true,
isPrimary: true,
@@ -182,13 +185,13 @@ export const moveDnDKitColumnVertical = (column, distance) => {
button: 0,
})
.wait(200)
- .trigger("pointermove", 0, distance, {
+ .trigger("pointermove", horizontal, vertical, {
force: true,
isPrimary: true,
button: 0,
})
.wait(200)
- .trigger("pointerup", 0, distance, {
+ .trigger("pointerup", horizontal, vertical, {
force: true,
isPrimary: true,
button: 0,
diff --git a/e2e/test/scenarios/dashboard-cards/visualization-options.cy.spec.js b/e2e/test/scenarios/dashboard-cards/visualization-options.cy.spec.js
index 84fb51696cf..0c34444e004 100644
--- a/e2e/test/scenarios/dashboard-cards/visualization-options.cy.spec.js
+++ b/e2e/test/scenarios/dashboard-cards/visualization-options.cy.spec.js
@@ -10,7 +10,7 @@ import {
saveDashboard,
getDashboardCardMenu,
getDraggableElements,
- moveDnDKitColumnVertical,
+ moveDnDKitElement,
} from "e2e/support/helpers";
describe("scenarios > dashboard cards > visualization options", () => {
@@ -51,7 +51,9 @@ describe("scenarios > dashboard cards > visualization options", () => {
getDashboardCard().realHover();
cy.findByLabelText("Show visualization options").click();
cy.findByTestId("chartsettings-sidebar").within(() => {
- moveDnDKitColumnVertical(getDraggableElements().contains("ID"), 100);
+ moveDnDKitElement(getDraggableElements().contains("ID"), {
+ vertical: 100,
+ });
/**
* When this issue gets fixed, it should be safe to uncomment the following assertion.
diff --git a/e2e/test/scenarios/question/notebook.cy.spec.js b/e2e/test/scenarios/question/notebook.cy.spec.js
index e45655ab063..ad34cc591ae 100644
--- a/e2e/test/scenarios/question/notebook.cy.spec.js
+++ b/e2e/test/scenarios/question/notebook.cy.spec.js
@@ -10,6 +10,7 @@ import {
getNotebookStep,
hovercard,
join,
+ moveDnDKitElement,
openNotebook,
openOrdersTable,
openProductsTable,
@@ -708,6 +709,68 @@ describe("scenarios > question > notebook", { tags: "@slow" }, () => {
assertTableRowCount(10);
});
});
+
+ it("should be able to drag-n-drop query clauses", () => {
+ function moveElement({ name, horizontal, vertical, index }) {
+ moveDnDKitElement(cy.findByText(name), {
+ horizontal,
+ vertical,
+ });
+ cy.findAllByTestId("notebook-cell-item")
+ .eq(index)
+ .should("have.text", name);
+ }
+
+ const questionDetails = {
+ query: {
+ "source-table": ORDERS_ID,
+ expressions: {
+ E1: ["+", ["field", ORDERS.ID, null], 1],
+ E2: ["+", ["field", ORDERS.ID, null], 2],
+ },
+ filter: [
+ "and",
+ ["=", ["field", ORDERS.ID, null], 1],
+ ["=", ["field", ORDERS.ID, null], 2],
+ ["=", ["field", ORDERS.ID, null], 3],
+ ],
+ breakout: [
+ ["field", ORDERS.ID, null],
+ ["field", ORDERS.PRODUCT_ID, null],
+ ],
+ aggregation: [
+ ["count"],
+ ["sum", ["field", ORDERS.TAX, null]],
+ ["sum", ["field", ORDERS.SUBTOTAL, null]],
+ ["sum", ["field", ORDERS.TOTAL, null]],
+ ["avg", ["field", ORDERS.TOTAL, null]],
+ ],
+ "order-by": [
+ ["asc", ["aggregation", 0]],
+ ["asc", ["aggregation", 4]],
+ ],
+ },
+ };
+ cy.createQuestion(questionDetails, { visitQuestion: true });
+ openNotebook();
+ getNotebookStep("expression").within(() => {
+ moveElement({ name: "E1", horizontal: 100, index: 1 });
+ });
+ getNotebookStep("filter").within(() => {
+ moveElement({ name: "ID is 2", horizontal: -100, index: 0 });
+ });
+ getNotebookStep("summarize").within(() => {
+ cy.findByTestId("aggregate-step").within(() => {
+ moveElement({ name: "Count", vertical: 100, index: 3 });
+ });
+ cy.findByTestId("breakout-step").within(() => {
+ moveElement({ name: "ID", horizontal: 100, index: 1 });
+ });
+ });
+ getNotebookStep("sort").within(() => {
+ moveElement({ name: "Average of Total", horizontal: -100, index: 0 });
+ });
+ });
});
function assertTableRowCount(expectedCount) {
diff --git a/e2e/test/scenarios/question/settings.cy.spec.js b/e2e/test/scenarios/question/settings.cy.spec.js
index 8d42d46a7e2..e304da8ca7e 100644
--- a/e2e/test/scenarios/question/settings.cy.spec.js
+++ b/e2e/test/scenarios/question/settings.cy.spec.js
@@ -9,7 +9,7 @@ import {
popover,
modal,
sidebar,
- moveDnDKitColumnVertical,
+ moveDnDKitElement,
} from "e2e/support/helpers";
const { ORDERS, ORDERS_ID, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
@@ -100,7 +100,7 @@ describe("scenarios > question > settings", () => {
getSidebarColumns().eq("5").as("total").contains("Total");
- moveDnDKitColumnVertical(cy.get("@total"), -100);
+ moveDnDKitElement(cy.get("@total"), { vertical: -100 });
getSidebarColumns().eq("3").should("contain.text", "Total");
@@ -114,7 +114,7 @@ describe("scenarios > question > settings", () => {
expect($el.scrollTop).to.eql(0);
});
- moveDnDKitColumnVertical(cy.get("@title"), 15);
+ moveDnDKitElement(cy.get("@title"), { vertical: 15 });
cy.findByTestId("chartsettings-sidebar").should(([$el]) => {
expect($el.scrollTop).to.be.greaterThan(0);
@@ -156,7 +156,7 @@ describe("scenarios > question > settings", () => {
.contains(/Products? → Category/);
// Drag and drop this column between "Tax" and "Discount" (index 5 in @sidebarColumns array)
- moveDnDKitColumnVertical(cy.get("@prod-category"), -360);
+ moveDnDKitElement(cy.get("@prod-category"), { vertical: -360 });
refreshResultsInHeader();
@@ -187,7 +187,7 @@ describe("scenarios > question > settings", () => {
findColumnAtIndex("User → Address", -1).as("user-address");
// Move it one place up
- moveDnDKitColumnVertical(cy.get("@user-address"), -100);
+ moveDnDKitElement(cy.get("@user-address"), { vertical: -100 });
findColumnAtIndex("User → Address", -3);
diff --git a/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js b/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
index a40971b9b51..f910df99af5 100644
--- a/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
@@ -8,7 +8,7 @@ import {
popover,
visitDashboard,
cypressWaitAll,
- moveDnDKitColumnVertical,
+ moveDnDKitElement,
} from "e2e/support/helpers";
const { ORDERS, ORDERS_ID, PEOPLE, PRODUCTS, PRODUCTS_ID } = SAMPLE_DATABASE;
@@ -143,7 +143,7 @@ describe("scenarios > visualizations > bar chart", () => {
});
it("should allow you to show/hide and reorder columns", () => {
- moveDnDKitColumnVertical(getDraggableElements().eq(0), 100);
+ moveDnDKitElement(getDraggableElements().eq(0), { vertical: 100 });
cy.findAllByTestId("legend-item").eq(0).should("contain.text", "Gadget");
cy.findAllByTestId("legend-item").eq(1).should("contain.text", "Gizmo");
@@ -192,7 +192,7 @@ describe("scenarios > visualizations > bar chart", () => {
});
it("should gracefully handle removing filtered items, and adding new items to the end of the list", () => {
- moveDnDKitColumnVertical(getDraggableElements().first(), 100);
+ moveDnDKitElement(getDraggableElements().first(), { vertical: 100 });
getDraggableElements()
.eq(1)
diff --git a/e2e/test/scenarios/visualizations-charts/funnel.cy.spec.js b/e2e/test/scenarios/visualizations-charts/funnel.cy.spec.js
index 22d20569a3a..b35c7ff2f84 100644
--- a/e2e/test/scenarios/visualizations-charts/funnel.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/funnel.cy.spec.js
@@ -6,7 +6,7 @@ import {
sidebar,
getDraggableElements,
popover,
- moveDnDKitColumnVertical,
+ moveDnDKitElement,
} from "e2e/support/helpers";
const { PEOPLE_ID, PEOPLE } = SAMPLE_DATABASE;
@@ -45,7 +45,7 @@ describe("scenarios > visualizations > funnel chart", () => {
.first()
.should("have.text", name);
- moveDnDKitColumnVertical(getDraggableElements().first(), 100);
+ moveDnDKitElement(getDraggableElements().first(), { vertical: 100 });
getDraggableElements().eq(2).should("have.text", name);
@@ -71,7 +71,7 @@ describe("scenarios > visualizations > funnel chart", () => {
});
it("should handle row items being filterd out and returned gracefully", () => {
- moveDnDKitColumnVertical(getDraggableElements().first(), 100);
+ moveDnDKitElement(getDraggableElements().first(), { vertical: 100 });
getDraggableElements()
.eq(1)
diff --git a/e2e/test/scenarios/visualizations-tabular/reproductions/28311-sorting-table-columns.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/reproductions/28311-sorting-table-columns.cy.spec.js
index a643221a07f..d693a599c45 100644
--- a/e2e/test/scenarios/visualizations-tabular/reproductions/28311-sorting-table-columns.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-tabular/reproductions/28311-sorting-table-columns.cy.spec.js
@@ -4,7 +4,7 @@ import {
restore,
visitQuestionAdhoc,
getDraggableElements,
- moveDnDKitColumnVertical,
+ moveDnDKitElement,
} from "e2e/support/helpers";
const { ORDERS_ID, ORDERS } = SAMPLE_DATABASE;
@@ -62,10 +62,9 @@ describe("issue 25250", () => {
cy.findByText("Product ID").should("be.visible");
cy.findByTestId("viz-settings-button").click();
- moveDnDKitColumnVertical(
- getDraggableElements().contains("Product ID"),
- -100,
- );
+ moveDnDKitElement(getDraggableElements().contains("Product ID"), {
+ vertical: -100,
+ });
getDraggableElements().eq(0).should("contain", "Product ID");
});
});
diff --git a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js
index 09b73e718d1..702bb7442c6 100644
--- a/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-tabular/table.cy.spec.js
@@ -17,7 +17,7 @@ import {
getTable,
leftSidebar,
sidebar,
- moveDnDKitColumnVertical,
+ moveDnDKitElement,
} from "e2e/support/helpers";
describe("scenarios > visualizations > table", () => {
@@ -431,10 +431,9 @@ describe("scenarios > visualizations > table > conditional formatting", () => {
.first()
.should("contain.text", "is less than 20");
- moveDnDKitColumnVertical(
- cy.findAllByTestId("formatting-rule-preview").eq(2),
- -300,
- );
+ moveDnDKitElement(cy.findAllByTestId("formatting-rule-preview").eq(2), {
+ vertical: -300,
+ });
cy.findAllByTestId("formatting-rule-preview")
.first()
diff --git a/frontend/src/metabase-lib/query.ts b/frontend/src/metabase-lib/query.ts
index 0b6e2907060..d069ff74ee1 100644
--- a/frontend/src/metabase-lib/query.ts
+++ b/frontend/src/metabase-lib/query.ts
@@ -81,6 +81,15 @@ export function replaceClause(
return ML.replace_clause(query, stageIndex, targetClause, newClause);
}
+export function swapClauses(
+ query: Query,
+ stageIndex: number,
+ sourceClause: Clause,
+ targetClause: Clause,
+): Query {
+ return ML.swap_clauses(query, stageIndex, sourceClause, targetClause);
+}
+
export function sourceTableOrCardId(query: Query): TableId | null {
return ML.source_table_or_card_id(query);
}
diff --git a/frontend/src/metabase-lib/types.ts b/frontend/src/metabase-lib/types.ts
index f7e1c57b43b..f9735278cf3 100644
--- a/frontend/src/metabase-lib/types.ts
+++ b/frontend/src/metabase-lib/types.ts
@@ -65,6 +65,8 @@ export type OrderByDirection = "asc" | "desc";
declare const FilterClause: unique symbol;
export type FilterClause = unknown & { _opaque: typeof FilterClause };
+export type Filterable = FilterClause | ExpressionClause | SegmentMetadata;
+
declare const Join: unique symbol;
export type Join = unknown & { _opaque: typeof Join };
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep/AggregateStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep/AggregateStep.tsx
index 039fcd7edcd..86cf095a3bd 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep/AggregateStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/AggregateStep/AggregateStep.tsx
@@ -39,8 +39,21 @@ export function AggregateStep({
updateQuery(nextQuery);
};
- const handleRemoveAggregation = (aggregation: Lib.AggregationClause) => {
- const nextQuery = Lib.removeClause(query, stageIndex, aggregation);
+ const handleReorderAggregation = (
+ sourceClause: Lib.AggregationClause,
+ targetClause: Lib.AggregationClause,
+ ) => {
+ const nextQuery = Lib.swapClauses(
+ query,
+ stageIndex,
+ sourceClause,
+ targetClause,
+ );
+ updateQuery(nextQuery);
+ };
+
+ const handleRemoveAggregation = (clause: Lib.AggregationClause) => {
+ const nextQuery = Lib.removeClause(query, stageIndex, clause);
updateQuery(nextQuery);
};
@@ -66,6 +79,7 @@ export function AggregateStep({
onClose={onClose}
/>
)}
+ onReorder={handleReorderAggregation}
onRemove={handleRemoveAggregation}
data-testid="aggregate-step"
/>
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.tsx
index 83b42d1343b..d4fd33c4da9 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.tsx
@@ -37,6 +37,19 @@ function BreakoutStep({
updateQuery(nextQuery);
};
+ const handleReorderBreakout = (
+ sourceClause: Lib.BreakoutClause,
+ targetClause: Lib.BreakoutClause,
+ ) => {
+ const nextQuery = Lib.swapClauses(
+ query,
+ stageIndex,
+ sourceClause,
+ targetClause,
+ );
+ updateQuery(nextQuery);
+ };
+
const handleRemoveBreakout = (clause: Lib.BreakoutClause) => {
const nextQuery = Lib.removeClause(query, stageIndex, clause);
updateQuery(nextQuery);
@@ -61,6 +74,7 @@ function BreakoutStep({
onClose={onClose}
/>
)}
+ onReorder={handleReorderBreakout}
onRemove={handleRemoveBreakout}
data-testid="breakout-step"
/>
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/ClauseStep/ClauseStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/ClauseStep/ClauseStep.tsx
index ea6113706cb..48ee0a0c7e6 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/ClauseStep/ClauseStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/ClauseStep/ClauseStep.tsx
@@ -1,3 +1,11 @@
+import type { DndContextProps } from "@dnd-kit/core";
+import { PointerSensor, useSensor, DndContext } from "@dnd-kit/core";
+import { restrictToParentElement } from "@dnd-kit/modifiers";
+import { SortableContext, useSortable } from "@dnd-kit/sortable";
+import { CSS } from "@dnd-kit/utilities";
+import type { ReactNode } from "react";
+import { useCallback } from "react";
+
import { Icon } from "metabase/ui";
import {
@@ -20,43 +28,35 @@ type RenderPopoverOpts<T> = {
onClose: () => void;
};
-export interface ClauseStepProps<T> {
+export type ClauseStepProps<T> = {
color: string;
items: T[];
- isLastOpened?: boolean;
- initialAddText?: string | null;
+ initialAddText?: string;
readOnly?: boolean;
+ isLastOpened?: boolean;
renderName: (item: T, index: number) => JSX.Element | string;
renderPopover: (opts: RenderPopoverOpts<T>) => JSX.Element | null;
- canRemove?: (item: T) => boolean;
- onRemove?: ((item: T, index: number) => void) | null;
+ onRemove: (item: T, index: number) => void;
+ onReorder: (sourceItem: T, targetItem: T) => void;
"data-testid"?: string;
-}
+};
export const ClauseStep = <T,>({
color,
items,
+ initialAddText,
+ readOnly = false,
isLastOpened = false,
- initialAddText = null,
- readOnly,
renderName,
renderPopover,
- canRemove,
- onRemove = null,
+ onRemove,
+ onReorder,
...props
}: ClauseStepProps<T>): JSX.Element => {
- const renderNewItem = ({ onOpen }: { onOpen?: () => void }) => (
- <NotebookCellAdd
- initialAddText={items.length === 0 && initialAddText}
- color={color}
- onClick={onOpen}
- />
- );
-
const renderItem = ({ item, index, onOpen }: RenderItemOpts<T>) => (
<NotebookCellItem color={color} readOnly={readOnly} onClick={onOpen}>
{renderName(item, index)}
- {!readOnly && onRemove && (!canRemove || canRemove(item)) && (
+ {!readOnly && (
<Icon
className="ml1"
name="close"
@@ -69,15 +69,26 @@ export const ClauseStep = <T,>({
</NotebookCellItem>
);
+ const renderNewItem = ({ onOpen }: { onOpen?: () => void }) => (
+ <NotebookCellAdd
+ initialAddText={items.length === 0 && initialAddText}
+ color={color}
+ onClick={onOpen}
+ />
+ );
+
return (
<NotebookCell color={color} data-testid={props["data-testid"]}>
- {items.map((item, index) => (
- <ClausePopover
- key={index}
- renderItem={onOpen => renderItem({ item, index, onOpen })}
- renderPopover={onClose => renderPopover({ item, index, onClose })}
- />
- ))}
+ <ClauseStepDndContext items={items} onReorder={onReorder}>
+ {items.map((item, index) => (
+ <ClauseStepDndItem key={index} index={index} readOnly={readOnly}>
+ <ClausePopover
+ renderItem={onOpen => renderItem({ item, index, onOpen })}
+ renderPopover={onClose => renderPopover({ item, index, onClose })}
+ />
+ </ClauseStepDndItem>
+ ))}
+ </ClauseStepDndContext>
{!readOnly && (
<ClausePopover
isInitiallyOpen={isLastOpened}
@@ -88,3 +99,87 @@ export const ClauseStep = <T,>({
</NotebookCell>
);
};
+
+type ClauseStepDndContextProps<T> = {
+ items: T[];
+ children: ReactNode;
+ onReorder: (sourceItem: T, targetItem: T) => void;
+};
+
+function ClauseStepDndContext<T>({
+ items,
+ children,
+ onReorder,
+}: ClauseStepDndContextProps<T>) {
+ const pointerSensor = useSensor(PointerSensor, {
+ activationConstraint: { distance: 0 },
+ });
+
+ const handleSortEnd: DndContextProps["onDragEnd"] = useCallback(
+ input => {
+ if (input.over) {
+ const sourceIndex = getItemIndexFromId(input.active.id);
+ const targetIndex = getItemIndexFromId(input.over.id);
+ onReorder(items[sourceIndex], items[targetIndex]);
+ }
+ },
+ [items, onReorder],
+ );
+
+ return (
+ <DndContext
+ sensors={[pointerSensor]}
+ modifiers={[restrictToParentElement]}
+ onDragEnd={handleSortEnd}
+ >
+ <SortableContext
+ items={items.map((_, index) => getItemIdFromIndex(index))}
+ >
+ {children}
+ </SortableContext>
+ </DndContext>
+ );
+}
+
+type ClauseStepDndItemProps = {
+ index: number;
+ readOnly: boolean;
+ children: ReactNode;
+};
+
+function ClauseStepDndItem({
+ index,
+ readOnly,
+ children,
+}: ClauseStepDndItemProps) {
+ const { attributes, listeners, setNodeRef, transform, transition } =
+ useSortable({
+ id: getItemIdFromIndex(index),
+ disabled: readOnly,
+ // disable animation after reordering because we don't have stable item ids
+ animateLayoutChanges: () => false,
+ });
+
+ return (
+ <div
+ ref={setNodeRef}
+ {...attributes}
+ {...listeners}
+ style={{
+ transition,
+ transform: CSS.Translate.toString(transform),
+ }}
+ >
+ {children}
+ </div>
+ );
+}
+
+// dnd-kit ignores `0` item, so we convert indexes to string `"0"`
+function getItemIdFromIndex(index: number) {
+ return String(index);
+}
+
+function getItemIndexFromId(id: string | number) {
+ return Number(id);
+}
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.tsx
index 040ef472f95..bc16416ff1f 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/ExpressionStep.tsx
@@ -17,8 +17,27 @@ export const ExpressionStep = ({
const { query, stageIndex } = step;
const expressions = Lib.expressions(query, stageIndex);
- const renderExpressionName = (expression: Lib.ExpressionClause) =>
- Lib.displayInfo(query, stageIndex, expression).longDisplayName;
+ const renderExpressionName = (expression: Lib.ExpressionClause) => {
+ return Lib.displayInfo(query, stageIndex, expression).longDisplayName;
+ };
+
+ const handleReorderExpression = (
+ sourceClause: Lib.ExpressionClause,
+ targetClause: Lib.ExpressionClause,
+ ) => {
+ const nextQuery = Lib.swapClauses(
+ query,
+ stageIndex,
+ sourceClause,
+ targetClause,
+ );
+ updateQuery(nextQuery);
+ };
+
+ const handleRemoveExpression = (clause: Lib.ExpressionClause) => {
+ const nextQuery = Lib.removeClause(query, stageIndex, clause);
+ updateQuery(nextQuery);
+ };
return (
<ClauseStep
@@ -71,10 +90,8 @@ export const ExpressionStep = ({
/>
)}
isLastOpened={isLastOpened}
- onRemove={clause => {
- const nextQuery = Lib.removeClause(query, stageIndex, clause);
- updateQuery(nextQuery);
- }}
+ onReorder={handleReorderExpression}
+ onRemove={handleRemoveExpression}
/>
);
};
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep/FilterStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep/FilterStep.tsx
index fa934103a9c..c03c3c300ed 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep/FilterStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/FilterStep/FilterStep.tsx
@@ -23,33 +23,44 @@ export function FilterStep({
[query, stageIndex],
);
- const handleAddFilter = (
- filter: Lib.ExpressionClause | Lib.FilterClause | Lib.SegmentMetadata,
- ) => {
- const nextQuery = Lib.filter(query, stageIndex, filter);
+ const renderFilterName = (filter: Lib.FilterClause) =>
+ Lib.displayInfo(query, stageIndex, filter).longDisplayName;
+
+ const handleAddFilter = (clause: Lib.Filterable) => {
+ const nextQuery = Lib.filter(query, stageIndex, clause);
updateQuery(nextQuery);
};
const handleUpdateFilter = (
- targetFilter: Lib.FilterClause,
- nextFilter: Lib.ExpressionClause | Lib.FilterClause | Lib.SegmentMetadata,
+ targetClause: Lib.FilterClause,
+ newClause: Lib.Filterable,
) => {
const nextQuery = Lib.replaceClause(
query,
stageIndex,
- targetFilter,
- nextFilter,
+ targetClause,
+ newClause,
);
updateQuery(nextQuery);
};
- const handleRemoveFilter = (filter: Lib.FilterClause) => {
- const nextQuery = Lib.removeClause(query, stageIndex, filter);
+ const handleReorderFilter = (
+ sourceClause: Lib.FilterClause,
+ targetClause: Lib.FilterClause,
+ ) => {
+ const nextQuery = Lib.swapClauses(
+ query,
+ stageIndex,
+ sourceClause,
+ targetClause,
+ );
updateQuery(nextQuery);
};
- const renderFilterName = (filter: Lib.FilterClause) =>
- Lib.displayInfo(query, stageIndex, filter).longDisplayName;
+ const handleRemoveFilter = (clause: Lib.FilterClause) => {
+ const nextQuery = Lib.removeClause(query, stageIndex, clause);
+ updateQuery(nextQuery);
+ };
return (
<ErrorBoundary>
@@ -71,6 +82,7 @@ export function FilterStep({
onClose={onClose}
/>
)}
+ onReorder={handleReorderFilter}
onRemove={handleRemoveFilter}
/>
</ErrorBoundary>
@@ -82,12 +94,10 @@ interface FilterPopoverProps {
stageIndex: number;
filter?: Lib.FilterClause;
filterIndex?: number;
- onAddFilter: (
- filter: Lib.ExpressionClause | Lib.FilterClause | Lib.SegmentMetadata,
- ) => void;
+ onAddFilter: (filter: Lib.Filterable) => void;
onUpdateFilter: (
targetFilter: Lib.FilterClause,
- nextFilter: Lib.ExpressionClause | Lib.FilterClause | Lib.SegmentMetadata,
+ nextFilter: Lib.Filterable,
) => void;
onClose?: () => void;
}
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/SortStep/SortStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/SortStep/SortStep.tsx
index 3a3a27cb1b0..00edddbf935 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/SortStep/SortStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/SortStep/SortStep.tsx
@@ -43,6 +43,19 @@ function SortStep({
updateQuery(nextQuery);
};
+ const handleReorderOrderBy = (
+ sourceClause: Lib.OrderByClause,
+ targetClause: Lib.OrderByClause,
+ ) => {
+ const nextQuery = Lib.swapClauses(
+ query,
+ stageIndex,
+ sourceClause,
+ targetClause,
+ );
+ updateQuery(nextQuery);
+ };
+
const handleRemoveOrderBy = (clause: Lib.OrderByClause) => {
const nextQuery = Lib.removeClause(query, stageIndex, clause);
updateQuery(nextQuery);
@@ -71,6 +84,7 @@ function SortStep({
onClose={onClose}
/>
)}
+ onReorder={handleReorderOrderBy}
onRemove={handleRemoveOrderBy}
/>
);
diff --git a/frontend/src/metabase/querying/components/FilterPicker/FilterPicker.tsx b/frontend/src/metabase/querying/components/FilterPicker/FilterPicker.tsx
index bc543741d45..3372d425772 100644
--- a/frontend/src/metabase/querying/components/FilterPicker/FilterPicker.tsx
+++ b/frontend/src/metabase/querying/components/FilterPicker/FilterPicker.tsx
@@ -15,9 +15,7 @@ export interface FilterPickerProps {
filter?: Lib.FilterClause;
filterIndex?: number;
- onSelect: (
- filter: Lib.ExpressionClause | Lib.FilterClause | Lib.SegmentMetadata,
- ) => void;
+ onSelect: (filter: Lib.Filterable) => void;
onClose?: () => void;
}
@@ -46,9 +44,7 @@ export function FilterPicker({
setFilter(initialFilter);
}, [initialFilter]);
- const handleChange = (
- filter: Lib.ExpressionClause | Lib.FilterClause | Lib.SegmentMetadata,
- ) => {
+ const handleChange = (filter: Lib.Filterable) => {
onSelect(filter);
onClose?.();
};
--
GitLab