Kimirock/metabase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Viz 1323 visualizer cant combine single series chart and breakout (#61216)

Browse Source 
* Closes VIZ-1322 VIZ-1323 looser compatibility

* fixup

* fixup

* fixed tests

* removed commented out blocks

* cleanup

* fixed unit test

---------

Co-authored-by: Tyler Plude <tsplude@gmail.com>
This commit is contained in:
Sébastien
2025-07-25 17:33:09 +02:00
committed by GitHub
parent dc973f0cb8
commit 832e2b7ac5
12 changed files with 189 additions and 347 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
bin/generate-country-code-docs.bb
View File

@@ -30,7 +30,7 @@
(if-not (.exists (io/file file-path))
(throw (ex-info (str "GeoJSON file not found: " file-path)
{:type :file-not-found :path file-path})))
(with-open [reader (io/reader file-path)]
(json/parse-stream reader true))
(catch Exception e
@@ -43,11 +43,11 @@
(when-not (map? geojson)
(throw (ex-info "Invalid GeoJSON structure: expected map"
{:type :invalid-structure})))
(when-not (sequential? (:features geojson))
(throw (ex-info "Invalid GeoJSON structure: missing or invalid features"
{:type :invalid-features})))
(let [countries (->> (:features geojson)
(map :properties)
(map (fn [{:keys [NAME ISO_A2]}]
@@ -55,14 +55,14 @@
(filter validate-country)
(sort-by :code))
invalid-count (- (count (:features geojson)) (count countries))]
(when (pos? invalid-count)
(println (format "Skipped %d entries with invalid or missing data" invalid-count)))
(when (empty? countries)
(throw (ex-info "No valid countries found in GeoJSON"
{:type :no-valid-countries})))
countries))
(defn- format-markdown-table
@@ -135,9 +135,9 @@
countries (extract-countries geojson)
countries-table (format-markdown-table countries table-headers)
markdown-content (generate-markdown-content template-path
countries-table
(count countries)
markdown-title)]
countries-table
(count countries)
markdown-title)]
(write-markdown-file markdown-content output-path)
(println (format "Generated country codes reference with %d countries" (count countries)))
(format-with-prettier output-path)
@@ -172,4 +172,4 @@
;; Auto-run when executed directly
(when (= *file* (System/getProperty "babashka.file"))
(-main))
(-main))

12
e2e/test/scenarios/dashboard/visualizer/cartesian.cy.spec.ts
View File

@@ -504,10 +504,12 @@ describe("scenarios > dashboard > visualizer > cartesian", () => {
H.chartLegend().should("not.exist");
// Remove all "category" columns from the well
H.horizontalWell()
.findByTestId("well-item")
.findByLabelText("Remove")
.click();
H.horizontalWell().within(() => {
cy.findAllByTestId("well-item")
.first()
.findByLabelText("Remove")
.click();
});
H.assertDataSourceColumnSelected(Q1_NAME, "Product → Category", false);
H.assertDataSourceColumnSelected(Q2_NAME, "Category", false);
H.chartLegend().should("not.exist");
@@ -520,7 +522,7 @@ describe("scenarios > dashboard > visualizer > cartesian", () => {
H.verticalWell().findAllByTestId("well-item").should("have.length", 2);
H.horizontalWell()
.findAllByTestId("well-item")
.should("have.length", 1);
.should("have.length", 2);
H.chartLegendItems().should("have.length", 2);
// Remove 2nd data source

59
frontend/src/metabase/visualizer/components/DataImporter/DatasetsList/DatasetsList.tsx
View File

@@ -33,7 +33,7 @@ import type {
VisualizerDataSourceId,
} from "metabase-types/api";
import { DatasetsListItem } from "./DatasetsListItem";
import { DatasetsListItem, type Item } from "./DatasetsListItem";
import { getIsCompatible } from "./getIsCompatible";
function shouldIncludeDashboardQuestion(
@@ -139,7 +139,8 @@ export function DatasetsList({
include_dashboard_questions: true,
include_metadata: true,
...(visualizationType &&
isCartesianChart(visualizationType) && {
isCartesianChart(visualizationType) &&
search.length === 0 && {
has_temporal_dim: timeDimensions.length > 0,
non_temporal_dim_ids: JSON.stringify(nonTemporalDimIds),
}),
@@ -172,7 +173,7 @@ export function DatasetsList({
[dataSources, handleAddDataSource, handleRemoveDataSource],
);
const items = useMemo(() => {
const items: Item[] | undefined = useMemo(() => {
if (!search && dataSources.length === 0) {
if (!allRecents) {
return;
@@ -186,6 +187,7 @@ export function DatasetsList({
...createDataSource("card", card.id, card.name),
display: card.display,
result_metadata: card.result_metadata,
notRecommended: false,
}));
}
@@ -193,7 +195,26 @@ export function DatasetsList({
return;
}
return visualizationSearchResult.data
const isCompatible = (item: Item) => {
if (!item.display || !item.result_metadata) {
return item;
}
return getIsCompatible({
currentDataset: {
display: visualizationType ?? null,
columns,
settings,
computedSettings,
},
targetDataset: {
fields: item.result_metadata,
},
datasets,
});
};
let results = visualizationSearchResult.data
.filter(
(maybeCard) =>
["card", "dataset", "metric"].includes(maybeCard.model) &&
@@ -204,26 +225,18 @@ export function DatasetsList({
display: card.display,
result_metadata: card.result_metadata,
}))
.filter((item) => {
// Filter out incompatible items using client-side compatibility check
if (!item.display || !item.result_metadata) {
return false;
}
return getIsCompatible({
currentDataset: {
display: visualizationType ?? null,
columns,
settings,
computedSettings,
},
targetDataset: {
fields: item.result_metadata,
},
datasets,
});
})
.sort((a, b) => a.name.localeCompare(b.name));
if (search.length > 0) {
results = results.map((item) => ({
...item,
notRecommended: !isCompatible(item),
}));
} else {
results = results.filter(isCompatible);
}
return results;
}, [
visualizationSearchResult,
dashboardId,

6
frontend/src/metabase/visualizer/components/DataImporter/DatasetsList/DatasetsListItem.module.css
View File

@@ -9,6 +9,12 @@
display: none;
}
.WarningIcon {
position: absolute;
top: 13px;
left: 19px;
}
.RemoveButton {
display: none;
}

31
frontend/src/metabase/visualizer/components/DataImporter/DatasetsList/DatasetsListItem.tsx
View File

@@ -1,5 +1,7 @@
import { t } from "ttag";
import { Ellipsified } from "metabase/common/components/Ellipsified";
import { Button, Icon } from "metabase/ui";
import { Box, Button, Icon, Tooltip } from "metabase/ui";
import type {
Field,
VisualizationDisplay,
@@ -8,11 +10,14 @@ import type {
import S from "./DatasetsListItem.module.css";
export type Item = VisualizerDataSource & {
notRecommended?: boolean;
display: VisualizationDisplay | null;
result_metadata?: Field[];
};
interface DatasetsListItemProps {
item: VisualizerDataSource & {
display: VisualizationDisplay | null;
result_metadata?: Field[];
};
item: Item;
onSwap?: (item: VisualizerDataSource) => void;
onToggle?: (item: VisualizerDataSource) => void;
onRemove?: (item: VisualizerDataSource) => void;
@@ -33,7 +38,21 @@ export const DatasetsListItem = (props: DatasetsListItemProps) => {
selected ? onRemove?.(item) : onToggle?.(item);
}}
leftSection={
<Icon color="inherit" className={S.TableIcon} name="table2" mr="xs" />
<Box>
<Icon color="inherit" className={S.TableIcon} name="table2" mr="xs" />
{item.notRecommended && (
<Tooltip
label={t`This dataset might not be fully compatible with your current selection.`}
>
<Icon
className={S.WarningIcon}
color="var(--mb-color-danger)"
name="warning_round_filled"
size={10}
/>
</Tooltip>
)}
</Box>
}
style={{ flex: 0, minHeight: 30, paddingLeft: 8, paddingRight: 8 }}
>

31
frontend/src/metabase/visualizer/components/DataImporter/DatasetsList/getIsCompatible.unit.spec.ts
View File

@@ -210,7 +210,6 @@ describe("getIsCompatible", () => {
const sameCategoryDimensionField = createMockCategoryField({
id: categoryDimensionColumn.id,
});
const otherCategoryDimensionField = createMockCategoryField({ id: 5 });
it("should return true if a data source has a matching time dimension", () => {
const settings = {
@@ -304,36 +303,6 @@ describe("getIsCompatible", () => {
}),
).toBe(true);
});
it("should return false if a data source doesn't have a matching category dimension", () => {
const settings = {
"graph.metrics": [metricColumn.name],
"graph.dimensions": [categoryDimensionColumn.name],
};
expect(
getIsCompatible({
currentDataset: {
display: "line",
columns: [metricColumn, categoryDimensionColumn],
settings,
computedSettings: settings,
},
targetDataset: { fields: [otherCategoryDimensionField] },
datasets: {
"1": defaultDataset,
"2": createMockDataset({
data: {
cols: [
createMockCategoryColumn({
id: otherCategoryDimensionField.id as number,
}),
],
},
}),
},
}),
).toBe(false);
});
});
});

2
frontend/src/metabase/visualizer/components/VisualizationCanvas/wells/HorizontalWell/CartesianHorizontallWell.tsx
View File

@@ -57,7 +57,7 @@ export function CartesianHorizontalWell({ style, ...props }: FlexProps) {
dimensions.push(timeDimensions[0]);
}
if (categoryDimensions.length > 0) {
dimensions.push(categoryDimensions[0]);
dimensions.push(...categoryDimensions); //.push(categoryDimensions[0]);
}
return dimensions;

80
frontend/src/metabase/visualizer/visualizations/cartesian.ts
View File

@@ -220,22 +220,21 @@ export function addDimensionColumnToCartesianChart(
*
* @param state the current state of the visualizer
* @param settings the computed visualization settings
* @param datasets the selected datasets
* @param datasets the selected datasets (not used )
* @param dataSourceColumns the current data source columns (not used )
* @param column the column to find a slot for
* @returns a string representing the slot where the column should be added,
* or undefined if no suitable slot is found.
*/
export function findColumnSlotForCartesianChart(
export function findColumnSlotForCartesianChart(parameters: {
state: Pick<
VisualizerVizDefinitionWithColumns,
"display" | "columns" | "settings"
>,
settings: ComputedVisualizationSettings,
datasets: Record<VisualizerDataSourceId, Dataset>,
dataSourceColumns: DatasetColumn[],
column: DatasetColumn,
) {
>;
settings: ComputedVisualizationSettings;
column: DatasetColumn;
}) {
const { state, settings, column } = parameters;
if (state.display === "scatter") {
const metrics = settings["graph.metrics"]?.filter(isNotNull) ?? [];
const dimensions = settings["graph.dimensions"]?.filter(isNotNull) ?? [];
@@ -252,53 +251,8 @@ export function findColumnSlotForCartesianChart(
return "scatter.bubble";
}
} else {
const metrics = settings["graph.metrics"] ?? [];
if (isDimension(column) && !isMetric(column)) {
// Filtering out nulls as computed 'graph.dimensions' can be `[null]` sometimes
const ownDimensions =
state.settings["graph.dimensions"] ??
settings["graph.dimensions"]?.filter(Boolean) ??
[];
if (ownDimensions.length === 0) {
return "graph.dimensions";
} else {
const isCompatibleWithUsedColumns = state.columns.some((col) => {
if (isDate(col)) {
return isDate(column);
} else {
return col.id === column.id;
}
});
if (isCompatibleWithUsedColumns) {
return "graph.dimensions";
}
// No breakout when there are more than one metric
if (ownDimensions.length > 0 && metrics.length > 1) {
if (!isDate(column)) {
return undefined;
}
}
// Handles potential new dimensions that are not yet used in a chart
// For example, a chart could show several metrics over time (from different data sources)
// And each data source can have a "User → Source" column. This check ensure that
// dimensions are considered mappable in this case if they're present in every data source.
const isCompatibleWithUnusedColumns = Object.values(datasets).every(
(dataset) =>
dataset.data.cols.some((col) => {
if (isDate(col)) {
return isDate(column);
} else {
return col.id === column.id;
}
}),
);
return isCompatibleWithUnusedColumns ? "graph.dimensions" : undefined;
}
return "graph.dimensions";
} else if (isMetric(column)) {
return "graph.metrics";
}
@@ -321,13 +275,11 @@ export function addColumnToCartesianChart(
columnRef: VisualizerColumnReference,
dataSource: VisualizerDataSource,
) {
const slot = findColumnSlotForCartesianChart(
const slot = findColumnSlotForCartesianChart({
state,
settings,
datasets,
dataSourceColumns,
column,
);
});
if (slot === "graph.dimensions") {
addDimensionColumnToCartesianChart(
state,
@@ -515,13 +467,11 @@ export function combineWithCartesianChart(
);
metrics.forEach((column) => {
const isCompatible = !!findColumnSlotForCartesianChart(
const isCompatible = !!findColumnSlotForCartesianChart({
state,
settings,
datasets,
dataset.data.cols,
column,
);
});
if (isCompatible) {
const columnRef = createVisualizerColumnReference(
dataSource,
@@ -539,13 +489,11 @@ export function combineWithCartesianChart(
});
dimensions.forEach((column) => {
const isCompatible = !!findColumnSlotForCartesianChart(
const isCompatible = !!findColumnSlotForCartesianChart({
state,
settings,
datasets,
dataset.data.cols,
column,
);
});
if (isCompatible) {
const columnRef = createVisualizerColumnReference(
dataSource,

227
frontend/src/metabase/visualizer/visualizations/cartesian.unit.spec.ts
View File

@@ -44,12 +44,6 @@ describe("cartesian", () => {
display_name: "Category",
});
const defaultDataset = createMockDataset({
data: {
cols: [metricColumn, timeDimensionColumn, categoryDimensionColumn],
},
});
const dateColumn = createMockDatetimeColumn({ id: 4 });
const sameCategoryDimensionColumn = createMockCategoryColumn({
id: categoryDimensionColumn.id,
@@ -62,22 +56,14 @@ describe("cartesian", () => {
display_name: "Other Metric",
});
const andAnotherMetricColumn = createMockNumericColumn({
id: 7,
name: "COLUMN_7",
display_name: "And Another Metric",
});
it("should return 'graph.metrics' for a metric column", () => {
const state = { display: "bar" as const, columns: [], settings: {} };
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
{},
{ "card:1": defaultDataset },
defaultDataset.data.cols,
metricColumn,
),
settings: {},
column: metricColumn,
}),
).toEqual("graph.metrics");
});
@@ -91,22 +77,18 @@ describe("cartesian", () => {
};
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset },
defaultDataset.data.cols,
timeDimensionColumn,
),
column: timeDimensionColumn,
}),
).toEqual("graph.dimensions");
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset },
defaultDataset.data.cols,
categoryDimensionColumn,
),
column: categoryDimensionColumn,
}),
).toEqual("graph.dimensions");
});
@@ -120,22 +102,17 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [dateColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset, "card:2": newDataset },
newDataset.data.cols,
dateColumn,
),
column: dateColumn,
}),
).toBe("graph.dimensions");
});
it("should return undefined for a non-date column when chart only has a time dimension", () => {
it("should return graph.dimensions for a non-date column even if a chart only has a time dimension", () => {
const settings = {
"graph.metrics": [metricColumn.name],
"graph.dimensions": [timeDimensionColumn.name],
@@ -145,22 +122,14 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const baseDataset = createMockDataset({
data: { cols: [metricColumn, timeDimensionColumn] },
});
const newDataset = createMockDataset({
data: { cols: [categoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": baseDataset, "card:2": newDataset },
newDataset.data.cols,
categoryDimensionColumn,
),
).toBeUndefined();
column: categoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
it("should return 'graph.dimensions' for a date column when chart has a time dimension and a category dimension", () => {
@@ -176,21 +145,13 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [dateColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{
"card:1": defaultDataset,
"card:2": newDataset,
},
newDataset.data.cols,
dateColumn,
),
column: dateColumn,
}),
).toBe("graph.dimensions");
});
@@ -204,22 +165,17 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [sameCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset, "card:2": newDataset },
newDataset.data.cols,
sameCategoryDimensionColumn,
),
column: sameCategoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
it("should return undefined for a category dimension when chart doesn't have the same dimension", () => {
it("should return graph.dimensions for a category dimension even if a chart doesn't have the same dimension", () => {
const settings = {
"graph.metrics": [metricColumn.name],
"graph.dimensions": [categoryDimensionColumn.name],
@@ -229,19 +185,14 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [otherCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset, "card:2": newDataset },
newDataset.data.cols,
otherCategoryDimensionColumn,
),
).toBeUndefined();
column: otherCategoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
it("should return 'graph.dimensions' for a category dimension when chart has a time dimension and the same category dimension", () => {
@@ -257,18 +208,13 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [sameCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset, "card:2": newDataset },
newDataset.data.cols,
sameCategoryDimensionColumn,
),
column: sameCategoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
@@ -285,19 +231,14 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [otherCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset, "card:2": newDataset },
newDataset.data.cols,
otherCategoryDimensionColumn,
),
).toBeUndefined();
column: otherCategoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
it("should return 'graph.dimensions' for a new time dimension when every data source has a time dimension", () => {
@@ -310,22 +251,17 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [dateColumn, sameCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset, "card:2": newDataset },
newDataset.data.cols,
dateColumn,
),
column: dateColumn,
}),
).toBe("graph.dimensions");
});
it("should return undefined for a new time dimension when not every data source has a time dimension", () => {
it("should return graph.dimensions for a new time dimension even if not every data source has a time dimension", () => {
const settings = {
"graph.metrics": [metricColumn.name],
"graph.dimensions": [categoryDimensionColumn.name],
@@ -335,22 +271,14 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const baseDataset = createMockDataset({
data: { cols: [metricColumn, categoryDimensionColumn] },
});
const newDataset = createMockDataset({
data: { cols: [dateColumn, sameCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": baseDataset, "card:2": newDataset },
newDataset.data.cols,
dateColumn,
),
).toBeUndefined();
column: dateColumn,
}),
).toBe("graph.dimensions");
});
it("should return 'graph.dimensions' for a new category dimension when it's present in every data source", () => {
@@ -363,22 +291,17 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const newDataset = createMockDataset({
data: { cols: [dateColumn, sameCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": defaultDataset, "card:2": newDataset },
newDataset.data.cols,
sameCategoryDimensionColumn,
),
column: sameCategoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
it("should return undefined for a new category dimension when it's not present in every data source", () => {
it("should return graph.dimensions for a new category dimension even if it's not present in every data source", () => {
const settings = {
"graph.metrics": [metricColumn.name],
"graph.dimensions": [timeDimensionColumn.name],
@@ -388,25 +311,17 @@ describe("cartesian", () => {
columns: [metricColumn],
settings,
};
const baseDataset = createMockDataset({
data: { cols: [metricColumn, timeDimensionColumn] },
});
const newDataset = createMockDataset({
data: { cols: [dateColumn, sameCategoryDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": baseDataset, "card:2": newDataset },
newDataset.data.cols,
sameCategoryDimensionColumn,
),
).toBeUndefined();
column: sameCategoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
it("should return undefined for a new category dimension when there are several metrics selected", () => {
it("should return graph.dimensions for a new category dimension when there are several metrics selected", () => {
const settings = {
"graph.metrics": [metricColumn.name, otherMetricColumn.name],
"graph.dimensions": [timeDimensionColumn.name],
@@ -416,26 +331,14 @@ describe("cartesian", () => {
columns: [metricColumn, otherMetricColumn, timeDimensionColumn],
settings,
};
const baseDataset = createMockDataset({
data: {
cols: [
metricColumn,
otherMetricColumn,
timeDimensionColumn,
sameCategoryDimensionColumn,
],
},
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": baseDataset },
baseDataset.data.cols,
sameCategoryDimensionColumn,
),
).toBeUndefined();
column: sameCategoryDimensionColumn,
}),
).toBe("graph.dimensions");
});
it("should return 'graph.dimensions' for a compatible date dimension when there are several metrics selected", () => {
@@ -448,23 +351,13 @@ describe("cartesian", () => {
columns: [metricColumn, otherMetricColumn],
settings,
};
const baseDataset = createMockDataset({
data: {
cols: [metricColumn, otherMetricColumn, timeDimensionColumn],
},
});
const newDataset = createMockDataset({
data: { cols: [andAnotherMetricColumn, timeDimensionColumn] },
});
expect(
findColumnSlotForCartesianChart(
findColumnSlotForCartesianChart({
state,
settings,
{ "card:1": baseDataset, "card:2": newDataset },
newDataset.data.cols,
timeDimensionColumn,
),
column: timeDimensionColumn,
}),
).toBe("graph.dimensions");
});
});

24
frontend/src/metabase/visualizer/visualizations/compat.ts
View File

@@ -14,16 +14,16 @@ import { findColumnSlotForCartesianChart } from "./cartesian";
import { findColumnSlotForFunnel } from "./funnel";
import { findColumnSlotForPieChart } from "./pie";
type CompatFn = (
type CompatFn = (parameters: {
state: Pick<
VisualizerVizDefinitionWithColumns,
"display" | "columns" | "settings"
>,
settings: ComputedVisualizationSettings,
datasets: Record<VisualizerDataSourceId, Dataset>,
dataSourceColumns: DatasetColumn[],
column: DatasetColumn,
) => string | undefined;
>;
settings: ComputedVisualizationSettings;
datasets: Record<VisualizerDataSourceId, Dataset>;
dataSourceColumns: DatasetColumn[];
column: DatasetColumn;
}) => string | undefined;
const vizMappingFn: Record<string, CompatFn> = {
cartesian: findColumnSlotForCartesianChart,
@@ -50,7 +50,7 @@ export function findSlotForColumn(
vizMappingFn[isCartesianChart(display) ? "cartesian" : display];
if (compatFn) {
return compatFn(state, settings, datasets, dataSourceColumns, column);
return compatFn({ state, settings, datasets, dataSourceColumns, column });
} else {
return undefined;
}
@@ -78,13 +78,13 @@ export function groupColumnsBySuitableVizSettings(
.map((column) => ({
column,
// TODO Fix type casting
slot: compatFn(
slot: compatFn({
state,
settings,
datasets,
columns as DatasetColumn[],
column as DatasetColumn,
),
dataSourceColumns: columns as DatasetColumn[],
column: column as DatasetColumn,
}),
}))
.filter((mapping) => !!mapping.slot);
const groupedMappings = _.groupBy(mapping, (m) => m.slot as string);

22
frontend/src/metabase/visualizer/visualizations/funnel.ts
View File

@@ -22,7 +22,6 @@ import type {
DatasetColumn,
VisualizerColumnReference,
VisualizerDataSource,
VisualizerDataSourceId,
} from "metabase-types/api";
import type { VisualizerVizDefinitionWithColumns } from "metabase-types/store/visualizer";
@@ -123,16 +122,16 @@ export function canCombineCardWithFunnel({ data }: Dataset) {
// instead of adding a column, so we need to use a special name for that
const SCALAR_FUNNEL_SLOT = "scalar_funnel";
export function findColumnSlotForFunnel(
export function findColumnSlotForFunnel(parameters: {
state: Pick<
VisualizerVizDefinitionWithColumns,
"display" | "columns" | "settings"
>,
settings: ComputedVisualizationSettings,
datasets: Record<VisualizerDataSourceId, Dataset>,
dataSourceColumns: DatasetColumn[],
column: DatasetColumn,
) {
>;
settings: ComputedVisualizationSettings;
dataSourceColumns: DatasetColumn[];
column: DatasetColumn;
}) {
const { state, settings, dataSourceColumns, column } = parameters;
const isEmpty = state.columns.length === 0;
if (
@@ -201,13 +200,12 @@ export function addColumnToFunnel(
dataset: Dataset,
dataSource: VisualizerDataSource,
) {
const slot = findColumnSlotForFunnel(
const slot = findColumnSlotForFunnel({
state,
settings,
datasets,
dataset.data.cols,
dataSourceColumns: dataset.data.cols,
column,
);
});
if (!slot) {
return;

22
frontend/src/metabase/visualizer/visualizations/pie.ts
View File

@@ -20,7 +20,6 @@ import type {
DatasetColumn,
VisualizerColumnReference,
VisualizerDataSource,
VisualizerVizDefinition,
} from "metabase-types/api";
import type { VisualizerVizDefinitionWithColumns } from "metabase-types/store/visualizer";
@@ -89,14 +88,12 @@ export const pieDropHandler = (
}
};
export function findColumnSlotForPieChart(
state: Pick<VisualizerVizDefinition, "settings">,
computedSettings: ComputedVisualizationSettings,
datasets: Record<string, Dataset>,
dataSourceColumns: DatasetColumn[],
column: DatasetColumn,
) {
const ownMetric = computedSettings["pie.metric"];
export function findColumnSlotForPieChart(parameters: {
settings: ComputedVisualizationSettings;
column: DatasetColumn;
}) {
const { settings, column } = parameters;
const ownMetric = settings["pie.metric"];
if (!ownMetric && isMetric(column)) {
return "pie.metric";
}
@@ -113,13 +110,10 @@ export function addColumnToPieChart(
column: DatasetColumn,
columnRef: VisualizerColumnReference,
) {
const slot = findColumnSlotForPieChart(
state,
const slot = findColumnSlotForPieChart({
settings,
datasets,
dataSourceColumns,
column,
);
});
if (slot) {
state.columns.push(column);
state.columnValuesMapping[column.name] = [columnRef];