Skip to content
Snippets Groups Projects
Unverified Commit 600fe62a authored by Aleksandr Lesnenko's avatar Aleksandr Lesnenko Committed by GitHub
Browse files

Fix combined cards crash when cards have invalid settings (#45115)

* fix combined cards crash when some cards have invalid viz settings

* fix mock column was not metric

* ensure combined scalar column has proper type to handle the case when the card returns empty results
parent 5f400a76
No related branches found
No related tags found
No related merge requests found
Showing
with 1000 additions and 47 deletions
.loki/reference/chrome_laptop_static_viz_ComboChart_Combined_With_Invalid_Settings.png

35.3 KiB

.loki/reference/chrome_laptop_viz_LineChart_Default.png

21.8 KiB | W: | H:

.loki/reference/chrome_laptop_viz_LineChart_Default.png

21.9 KiB | W: | H:

.loki/reference/chrome_laptop_viz_LineChart_Default.png
.loki/reference/chrome_laptop_viz_LineChart_Default.png
.loki/reference/chrome_laptop_viz_LineChart_Default.png
.loki/reference/chrome_laptop_viz_LineChart_Default.png
  • 2-up
  • Swipe
  • Onion skin
......@@ -822,6 +822,13 @@ SafariNonIanaTimezoneRepro44128.args = {
renderingContext,
};
export const CombinedWithInvalidSettings = Template.bind({});
CombinedWithInvalidSettings.args = {
rawSeries: data.combinedWithInvalidSettings as any,
dashcardSettings: {},
renderingContext,
};
export const Default = Template.bind({});
Default.args = {
rawSeries: data.messedUpAxis as any,
......
......@@ -9,7 +9,6 @@ import {
getAreDimensionsAndMetricsValid,
getDefaultBubbleSizeCol,
getDefaultDataLabelsFrequency,
getDefaultDimensionFilter,
getDefaultDimensions,
getDefaultGoalLabel,
getDefaultIsAutoSplitEnabled,
......@@ -17,7 +16,6 @@ import {
getDefaultIsNumeric,
getDefaultIsTimeSeries,
getDefaultLegendIsReversed,
getDefaultMetricFilter,
getDefaultMetrics,
getDefaultShowDataLabels,
getDefaultShowStackValues,
......@@ -131,22 +129,8 @@ export const computeStaticComboChartSettings = (
): ComputedVisualizationSettings => {
const { card: mainCard, data: mainDataset } = rawSeries[0];
const settings = getCommonStaticVizSettings(rawSeries, dashcardSettings);
fillWithDefaultValue(
settings,
"graph._dimension_filter",
getDefaultDimensionFilter(mainCard.display),
);
fillWithDefaultValue(
settings,
"graph._metric_filter",
getDefaultMetricFilter(mainCard.display),
);
const areDimensionsAndMetricsValid = getAreDimensionsAndMetricsValid(
rawSeries,
settings,
);
const areDimensionsAndMetricsValid =
getAreDimensionsAndMetricsValid(rawSeries);
fillWithDefaultValue(
settings,
......
......@@ -49,6 +49,7 @@ import barTwoDaysOfWeek from "./bar-two-days-of-week.json";
import barsBreakoutSortedWithNegativeValuesPowerYAxis from "./bars-breakout-sorted-with-negative-values-power-y-axis.json";
import breakoutNullAndEmptyString from "./breakout-null-and-empty-string.json";
import combinedBarTimeSeriesDifferentGranularityWithBreakout from "./combined-bar-timeseries-different-granularity-with-breakout.json";
import combinedWithInvalidSettings from "./combined-with-invalid-settings.json";
import comboDataLabelsAutoCompactnessPropagatesFromLine from "./combo-data-labels-auto-compactness-propagates-from-line.json";
import comboDataLabelsAutoCompactnessPropagatesFromTotals from "./combo-data-labels-auto-compactness-propagates-from-totals.json";
import comboHistogram from "./combo-histogram.json";
......@@ -226,4 +227,5 @@ export const data = {
areaChartSteppedNullsInterpolated,
areaChartSteppedNullsSkipped,
safariNonIanaTimezoneRepro44128,
combinedWithInvalidSettings,
};
......@@ -22,6 +22,7 @@ import type {
} from "metabase/visualizations/echarts/cartesian/model/types";
import { getCartesianChartColumns } from "metabase/visualizations/lib/graph/columns";
import { getSingleSeriesDimensionsAndMetrics } from "metabase/visualizations/lib/utils";
import { getAreDimensionsAndMetricsValid } from "metabase/visualizations/shared/settings/cartesian-chart";
import type {
ComputedVisualizationSettings,
RenderingContext,
......@@ -40,18 +41,15 @@ const getSettingsWithDefaultMetricsAndDimensions = (series: SingleSeries) => {
const {
card: { visualization_settings: settings },
} = series;
if (
settings["graph.dimensions"] != null &&
settings["graph.metrics"] != null
) {
if (getAreDimensionsAndMetricsValid([series])) {
return settings;
}
const { dimensions, metrics } = getSingleSeriesDimensionsAndMetrics(series);
const settingsWithDefaults = { ...settings };
settingsWithDefaults["graph.dimensions"] ??= dimensions;
settingsWithDefaults["graph.metrics"] ??= metrics;
settingsWithDefaults["graph.dimensions"] = dimensions;
settingsWithDefaults["graph.metrics"] = metrics;
return settingsWithDefaults;
};
......
......@@ -71,14 +71,6 @@ export const GRAPH_DATA_SETTINGS = {
]) => cols,
hidden: true,
}),
"graph._dimension_filter": {
getDefault: ([{ card }]) => getDefaultDimensionFilter(card.display),
useRawSeries: true,
},
"graph._metric_filter": {
getDefault: ([{ card }]) => getDefaultMetricFilter(card.display),
useRawSeries: true,
},
"graph.dimensions": {
section: t`Data`,
title: t`X-axis`,
......@@ -97,7 +89,7 @@ export const GRAPH_DATA_SETTINGS = {
const addedDimensions = vizSettings["graph.dimensions"];
const maxDimensionsSupported = getMaxDimensionsSupported(card.display);
const options = data.cols
.filter(vizSettings["graph._dimension_filter"])
.filter(getDefaultDimensionFilter(card.display))
.map(getOptionFromColumn);
return {
options,
......@@ -116,7 +108,6 @@ export const GRAPH_DATA_SETTINGS = {
showColumnSettingForIndicies: [0],
};
},
readDependencies: ["graph._dimension_filter", "graph._metric_filter"],
writeDependencies: ["graph.metrics"],
eraseDependencies: ["graph.series_order_dimension", "graph.series_order"],
dashboard: false,
......@@ -156,7 +147,7 @@ export const GRAPH_DATA_SETTINGS = {
persistDefault: true,
getProps: ([{ card, data }], vizSettings, _onChange, extra) => {
const options = data.cols
.filter(vizSettings["graph._metric_filter"])
.filter(getDefaultMetricFilter(card.display))
.map(getOptionFromColumn);
const addedMetrics = vizSettings["graph.metrics"];
......@@ -181,11 +172,7 @@ export const GRAPH_DATA_SETTINGS = {
series: extra.transformedSeries,
};
},
readDependencies: [
"graph._dimension_filter",
"graph._metric_filter",
"series_settings.colors",
],
readDependencies: ["series_settings.colors"],
writeDependencies: ["graph.dimensions"],
dashboard: false,
useRawSeries: true,
......
......@@ -35,21 +35,18 @@ export function getDefaultMetricFilter(display: string) {
return display === "scatter" ? isNumeric : isMetric;
}
export function getAreDimensionsAndMetricsValid(
rawSeries: RawSeries,
settings: ComputedVisualizationSettings,
) {
export function getAreDimensionsAndMetricsValid(rawSeries: RawSeries) {
return rawSeries.some(
({ card, data }) =>
columnsAreValid(
card.visualization_settings["graph.dimensions"],
data,
settings["graph._dimension_filter"],
getDefaultDimensionFilter(card.display),
) &&
columnsAreValid(
card.visualization_settings["graph.metrics"],
data,
settings["graph._metric_filter"],
getDefaultMetricFilter(card.display),
),
);
}
......
......@@ -27,7 +27,12 @@ const setup = (funnelProps, visualizationSettings = {}) => {
data: createMockDatasetData({
cols: [
createMockColumn({ id: 1, name: "foo", display_name: "foo" }),
createMockColumn({ id: 2, name: "bar", display_name: "bar" }),
createMockColumn({
id: 2,
name: "bar",
display_name: "bar",
semantic_type: "type/Number",
}),
],
rows: [
[10, 20],
......
......@@ -25,6 +25,7 @@ export const scalarToBarTransform: TransformSeries = rawSeries => {
{
...data.cols[metricColumnIndex],
name: card.name,
semantic_type: "type/Number",
},
],
rows: [[card.name, data.rows[0][metricColumnIndex]]],
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment