Skip to content

Commit 1b139d0

Browse files
villebrovillebro
authored
feat(generic-x-axis): add x sorting on series limit metric (#23274)
1 parent 71a9d0d commit 1b139d0

File tree

11 files changed

+294
-76
lines changed

11 files changed

+294
-76
lines changed

‎superset-frontend/packages/superset-ui-chart-controls/src/operators/pivotOperator.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,16 @@ import {
2424
getXAxisLabel,
2525
} from '@superset-ui/core';
2626
import { PostProcessingFactory } from './types';
27+
import { extractExtraMetrics } from './utils';
2728

2829
export const pivotOperator: PostProcessingFactory<PostProcessingPivot> = (
2930
formData,
3031
queryObject,
3132
) => {
32-
const metricLabels = ensureIsArray(queryObject.metrics).map(getMetricLabel);
33+
const metricLabels = [
34+
...ensureIsArray(queryObject.metrics),
35+
...extractExtraMetrics(formData),
36+
].map(getMetricLabel);
3337
const xAxisLabel = getXAxisLabel(formData);
3438
const columns = queryObject.series_columns || queryObject.columns;
3539

‎superset-frontend/packages/superset-ui-chart-controls/src/operators/sortOperator.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
PostProcessingSort,
2727
} from '@superset-ui/core';
2828
import { PostProcessingFactory } from './types';
29+
import { extractExtraMetrics } from './utils';
2930

3031
export const sortOperator: PostProcessingFactory<PostProcessingSort> = (
3132
formData,
@@ -34,7 +35,8 @@ export const sortOperator: PostProcessingFactory<PostProcessingSort> = (
3435
// the sortOperator only used in the barchart v2
3536
const sortableLabels = [
3637
getXAxisLabel(formData),
37-
...ensureIsArray(formData.metrics).map(metric => getMetricLabel(metric)),
38+
...ensureIsArray(formData.metrics).map(getMetricLabel),
39+
...extractExtraMetrics(formData).map(getMetricLabel),
3840
].filter(Boolean);
3941

4042
if (

‎superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/extractExtraMetrics.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
import {
20+
getMetricLabel,
21+
QueryFormData,
22+
QueryFormMetric,
23+
} from '@superset-ui/core';
24+
25+
export function extractExtraMetrics(
26+
formData: QueryFormData,
27+
): QueryFormMetric[] {
28+
const { groupby, timeseries_limit_metric, x_axis_sort } = formData;
29+
const extra_metrics: QueryFormMetric[] = [];
30+
if (
31+
!(groupby || []).length &&
32+
timeseries_limit_metric &&
33+
getMetricLabel(timeseries_limit_metric) === x_axis_sort
34+
) {
35+
extra_metrics.push(timeseries_limit_metric);
36+
}
37+
return extra_metrics;
38+
}

‎superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@
2020
export { getMetricOffsetsMap } from './getMetricOffsetsMap';
2121
export { isTimeComparison } from './isTimeComparison';
2222
export { isDerivedSeries } from './isDerivedSeries';
23+
export { extractExtraMetrics } from './extractExtraMetrics';
2324
export { TIME_COMPARISON_SEPARATOR } from './constants';

‎superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx‎

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,16 @@ import {
2323
getColumnLabel,
2424
getMetricLabel,
2525
isDefined,
26-
isEqualArray,
2726
QueryFormColumn,
2827
QueryFormMetric,
2928
t,
3029
} from '@superset-ui/core';
31-
import { ControlPanelState, ControlState, ControlStateMapping } from '../types';
30+
import {
31+
ControlPanelState,
32+
ControlState,
33+
ControlStateMapping,
34+
isDataset,
35+
} from '../types';
3236
import { isTemporalColumn } from '../utils';
3337

3438
export const contributionModeControl = {
@@ -59,39 +63,42 @@ export const xAxisSortControl = {
5963
name: 'x_axis_sort',
6064
config: {
6165
type: 'XAxisSortControl',
62-
label: t('X-Axis Sort By'),
63-
description: t('Whether to sort descending or ascending on the X-Axis.'),
64-
shouldMapStateToProps: (
65-
prevState: ControlPanelState,
66-
state: ControlPanelState,
67-
) => {
68-
const prevOptions = [
69-
getColumnLabel(prevState?.controls?.x_axis?.value as QueryFormColumn),
70-
...ensureIsArray(prevState?.controls?.metrics?.value).map(metric =>
71-
getMetricLabel(metric as QueryFormMetric),
72-
),
73-
];
74-
const currOptions = [
75-
getColumnLabel(state?.controls?.x_axis?.value as QueryFormColumn),
76-
...ensureIsArray(state?.controls?.metrics?.value).map(metric =>
77-
getMetricLabel(metric as QueryFormMetric),
78-
),
79-
];
80-
return !isEqualArray(prevOptions, currOptions);
81-
},
82-
mapStateToProps: (
83-
{ controls }: { controls: ControlStateMapping },
84-
controlState: ControlState,
85-
) => {
86-
const choices = [
87-
getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
88-
...ensureIsArray(controls?.metrics?.value).map(metric =>
89-
getMetricLabel(metric as QueryFormMetric),
90-
),
66+
label: (state: ControlPanelState) =>
67+
state.form_data?.orientation === 'horizontal'
68+
? t('Y-Axis Sort By')
69+
: t('X-Axis Sort By'),
70+
description: t('Decides which column to sort the base axis by.'),
71+
shouldMapStateToProps: () => true,
72+
mapStateToProps: (state: ControlPanelState, controlState: ControlState) => {
73+
const { controls, datasource } = state;
74+
const dataset = isDataset(datasource) ? datasource : undefined;
75+
const columns = [controls?.x_axis?.value as QueryFormColumn].filter(
76+
Boolean,
77+
);
78+
const metrics = [
79+
...ensureIsArray(controls?.metrics?.value as QueryFormMetric),
80+
controls?.timeseries_limit_metric?.value as QueryFormMetric,
9181
].filter(Boolean);
82+
const options = [
83+
...columns.map(column => {
84+
const value = getColumnLabel(column);
85+
return {
86+
value,
87+
label: dataset?.verbose_map?.[value] || value,
88+
};
89+
}),
90+
...metrics.map(metric => {
91+
const value = getMetricLabel(metric);
92+
return {
93+
value,
94+
label: dataset?.verbose_map?.[value] || value,
95+
};
96+
}),
97+
];
98+
9299
const shouldReset = !(
93100
typeof controlState.value === 'string' &&
94-
choices.includes(controlState.value) &&
101+
options.map(option => option.value).includes(controlState.value) &&
95102
!isTemporalColumn(
96103
getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
97104
controls?.datasource?.datasource,
@@ -100,10 +107,7 @@ export const xAxisSortControl = {
100107

101108
return {
102109
shouldReset,
103-
options: choices.map(entry => ({
104-
value: entry,
105-
label: entry,
106-
})),
110+
options,
107111
};
108112
},
109113
visibility: xAxisSortVisibility,
@@ -114,9 +118,12 @@ export const xAxisSortAscControl = {
114118
name: 'x_axis_sort_asc',
115119
config: {
116120
type: 'CheckboxControl',
117-
label: t('X-Axis Sort Ascending'),
121+
label: (state: ControlPanelState) =>
122+
state.form_data?.orientation === 'horizontal'
123+
? t('Y-Axis Sort Ascending')
124+
: t('X-Axis Sort Ascending'),
118125
default: true,
119-
description: t('Whether to sort descending or ascending on the X-Axis.'),
126+
description: t('Whether to sort ascending or descending on the base Axis.'),
120127
visibility: xAxisSortVisibility,
121128
},
122129
};

‎superset-frontend/packages/superset-ui-chart-controls/test/operators/pivotOperator.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,3 +185,33 @@ test('pivot by adhoc x_axis', () => {
185185
},
186186
});
187187
});
188+
189+
test('pivot by x_axis with extra metrics', () => {
190+
expect(
191+
pivotOperator(
192+
{
193+
...formData,
194+
x_axis: 'foo',
195+
x_axis_sort: 'bar',
196+
groupby: [],
197+
timeseries_limit_metric: 'bar',
198+
},
199+
{
200+
...queryObject,
201+
series_columns: [],
202+
},
203+
),
204+
).toEqual({
205+
operation: 'pivot',
206+
options: {
207+
index: ['foo'],
208+
columns: [],
209+
aggregates: {
210+
'count(*)': { operator: 'mean' },
211+
'sum(val)': { operator: 'mean' },
212+
bar: { operator: 'mean' },
213+
},
214+
drop_missing_columns: false,
215+
},
216+
});
217+
});

‎superset-frontend/packages/superset-ui-chart-controls/test/operators/sortOperator.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,28 @@ test('should sort by axis', () => {
146146
},
147147
});
148148
});
149+
150+
test('should sort by extra metric', () => {
151+
Object.defineProperty(supersetCoreModule, 'hasGenericChartAxes', {
152+
value: true,
153+
});
154+
expect(
155+
sortOperator(
156+
{
157+
...formData,
158+
x_axis_sort: 'my_limit_metric',
159+
x_axis_sort_asc: true,
160+
x_axis: 'Categorical Column',
161+
groupby: [],
162+
timeseries_limit_metric: 'my_limit_metric',
163+
},
164+
queryObject,
165+
),
166+
).toEqual({
167+
operation: 'sort',
168+
options: {
169+
by: 'my_limit_metric',
170+
ascending: true,
171+
},
172+
});
173+
});

‎superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/extractExtraMetrics.test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
import { QueryFormData, QueryFormMetric } from '@superset-ui/core';
20+
import { extractExtraMetrics } from '@superset-ui/chart-controls';
21+
22+
const baseFormData: QueryFormData = {
23+
datasource: 'dummy',
24+
viz_type: 'table',
25+
metrics: ['a', 'b'],
26+
columns: ['foo', 'bar'],
27+
limit: 100,
28+
metrics_b: ['c', 'd'],
29+
columns_b: ['hello', 'world'],
30+
limit_b: 200,
31+
};
32+
33+
const metric: QueryFormMetric = {
34+
expressionType: 'SQL',
35+
sqlExpression: 'case when 1 then 1 else 2 end',
36+
label: 'foo',
37+
};
38+
39+
test('returns empty array if relevant controls missing', () => {
40+
expect(
41+
extractExtraMetrics({
42+
...baseFormData,
43+
}),
44+
).toEqual([]);
45+
});
46+
47+
test('returns empty array if x_axis_sort is not same as timeseries_limit_metric', () => {
48+
expect(
49+
extractExtraMetrics({
50+
...baseFormData,
51+
timeseries_limit_metric: 'foo',
52+
x_axis_sort: 'bar',
53+
}),
54+
).toEqual([]);
55+
});
56+
57+
test('returns correct column if sort columns match', () => {
58+
expect(
59+
extractExtraMetrics({
60+
...baseFormData,
61+
timeseries_limit_metric: 'foo',
62+
x_axis_sort: 'foo',
63+
}),
64+
).toEqual(['foo']);
65+
});
66+
67+
test('handles adhoc metrics correctly', () => {
68+
expect(
69+
extractExtraMetrics({
70+
...baseFormData,
71+
timeseries_limit_metric: metric,
72+
x_axis_sort: 'foo',
73+
}),
74+
).toEqual([metric]);
75+
76+
expect(
77+
extractExtraMetrics({
78+
...baseFormData,
79+
timeseries_limit_metric: metric,
80+
x_axis_sort: 'bar',
81+
}),
82+
).toEqual([]);
83+
});
84+
85+
test('returns empty array if groupby populated', () => {
86+
expect(
87+
extractExtraMetrics({
88+
...baseFormData,
89+
groupby: ['bar'],
90+
timeseries_limit_metric: 'foo',
91+
x_axis_sort: 'foo',
92+
}),
93+
).toEqual([]);
94+
});

0 commit comments

Comments
 (0)