diff --git a/e2e/test/scenarios/question/column-compare.cy.spec.ts b/e2e/test/scenarios/question/column-compare.cy.spec.ts
index d206fb786c824868fda85bd5c82f7026c70bfdc7..a0678562a77aa4cee756d16ac82f4af56242d497 100644
--- a/e2e/test/scenarios/question/column-compare.cy.spec.ts
+++ b/e2e/test/scenarios/question/column-compare.cy.spec.ts
@@ -16,7 +16,7 @@ import {
} from "e2e/support/helpers";
import type { FieldReference, StructuredQuery } from "metabase-types/api";
-const { PRODUCTS_ID, PRODUCTS } = SAMPLE_DATABASE;
+const { PRODUCTS_ID, PRODUCTS, ORDERS, ORDERS_ID, PEOPLE } = SAMPLE_DATABASE;
const FIELD_PRICE: FieldReference = [
"field",
@@ -42,6 +42,16 @@ const BREAKOUT_NON_DATETIME: FieldReference = [
{ "base-type": "type/Text" },
];
+const BREAKOUT_OTHER_DATETIME: FieldReference = [
+ "field",
+ PEOPLE.CREATED_AT,
+ {
+ "base-type": "type/DateTime",
+ "temporal-unit": "month",
+ "source-field": ORDERS.USER_ID,
+ },
+];
+
const QUERY_NO_AGGREGATION: StructuredQuery = {
"source-table": PRODUCTS_ID,
};
@@ -62,6 +72,12 @@ const QUERY_SINGLE_AGGREGATION_BINNED_DATETIME_BREAKOUT: StructuredQuery = {
breakout: [BREAKOUT_BINNED_DATETIME],
};
+const QUERY_SINGLE_AGGREGATION_OTHER_DATETIME: StructuredQuery = {
+ "source-table": ORDERS_ID,
+ aggregation: [["count"]],
+ breakout: [BREAKOUT_OTHER_DATETIME],
+};
+
const QUERY_SINGLE_AGGREGATION_NON_BINNED_DATETIME_BREAKOUT: StructuredQuery = {
"source-table": PRODUCTS_ID,
aggregation: [["count"]],
@@ -93,6 +109,22 @@ const QUERY_MULTIPLE_AGGREGATIONS_NON_DATETIME_BREAKOUT: StructuredQuery = {
breakout: [BREAKOUT_NON_DATETIME],
};
+const QUERY_MULTIPLE_BREAKOUTS: StructuredQuery = {
+ "source-table": PRODUCTS_ID,
+ aggregation: [["count"]],
+ breakout: [BREAKOUT_NON_DATETIME, BREAKOUT_BINNED_DATETIME],
+};
+
+const QUERY_MULTIPLE_TEMPORAL_BREAKOUTS: StructuredQuery = {
+ "source-table": PRODUCTS_ID,
+ aggregation: [["count"]],
+ breakout: [
+ BREAKOUT_NON_DATETIME,
+ BREAKOUT_BINNED_DATETIME,
+ BREAKOUT_NON_BINNED_DATETIME,
+ ],
+};
+
const CUSTOM_EXPRESSIONS_USED = [
"offset",
"count",
@@ -249,21 +281,25 @@ describeWithSnowplow("scenarios > question > column compare TODO", () => {
});
});
+ verifyBreakoutExistsAndIsFirst({
+ column: "Created At",
+ bucket: "Month",
+ });
+
verifyAggregations([
{
- name: "Count (previous period)",
+ name: "Count (previous month)",
expression: "Offset(Count, -1)",
},
{
- name: "Count (vs previous period)",
+ name: "Count (vs previous month)",
expression: "Count - Offset(Count, -1)",
},
{
- name: "Count (% vs previous period)",
+ name: "Count (% vs previous month)",
expression: "Count / Offset(Count, -1) - 1",
},
]);
- verifyBreakoutRequiredError();
});
it("breakout on binned datetime column", () => {
@@ -325,6 +361,7 @@ describeWithSnowplow("scenarios > question > column compare TODO", () => {
expression: "Count / Offset(Count, -1) - 1",
},
]);
+ verifyBreakoutExistsAndIsFirst({ column: "Created At", bucket: "Month" });
verifyColumns([
"Count (previous month)",
@@ -455,22 +492,224 @@ describeWithSnowplow("scenarios > question > column compare TODO", () => {
verifyAggregations([
{
- name: "Count (previous value)",
+ name: "Count (previous month)",
+ expression: "Offset(Count, -1)",
+ },
+ {
+ name: "Count (vs previous month)",
+ expression: "Count - Offset(Count, -1)",
+ },
+ {
+ name: "Count (% vs previous month)",
+ expression: "Count / Offset(Count, -1) - 1",
+ },
+ ]);
+
+ verifyBreakoutExistsAndIsFirst({
+ column: "Created At",
+ bucket: "Month",
+ });
+
+ verifyColumns([
+ "Count (previous month)",
+ "Count (vs previous month)",
+ "Count (% vs previous month)",
+ ]);
+ });
+
+ it("multiple breakouts", () => {
+ createQuestion(
+ { query: QUERY_MULTIPLE_BREAKOUTS },
+ { visitQuestion: true, wrapId: true, idAlias: "questionId" },
+ );
+
+ verifySummarizeText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "rows above based on “Categoryâ€",
+ });
+
+ verifyPlusButtonText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "rows above based on “Categoryâ€",
+ });
+
+ verifyNotebookText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "rows above based on “Categoryâ€",
+ });
+
+ toggleColumnPickerItems(["Value difference"]);
+ popover().button("Done").click();
+
+ cy.get("@questionId").then(questionId => {
+ expectGoodSnowplowEvent({
+ event: "column_compare_via_shortcut",
+ custom_expressions_used: CUSTOM_EXPRESSIONS_USED,
+ database_id: SAMPLE_DB_ID,
+ question_id: questionId,
+ });
+ });
+
+ verifyAggregations([
+ {
+ name: "Count (previous month)",
+ expression: "Offset(Count, -1)",
+ },
+ {
+ name: "Count (vs previous month)",
+ expression: "Count - Offset(Count, -1)",
+ },
+ {
+ name: "Count (% vs previous month)",
+ expression: "Count / Offset(Count, -1) - 1",
+ },
+ ]);
+
+ verifyBreakoutExistsAndIsFirst({ column: "Created At", bucket: "Month" });
+ breakout({ column: "Category" }).should("exist");
+
+ verifyColumns([
+ "Count (previous month)",
+ "Count (vs previous month)",
+ "Count (% vs previous month)",
+ ]);
+ });
+
+ it("multiple temporal breakouts", () => {
+ createQuestion(
+ { query: QUERY_MULTIPLE_TEMPORAL_BREAKOUTS },
+ { visitQuestion: true, wrapId: true, idAlias: "questionId" },
+ );
+
+ verifySummarizeText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "rows above based on “Categoryâ€",
+ });
+
+ verifyPlusButtonText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "rows above based on “Categoryâ€",
+ });
+
+ verifyNotebookText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "rows above based on “Categoryâ€",
+ });
+
+ toggleColumnPickerItems(["Value difference"]);
+ popover().button("Done").click();
+
+ cy.get("@questionId").then(questionId => {
+ expectGoodSnowplowEvent({
+ event: "column_compare_via_shortcut",
+ custom_expressions_used: CUSTOM_EXPRESSIONS_USED,
+ database_id: SAMPLE_DB_ID,
+ question_id: questionId,
+ });
+ });
+
+ verifyAggregations([
+ {
+ name: "Count (previous month)",
expression: "Offset(Count, -1)",
},
{
- name: "Count (vs previous value)",
+ name: "Count (vs previous month)",
expression: "Count - Offset(Count, -1)",
},
{
- name: "Count (% vs previous value)",
+ name: "Count (% vs previous month)",
expression: "Count / Offset(Count, -1) - 1",
},
]);
+
+ verifyBreakoutExistsAndIsFirst({ column: "Created At", bucket: "Month" });
+ breakout({ column: "Category" }).should("exist");
+ breakout({ column: "Created At" }).should("exist");
+
verifyColumns([
- "Count (previous value)",
- "Count (vs previous value)",
- "Count (% vs previous value)",
+ "Count (previous month)",
+ "Count (vs previous month)",
+ "Count (% vs previous month)",
+ ]);
+ });
+
+ it("one breakout on non-default datetime column", () => {
+ createQuestion(
+ { query: QUERY_SINGLE_AGGREGATION_OTHER_DATETIME },
+ { visitQuestion: true, wrapId: true, idAlias: "questionId" },
+ );
+
+ verifySummarizeText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "months ago based on “Created Atâ€",
+ });
+
+ tableHeaderClick("Count");
+ verifyNoColumnCompareShortcut();
+
+ verifyColumnDrillText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "months ago based on “Created Atâ€",
+ });
+
+ verifyPlusButtonText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "months ago based on “Created Atâ€",
+ });
+
+ verifyNotebookText({
+ itemName: "Compare to the past",
+ step2Title: "Compare “Count†to the past",
+ offsetHelp: "months ago based on “Created Atâ€",
+ });
+
+ toggleColumnPickerItems(["Value difference"]);
+ popover().button("Done").click();
+
+ cy.get("@questionId").then(questionId => {
+ expectGoodSnowplowEvent({
+ event: "column_compare_via_shortcut",
+ custom_expressions_used: CUSTOM_EXPRESSIONS_USED,
+ database_id: SAMPLE_DB_ID,
+ question_id: questionId,
+ });
+ });
+
+ verifyAggregations([
+ {
+ name: "Count (previous month)",
+ expression: "Offset(Count, -1)",
+ },
+ {
+ name: "Count (vs previous month)",
+ expression: "Count - Offset(Count, -1)",
+ },
+ {
+ name: "Count (% vs previous month)",
+ expression: "Count / Offset(Count, -1) - 1",
+ },
+ ]);
+
+ verifyBreakoutExistsAndIsFirst({
+ column: "User → Created At",
+ bucket: "Month",
+ });
+ breakout({ column: "Created At", bucket: "Month" }).should("not.exist");
+
+ verifyColumns([
+ "Count (previous month)",
+ "Count (vs previous month)",
+ "Count (% vs previous month)",
]);
});
});
@@ -521,21 +760,21 @@ describeWithSnowplow("scenarios > question > column compare TODO", () => {
});
});
+ verifyBreakoutExistsAndIsFirst({ column: "Created At", bucket: "Month" });
verifyAggregations([
{
- name: "Count (previous period)",
+ name: "Count (previous month)",
expression: "Offset(Count, -1)",
},
{
- name: "Count (vs previous period)",
+ name: "Count (vs previous month)",
expression: "Count - Offset(Count, -1)",
},
{
- name: "Count (% vs previous period)",
+ name: "Count (% vs previous month)",
expression: "Count / Offset(Count, -1) - 1",
},
]);
- verifyBreakoutRequiredError();
});
it("breakout on binned datetime column", () => {
@@ -728,23 +967,23 @@ describeWithSnowplow("scenarios > question > column compare TODO", () => {
verifyAggregations([
{
- name: "Count (previous value)",
+ name: "Count (previous month)",
expression: "Offset(Count, -1)",
},
{
- name: "Count (vs previous value)",
+ name: "Count (vs previous month)",
expression: "Count - Offset(Count, -1)",
},
{
- name: "Count (% vs previous value)",
+ name: "Count (% vs previous month)",
expression: "Count / Offset(Count, -1) - 1",
},
]);
verifyColumns([
- "Count (previous value)",
- "Count (vs previous value)",
- "Count (% vs previous value)",
+ "Count (previous month)",
+ "Count (vs previous month)",
+ "Count (% vs previous month)",
]);
});
});
@@ -821,7 +1060,8 @@ function verifyPlusButtonText(options: CheckTextOpts) {
function verifyNotebookText(options: CheckTextOpts) {
openNotebook();
getNotebookStep("summarize")
- .findByTestId("aggregate-step")
+ .findAllByTestId("aggregate-step")
+ .last()
.icon("add")
.click();
@@ -865,18 +1105,20 @@ function verifyColumns(names: string[]) {
}
}
-function verifyBreakoutRequiredError() {
- visualize();
+function breakout({ column, bucket }: { column: string; bucket?: string }) {
+ const name = bucket ? `${column}: ${bucket}` : column;
+ return cy.findByTestId("breakout-step").findByText(name);
+}
- cy.get("main")
- .findByText("There was a problem with your question")
- .should("be.visible");
- cy.get("main").findByText("Show error details").click();
- cy.get("main")
- .findByText(
- "Window function requires either breakouts or order by in the query",
- )
- .should("be.visible");
+function verifyBreakoutExistsAndIsFirst(options: {
+ column: string;
+ bucket?: string;
+}) {
+ breakout(options)
+ .should("exist")
+ .parent()
+ .parent()
+ .should("match", ":first-child");
}
function openVisualization() {
diff --git a/enterprise/frontend/src/embedding-sdk/components/public/InteractiveQuestion/components/Summarize.tsx b/enterprise/frontend/src/embedding-sdk/components/public/InteractiveQuestion/components/Summarize.tsx
index d4a1387cdb6a186c33eb480aa72d0468fb6b8981..ca457768a498f2def20a3aa9b3f594cf119ed9fe 100644
--- a/enterprise/frontend/src/embedding-sdk/components/public/InteractiveQuestion/components/Summarize.tsx
+++ b/enterprise/frontend/src/embedding-sdk/components/public/InteractiveQuestion/components/Summarize.tsx
@@ -56,12 +56,10 @@ const SummarizeInner = ({
query,
stageIndex,
aggregations,
- handleAddAggregations,
handleAddBreakout,
- handleRemoveAggregation,
+ handleQueryChange,
handleRemoveBreakout,
handleReplaceBreakouts,
- handleUpdateAggregation,
handleUpdateBreakout,
hasAggregations,
} = useSummarizeQuery({
@@ -76,9 +74,7 @@ const SummarizeInner = ({
query={query}
stageIndex={stageIndex}
aggregations={aggregations}
- onAddAggregations={handleAddAggregations}
- onUpdateAggregation={handleUpdateAggregation}
- onRemoveAggregation={handleRemoveAggregation}
+ onQueryChange={handleQueryChange}
/>
<Divider my="lg" />
{hasAggregations && (
diff --git a/frontend/src/metabase-lib/temporal_bucket.ts b/frontend/src/metabase-lib/temporal_bucket.ts
index 6b8e78bceb5c3fa684525617bbabb9689fb6b499..207297976fb80dbe6adb54e3cbfd007ff9096f46 100644
--- a/frontend/src/metabase-lib/temporal_bucket.ts
+++ b/frontend/src/metabase-lib/temporal_bucket.ts
@@ -36,11 +36,21 @@ export function withDefaultTemporalBucket(
stageIndex: number,
column: ColumnMetadata,
): ColumnMetadata {
+ const defaultBucket = defaultTemporalBucket(query, stageIndex, column);
+ return defaultBucket ? withTemporalBucket(column, defaultBucket) : column;
+}
+
+export function defaultTemporalBucket(
+ query: Query,
+ stageIndex: number,
+ column: ColumnMetadata,
+): Bucket | null {
const buckets = availableTemporalBuckets(query, stageIndex, column);
const defaultBucket = buckets.find(
bucket => displayInfo(query, stageIndex, bucket).default,
);
- return defaultBucket ? withTemporalBucket(column, defaultBucket) : column;
+
+ return defaultBucket ?? null;
}
type IntervalAmount = number | "current" | "next" | "last";
diff --git a/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.tsx b/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.tsx
index 4f9f7452e7edfc7d44f12b9cc1585fa8322e2d90..39a5ae5a2984338d1e0c8b1077630c833aa031d3 100644
--- a/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.tsx
+++ b/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.tsx
@@ -33,9 +33,8 @@ interface AggregationPickerProps {
clauseIndex?: number;
operators: Lib.AggregationOperator[];
hasExpressionInput?: boolean;
- onAdd: (aggregations: Lib.Aggregable[]) => void;
- onSelect: (aggregation: Lib.Aggregable) => void;
onClose?: () => void;
+ onQueryChange: (query: Lib.Query) => void;
}
type OperatorListItem = Lib.AggregationOperatorDisplayInfo & {
@@ -67,9 +66,8 @@ export function AggregationPicker({
clauseIndex,
operators,
hasExpressionInput = true,
- onAdd,
- onSelect,
onClose,
+ onQueryChange,
}: AggregationPickerProps) {
const question = useSelector(getQuestion);
const metadata = useSelector(getMetadata);
@@ -101,6 +99,25 @@ export function AggregationPicker({
return Lib.aggregations(query, stageIndex);
}, [query, stageIndex]);
+ const onSelect = useCallback(
+ function (aggregation: Lib.Aggregable) {
+ const isUpdate = clause != null && clauseIndex != null;
+ if (isUpdate) {
+ const nextQuery = Lib.replaceClause(
+ query,
+ stageIndex,
+ clause,
+ aggregation,
+ );
+ onQueryChange(nextQuery);
+ } else {
+ const nextQuery = Lib.aggregate(query, stageIndex, aggregation);
+ onQueryChange(nextQuery);
+ }
+ },
+ [query, stageIndex, clause, clauseIndex, onQueryChange],
+ );
+
const sections = useMemo(() => {
const sections: Section[] = [];
@@ -239,8 +256,8 @@ export function AggregationPicker({
);
const handleCompareSubmit = useCallback(
- (aggregations: Lib.ExpressionClause[]) => {
- onAdd(aggregations);
+ (query: Lib.Query, aggregations: Lib.ExpressionClause[]) => {
+ onQueryChange(query);
if (question) {
trackColumnCompareViaShortcut(
@@ -253,7 +270,7 @@ export function AggregationPicker({
onClose?.();
},
- [query, stageIndex, question, onAdd, onClose],
+ [stageIndex, question, onClose, onQueryChange],
);
if (isComparing) {
diff --git a/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.unit.spec.tsx b/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.unit.spec.tsx
index 9156664f161581490364d64c5672f5fb9565bf46..30be8c26d5c64e51dd18143680b3ddd7043e70ec 100644
--- a/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.unit.spec.tsx
+++ b/frontend/src/metabase/common/components/AggregationPicker/AggregationPicker.unit.spec.tsx
@@ -152,8 +152,7 @@ function setup({
? Lib.selectedAggregationOperators(baseOperators, clause)
: baseOperators;
- const onSelect = jest.fn();
- const onAdd = jest.fn();
+ const onQueryChange = jest.fn();
renderWithProviders(
<AggregationPicker
@@ -162,20 +161,23 @@ function setup({
stageIndex={stageIndex}
operators={operators}
hasExpressionInput={hasExpressionInput}
- onAdd={onAdd}
- onSelect={onSelect}
+ onQueryChange={onQueryChange}
/>,
{ storeInitialState: state },
);
- function getRecentClause(): Lib.Clause {
- expect(onSelect).toHaveBeenCalledWith(expect.anything());
- const [clause] = onSelect.mock.lastCall;
- return clause;
+ function getRecentClause(index: number = -1): Lib.Clause | undefined {
+ expect(onQueryChange).toHaveBeenCalledWith(expect.anything());
+ const [query] = onQueryChange.mock.lastCall;
+ return Lib.aggregations(query, stageIndex).at(index);
}
- function getRecentClauseInfo() {
- return Lib.displayInfo(query, stageIndex, getRecentClause());
+ function getRecentClauseInfo(index: number = -1) {
+ const clause = getRecentClause(index);
+ if (clause) {
+ return Lib.displayInfo(query, stageIndex, clause);
+ }
+ return null;
}
return {
@@ -183,8 +185,7 @@ function setup({
query,
stageIndex,
getRecentClauseInfo,
- onAdd,
- onSelect,
+ onQueryChange,
};
}
@@ -320,7 +321,7 @@ describe("AggregationPicker", () => {
await userEvent.type(screen.getByLabelText("Expression"), expression);
await userEvent.type(screen.getByLabelText("Name"), expressionName);
await userEvent.click(screen.getByRole("button", { name: "Done" }));
- expect(getRecentClauseInfo().displayName).toBe(expressionName);
+ expect(getRecentClauseInfo()?.displayName).toBe(expressionName);
});
it("should open the editor when a named expression without operator is used", async () => {
@@ -392,22 +393,15 @@ describe("AggregationPicker", () => {
expect(screen.getByText("Compare to the past")).toBeInTheDocument();
});
- it("calls 'onAdd' on submit", async () => {
- const { onAdd } = setup({ query: createQueryWithCountAggregation() });
-
- await userEvent.click(screen.getByText("Compare to the past"));
- await userEvent.click(screen.getByText("Done"));
-
- expect(onAdd).toHaveBeenCalled();
- });
-
- it("does not call 'onSelect' on submit", async () => {
- const { onSelect } = setup({ query: createQueryWithCountAggregation() });
+ it("calls 'onQueryChange' on submit", async () => {
+ const { onQueryChange } = setup({
+ query: createQueryWithCountAggregation(),
+ });
await userEvent.click(screen.getByText("Compare to the past"));
await userEvent.click(screen.getByText("Done"));
- expect(onSelect).not.toHaveBeenCalled();
+ expect(onQueryChange).toHaveBeenCalled();
});
});
});
diff --git a/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.tsx b/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.tsx
index efb3de3968a5b2e8bc182215f1607b4e54ad19c4..c8e57fa61200db0851f7421d151e8153ffbd0ce0 100644
--- a/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.tsx
+++ b/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.tsx
@@ -13,14 +13,19 @@ import {
ReferenceAggregationPicker,
} from "./components";
import type { ColumnType } from "./types";
-import { canSubmit, getAggregations, getTitle } from "./utils";
+import {
+ canSubmit,
+ getBreakout,
+ getTitle,
+ updateQueryWithCompareOffsetAggregations,
+} from "./utils";
interface Props {
aggregations: Lib.AggregationClause[];
query: Lib.Query;
stageIndex: number;
onClose: () => void;
- onSubmit: (aggregations: Lib.ExpressionClause[]) => void;
+ onSubmit: (query: Lib.Query, aggregations: Lib.ExpressionClause[]) => void;
}
const DEFAULT_OFFSET = 1;
@@ -39,6 +44,11 @@ export const CompareAggregations = ({
const [aggregation, setAggregation] = useState<
Lib.AggregationClause | Lib.ExpressionClause | undefined
>(hasManyAggregations ? undefined : aggregations[0]);
+ const columnAndBucket = useMemo(
+ () => getBreakout(query, stageIndex),
+ [query, stageIndex],
+ );
+
const [offset, setOffset] = useState<number | "">(DEFAULT_OFFSET);
const [columns, setColumns] = useState<ColumnType[]>(DEFAULT_COLUMNS);
const width = aggregation ? STEP_2_WIDTH : STEP_1_WIDTH;
@@ -59,17 +69,25 @@ export const CompareAggregations = ({
const handleSubmit = (event: FormEvent) => {
event.preventDefault();
- if (aggregation && offset !== "") {
- const aggregations = getAggregations(
- query,
- stageIndex,
- aggregation,
- columns,
- offset,
- );
- onSubmit(aggregations);
- onClose();
+ if (!aggregation || !columnAndBucket) {
+ return;
}
+
+ const next = updateQueryWithCompareOffsetAggregations(
+ query,
+ stageIndex,
+ aggregation,
+ offset,
+ columns,
+ columnAndBucket,
+ );
+
+ if (!next) {
+ return;
+ }
+
+ onSubmit(next.query, next.addedAggregations);
+ onClose();
};
return (
diff --git a/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.unit.spec.tsx b/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.unit.spec.tsx
index e1af040e3c9bb1e64131c87e9a900e6e8e75e466..ef92cb39f772a486fca1b1d19d6adbbb3a2f854f 100644
--- a/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.unit.spec.tsx
+++ b/frontend/src/metabase/query_builder/components/CompareAggregations/CompareAggregations.unit.spec.tsx
@@ -184,7 +184,7 @@ describe("CompareAggregations", () => {
await userEvent.click(within(listBox).getByText("Previous value"));
await userEvent.click(screen.getByRole("button", { name: "Done" }));
- const [aggregations] = onSubmit.mock.lastCall;
+ const [_, aggregations] = onSubmit.mock.lastCall;
expect(onSubmit).toHaveBeenCalled();
expect(aggregations).toHaveLength(1);
});
@@ -194,7 +194,7 @@ describe("CompareAggregations", () => {
await userEvent.click(screen.getByRole("button", { name: "Done" }));
- const [aggregations] = onSubmit.mock.lastCall;
+ const [_, aggregations] = onSubmit.mock.lastCall;
expect(onSubmit).toHaveBeenCalled();
expect(aggregations).toHaveLength(2);
});
diff --git a/frontend/src/metabase/query_builder/components/CompareAggregations/utils.ts b/frontend/src/metabase/query_builder/components/CompareAggregations/utils.ts
index f0efefb0738e0bbf997fd59edcb6ca1f17d22dc1..a44a07c10160ac396a581777ce51ef99f6d2701f 100644
--- a/frontend/src/metabase/query_builder/components/CompareAggregations/utils.ts
+++ b/frontend/src/metabase/query_builder/components/CompareAggregations/utils.ts
@@ -82,6 +82,52 @@ export const getAggregations = (
return aggregations;
};
+type BreakoutColumnAndBucket = {
+ breakoutIndex: number | null;
+ column: Lib.ColumnMetadata;
+ bucket: Lib.Bucket | null;
+};
+
+export function getBreakout(
+ query: Lib.Query,
+ stageIndex: number,
+): BreakoutColumnAndBucket | null {
+ const breakouts = Lib.breakouts(query, stageIndex);
+ const breakoutIndex = breakouts.findIndex(breakout =>
+ isTemporal(query, stageIndex, breakout),
+ );
+ if (breakoutIndex >= 0) {
+ const breakout = breakouts[breakoutIndex];
+ return {
+ breakoutIndex,
+ column: Lib.breakoutColumn(query, stageIndex, breakout),
+ bucket: Lib.temporalBucket(breakout),
+ };
+ }
+
+ const columns = Lib.breakoutableColumns(query, stageIndex);
+ const temporalColumn = columns.find(column => Lib.isTemporal(column));
+
+ if (temporalColumn) {
+ return {
+ breakoutIndex: null,
+ column: temporalColumn,
+ bucket: Lib.defaultTemporalBucket(query, stageIndex, temporalColumn),
+ };
+ }
+
+ return null;
+}
+
+function isTemporal(
+ query: Lib.Query,
+ stageIndex: number,
+ breakout: Lib.BreakoutClause,
+) {
+ const column = Lib.breakoutColumn(query, stageIndex, breakout);
+ return Lib.isTemporal(column);
+}
+
export const canSubmit = (
period: number | "",
columns: ColumnType[],
@@ -91,6 +137,71 @@ export const canSubmit = (
return isPeriodValid && areColumnsValid;
};
+type UpdatedQuery = {
+ query: Lib.Query;
+ addedAggregations: Lib.ExpressionClause[];
+};
+
+export function updateQueryWithCompareOffsetAggregations(
+ query: Lib.Query,
+ stageIndex: number,
+ aggregation: Lib.AggregationClause | Lib.ExpressionClause,
+ offset: "" | number,
+ columns: ColumnType[],
+ columnAndBucket: BreakoutColumnAndBucket,
+): UpdatedQuery | null {
+ if (!aggregation || offset === "") {
+ return null;
+ }
+
+ let nextQuery = query;
+ const column = Lib.withTemporalBucket(
+ columnAndBucket.column,
+ columnAndBucket.bucket,
+ );
+
+ let { breakoutIndex } = columnAndBucket;
+ if (breakoutIndex !== null) {
+ // replace the breakout
+ const breakout = Lib.breakouts(nextQuery, stageIndex)[breakoutIndex];
+ nextQuery = Lib.replaceClause(nextQuery, stageIndex, breakout, column);
+ } else {
+ // add the breakout
+ nextQuery = Lib.breakout(nextQuery, stageIndex, column);
+ breakoutIndex = Lib.breakouts(nextQuery, stageIndex).length - 1;
+ }
+
+ if (breakoutIndex > 0) {
+ const breakouts = Lib.breakouts(nextQuery, stageIndex);
+
+ // move the breakout to the front
+ nextQuery = Lib.swapClauses(
+ nextQuery,
+ stageIndex,
+ breakouts[0],
+ breakouts[breakoutIndex],
+ );
+ }
+
+ const aggregations = getAggregations(
+ nextQuery,
+ stageIndex,
+ aggregation,
+ columns,
+ offset,
+ );
+
+ nextQuery = aggregations.reduce(
+ (query, aggregation) => Lib.aggregate(query, stageIndex, aggregation),
+ nextQuery,
+ );
+
+ return {
+ query: nextQuery,
+ addedAggregations: aggregations,
+ };
+}
+
export function canAddTemporalCompareAggregation(
query: Lib.Query,
stageIndex: number,
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 d7267211f7ae97b6fab4adb709ba2128f2593ea6..ea6a0d8f5ee07ff16b05921daf960890cc03e3c0 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
@@ -21,27 +21,6 @@ export function AggregateStep({
return Lib.aggregations(query, stageIndex);
}, [query, stageIndex]);
- const handleAddAggregations = (aggregations: Lib.Aggregable[]) => {
- const nextQuery = aggregations.reduce(
- (query, aggregation) => Lib.aggregate(query, stageIndex, aggregation),
- query,
- );
- updateQuery(nextQuery);
- };
-
- const handleUpdateAggregation = (
- currentClause: Lib.AggregationClause,
- nextClause: Lib.Aggregable,
- ) => {
- const nextQuery = Lib.replaceClause(
- query,
- stageIndex,
- currentClause,
- nextClause,
- );
- updateQuery(nextQuery);
- };
-
const handleReorderAggregation = (
sourceClause: Lib.AggregationClause,
targetClause: Lib.AggregationClause,
@@ -77,8 +56,7 @@ export function AggregateStep({
stageIndex={stageIndex}
clause={aggregation}
clauseIndex={index}
- onAddAggregations={handleAddAggregations}
- onUpdateAggregation={handleUpdateAggregation}
+ onQueryChange={updateQuery}
onClose={onClose}
/>
)}
@@ -93,14 +71,8 @@ interface AggregationPopoverProps {
query: Lib.Query;
stageIndex: number;
clause?: Lib.AggregationClause;
- onUpdateAggregation: (
- currentClause: Lib.AggregationClause,
- nextClause: Lib.Aggregable,
- ) => void;
- onAddAggregations: (aggregations: Lib.Aggregable[]) => void;
-
clauseIndex?: number;
-
+ onQueryChange: (query: Lib.Query) => void;
onClose: () => void;
}
@@ -109,8 +81,7 @@ function AggregationPopover({
stageIndex,
clause,
clauseIndex,
- onAddAggregations,
- onUpdateAggregation,
+ onQueryChange,
onClose,
}: AggregationPopoverProps) {
const isUpdate = clause != null && clauseIndex != null;
@@ -129,14 +100,7 @@ function AggregationPopover({
clause={clause}
clauseIndex={clauseIndex}
operators={operators}
- onAdd={onAddAggregations}
- onSelect={aggregation => {
- if (isUpdate) {
- onUpdateAggregation(clause, aggregation);
- } else {
- onAddAggregations([aggregation]);
- }
- }}
+ onQueryChange={onQueryChange}
onClose={onClose}
/>
);
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.tsx
index 0b67b1a282b234f5a72def8fcccbe26a1e2bda5e..f1e7b291a45e041759d01699594521ed052765ad 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AddAggregationButton/AddAggregationButton.tsx
@@ -12,13 +12,13 @@ import { AddAggregationButtonRoot } from "./AddAggregationButton.styled";
interface AddAggregationButtonProps {
query: Lib.Query;
stageIndex: number;
- onAddAggregations: (aggregation: Lib.Aggregable[]) => void;
+ onQueryChange: (query: Lib.Query) => void;
}
export function AddAggregationButton({
query,
stageIndex,
- onAddAggregations,
+ onQueryChange,
}: AddAggregationButtonProps) {
const [isOpened, setIsOpened] = useState(false);
const hasAggregations = Lib.aggregations(query, stageIndex).length > 0;
@@ -53,12 +53,8 @@ export function AddAggregationButton({
stageIndex={stageIndex}
operators={operators}
hasExpressionInput={false}
- onAdd={aggregations => {
- onAddAggregations(aggregations);
- setIsOpened(false);
- }}
- onSelect={aggregation => {
- onAddAggregations([aggregation]);
+ onQueryChange={query => {
+ onQueryChange(query);
setIsOpened(false);
}}
/>
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.tsx
index 9d49652a6fc41bea395682611c453cb7ea790061..ff7daabfc87279728d9cce34f80c2520245d256f 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/AggregationItem/AggregationItem.tsx
@@ -1,4 +1,4 @@
-import { useState } from "react";
+import { useState, useCallback } from "react";
import { Popover } from "metabase/ui";
import * as Lib from "metabase-lib";
@@ -12,9 +12,7 @@ interface AggregationItemProps {
stageIndex: number;
aggregation: Lib.AggregationClause;
aggregationIndex: number;
- onAdd: (aggregations: Lib.Aggregable[]) => void;
- onUpdate: (nextAggregation: Lib.Aggregable) => void;
- onRemove: () => void;
+ onQueryChange: (query: Lib.Query) => void;
}
export function AggregationItem({
@@ -22,9 +20,7 @@ export function AggregationItem({
stageIndex,
aggregation,
aggregationIndex,
- onAdd,
- onUpdate,
- onRemove,
+ onQueryChange,
}: AggregationItemProps) {
const [isOpened, setIsOpened] = useState(false);
const { displayName } = Lib.displayInfo(query, stageIndex, aggregation);
@@ -34,6 +30,11 @@ export function AggregationItem({
aggregation,
);
+ const handleRemove = useCallback(() => {
+ const nextQuery = Lib.removeClause(query, stageIndex, aggregation);
+ onQueryChange(nextQuery);
+ }, [query, stageIndex, aggregation, onQueryChange]);
+
return (
<Popover opened={isOpened} onChange={setIsOpened}>
<Popover.Target>
@@ -43,7 +44,7 @@ export function AggregationItem({
onClick={() => setIsOpened(!isOpened)}
>
<AggregationName>{displayName}</AggregationName>
- <RemoveIcon name="close" onClick={onRemove} />
+ <RemoveIcon name="close" onClick={handleRemove} />
</Root>
</Popover.Target>
<Popover.Dropdown>
@@ -54,11 +55,7 @@ export function AggregationItem({
clauseIndex={aggregationIndex}
operators={operators}
hasExpressionInput={false}
- onAdd={onAdd}
- onSelect={nextAggregation => {
- onUpdate(nextAggregation);
- setIsOpened(false);
- }}
+ onQueryChange={onQueryChange}
/>
</Popover.Dropdown>
</Popover>
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeAggregationItemList.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeAggregationItemList.tsx
index 115b8e871c3eb6c8932296221f74895321079194..a7c1436857dd14d7248984e3f597d1a13f3253ea 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeAggregationItemList.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeAggregationItemList.tsx
@@ -8,21 +8,14 @@ type SummarizeAggregationItemListProps = {
query: Lib.Query;
stageIndex: number;
aggregations: Lib.AggregationClause[];
- onAddAggregations: (aggregations: Lib.Aggregable[]) => void;
- onUpdateAggregation: (
- aggregation: Lib.AggregationClause,
- nextAggregation: Lib.Aggregable,
- ) => void;
- onRemoveAggregation: (aggregation: Lib.AggregationClause) => void;
+ onQueryChange: (query: Lib.Query) => void;
} & GroupProps;
export const SummarizeAggregationItemList = ({
query,
stageIndex,
aggregations,
- onAddAggregations,
- onUpdateAggregation,
- onRemoveAggregation,
+ onQueryChange,
...containerProps
}: SummarizeAggregationItemListProps) => (
<Group
@@ -38,17 +31,13 @@ export const SummarizeAggregationItemList = ({
stageIndex={stageIndex}
aggregation={aggregation}
aggregationIndex={aggregationIndex}
- onAdd={onAddAggregations}
- onUpdate={nextAggregation =>
- onUpdateAggregation(aggregation, nextAggregation)
- }
- onRemove={() => onRemoveAggregation(aggregation)}
+ onQueryChange={onQueryChange}
/>
))}
<AddAggregationButton
query={query}
stageIndex={stageIndex}
- onAddAggregations={onAddAggregations}
+ onQueryChange={onQueryChange}
/>
</Group>
);
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/use-summarize-query.ts b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/use-summarize-query.ts
index 6b04d5d9c6ce096c25ad7f56983f53d8da303e4b..dfbf3d3aec1502b74b836baebed6cbbbe9b23e18 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/use-summarize-query.ts
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/use-summarize-query.ts
@@ -36,39 +36,16 @@ export const useSummarizeQuery = ({
[onQueryChange],
);
- const handleAddAggregations = useCallback(
- (aggregations: Lib.Aggregable[]) => {
- const nextQuery = aggregations.reduce(
- (query, aggregation) => Lib.aggregate(query, STAGE_INDEX, aggregation),
- query,
- );
- handleChange(nextQuery);
- },
- [query, handleChange],
- );
-
- const handleUpdateAggregation = useCallback(
- (aggregation: Lib.AggregationClause, nextAggregation: Lib.Aggregable) => {
- const nextQuery = Lib.replaceClause(
- query,
- STAGE_INDEX,
- aggregation,
- nextAggregation,
- );
- handleChange(nextQuery);
- },
- [query, handleChange],
- );
-
- const handleRemoveAggregation = useCallback(
- (aggregation: Lib.AggregationClause) => {
- if (hasDefaultAggregation) {
+ const handleQueryChange = useCallback(
+ (nextQuery: Lib.Query) => {
+ const newAggregations = Lib.aggregations(nextQuery, STAGE_INDEX);
+ if (newAggregations.length === 0) {
setHasDefaultAggregation(false);
} else {
- handleChange(Lib.removeClause(query, STAGE_INDEX, aggregation));
+ handleChange(nextQuery);
}
},
- [query, hasDefaultAggregation, handleChange],
+ [handleChange],
);
const handleAddBreakout = useCallback(
@@ -107,9 +84,7 @@ export const useSummarizeQuery = ({
stageIndex: STAGE_INDEX,
aggregations,
hasAggregations,
- handleAddAggregations,
- handleUpdateAggregation,
- handleRemoveAggregation,
+ handleQueryChange,
handleAddBreakout,
handleUpdateBreakout,
handleRemoveBreakout,
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.tsx
index 4c9a033b19fb37fa24d38483ab9d0bdab80fc752..83bffdb16b33c2e3878a41c5f1eb7d93ec464ba8 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.tsx
@@ -30,9 +30,7 @@ export function SummarizeSidebar({
stageIndex,
aggregations,
hasAggregations,
- handleAddAggregations,
- handleUpdateAggregation,
- handleRemoveAggregation,
+ handleQueryChange,
handleAddBreakout,
handleUpdateBreakout,
handleRemoveBreakout,
@@ -59,9 +57,7 @@ export function SummarizeSidebar({
query={query}
stageIndex={stageIndex}
aggregations={aggregations}
- onAddAggregations={handleAddAggregations}
- onUpdateAggregation={handleUpdateAggregation}
- onRemoveAggregation={handleRemoveAggregation}
+ onQueryChange={handleQueryChange}
/>
<Divider my="lg" />
{hasAggregations && (
diff --git a/frontend/src/metabase/querying/utils/drills/compare-aggregations-drill/compare-aggregations-drill.tsx b/frontend/src/metabase/querying/utils/drills/compare-aggregations-drill/compare-aggregations-drill.tsx
index e05b6a10e13aa73570b881f0ed19a2144ef2f84c..d8a2595c9c737e3312ed3d9d8e570fca0958e3d5 100644
--- a/frontend/src/metabase/querying/utils/drills/compare-aggregations-drill/compare-aggregations-drill.tsx
+++ b/frontend/src/metabase/querying/utils/drills/compare-aggregations-drill/compare-aggregations-drill.tsx
@@ -34,13 +34,7 @@ export const compareAggregationsDrill: Drill<
query={query}
stageIndex={stageIndex}
onClose={onClose}
- onSubmit={aggregations => {
- const nextQuery = aggregations.reduce(
- (query, aggregation) =>
- Lib.aggregate(query, stageIndex, aggregation),
- query,
- );
-
+ onSubmit={(nextQuery, aggregations) => {
const nextQuestion = question.setQuery(nextQuery);
const nextCard = nextQuestion.card();
diff --git a/frontend/src/metabase/visualizations/click-actions/actions/CompareAggregationsAction/CompareAggregationsAction.tsx b/frontend/src/metabase/visualizations/click-actions/actions/CompareAggregationsAction/CompareAggregationsAction.tsx
index c69a573b766fbfe47a45277cd1cb914b5f3ddec2..c68257e51810d4791e3973c7f15b7d68a7da6aa8 100644
--- a/frontend/src/metabase/visualizations/click-actions/actions/CompareAggregationsAction/CompareAggregationsAction.tsx
+++ b/frontend/src/metabase/visualizations/click-actions/actions/CompareAggregationsAction/CompareAggregationsAction.tsx
@@ -41,15 +41,12 @@ export const CompareAggregationsAction: LegacyDrill = ({
const dispatch = useDispatch();
const aggregations = Lib.aggregations(query, stageIndex);
- function handleSubmit(aggregations: Lib.ExpressionClause[]) {
- const nextQuery = aggregations.reduce(
- (query, aggregation) => Lib.aggregate(query, stageIndex, aggregation),
- query,
- );
-
+ function handleSubmit(
+ nextQuery: Lib.Query,
+ aggregations: Lib.ExpressionClause[],
+ ) {
const nextQuestion = checkNotNull(currentQuestion).setQuery(nextQuery);
const nextCard = nextQuestion.card();
-
trackColumnCompareViaPlusModal(
nextQuery,
stageIndex,