From ad5af66b35e8f488d88462c28c2bc2254e95e22e Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 14 Mar 2023 14:23:58 +0100 Subject: [PATCH 1/4] fix(chart-controls): Error if x_axis_sort and sort_by metrics are included in main metrics --- .../src/operators/utils/extractExtraMetrics.ts | 5 +++-- .../test/operators/utils/extractExtraMetrics.test.ts | 10 ++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts index 74928f836f861..28d9ec40ad926 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts @@ -25,12 +25,13 @@ import { export function extractExtraMetrics( formData: QueryFormData, ): QueryFormMetric[] { - const { groupby, timeseries_limit_metric, x_axis_sort } = formData; + const { groupby, timeseries_limit_metric, x_axis_sort, metrics } = formData; const extra_metrics: QueryFormMetric[] = []; if ( !(groupby || []).length && timeseries_limit_metric && - getMetricLabel(timeseries_limit_metric) === x_axis_sort + getMetricLabel(timeseries_limit_metric) === x_axis_sort && + !metrics?.some(metric => getMetricLabel(metric) === x_axis_sort) ) { extra_metrics.push(timeseries_limit_metric); } diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts index 89f4c11181192..8bba8883c5c65 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts @@ -92,3 +92,13 @@ test('returns empty array if groupby populated', () => { }), ).toEqual([]); }); + +test('returns empty array if timeseries_limit_metric and x_axis_sort are included in main metrics array', () => { + expect( + extractExtraMetrics({ + ...baseFormData, + timeseries_limit_metric: 'a', + x_axis_sort: 'a', + }), + ).toEqual([]); +}); From 33aa194873c9a644d0176d256de3d80ebff2e1d6 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 14 Mar 2023 14:27:53 +0100 Subject: [PATCH 2/4] Add test --- .../utils/extractExtraMetrics.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts index 8bba8883c5c65..b7c1bf047592d 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts @@ -102,3 +102,25 @@ test('returns empty array if timeseries_limit_metric and x_axis_sort are include }), ).toEqual([]); }); + +test('returns empty array if timeseries_limit_metric and x_axis_sort are included in main metrics array with adhoc metrics', () => { + expect( + extractExtraMetrics({ + ...baseFormData, + metrics: [ + 'a', + { + expressionType: 'SIMPLE', + aggregate: 'SUM', + column: { column_name: 'num' }, + }, + ], + timeseries_limit_metric: { + expressionType: 'SIMPLE', + aggregate: 'SUM', + column: { column_name: 'num' }, + }, + x_axis_sort: 'SUM(num)', + }), + ).toEqual([]); +}); From 6e67423019a1f2dc13aa8c70aec9115a6e69e046 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 14 Mar 2023 14:38:34 +0100 Subject: [PATCH 3/4] Remove duplicates from x axis dropdown --- .../src/shared-controls/customControls.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx index 8e8f4d8400ca5..333d5639ebe43 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx @@ -79,6 +79,7 @@ export const xAxisSortControl = { ...ensureIsArray(controls?.metrics?.value as QueryFormMetric), controls?.timeseries_limit_metric?.value as QueryFormMetric, ].filter(Boolean); + const metricLabels = [...new Set(metrics.map(getMetricLabel))]; const options = [ ...columns.map(column => { const value = getColumnLabel(column); @@ -87,8 +88,7 @@ export const xAxisSortControl = { label: dataset?.verbose_map?.[value] || value, }; }), - ...metrics.map(metric => { - const value = getMetricLabel(metric); + ...metricLabels.map(value => { return { value, label: dataset?.verbose_map?.[value] || value, From 38acc76c69dacac3f94bc2a4e756b257b2de4ab4 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 14 Mar 2023 14:45:53 +0100 Subject: [PATCH 4/4] Fix lint --- .../src/shared-controls/customControls.tsx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx index 333d5639ebe43..5ac303f54dcb1 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx @@ -88,12 +88,10 @@ export const xAxisSortControl = { label: dataset?.verbose_map?.[value] || value, }; }), - ...metricLabels.map(value => { - return { - value, - label: dataset?.verbose_map?.[value] || value, - }; - }), + ...metricLabels.map(value => ({ + value, + label: dataset?.verbose_map?.[value] || value, + })), ]; const shouldReset = !(