From cb7978315861bd5e4cd21d2c8c914f386c9c9b82 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Mon, 12 Aug 2024 16:54:37 -0400
Subject: [PATCH] FE - Support multiple breakout positions (#46555)
---
.../src/metabase-lib/breakout.unit.spec.ts | 10 +-
frontend/src/metabase-lib/types.ts | 2 +-
.../QueryColumnPicker.unit.spec.tsx | 4 +-
.../components/notebook/NotebookStep/steps.ts | 2 +-
.../steps/BreakoutStep/BreakoutStep.tsx | 32 +--
.../BreakoutStep/BreakoutStep.unit.spec.tsx | 2 +-
.../notebook/steps/BreakoutStep/index.ts | 3 +-
.../notebook/steps/SummarizeStep.tsx | 2 +-
.../BreakoutColumnList/BreakoutColumnList.tsx | 155 +++++++-------
.../BreakoutColumnListItem.tsx | 39 ++--
.../SummarizeBreakoutColumnList.tsx | 4 +-
.../SummarizeContent/use-summarize-query.ts | 11 +-
.../SummarizeSidebar.unit.spec.tsx | 198 ++++++++++++------
.../TimeseriesBucketPicker.tsx | 18 +-
.../TimeseriesBucketPicker.unit.spec.tsx | 57 +++--
.../TimeseriesChrome/TimeseriesChrome.tsx | 15 +-
.../TimeseriesChrome.unit.spec.tsx | 100 +++++++++
.../components/TimeseriesChrome/utils.ts | 26 +--
src/metabase/lib/breakout.cljc | 76 ++++---
src/metabase/lib/metadata/calculation.cljc | 2 +-
src/metabase/lib/remove_replace.cljc | 39 ++--
test/metabase/lib/binning_test.cljc | 12 +-
test/metabase/lib/breakout_test.cljc | 128 +++++++++--
test/metabase/lib/equality_test.cljc | 2 +-
test/metabase/lib/remove_replace_test.cljc | 22 +-
25 files changed, 628 insertions(+), 333 deletions(-)
create mode 100644 frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.unit.spec.tsx
diff --git a/frontend/src/metabase-lib/breakout.unit.spec.ts b/frontend/src/metabase-lib/breakout.unit.spec.ts
index 25d10b0297f..96bd13f1382 100644
--- a/frontend/src/metabase-lib/breakout.unit.spec.ts
+++ b/frontend/src/metabase-lib/breakout.unit.spec.ts
@@ -34,9 +34,9 @@ describe("breakout", () => {
"TAX",
);
- expect(
- Lib.displayInfo(nextQuery, 0, nextTaxColumn).breakoutPosition,
- ).toBe(0);
+ expect(Lib.displayInfo(nextQuery, 0, nextTaxColumn)).toMatchObject({
+ breakoutPositions: [0],
+ });
const roundtripQuery = createQuery({
query: Lib.toLegacyQuery(nextQuery),
@@ -48,8 +48,8 @@ describe("breakout", () => {
)("ORDERS", "TAX");
expect(
- Lib.displayInfo(roundtripQuery, 0, roundtripTaxColumn).breakoutPosition,
- ).toBe(0);
+ Lib.displayInfo(roundtripQuery, 0, roundtripTaxColumn),
+ ).toMatchObject({ breakoutPositions: [0] });
});
it("should note whether the temporal unit is for extraction in the displayInfo", () => {
diff --git a/frontend/src/metabase-lib/types.ts b/frontend/src/metabase-lib/types.ts
index 1186395a319..9637cd3149c 100644
--- a/frontend/src/metabase-lib/types.ts
+++ b/frontend/src/metabase-lib/types.ts
@@ -160,7 +160,7 @@ export type ColumnDisplayInfo = {
table?: TableInlineDisplayInfo;
fingerprint?: FingerprintDisplayInfo;
- breakoutPosition?: number;
+ breakoutPositions?: number[];
filterPositions?: number[];
orderByPosition?: number;
selected?: boolean; // used in aggregation and field clauses
diff --git a/frontend/src/metabase/common/components/QueryColumnPicker/QueryColumnPicker.unit.spec.tsx b/frontend/src/metabase/common/components/QueryColumnPicker/QueryColumnPicker.unit.spec.tsx
index 67d6ae067a3..75a03cee24e 100644
--- a/frontend/src/metabase/common/components/QueryColumnPicker/QueryColumnPicker.unit.spec.tsx
+++ b/frontend/src/metabase/common/components/QueryColumnPicker/QueryColumnPicker.unit.spec.tsx
@@ -49,7 +49,9 @@ function setup({
columnGroups={Lib.groupColumns(columns)}
hasBinning={hasBinning}
hasTemporalBucketing={hasTemporalBucketing}
- checkIsColumnSelected={item => item.breakoutPosition === 0}
+ checkIsColumnSelected={({ breakoutPositions = [] }) =>
+ breakoutPositions.length > 0
+ }
onSelect={onSelect}
onClose={onClose}
/>,
diff --git a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/steps.ts b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/steps.ts
index e5135c01c06..2806a94db1d 100644
--- a/frontend/src/metabase/query_builder/components/notebook/NotebookStep/steps.ts
+++ b/frontend/src/metabase/query_builder/components/notebook/NotebookStep/steps.ts
@@ -5,7 +5,7 @@ import { color } from "metabase/lib/colors";
import type { IconName } from "metabase/ui";
import { AggregateStep } from "../steps/AggregateStep";
-import BreakoutStep from "../steps/BreakoutStep";
+import { BreakoutStep } from "../steps/BreakoutStep";
import { DataStep } from "../steps/DataStep";
import { ExpressionStep } from "../steps/ExpressionStep";
import { FilterStep } from "../steps/FilterStep";
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 d4fd33c4da9..3f671b1d11a 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
@@ -7,7 +7,7 @@ import * as Lib from "metabase-lib";
import type { NotebookStepUiComponentProps } from "../../types";
import { ClauseStep } from "../ClauseStep";
-function BreakoutStep({
+export function BreakoutStep({
query,
step,
color,
@@ -105,16 +105,21 @@ const BreakoutPopover = ({
}: BreakoutPopoverProps) => {
const columnGroups = useMemo(() => {
const columns = Lib.breakoutableColumns(query, stageIndex);
-
- const filteredColumns = columns.filter(column => {
- const columnInfo = Lib.displayInfo(query, stageIndex, column);
- const isAlreadyUsed = columnInfo.breakoutPosition != null;
- const isSelected = checkColumnSelected(columnInfo, breakoutIndex);
- return isSelected || !isAlreadyUsed;
- });
-
+ const filteredColumns = columns.reduce(
+ (columns: Lib.ColumnMetadata[], column) => {
+ const columnInfo = Lib.displayInfo(query, stageIndex, column);
+ const { breakoutPositions = [] } = columnInfo;
+ if (breakout && checkColumnSelected(columnInfo, breakoutIndex)) {
+ columns.push(Lib.breakoutColumn(query, stageIndex, breakout));
+ } else if (breakoutPositions.length === 0) {
+ columns.push(column);
+ }
+ return columns;
+ },
+ [],
+ );
return Lib.groupColumns(filteredColumns);
- }, [query, stageIndex, breakoutIndex]);
+ }, [query, stageIndex, breakout, breakoutIndex]);
return (
<QueryColumnPicker
@@ -140,11 +145,8 @@ const BreakoutPopover = ({
};
const checkColumnSelected = (
- columnInfo: Lib.ColumnDisplayInfo,
+ { breakoutPositions = [] }: Lib.ColumnDisplayInfo,
breakoutIndex?: number,
) => {
- return breakoutIndex != null && columnInfo.breakoutPosition === breakoutIndex;
+ return breakoutIndex != null && breakoutPositions.includes(breakoutIndex);
};
-
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export default BreakoutStep;
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.unit.spec.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.unit.spec.tsx
index 60a3db89655..ebf29b553ba 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.unit.spec.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/BreakoutStep.unit.spec.tsx
@@ -11,7 +11,7 @@ import {
import { createMockNotebookStep } from "../../test-utils";
-import BreakoutStep from "./BreakoutStep";
+import { BreakoutStep } from "./BreakoutStep";
function createQueryWithBreakout() {
const initialQuery = createQuery();
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/index.ts b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/index.ts
index 0f673eb8bc2..5f8dd8f4afb 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/index.ts
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/BreakoutStep/index.ts
@@ -1,2 +1 @@
-// eslint-disable-next-line import/no-default-export -- deprecated usage
-export { default } from "./BreakoutStep";
+export * from "./BreakoutStep";
diff --git a/frontend/src/metabase/query_builder/components/notebook/steps/SummarizeStep.tsx b/frontend/src/metabase/query_builder/components/notebook/steps/SummarizeStep.tsx
index b9e2a7a0bc3..b1a4214c4bd 100644
--- a/frontend/src/metabase/query_builder/components/notebook/steps/SummarizeStep.tsx
+++ b/frontend/src/metabase/query_builder/components/notebook/steps/SummarizeStep.tsx
@@ -5,7 +5,7 @@ import { Box, Flex } from "metabase/ui";
import type { NotebookStepUiComponentProps } from "../types";
import { AggregateStep } from "./AggregateStep";
-import BreakoutStep from "./BreakoutStep";
+import { BreakoutStep } from "./BreakoutStep";
function SummarizeStep({
color,
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnList.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnList.tsx
index 5f893f8b66d..707e5849733 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnList.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnList.tsx
@@ -1,6 +1,5 @@
-import { useCallback, useMemo, useState } from "react";
+import { type ChangeEvent, useCallback, useMemo, useState } from "react";
import { t } from "ttag";
-import _ from "underscore";
import { getColumnGroupName } from "metabase/common/utils/column-groups";
import Input from "metabase/core/components/Input";
@@ -18,10 +17,10 @@ export interface BreakoutColumnListProps {
onAddBreakout: (column: Lib.ColumnMetadata) => void;
onUpdateBreakout: (
breakout: Lib.BreakoutClause,
- nextColumn: Lib.ColumnMetadata,
+ column: Lib.ColumnMetadata,
) => void;
- onRemoveBreakout: (column: Lib.ColumnMetadata) => void;
- onReplaceBreakout: (column: Lib.ColumnMetadata) => void;
+ onRemoveBreakout: (breakout: Lib.BreakoutClause) => void;
+ onReplaceBreakouts: (column: Lib.ColumnMetadata) => void;
}
export function BreakoutColumnList({
@@ -30,7 +29,7 @@ export function BreakoutColumnList({
onAddBreakout,
onUpdateBreakout,
onRemoveBreakout,
- onReplaceBreakout,
+ onReplaceBreakouts,
}: BreakoutColumnListProps) {
const [searchQuery, setSearchQuery] = useState("");
const debouncedSearchQuery = useDebouncedValue(
@@ -40,27 +39,27 @@ export function BreakoutColumnList({
const isSearching = searchQuery.trim().length > 0;
const breakouts = Lib.breakouts(query, stageIndex);
- const [pinnedBreakouts, setPinnedBreakouts] = useState(breakouts);
+ const [pinnedItemCount, setPinnedItemCount] = useState(breakouts.length);
+
+ const pinnedItems = useMemo(
+ () =>
+ breakouts
+ .slice(0, pinnedItemCount)
+ .map(breakout => getBreakoutListItem(query, stageIndex, breakout)),
+ [query, stageIndex, breakouts, pinnedItemCount],
+ );
const allColumns = useMemo(
() => Lib.breakoutableColumns(query, stageIndex),
[query, stageIndex],
);
- const [pinnedColumns, unpinnedColumns] = useMemo(
- () =>
- _.partition(allColumns, column =>
- isPinnedColumn(query, stageIndex, pinnedBreakouts, column),
- ),
- [query, stageIndex, pinnedBreakouts, allColumns],
- );
-
- const pinnedItems = useMemo(
+ const unpinnedColumns = useMemo(
() =>
- pinnedColumns.map(column =>
- getColumnListItem(query, stageIndex, breakouts, column),
+ allColumns.filter(
+ column => !isPinnedColumn(query, stageIndex, column, pinnedItemCount),
),
- [query, stageIndex, breakouts, pinnedColumns],
+ [query, stageIndex, allColumns, pinnedItemCount],
);
const sections = useMemo(
@@ -82,31 +81,23 @@ export function BreakoutColumnList({
);
const handleRemovePinnedBreakout = useCallback(
- (column: Lib.ColumnMetadata) => {
- const { breakoutPosition } = Lib.displayInfo(query, stageIndex, column);
- const isPinned =
- breakoutPosition != null && breakoutPosition < pinnedBreakouts.length;
-
- if (isPinned) {
- const breakout = pinnedBreakouts[breakoutPosition];
- setPinnedBreakouts(breakouts => breakouts.filter(b => b !== breakout));
- }
-
- onRemoveBreakout(column);
+ (breakout: Lib.BreakoutClause) => {
+ setPinnedItemCount(pinnedItemCount - 1);
+ onRemoveBreakout(breakout);
},
- [query, stageIndex, pinnedBreakouts, onRemoveBreakout],
+ [pinnedItemCount, onRemoveBreakout],
);
- const handleReplaceBreakout = useCallback(
+ const handleReplaceBreakouts = useCallback(
(column: Lib.ColumnMetadata) => {
- onReplaceBreakout(column);
- setPinnedBreakouts([]);
+ onReplaceBreakouts(column);
+ setPinnedItemCount(0);
},
- [onReplaceBreakout],
+ [onReplaceBreakouts],
);
const handleChangeSearchQuery = useCallback(
- (event: React.ChangeEvent<HTMLInputElement>) => {
+ (event: ChangeEvent<HTMLInputElement>) => {
setSearchQuery(event.target.value);
},
[],
@@ -137,15 +128,9 @@ export function BreakoutColumnList({
item={item}
breakout={item.breakout}
isPinned
- onAddColumn={onAddBreakout}
- onUpdateColumn={column => {
- if (item.breakout) {
- onUpdateBreakout(item.breakout, column);
- } else {
- onAddBreakout(column);
- }
- }}
- onRemoveColumn={handleRemovePinnedBreakout}
+ onAddBreakout={onAddBreakout}
+ onUpdateBreakout={onUpdateBreakout}
+ onRemoveBreakout={handleRemovePinnedBreakout}
/>
))}
</ul>
@@ -164,16 +149,10 @@ export function BreakoutColumnList({
stageIndex={stageIndex}
item={item}
breakout={item.breakout}
- onAddColumn={onAddBreakout}
- onUpdateColumn={column => {
- if (item.breakout) {
- onUpdateBreakout(item.breakout, column);
- } else {
- onAddBreakout(column);
- }
- }}
- onRemoveColumn={onRemoveBreakout}
- onReplaceColumns={handleReplaceBreakout}
+ onAddBreakout={onAddBreakout}
+ onUpdateBreakout={onUpdateBreakout}
+ onRemoveBreakout={onRemoveBreakout}
+ onReplaceBreakouts={handleReplaceBreakouts}
/>
))}
</ul>
@@ -185,22 +164,46 @@ export function BreakoutColumnList({
);
}
-function getColumnListItem(
+type ListItem = Lib.ColumnDisplayInfo & {
+ column: Lib.ColumnMetadata;
+ breakout?: Lib.BreakoutClause;
+};
+
+type ListSection = {
+ name: string;
+ items: ListItem[];
+};
+
+function getBreakoutListItem(
+ query: Lib.Query,
+ stageIndex: number,
+ breakout: Lib.BreakoutClause,
+): ListItem {
+ const column = Lib.breakoutColumn(query, stageIndex, breakout);
+ const columnInfo = Lib.displayInfo(query, stageIndex, column);
+ return { ...columnInfo, column, breakout };
+}
+
+function getColumnListItems(
query: Lib.Query,
stageIndex: number,
breakouts: Lib.BreakoutClause[],
column: Lib.ColumnMetadata,
-) {
- const displayInfo = Lib.displayInfo(query, stageIndex, column);
- const breakout =
- displayInfo.breakoutPosition != null
- ? breakouts[displayInfo.breakoutPosition]
- : undefined;
- return {
- ...displayInfo,
- column,
- breakout,
- };
+): ListItem[] {
+ const columnInfo = Lib.displayInfo(query, stageIndex, column);
+ const { breakoutPositions = [] } = columnInfo;
+ if (breakoutPositions.length === 0) {
+ return [{ ...columnInfo, column }];
+ }
+
+ return breakoutPositions.map(index => {
+ const breakout = breakouts[index];
+ return {
+ ...columnInfo,
+ column: Lib.breakoutColumn(query, stageIndex, breakout),
+ breakout,
+ };
+ });
}
function getColumnSections(
@@ -208,7 +211,7 @@ function getColumnSections(
stageIndex: number,
columns: Lib.ColumnMetadata[],
searchQuery: string,
-) {
+): ListSection[] {
const breakouts = Lib.breakouts(query, stageIndex);
const formattedSearchQuery = searchQuery.trim().toLowerCase();
@@ -223,8 +226,8 @@ function getColumnSections(
return Lib.groupColumns(filteredColumns).map(group => {
const groupInfo = Lib.displayInfo(query, stageIndex, group);
- const items = Lib.getColumnsFromColumnGroup(group).map(column =>
- getColumnListItem(query, stageIndex, breakouts, column),
+ const items = Lib.getColumnsFromColumnGroup(group).flatMap(column =>
+ getColumnListItems(query, stageIndex, breakouts, column),
);
return {
@@ -237,10 +240,12 @@ function getColumnSections(
function isPinnedColumn(
query: Lib.Query,
stageIndex: number,
- pinnedBreakouts: Lib.BreakoutClause[],
column: Lib.ColumnMetadata,
-) {
- const { breakoutPosition } = Lib.displayInfo(query, stageIndex, column);
- const maxPinnedBreakoutIndex = pinnedBreakouts.length - 1;
- return breakoutPosition != null && breakoutPosition <= maxPinnedBreakoutIndex;
+ pinnedItemCount: number,
+): boolean {
+ const { breakoutPositions = [] } = Lib.displayInfo(query, stageIndex, column);
+ return (
+ breakoutPositions.length > 0 &&
+ breakoutPositions.every(breakoutIndex => breakoutIndex < pinnedItemCount)
+ );
}
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnListItem/BreakoutColumnListItem.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnListItem/BreakoutColumnListItem.tsx
index 54fc0128846..ac89c74e2c3 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnListItem/BreakoutColumnListItem.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/BreakoutColumnList/BreakoutColumnListItem/BreakoutColumnListItem.tsx
@@ -23,10 +23,13 @@ interface BreakoutColumnListItemProps {
item: Lib.ColumnDisplayInfo & { column: Lib.ColumnMetadata };
breakout?: Lib.BreakoutClause;
isPinned?: boolean;
- onAddColumn: (column: Lib.ColumnMetadata) => void;
- onUpdateColumn: (column: Lib.ColumnMetadata) => void;
- onRemoveColumn: (column: Lib.ColumnMetadata) => void;
- onReplaceColumns?: (column: Lib.ColumnMetadata) => void;
+ onAddBreakout: (column: Lib.ColumnMetadata) => void;
+ onUpdateBreakout: (
+ breakout: Lib.BreakoutClause,
+ column: Lib.ColumnMetadata,
+ ) => void;
+ onRemoveBreakout: (breakout: Lib.BreakoutClause) => void;
+ onReplaceBreakouts?: (column: Lib.ColumnMetadata) => void;
}
export function BreakoutColumnListItem({
@@ -35,27 +38,29 @@ export function BreakoutColumnListItem({
item,
breakout,
isPinned = false,
- onAddColumn,
- onUpdateColumn,
- onRemoveColumn,
- onReplaceColumns,
+ onAddBreakout,
+ onUpdateBreakout,
+ onRemoveBreakout,
+ onReplaceBreakouts,
}: BreakoutColumnListItemProps) {
- const isSelected = typeof item.breakoutPosition === "number";
+ const isSelected = breakout != null;
const handleAddClick = useCallback(() => {
- onAddColumn(Lib.withDefaultBucket(query, stageIndex, item.column));
- }, [query, stageIndex, item.column, onAddColumn]);
+ onAddBreakout(Lib.withDefaultBucket(query, stageIndex, item.column));
+ }, [query, stageIndex, item.column, onAddBreakout]);
const handleListItemClick = useCallback(() => {
- onReplaceColumns?.(Lib.withDefaultBucket(query, stageIndex, item.column));
- }, [query, stageIndex, item.column, onReplaceColumns]);
+ onReplaceBreakouts?.(Lib.withDefaultBucket(query, stageIndex, item.column));
+ }, [query, stageIndex, item.column, onReplaceBreakouts]);
const handleRemoveColumn = useCallback(
(event: MouseEvent) => {
event.stopPropagation();
- onRemoveColumn(item.column);
+ if (breakout) {
+ onRemoveBreakout(breakout);
+ }
},
- [item.column, onRemoveColumn],
+ [breakout, onRemoveBreakout],
);
const displayName = isPinned ? item.longDisplayName : item.displayName;
@@ -89,7 +94,9 @@ export function BreakoutColumnListItem({
hasBinning
hasTemporalBucketing
onSelect={column =>
- breakout ? onUpdateColumn(column) : onAddColumn(column)
+ breakout
+ ? onUpdateBreakout(breakout, column)
+ : onAddBreakout(column)
}
/>
{isSelected && (
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeBreakoutColumnList.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeBreakoutColumnList.tsx
index e34a62777e9..e909a59f0c1 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeBreakoutColumnList.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeContent/SummarizeBreakoutColumnList.tsx
@@ -14,7 +14,7 @@ type SummarizeBreakoutColumnListProps = {
clause: Lib.BreakoutClause,
column: Lib.ColumnMetadata,
) => void;
- onRemoveBreakout: (column: Lib.ColumnMetadata) => void;
+ onRemoveBreakout: (clause: Lib.BreakoutClause) => void;
onReplaceBreakouts: (column: Lib.ColumnMetadata) => void;
} & StackProps;
@@ -40,7 +40,7 @@ export const SummarizeBreakoutColumnList = ({
onAddBreakout={onAddBreakout}
onUpdateBreakout={onUpdateBreakout}
onRemoveBreakout={onRemoveBreakout}
- onReplaceBreakout={onReplaceBreakouts}
+ onReplaceBreakouts={onReplaceBreakouts}
/>
</Stack>
);
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 be17671f5bf..b68c5d75ca7 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
@@ -88,14 +88,9 @@ export const useSummarizeQuery = ({
);
const handleRemoveBreakout = useCallback(
- (column: Lib.ColumnMetadata) => {
- const { breakoutPosition } = Lib.displayInfo(query, STAGE_INDEX, column);
- if (breakoutPosition != null) {
- const breakouts = Lib.breakouts(query, STAGE_INDEX);
- const clause = breakouts[breakoutPosition];
- const nextQuery = Lib.removeClause(query, STAGE_INDEX, clause);
- handleChange(nextQuery);
- }
+ (clause: Lib.BreakoutClause) => {
+ const nextQuery = Lib.removeClause(query, STAGE_INDEX, clause);
+ handleChange(nextQuery);
},
[query, handleChange],
);
diff --git a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.unit.spec.tsx b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.unit.spec.tsx
index 2e4e301ec2d..d05adb145b1 100644
--- a/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.unit.spec.tsx
+++ b/frontend/src/metabase/query_builder/components/view/sidebars/SummarizeSidebar/SummarizeSidebar.unit.spec.tsx
@@ -1,23 +1,10 @@
import userEvent from "@testing-library/user-event";
import { useState } from "react";
-import { createMockMetadata } from "__support__/metadata";
import { renderWithProviders, screen, waitFor, within } from "__support__/ui";
-import { checkNotNull } from "metabase/lib/types";
import * as Lib from "metabase-lib";
-import Question from "metabase-lib/v1/Question";
-import type { Card, UnsavedCard } from "metabase-types/api";
+import { createQuery, createQueryWithClauses } from "metabase-lib/test-helpers";
import { createMockCard } from "metabase-types/api/mocks";
-import {
- ORDERS,
- ORDERS_ID,
- PEOPLE_ID,
- PRODUCTS,
- PRODUCTS_ID,
- SAMPLE_DB_ID,
- createAdHocCard,
- createSampleDatabase,
-} from "metabase-types/api/mocks/presets";
import {
createMockQueryBuilderState,
createMockState,
@@ -26,46 +13,56 @@ import {
import { SummarizeSidebar } from "./SummarizeSidebar";
type SetupOpts = {
- card?: Card | UnsavedCard;
+ query?: Lib.Query;
withDefaultAggregation?: boolean;
};
-function createSummarizedCard() {
- return createAdHocCard({
- dataset_query: {
- type: "query",
- database: SAMPLE_DB_ID,
- query: {
- "source-table": ORDERS_ID,
- aggregation: [["max", ["field", ORDERS.QUANTITY, null]]],
- breakout: [
- ["field", ORDERS.CREATED_AT, { "temporal-unit": "month" }],
- ["field", PRODUCTS.CATEGORY, { "source-field": ORDERS.PRODUCT_ID }],
- ],
+function createSummarizedQuery() {
+ return createQueryWithClauses({
+ aggregations: [
+ { operatorName: "max", tableName: "ORDERS", columnName: "QUANTITY" },
+ ],
+ breakouts: [
+ {
+ tableName: "ORDERS",
+ columnName: "CREATED_AT",
+ temporalBucketName: "Month",
+ },
+ {
+ tableName: "PRODUCTS",
+ columnName: "CATEGORY",
},
- },
+ ],
+ });
+}
+
+function createQueryWithBreakoutsForSameColumn() {
+ return createQueryWithClauses({
+ aggregations: [{ operatorName: "count" }],
+ breakouts: [
+ {
+ tableName: "ORDERS",
+ columnName: "CREATED_AT",
+ temporalBucketName: "Year",
+ },
+ {
+ tableName: "ORDERS",
+ columnName: "CREATED_AT",
+ temporalBucketName: "Month of year",
+ },
+ ],
});
}
async function setup({
- card = createAdHocCard(),
+ query: initialQuery = createQuery(),
withDefaultAggregation = true,
}: SetupOpts = {}) {
const onQueryChange = jest.fn();
const onClose = jest.fn();
- const metadata = createMockMetadata({
- databases: [createSampleDatabase()],
- questions: "id" in card ? [card] : [],
- });
-
- const question =
- "id" in card
- ? checkNotNull(metadata.question(card.id))
- : new Question(card, metadata);
-
function Wrapper() {
- const [query, setQuery] = useState(question.query());
+ const [query, setQuery] = useState(initialQuery);
return (
<SummarizeSidebar
@@ -112,7 +109,6 @@ async function setup({
}
return {
- metadata,
getNextAggregations,
getNextBreakouts,
onQueryChange,
@@ -161,28 +157,22 @@ describe("SummarizeSidebar", () => {
});
it("shouldn't apply default aggregation if a query is already aggregated", async () => {
- await setup({ card: createSummarizedCard() });
+ await setup({ query: createSummarizedQuery() });
expect(screen.queryByLabelText("Count")).not.toBeInTheDocument();
});
});
it("should list breakoutable columns", async () => {
- const { metadata } = await setup();
- const ordersTable = checkNotNull(metadata.table(ORDERS_ID));
- const productsTable = checkNotNull(metadata.table(PRODUCTS_ID));
- const peopleTable = checkNotNull(metadata.table(PEOPLE_ID));
- const expectedColumnCount = [
- ordersTable.fields,
- productsTable.fields,
- peopleTable.fields,
- ].flat().length;
+ const query = createQuery();
+ const columns = Lib.breakoutableColumns(query, -1);
+ await setup({ query });
expect(screen.getByText("Group by")).toBeInTheDocument();
expect(screen.getByText("Discount")).toBeInTheDocument();
expect(screen.getByText("Category")).toBeInTheDocument();
expect(screen.getByText("Email")).toBeInTheDocument();
expect(screen.getAllByTestId("dimension-list-item")).toHaveLength(
- expectedColumnCount,
+ columns.length,
);
});
@@ -213,7 +203,7 @@ describe("SummarizeSidebar", () => {
});
it("should highlight selected breakout columns", async () => {
- await setup({ card: createSummarizedCard() });
+ await setup({ query: createSummarizedQuery() });
const [ordersCreatedAt, peopleCreatedAt] =
screen.getAllByLabelText("Created At");
@@ -225,26 +215,88 @@ describe("SummarizeSidebar", () => {
});
it("should list breakouts added before opening the sidebar in a separate section", async () => {
- await setup({ card: createSummarizedCard() });
-
- const pinnedColumnList = screen.getByTestId("pinned-dimensions");
- const unpinnedColumnList = screen.getByTestId("unpinned-dimensions");
+ await setup({ query: createSummarizedQuery() });
expect(
- within(pinnedColumnList).getByText("Product → Category"),
+ within(getPinnedColumnList()).getByText("Product → Category"),
).toBeInTheDocument();
expect(
- within(pinnedColumnList).getByText("Created At"),
+ within(getPinnedColumnList()).getByText("Created At"),
).toBeInTheDocument();
expect(
- within(unpinnedColumnList).queryByText("Category"),
+ within(getUnpinnedColumnList()).queryByText("Category"),
).not.toBeInTheDocument();
// "Product → Created At" and "User → Created At" should still be there
- expect(within(unpinnedColumnList).getAllByText("Created At")).toHaveLength(
- 2,
+ expect(
+ within(getUnpinnedColumnList()).getAllByText("Created At"),
+ ).toHaveLength(2);
+ });
+
+ it("should show multiple breakouts of the same column", async () => {
+ await setup({
+ query: createQueryWithBreakoutsForSameColumn(),
+ });
+ expect(
+ within(getPinnedColumnList()).getAllByText("Created At"),
+ ).toHaveLength(2);
+ });
+
+ it("should allow to modify temporal units for breakouts of the same column", async () => {
+ const { getNextBreakouts } = await setup({
+ query: createQueryWithBreakoutsForSameColumn(),
+ });
+
+ await userEvent.click(await screen.findByText("by year"));
+ await userEvent.click(await screen.findByText("Quarter"));
+ expect(getNextBreakouts()).toMatchObject([
+ { displayName: "Created At: Quarter" },
+ { displayName: "Created At: Month of year" },
+ ]);
+
+ await userEvent.click(await screen.findByText("by month of year"));
+ await userEvent.click(await screen.findByText("Quarter of year"));
+ expect(getNextBreakouts()).toMatchObject([
+ { displayName: "Created At: Quarter" },
+ { displayName: "Created At: Quarter of year" },
+ ]);
+ });
+
+ it("should allow to remove breakouts of the same column", async () => {
+ const { getNextBreakouts } = await setup({
+ query: createQueryWithBreakoutsForSameColumn(),
+ });
+
+ const [firstBreakout] = within(getPinnedColumnList()).getAllByLabelText(
+ "Created At",
+ );
+ await userEvent.click(
+ within(firstBreakout).getByLabelText("Remove dimension"),
+ );
+ expect(
+ within(getPinnedColumnList()).getByText("Created At"),
+ ).toBeInTheDocument();
+ expect(
+ within(getUnpinnedColumnList()).getAllByText("Created At"),
+ ).toHaveLength(2);
+ expect(getNextBreakouts()).toMatchObject([
+ { displayName: "Created At: Month of year" },
+ ]);
+
+ const [secondBreakout] = within(getPinnedColumnList()).getAllByLabelText(
+ "Created At",
+ );
+ await userEvent.click(
+ within(secondBreakout).getByLabelText("Remove dimension"),
);
+ expect(
+ within(getPinnedColumnList()).queryByText("Created At"),
+ ).not.toBeInTheDocument();
+ expect(
+ within(getUnpinnedColumnList()).getAllByText("Created At"),
+ ).toHaveLength(3);
+ expect(getNextBreakouts()).toEqual([]);
});
it("should add an aggregation", async () => {
@@ -288,7 +340,7 @@ describe("SummarizeSidebar", () => {
});
it("shouldn't allow changing an aggregation to an expression", async () => {
- await setup({ card: createSummarizedCard() });
+ await setup({ query: createSummarizedQuery() });
await userEvent.click(screen.getByText("Max of Quantity"));
await userEvent.click(await screen.findByLabelText("Back"));
@@ -362,7 +414,9 @@ describe("SummarizeSidebar", () => {
});
it("should add a new column instead of replacing when adding a bucketed column", async () => {
- const { getNextBreakouts } = await setup({ card: createSummarizedCard() });
+ const { getNextBreakouts } = await setup({
+ query: createSummarizedQuery(),
+ });
const [total] = screen.getAllByLabelText("Total");
await userEvent.hover(total);
@@ -374,7 +428,9 @@ describe("SummarizeSidebar", () => {
});
it("should remove breakout", async () => {
- const { getNextBreakouts } = await setup({ card: createSummarizedCard() });
+ const { getNextBreakouts } = await setup({
+ query: createSummarizedQuery(),
+ });
const [breakout] = screen.getAllByLabelText("Created At");
await userEvent.click(within(breakout).getByLabelText("Remove dimension"));
@@ -384,7 +440,9 @@ describe("SummarizeSidebar", () => {
});
it("should replace breakouts by clicking on a column", async () => {
- const { getNextBreakouts } = await setup({ card: createSummarizedCard() });
+ const { getNextBreakouts } = await setup({
+ query: createSummarizedQuery(),
+ });
await userEvent.click(screen.getByText("Quantity"));
@@ -401,3 +459,11 @@ describe("SummarizeSidebar", () => {
expect(onClose).toHaveBeenCalled();
});
});
+
+function getPinnedColumnList() {
+ return screen.getByTestId("pinned-dimensions");
+}
+
+function getUnpinnedColumnList() {
+ return screen.getByTestId("unpinned-dimensions");
+}
diff --git a/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.tsx b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.tsx
index 938b68a68d2..45ab52e7a86 100644
--- a/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.tsx
+++ b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.tsx
@@ -11,6 +11,7 @@ interface TimeseriesBucketPickerProps {
query: Lib.Query;
stageIndex: number;
column: Lib.ColumnMetadata;
+ breakout: Lib.BreakoutClause;
onChange: (newColumn: Lib.ColumnMetadata) => void;
}
@@ -18,14 +19,15 @@ export function TimeseriesBucketPicker({
query,
stageIndex,
column,
+ breakout,
onChange,
}: TimeseriesBucketPickerProps) {
const [isOpened, setIsOpened] = useState(false);
- const columnBucketInfo = useMemo(() => {
- const bucket = Lib.temporalBucket(column);
+ const bucketInfo = useMemo(() => {
+ const bucket = Lib.temporalBucket(breakout);
return bucket ? Lib.displayInfo(query, stageIndex, bucket) : undefined;
- }, [query, stageIndex, column]);
+ }, [query, stageIndex, breakout]);
const handleChange = (newColumn: Lib.ColumnMetadata) => {
onChange(newColumn);
@@ -40,7 +42,7 @@ export function TimeseriesBucketPicker({
data-testid="timeseries-bucket-button"
onClick={() => setIsOpened(!isOpened)}
>
- {columnBucketInfo ? columnBucketInfo.displayName : t`Unbinned`}
+ {bucketInfo ? bucketInfo.displayName : t`Unbinned`}
</Button>
</Popover.Target>
<Popover.Dropdown>
@@ -48,7 +50,7 @@ export function TimeseriesBucketPicker({
query={query}
stageIndex={stageIndex}
column={column}
- columnBucketInfo={columnBucketInfo}
+ bucketInfo={bucketInfo}
onChange={handleChange}
/>
</Popover.Dropdown>
@@ -60,7 +62,7 @@ interface TimeseriesBucketDropdownProps {
query: Lib.Query;
stageIndex: number;
column: Lib.ColumnMetadata;
- columnBucketInfo: Lib.BucketDisplayInfo | undefined;
+ bucketInfo: Lib.BucketDisplayInfo | undefined;
onChange: (newColumn: Lib.ColumnMetadata) => void;
}
@@ -68,7 +70,7 @@ function TimeseriesBucketDropdown({
query,
stageIndex,
column,
- columnBucketInfo,
+ bucketInfo,
onChange,
}: TimeseriesBucketDropdownProps) {
const availableBuckets = Lib.availableTemporalBuckets(
@@ -100,7 +102,7 @@ function TimeseriesBucketDropdown({
return (
<TemporalUnitPicker
- value={columnBucketInfo?.shortName}
+ value={bucketInfo?.shortName}
availableItems={availableItems}
canRemove
onChange={handleChange}
diff --git a/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.unit.spec.tsx b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.unit.spec.tsx
index 5197d3f0bba..f7ea81f58d3 100644
--- a/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.unit.spec.tsx
+++ b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesBucketPicker/TimeseriesBucketPicker.unit.spec.tsx
@@ -22,6 +22,7 @@ function findMonthBucket(query: Lib.Query, column: Lib.ColumnMetadata) {
interface QueryWithBreakoutOpts {
query?: Lib.Query;
+ stageIndex?: number;
column?: Lib.ColumnMetadata;
bucket?: Lib.Bucket | null;
}
@@ -30,28 +31,36 @@ function createQueryWithBreakout({
query: initialQuery = createQuery(),
column = findBreakoutColumn(initialQuery),
bucket = findMonthBucket(initialQuery, column),
+ stageIndex = -1,
}: QueryWithBreakoutOpts = {}) {
const query = Lib.breakout(
initialQuery,
- 0,
+ stageIndex,
Lib.withTemporalBucket(column, bucket),
);
-
- return { query, column: findBreakoutColumn(query) };
+ const [breakout] = Lib.breakouts(query, stageIndex);
+ return {
+ query,
+ breakout,
+ column: Lib.breakoutColumn(query, stageIndex, breakout),
+ };
}
interface SetupOpts {
query: Lib.Query;
+ stageIndex?: number;
+ breakout: Lib.BreakoutClause;
column: Lib.ColumnMetadata;
}
-function setup({ query, column }: SetupOpts) {
+function setup({ query, breakout, column, stageIndex = -1 }: SetupOpts) {
const onChange = jest.fn();
renderWithProviders(
<TimeseriesBucketPicker
query={query}
- stageIndex={0}
+ stageIndex={stageIndex}
+ breakout={breakout}
column={column}
onChange={onChange}
/>,
@@ -68,8 +77,10 @@ function setup({ query, column }: SetupOpts) {
describe("TimeseriesBucketPicker", () => {
it("should allow to add a temporal bucket", async () => {
- const { query, column } = createQueryWithBreakout({ bucket: null });
- const { getNextBucketName } = setup({ query, column });
+ const { query, breakout, column } = createQueryWithBreakout({
+ bucket: null,
+ });
+ const { getNextBucketName } = setup({ query, breakout, column });
await userEvent.click(screen.getByText("Unbinned"));
await userEvent.click(await screen.findByText("Month"));
@@ -78,8 +89,8 @@ describe("TimeseriesBucketPicker", () => {
});
it("should allow to update a temporal bucket", async () => {
- const { query, column } = createQueryWithBreakout();
- const { getNextBucketName } = setup({ query, column });
+ const { query, breakout, column } = createQueryWithBreakout();
+ const { getNextBucketName } = setup({ query, breakout, column });
await userEvent.click(screen.getByText("Month"));
await userEvent.click(await screen.findByText("Year"));
@@ -88,8 +99,8 @@ describe("TimeseriesBucketPicker", () => {
});
it("should allow to show more binning options", async () => {
- const { query, column } = createQueryWithBreakout();
- const { getNextBucketName } = setup({ query, column });
+ const { query, breakout, column } = createQueryWithBreakout();
+ const { getNextBucketName } = setup({ query, breakout, column });
await userEvent.click(screen.getByText("Month"));
await userEvent.click(screen.getByText("More…"));
@@ -99,8 +110,8 @@ describe("TimeseriesBucketPicker", () => {
});
it("should allow to remove a temporal bucket", async () => {
- const { query, column } = createQueryWithBreakout();
- const { getNextBucketName } = setup({ query, column });
+ const { query, breakout, column } = createQueryWithBreakout();
+ const { getNextBucketName } = setup({ query, breakout, column });
await userEvent.click(screen.getByText("Month"));
await userEvent.click(screen.getByText("More…"));
@@ -110,17 +121,21 @@ describe("TimeseriesBucketPicker", () => {
});
it("should show all options when the current bucket is below the More button", async () => {
- const _query = createQuery();
- const _column = findBreakoutColumn(_query);
- const bucket = findTemporalBucket(_query, _column, "Quarter of year");
-
- const { query, column } = createQueryWithBreakout({
- query: _query,
- column: _column,
+ const initialQuery = createQuery();
+ const initialColumn = findBreakoutColumn(initialQuery);
+ const bucket = findTemporalBucket(
+ initialQuery,
+ initialColumn,
+ "Quarter of year",
+ );
+
+ const { query, breakout, column } = createQueryWithBreakout({
+ query: initialQuery,
+ column: initialColumn,
bucket,
});
- setup({ query, column });
+ setup({ query, breakout, column });
await userEvent.click(screen.getByText("Quarter of year"));
expect(await screen.findByText("Month of year")).toBeInTheDocument();
diff --git a/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.tsx b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.tsx
index 7b17d3d2d1f..f18b787e139 100644
--- a/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.tsx
+++ b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.tsx
@@ -9,7 +9,6 @@ import { TimeseriesBucketPicker } from "./TimeseriesBucketPicker";
import { TimeseriesFilterPicker } from "./TimeseriesFilterPicker";
import {
findBreakoutClause,
- findBreakoutColumn,
findFilterClause,
findFilterColumn,
} from "./utils";
@@ -55,15 +54,14 @@ function TimeseriesControls({
stageIndex,
onChange,
}: TimeseriesControlsProps) {
- const breakoutColumn = useMemo(
- () => findBreakoutColumn(query, stageIndex),
+ const breakout = useMemo(
+ () => findBreakoutClause(query, stageIndex),
[query, stageIndex],
);
- const breakout = useMemo(
- () =>
- breakoutColumn && findBreakoutClause(query, stageIndex, breakoutColumn),
- [query, stageIndex, breakoutColumn],
+ const breakoutColumn = useMemo(
+ () => breakout && Lib.breakoutColumn(query, stageIndex, breakout),
+ [query, stageIndex, breakout],
);
const isTemporalBucketable = useMemo(
@@ -99,7 +97,7 @@ function TimeseriesControls({
}
};
- if (!breakoutColumn || !filterColumn) {
+ if (!breakout || !breakoutColumn || !filterColumn) {
return null;
}
@@ -125,6 +123,7 @@ function TimeseriesControls({
query={query}
stageIndex={stageIndex}
column={breakoutColumn}
+ breakout={breakout}
onChange={handleBreakoutChange}
/>
</>
diff --git a/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.unit.spec.tsx b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.unit.spec.tsx
new file mode 100644
index 00000000000..a0f8fcc8fe1
--- /dev/null
+++ b/frontend/src/metabase/querying/components/TimeseriesChrome/TimeseriesChrome.unit.spec.tsx
@@ -0,0 +1,100 @@
+import { userEvent } from "@storybook/testing-library";
+
+import { renderWithProviders, screen } from "__support__/ui";
+import * as Lib from "metabase-lib";
+import {
+ createQuery,
+ createQueryWithClauses,
+ SAMPLE_METADATA,
+} from "metabase-lib/test-helpers";
+import Question from "metabase-lib/v1/Question";
+import { createMockCard } from "metabase-types/api/mocks";
+
+import { TimeseriesChrome } from "./TimeseriesChrome";
+
+interface SetupOpts {
+ query?: Lib.Query;
+}
+
+function setup({ query = createQuery() }: SetupOpts = {}) {
+ const question = new Question(createMockCard(), SAMPLE_METADATA).setQuery(
+ query,
+ );
+ const stageIndex = -1;
+ const updateQuestion = jest.fn();
+
+ const getNextBreakouts = () => {
+ const nextQuestion = updateQuestion.mock.calls[0][0];
+ const nextQuery = nextQuestion.query();
+ return Lib.breakouts(nextQuery, stageIndex).map(breakout =>
+ Lib.displayInfo(nextQuery, stageIndex, breakout),
+ );
+ };
+
+ renderWithProviders(
+ <TimeseriesChrome question={question} updateQuestion={updateQuestion} />,
+ );
+
+ return { getNextBreakouts };
+}
+
+describe("TimeseriesChrome", () => {
+ it("should render nothing if there are no breakouts", () => {
+ setup();
+ expect(screen.queryByText("View")).not.toBeInTheDocument();
+ });
+
+ it("should render nothing if there are no breakouts on a temporal column", () => {
+ const query = createQueryWithClauses({
+ breakouts: [{ tableName: "PRODUCTS", columnName: "CATEGORY" }],
+ });
+ setup({ query });
+ expect(screen.queryByText("View")).not.toBeInTheDocument();
+ });
+
+ it("should allow to change the temporal unit for a breakout", async () => {
+ const query = createQueryWithClauses({
+ breakouts: [
+ {
+ tableName: "PRODUCTS",
+ columnName: "CREATED_AT",
+ temporalBucketName: "Month",
+ },
+ ],
+ });
+ const { getNextBreakouts } = setup({ query });
+
+ await userEvent.click(screen.getByText("Month"));
+ await userEvent.click(await screen.findByText("Year"));
+
+ expect(getNextBreakouts()).toMatchObject([
+ { displayName: "Created At: Year" },
+ ]);
+ });
+
+ it("should allow to change the temporal unit for a breakout when there are multiple breakouts of the same column", async () => {
+ const query = createQueryWithClauses({
+ breakouts: [
+ {
+ tableName: "PRODUCTS",
+ columnName: "CREATED_AT",
+ temporalBucketName: "Month",
+ },
+ {
+ tableName: "PRODUCTS",
+ columnName: "CREATED_AT",
+ temporalBucketName: "Year",
+ },
+ ],
+ });
+ const { getNextBreakouts } = setup({ query });
+
+ await userEvent.click(screen.getByText("Month"));
+ await userEvent.click(await screen.findByText("Quarter"));
+
+ expect(getNextBreakouts()).toMatchObject([
+ { displayName: "Created At: Quarter" },
+ { displayName: "Created At: Year" },
+ ]);
+ });
+});
diff --git a/frontend/src/metabase/querying/components/TimeseriesChrome/utils.ts b/frontend/src/metabase/querying/components/TimeseriesChrome/utils.ts
index 0804e48a85e..1547bffad69 100644
--- a/frontend/src/metabase/querying/components/TimeseriesChrome/utils.ts
+++ b/frontend/src/metabase/querying/components/TimeseriesChrome/utils.ts
@@ -1,20 +1,5 @@
import * as Lib from "metabase-lib";
-export function findBreakoutColumn(
- query: Lib.Query,
- stageIndex: number,
-): Lib.ColumnMetadata | undefined {
- const columns = Lib.breakoutableColumns(query, stageIndex);
- return columns.find(column => {
- if (!Lib.isTemporal(column)) {
- return false;
- }
-
- const { breakoutPosition } = Lib.displayInfo(query, stageIndex, column);
- return breakoutPosition != null;
- });
-}
-
export function findFilterColumn(
query: Lib.Query,
stageIndex: number,
@@ -46,13 +31,10 @@ export function findFilterClause(
export function findBreakoutClause(
query: Lib.Query,
stageIndex: number,
- breakoutColumn: Lib.ColumnMetadata,
): Lib.BreakoutClause | undefined {
const breakouts = Lib.breakouts(query, stageIndex);
- const { breakoutPosition } = Lib.displayInfo(
- query,
- stageIndex,
- breakoutColumn,
- );
- return breakoutPosition != null ? breakouts[breakoutPosition] : undefined;
+ return breakouts.find(breakout => {
+ const column = Lib.breakoutColumn(query, stageIndex, breakout);
+ return Lib.isTemporal(column);
+ });
}
diff --git a/src/metabase/lib/breakout.cljc b/src/metabase/lib/breakout.cljc
index 539cf243918..13201c0426f 100644
--- a/src/metabase/lib/breakout.cljc
+++ b/src/metabase/lib/breakout.cljc
@@ -10,6 +10,7 @@
[metabase.lib.schema.expression :as lib.schema.expression]
[metabase.lib.schema.metadata :as lib.schema.metadata]
[metabase.lib.schema.ref :as lib.schema.ref]
+ [metabase.lib.schema.util :as lib.schema.util]
[metabase.lib.temporal-bucket :as lib.temporal-bucket]
[metabase.lib.util :as lib.util]
[metabase.shared.util.i18n :as i18n]
@@ -23,19 +24,6 @@
(for [breakout breakouts]
(lib.metadata.calculation/display-name query stage-number breakout :long))))))
-(mu/defn breakout :- ::lib.schema/query
- "Add a new breakout on an expression, presumably a Field reference."
- ([query expr]
- (breakout query -1 expr))
-
- ([query :- ::lib.schema/query
- stage-number :- :int
- expr :- some?]
- (let [expr (lib.ref/ref (if (fn? expr)
- (expr query stage-number)
- expr))]
- (lib.util/add-summary-clause query stage-number :breakout expr))))
-
(mu/defn breakouts :- [:maybe [:sequential ::lib.schema.expression/expression]]
"Return the current breakouts"
([query]
@@ -55,6 +43,18 @@
(-> (lib.metadata.calculation/metadata query stage-number field-ref)
(assoc :lib/source :source/breakouts)))))))
+(mu/defn breakout :- ::lib.schema/query
+ "Add a new breakout on an expression, presumably a Field reference. Ignores attempts to add a duplicate breakout."
+ ([query expr]
+ (breakout query -1 expr))
+ ([query :- ::lib.schema/query
+ stage-number :- :int
+ expr :- some?]
+ (let [expr (if (fn? expr) (expr query stage-number) expr)]
+ (if (lib.schema.util/distinct-refs? (map lib.ref/ref (cons expr (breakouts query stage-number))))
+ (lib.util/add-summary-clause query stage-number :breakout expr)
+ query))))
+
(mu/defn breakoutable-columns :- [:sequential ::lib.schema.metadata/column]
"Get column metadata for all the columns that can be broken out by in
the stage number `stage-number` of the query `query`
@@ -75,24 +75,23 @@
([query :- ::lib.schema/query
stage-number :- :int]
- (let [cols (let [stage (lib.util/query-stage query stage-number)
- options {:include-implicitly-joinable-for-source-card? false}]
- (lib.metadata.calculation/visible-columns query stage-number stage options))]
- (when (seq cols)
- (let [matching (into {} (keep-indexed (fn [index a-breakout]
- (when-let [col (lib.equality/find-matching-column
- query stage-number a-breakout cols
- {:generous? true})]
- [col [index a-breakout]]))
- (or (breakouts query stage-number) [])))]
- (mapv #(let [[pos a-breakout] (matching %)
- binning (lib.binning/binning a-breakout)
- {:keys [unit]} (lib.temporal-bucket/temporal-bucket a-breakout)]
- (cond-> (assoc % :lib/hide-bin-bucket? true)
- binning (lib.binning/with-binning binning)
- unit (lib.temporal-bucket/with-temporal-bucket unit)
- pos (assoc :breakout-position pos)))
- cols))))))
+ (let [columns (let [stage (lib.util/query-stage query stage-number)
+ options {:include-implicitly-joinable-for-source-card? false}]
+ (lib.metadata.calculation/visible-columns query stage-number stage options))]
+ (when (seq columns)
+ (let [existing-breakouts (breakouts query stage-number)
+ column->breakout-positions (group-by
+ (fn [position]
+ (lib.equality/find-matching-column query
+ stage-number
+ (get existing-breakouts position)
+ columns
+ {:generous? true}))
+ (range (count existing-breakouts)))]
+ (mapv #(let [positions (column->breakout-positions %)]
+ (cond-> (assoc % :lib/hide-bin-bucket? true)
+ positions (assoc :breakout-positions positions)))
+ columns))))))
(mu/defn existing-breakouts :- [:maybe [:sequential {:min 1} ::lib.schema.ref/ref]]
"Returns existing breakouts (as MBQL expressions) for `column` in a stage if there are any. Returns `nil` if there
@@ -138,11 +137,20 @@
(mu/defn breakout-column :- ::lib.schema.metadata/column
"Returns the input column used for this breakout."
- [query :- ::lib.schema/query
+ ([query :- ::lib.schema/query
+ breakout-ref :- ::lib.schema.ref/ref]
+ (breakout-column query -1 breakout-ref))
+ ([query :- ::lib.schema/query
stage-number :- :int
breakout-ref :- ::lib.schema.ref/ref]
- (->> (breakoutable-columns query stage-number)
- (lib.equality/find-matching-column breakout-ref)))
+ (when-let [column (lib.equality/find-matching-column breakout-ref
+ (breakoutable-columns query stage-number)
+ {:generous? true})]
+ (let [binning (lib.binning/binning breakout-ref)
+ bucket (lib.temporal-bucket/temporal-bucket breakout-ref)]
+ (cond-> column
+ binning (lib.binning/with-binning binning)
+ bucket (lib.temporal-bucket/with-temporal-bucket bucket))))))
(mu/defn remove-all-breakouts :- ::lib.schema/query
"Remove all breakouts from a query stage."
diff --git a/src/metabase/lib/metadata/calculation.cljc b/src/metabase/lib/metadata/calculation.cljc
index be72f4942c4..523e2315667 100644
--- a/src/metabase/lib/metadata/calculation.cljc
+++ b/src/metabase/lib/metadata/calculation.cljc
@@ -387,7 +387,7 @@
{:is-temporal-extraction
(and (contains? lib.schema.temporal-bucketing/datetime-extraction-units temporal-unit)
(not (contains? lib.schema.temporal-bucketing/datetime-truncation-units temporal-unit)))})
- (select-keys x-metadata [:breakout-position :order-by-position :filter-positions]))))
+ (select-keys x-metadata [:breakout-positions :order-by-position :filter-positions]))))
(defmethod display-info-method :default
[query stage-number x]
diff --git a/src/metabase/lib/remove_replace.cljc b/src/metabase/lib/remove_replace.cljc
index f4085a3732a..4517f632fb9 100644
--- a/src/metabase/lib/remove_replace.cljc
+++ b/src/metabase/lib/remove_replace.cljc
@@ -14,6 +14,7 @@
[metabase.lib.options :as lib.options]
[metabase.lib.ref :as lib.ref]
[metabase.lib.schema :as lib.schema]
+ [metabase.lib.schema.util :as lib.schema.util]
[metabase.lib.util :as lib.util]
[metabase.lib.util.match :as lib.util.match]
[metabase.util :as u]
@@ -186,23 +187,27 @@
(first replacement-clause))
(= (last target-clause)
(last replacement-clause))))
- query (cond
- sync-breakout-ordering?
- (sync-order-by-options-with-breakout
- query
- stage-number
- target-clause
- (select-keys (second replacement-clause) [:binning :temporal-unit]))
-
- changing-breakout?
- (remove-breakout-order-by query stage-number target-clause)
-
- :else
- query)]
- (if location
- (-> query
- (remove-replace-location stage-number query location target-clause remove-replace-fn)
- (normalize-fields-clauses location))
+ new-query (cond
+ sync-breakout-ordering?
+ (sync-order-by-options-with-breakout
+ query
+ stage-number
+ target-clause
+ (select-keys (second replacement-clause) [:binning :temporal-unit]))
+
+ changing-breakout?
+ (remove-breakout-order-by query stage-number target-clause)
+
+ :else
+ query)
+ new-query (if location
+ (-> new-query
+ (remove-replace-location stage-number new-query location target-clause remove-replace-fn)
+ (normalize-fields-clauses location))
+ new-query)
+ new-stage (lib.util/query-stage new-query stage-number)]
+ (if (or (not changing-breakout?) (lib.schema.util/distinct-refs? (:breakout new-stage)))
+ new-query
query))))
(mu/defn remove-clause :- :metabase.lib.schema/query
diff --git a/test/metabase/lib/binning_test.cljc b/test/metabase/lib/binning_test.cljc
index a9fabca5794..5c9da6b0874 100644
--- a/test/metabase/lib/binning_test.cljc
+++ b/test/metabase/lib/binning_test.cljc
@@ -31,7 +31,7 @@
(is (= {:bin-width 12.5, :min-value 15, :max-value 27.5}
(lib.binning/resolve-bin-width query col-quantity 15)))))
-(deftest ^:parallel binning-and-bucketing-only-show-up-for-returned-and-breakoutable-columns
+(deftest ^:parallel binning-and-bucketing-only-show-up-for-returned-and-breakout-columns
(testing "Within the stage, binning and bucketing at breakout should be invisible, outside the stage it should be visible"
(let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :orders))
(lib/aggregate (lib/count)))
@@ -45,8 +45,11 @@
(complement :metabase.lib.field/binning)
(m/find-first (comp #{"TOTAL"} :name) (lib/visible-columns query))))
(is (=?
- {:metabase.lib.field/binning {:strategy :default}}
+ (complement :metabase.lib.field/binning)
(m/find-first (comp #{"TOTAL"} :name) (lib/breakoutable-columns query))))
+ (is (=?
+ {:metabase.lib.field/binning {:strategy :default}}
+ (lib/breakout-column query -1 (first (lib/breakouts query)))))
(is (=?
{:metabase.lib.field/binning {:strategy :default}}
(m/find-first (comp #{"TOTAL"} :name) (lib/returned-columns query))))
@@ -54,8 +57,11 @@
(complement :metabase.lib.field/temporal-unit)
(m/find-first (comp #{"CREATED_AT"} :name) (lib/visible-columns query))))
(is (=?
- {:metabase.lib.field/temporal-unit :minute}
+ (complement :metabase.lib.field/temporal-unit)
(m/find-first (comp #{"CREATED_AT"} :name) (lib/breakoutable-columns query))))
+ (is (=?
+ {:metabase.lib.field/temporal-unit :minute}
+ (lib/breakout-column query -1 (second (lib/breakouts query)))))
(is (=?
{:metabase.lib.field/temporal-unit :minute}
(m/find-first (comp #{"CREATED_AT"} :name) (lib/returned-columns query)))))))
diff --git a/test/metabase/lib/breakout_test.cljc b/test/metabase/lib/breakout_test.cljc
index 2b0fbc9d305..52138838844 100644
--- a/test/metabase/lib/breakout_test.cljc
+++ b/test/metabase/lib/breakout_test.cljc
@@ -6,6 +6,7 @@
[metabase.lib.breakout :as lib.breakout]
[metabase.lib.card :as lib.card]
[metabase.lib.core :as lib]
+ [metabase.lib.query :as lib.query]
[metabase.lib.test-metadata :as meta]
[metabase.lib.test-util :as lib.tu]
[metabase.lib.test-util.mocks-31368 :as lib.tu.mocks-31368]
@@ -169,13 +170,74 @@
(is (=? [{:display-name "ID"}
{:display-name "Name"}
{:display-name "Category ID"}
- {:display-name "Latitude", :breakout-position 1}
+ {:display-name "Latitude", :breakout-positions [1]}
{:display-name "Longitude"}
- {:display-name "Price", :breakout-position 0}
+ {:display-name "Price", :breakout-positions [0]}
{:display-name "ID"}
{:display-name "Name"}]
(lib/breakoutable-columns query)))))))
+(deftest ^:parallel multiple-breakouts-for-the-same-column-test
+ (testing "multiple breakout positions for the same column"
+ (let [base-query lib.tu/venues-query
+ column (meta/field-metadata :venues :price)
+ binning-strategies (lib/available-binning-strategies base-query column)
+ query (-> base-query
+ (lib/breakout (lib/with-binning column (first binning-strategies)))
+ (lib/breakout (lib/with-binning column (second binning-strategies))))]
+ (is (=? [{:display-name "ID"}
+ {:display-name "Name"}
+ {:display-name "Category ID"}
+ {:display-name "Latitude"}
+ {:display-name "Longitude"}
+ {:display-name "Price", :breakout-positions [0 1]}
+ {:display-name "ID"}
+ {:display-name "Name"}]
+ (lib/breakoutable-columns query)))))
+ (testing "should ignore duplicate breakouts without binning strategy or temporal bucket"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (first (lib/breakoutable-columns base-query))
+ query (-> base-query
+ (lib/breakout column)
+ (lib/breakout column))
+ breakouts (lib/breakouts query)]
+ (is (= 1 (count breakouts)))))
+ (testing "should ignore duplicate breakouts with the same binning strategy"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (meta/field-metadata :people :latitude)
+ binning-strategy (first (lib/available-binning-strategies base-query column))
+ query (-> base-query
+ (lib/breakout (lib/with-binning column binning-strategy))
+ (lib/breakout (lib/with-binning column binning-strategy)))
+ breakouts (lib/breakouts query)]
+ (is (= 1 (count breakouts)))))
+ (testing "should ignore duplicate breakouts with the same temporal bucket"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (meta/field-metadata :people :birth-date)
+ query (-> base-query
+ (lib/breakout (lib/with-temporal-bucket column :month))
+ (lib/breakout (lib/with-temporal-bucket column :month)))
+ breakouts (lib/breakouts query)]
+ (is (= 1 (count breakouts)))))
+ (testing "should ignore duplicate breakouts with the same temporal bucket when converting from legacy MBQL"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (meta/field-metadata :people :birth-date)
+ query (lib/breakout (->> (lib/breakout base-query (lib/with-temporal-bucket column :year))
+ (lib.query/->legacy-MBQL)
+ (lib/query meta/metadata-provider))
+ (lib/with-temporal-bucket column :year))
+ breakouts (lib/breakouts query)]
+ (is (= 1 (count breakouts)))))
+ (testing "should allow multiple breakouts with different temporal buckets when converting from legacy MBQL"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (meta/field-metadata :people :birth-date)
+ query (lib/breakout (->> (lib/breakout base-query (lib/with-temporal-bucket column :year))
+ (lib.query/->legacy-MBQL)
+ (lib/query meta/metadata-provider))
+ (lib/with-temporal-bucket column :month))
+ breakouts (lib/breakouts query)]
+ (is (= 2 (count breakouts))))))
+
(deftest ^:parallel breakoutable-explicit-joins-test
(testing "breakoutable-columns should include columns from explicit joins"
(let [query (-> lib.tu/venues-query
@@ -335,11 +397,11 @@
{:display-name "Category ID", :lib/source :source/table-defaults}
{:display-name "Latitude", :lib/source :source/table-defaults}
{:display-name "Longitude", :lib/source :source/table-defaults}
- {:display-name "Price" :lib/source :source/table-defaults, :breakout-position 1}
+ {:display-name "Price" :lib/source :source/table-defaults, :breakout-positions [1]}
{:display-name "ID", :lib/source :source/implicitly-joinable}
- {:display-name "Name", :lib/source :source/implicitly-joinable, :breakout-position 0}]
+ {:display-name "Name", :lib/source :source/implicitly-joinable, :breakout-positions [0]}]
breakoutables'))
- (is (= 2 (count (filter :breakout-position breakoutables'))))
+ (is (= 2 (count (filter :breakout-positions breakoutables'))))
(is (=? [{:table {:name "VENUES", :display-name "Venues", :is-source-table true}
:semantic-type :type/PK
:name "ID"
@@ -394,7 +456,7 @@
:is-from-previous-stage false
:is-calculated false
:is-implicitly-joinable false
- :breakout-position 1}
+ :breakout-positions [1]}
{:table {:name "CATEGORIES", :display-name "Categories", :is-source-table false}
:semantic-type :type/PK
:name "ID"
@@ -413,7 +475,7 @@
:is-from-previous-stage false
:is-calculated false
:is-implicitly-joinable true
- :breakout-position 0}]
+ :breakout-positions [0]}]
(map #(lib/display-info query' %) breakoutables'))))))
(deftest ^:parallel breakoutable-columns-with-source-card-e2e-test
@@ -530,8 +592,8 @@
(lib/breakout field-ref))]
(is (= [field-ref]
(lib/breakouts query)))
- (is (=? {:name "NAME"
- :breakout-position 0}
+ (is (=? {:name "NAME"
+ :breakout-positions [0]}
(m/find-first #(= (:id %) (meta/id :categories :name))
(lib/breakoutable-columns query)))))))))
@@ -558,8 +620,8 @@
(deftest ^:parallel legacy-query-with-broken-breakout-breakoutable-columns-test
(testing "Handle busted references to joined Fields in broken breakouts from broken drill-thrus (#31482)"
- (is (=? {:display-name "Products → Category"
- :breakout-position 0}
+ (is (=? {:display-name "Products → Category"
+ :breakout-positions [0]}
(m/find-first #(= (:id %) (meta/id :products :category))
(lib/breakoutable-columns (legacy-query-with-broken-breakout)))))))
@@ -605,15 +667,35 @@
(lib.breakout/remove-existing-breakouts-for-column query' (meta/field-metadata :people :latitude)))))))
(deftest ^:parallel breakout-column-test
- (let [query (-> lib.tu/venues-query
- (lib/breakout (meta/field-metadata :venues :category-id))
- (lib/breakout (meta/field-metadata :venues :price))
- (lib/aggregate (lib/count)))
- category (m/find-first #(= (:name %) "CATEGORY_ID") (lib/visible-columns query))
- price (m/find-first #(= (:name %) "PRICE") (lib/visible-columns query))
- breakouts (lib/breakouts query)]
- (is (= (count breakouts) 2))
- (is (=? category
- (lib.breakout/breakout-column query -1 (first breakouts))))
- (is (=? price
- (lib.breakout/breakout-column query -1 (second breakouts))))))
+ (testing "should find the correct column"
+ (let [query (-> lib.tu/venues-query
+ (lib/breakout (meta/field-metadata :venues :category-id))
+ (lib/breakout (meta/field-metadata :venues :price))
+ (lib/aggregate (lib/count)))
+ category (meta/field-metadata :venues :category-id)
+ price (meta/field-metadata :venues :price)
+ breakouts (lib/breakouts query)]
+ (is (= (count breakouts) 2))
+ (is (=? category
+ (lib.breakout/breakout-column query (first breakouts))))
+ (is (=? price
+ (lib.breakout/breakout-column query (second breakouts))))))
+ (testing "should set the binning strategy from the breakout clause"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (meta/field-metadata :people :latitude)
+ binning-strategy (first (lib/available-binning-strategies base-query column))
+ query (->> (lib/with-binning column binning-strategy)
+ (lib/breakout base-query))
+ breakout (first (lib/breakouts query))]
+ (is (=? {:strategy :default}
+ (->> (lib/breakout-column query breakout)
+ (lib/binning))))))
+ (testing "should set the temporal unit from the breakout clause"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (meta/field-metadata :people :birth-date)
+ query (->> (lib/with-temporal-bucket column :month)
+ (lib/breakout base-query))
+ breakout (first (lib/breakouts query))]
+ (is (=? {:unit :month}
+ (->> (lib/breakout-column query breakout)
+ (lib/temporal-bucket)))))))
diff --git a/test/metabase/lib/equality_test.cljc b/test/metabase/lib/equality_test.cljc
index c5c94d1af53..4bc051bf47f 100644
--- a/test/metabase/lib/equality_test.cljc
+++ b/test/metabase/lib/equality_test.cljc
@@ -291,7 +291,7 @@
(let [query (-> lib.tu/venues-query
(lib/breakout (meta/field-metadata :venues :id)))
filterable-cols (lib/filterable-columns query)
- matched-from-col (lib.equality/find-matching-column query -1 (m/find-first :breakout-position (lib/breakoutable-columns query)) filterable-cols)
+ matched-from-col (lib.equality/find-matching-column query -1 (m/find-first :breakout-positions (lib/breakoutable-columns query)) filterable-cols)
matched-from-ref (lib.equality/find-matching-column query -1 (first (lib/breakouts query)) filterable-cols)]
(is (=?
{:id (meta/id :venues :id)}
diff --git a/test/metabase/lib/remove_replace_test.cljc b/test/metabase/lib/remove_replace_test.cljc
index 496244637bc..9dd7ea2668a 100644
--- a/test/metabase/lib/remove_replace_test.cljc
+++ b/test/metabase/lib/remove_replace_test.cljc
@@ -7,6 +7,7 @@
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.options :as lib.options]
+ [metabase.lib.query :as lib.query]
[metabase.lib.remove-replace :as lib.remove-replace]
[metabase.lib.test-metadata :as meta]
[metabase.lib.test-util :as lib.tu]
@@ -386,7 +387,26 @@
(lib/append-stage)
(lib/filter (lib/= [:field {:lib/uuid (str (random-uuid)) :base-type :type/Integer} "ID"] 1))
(lib/replace-clause 0 (second breakouts) (meta/field-metadata :venues :price))
- (lib/breakouts 0)))))))
+ (lib/breakouts 0)))))
+ (testing "should ignore duplicate breakouts"
+ (let [id-column (meta/field-metadata :venues :id)
+ price-column (meta/field-metadata :venues :price)
+ query (-> lib.tu/venues-query
+ (lib/breakout id-column)
+ (lib/breakout price-column))
+ breakouts (lib/breakouts query)]
+ (is (= query (lib/replace-clause query (first breakouts) price-column)))))
+ (testing "should ignore duplicate breakouts with the same temporal bucket when converting from legacy MBQL"
+ (let [base-query (lib/query meta/metadata-provider (meta/table-metadata :people))
+ column (meta/field-metadata :people :birth-date)
+ query (-> base-query
+ (lib/breakout (lib/with-temporal-bucket column :year))
+ (lib/breakout (lib/with-temporal-bucket column :month)))
+ query (->> query
+ (lib.query/->legacy-MBQL)
+ (lib/query meta/metadata-provider))]
+ (is (= query (lib/replace-clause query (first breakouts)
+ (lib/with-temporal-bucket column :month))))))))
(deftest ^:parallel replace-clause-fields-test
(let [query (-> lib.tu/venues-query
--
GitLab