You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/09/23 00:43:36 UTC
(superset) branch adopt/37712-x-axis-sort-with-dimensions created (now 15361212dd5)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch adopt/37712-x-axis-sort-with-dimensions
in repository https://gitbox.apache.org/repos/asf/superset.git
at 15361212dd5 fix(echarts-bar): sort the x-axis by any column or metric when dimensions are set
This branch includes the following new commits:
new 15361212dd5 fix(echarts-bar): sort the x-axis by any column or metric when dimensions are set
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/01: fix(echarts-bar): sort the x-axis by any column or metric when dimensions are set
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch adopt/37712-x-axis-sort-with-dimensions
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 15361212dd5883dc2f445e0d1e164a999a6c7702
Author: rusackas <ev...@rusackas.com>
AuthorDate: Tue Sep 22 17:42:12 2026 -0700
fix(echarts-bar): sort the x-axis by any column or metric when dimensions are set
The "X-Axis Sort By" control on ECharts bar charts offered a different set of
choices once a dimension was added (only the series aggregates), and even a
metric that was selected before adding the dimension was silently ignored by
the multi-series render path, which only understood `SortSeriesType`.
Carry the chosen field through the grouped sort path instead of guessing an
aggregate for it:
- the control offers the x-axis column and every metric (the query "Sort By"
metric included) with or without dimensions, adding the series aggregates
for multi-series charts and deduping by value;
- the sort-only limit metric is queried with dimensions too, so the pivot
returns one `<metric>, <dimension values>` column per series;
- transformProps resolves a metric's pivoted columns through the response's
`label_map` and sorts the rows by their sum (`sortRows` gains a typed
`{ sumOfColumns }` mode), sorts by axis value for the x-axis column, and
keeps the aggregate behaviour for `SortSeriesType` values; the same lookup
keeps the sort-only metric's columns out of the rendered series and the
stacked totals.
Adopted from #37712.
Fixes #34352
Fixes #38249
Co-authored-by: Yousuf Ansari <an...@gmail.com>
Co-Authored-By: Evan Rusackas <ev...@preset.io>
Co-Authored-By: Claude Fable 5.1 <no...@anthropic.com>
---
.../src/operators/utils/extractExtraMetrics.ts | 10 +-
.../src/shared-controls/customControls.tsx | 59 +++--
.../operators/utils/extractExtraMetrics.test.ts | 13 +-
.../test/shared-controls/xAxisSortControl.test.ts | 161 +++++++++++++
.../src/Timeseries/transformProps.ts | 61 ++++-
.../plugin-chart-echarts/src/utils/series.ts | 43 +++-
.../test/Timeseries/buildQuery.test.ts | 43 ++++
.../Timeseries/xAxisSortWithDimensions.test.ts | 250 +++++++++++++++++++++
.../plugin-chart-echarts/test/utils/series.test.ts | 91 ++++++++
9 files changed, 694 insertions(+), 37 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 4344d088af3..35de84de593 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
@@ -26,11 +26,15 @@ import {
export function extractExtraMetrics(
formData: QueryFormData,
): QueryFormMetric[] {
- const { groupby, timeseries_limit_metric, x_axis_sort, metrics } = formData;
+ const { timeseries_limit_metric, x_axis_sort, metrics } = formData;
const extra_metrics: QueryFormMetric[] = [];
- const limitMetric = ensureIsArray(timeseries_limit_metric)[0];
+ const [limitMetric] = ensureIsArray(timeseries_limit_metric);
+ // The "Sort By" limit metric is only queried when the axis is sorted by it
+ // and it is not already a value metric. This applies with dimensions too:
+ // the pivot then yields one `<limit metric>, <dimension values>` column
+ // per series, which the chart sums client-side to order the axis (the
+ // backend sort operator only handles the single-series case).
if (
- !(groupby || []).length &&
limitMetric &&
getMetricLabel(limitMetric) === x_axis_sort &&
!metrics?.some(metric => getMetricLabel(metric) === x_axis_sort)
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 3604b099359..929ac839800 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
@@ -146,9 +146,7 @@ export const xAxisSortControl = {
const columns = [controls?.x_axis?.value as QueryFormColumn].filter(
Boolean,
);
- const isSingleSortAvailable =
- ensureIsArray(controls?.groupby?.value).length === 0;
- const isMultiSortAvailable =
+ const isMultiSeries =
!!ensureIsArray(controls?.groupby?.value).length ||
ensureIsArray(controls?.metrics?.value).length > 1;
const metrics = [
@@ -156,25 +154,44 @@ export const xAxisSortControl = {
controls?.timeseries_limit_metric?.value as QueryFormMetric,
].filter(Boolean);
const metricLabels = [...new Set(metrics.map(getMetricLabel))];
+ // The x-axis column and every metric (the "Sort By" limit metric
+ // included) can order the axis with or without dimensions: the backend
+ // sort operator handles the single-series case, and the chart resolves
+ // a metric's pivoted columns through `label_map` when dimensions split
+ // it into several series.
+ const fieldOptions = [
+ ...columns.map(column => {
+ const value = getColumnLabel(column);
+ return { value, label: dataset?.verbose_map?.[value] || value };
+ }),
+ ...metricLabels.map(value => ({
+ value,
+ label: dataset?.verbose_map?.[value] || value,
+ })),
+ ];
+ // Aggregating across series only means something once there is more
+ // than one series per x-axis value.
+ const aggregateOptions = isMultiSeries
+ ? SORT_SERIES_CHOICES.map(choice => ({
+ value: choice[0],
+ label: choice[1],
+ }))
+ : [];
+ // Option values double as the stored `x_axis_sort`, and the aggregate
+ // values (`name`, `sum`, ...) are what the chart matches first, so a
+ // column or metric whose label collides with one is dropped rather
+ // than shadowing the aggregate. The same set also dedupes a column
+ // that shares its label with a metric.
+ const seenValues = new Set(aggregateOptions.map(option => option.value));
const options = [
- ...(isSingleSortAvailable
- ? [
- ...columns.map(column => {
- const value = getColumnLabel(column);
- return { value, label: dataset?.verbose_map?.[value] || value };
- }),
- ...metricLabels.map(value => ({
- value,
- label: dataset?.verbose_map?.[value] || value,
- })),
- ]
- : []),
- ...(isMultiSortAvailable
- ? SORT_SERIES_CHOICES.map(choice => ({
- value: choice[0],
- label: choice[1],
- }))
- : []),
+ ...fieldOptions.filter(option => {
+ if (seenValues.has(option.value)) {
+ return false;
+ }
+ seenValues.add(option.value);
+ return true;
+ }),
+ ...aggregateOptions,
];
const shouldReset = !(
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 dd2a10cbe55..7832bc684e1 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
@@ -82,7 +82,7 @@ test('handles adhoc metrics correctly', () => {
).toEqual([]);
});
-test('returns empty array if groupby populated', () => {
+test('returns the limit metric when groupby is populated so the chart can sort by its pivoted columns', () => {
expect(
extractExtraMetrics({
...baseFormData,
@@ -90,6 +90,17 @@ test('returns empty array if groupby populated', () => {
timeseries_limit_metric: 'foo',
x_axis_sort: 'foo',
}),
+ ).toEqual(['foo']);
+});
+
+test('returns empty array with groupby when the axis is sorted by a series aggregate', () => {
+ expect(
+ extractExtraMetrics({
+ ...baseFormData,
+ groupby: ['bar'],
+ timeseries_limit_metric: 'foo',
+ x_axis_sort: 'sum',
+ }),
).toEqual([]);
});
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/xAxisSortControl.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/xAxisSortControl.test.ts
new file mode 100644
index 00000000000..ac967c3975b
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/shared-controls/xAxisSortControl.test.ts
@@ -0,0 +1,161 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import type { QueryFormMetric } from '@superset-ui/core';
+import { xAxisSortControl } from '../../src/shared-controls/customControls';
+import { SORT_SERIES_CHOICES } from '../../src/constants';
+import { SortSeriesType } from '../../src/types';
+import type {
+ ControlPanelState,
+ ControlState,
+ ControlStateMapping,
+ Dataset,
+} from '../../src/types';
+
+const naSales: QueryFormMetric = {
+ expressionType: 'SIMPLE',
+ aggregate: 'SUM',
+ column: { column_name: 'na_sales' },
+ label: 'SUM(na_sales)',
+};
+
+const dataset = {
+ id: 1,
+ type: 'table',
+ columns: [
+ { column_name: 'genre', type: 'STRING' },
+ { column_name: 'platform', type: 'STRING' },
+ { column_name: 'na_sales', type: 'FLOAT' },
+ { column_name: 'sum', type: 'STRING' },
+ ],
+ metrics: [{ metric_name: 'count', expression: 'COUNT(*)' }],
+ verbose_map: {
+ genre: 'Genre',
+ count: 'Count',
+ 'SUM(na_sales)': 'NA Sales',
+ },
+} as unknown as Dataset;
+
+const aggregateOptions = SORT_SERIES_CHOICES.map(([value, label]) => ({
+ value,
+ label,
+}));
+
+function mapStateToProps(
+ controlValues: Record<string, unknown>,
+ value?: string,
+) {
+ const controls = Object.fromEntries(
+ Object.entries(controlValues).map(([name, controlValue]) => [
+ name,
+ { value: controlValue },
+ ]),
+ );
+ const state = {
+ controls: {
+ ...controls,
+ datasource: { datasource: dataset },
+ } as unknown as ControlStateMapping,
+ datasource: dataset,
+ } as unknown as ControlPanelState;
+ const controlState = {
+ value,
+ type: 'XAxisSortControl',
+ } as unknown as ControlState;
+ return xAxisSortControl.config.mapStateToProps(state, controlState);
+}
+
+const singleSeriesControls = {
+ x_axis: 'genre',
+ metrics: ['count'],
+ timeseries_limit_metric: naSales,
+ groupby: [],
+};
+
+const fieldOptions = [
+ { value: 'genre', label: 'Genre' },
+ { value: 'count', label: 'Count' },
+ { value: 'SUM(na_sales)', label: 'NA Sales' },
+];
+
+test('offers the x-axis column and every metric with verbose labels when there are no dimensions', () => {
+ expect(mapStateToProps(singleSeriesControls).options).toEqual(fieldOptions);
+});
+
+test('keeps the same field options and adds the series aggregates when a dimension is set', () => {
+ expect(
+ mapStateToProps({ ...singleSeriesControls, groupby: ['platform'] }).options,
+ ).toEqual([...fieldOptions, ...aggregateOptions]);
+});
+
+test('adds the series aggregates when there are several metrics and no dimension', () => {
+ expect(
+ mapStateToProps({ ...singleSeriesControls, metrics: ['count', naSales] })
+ .options,
+ ).toEqual([...fieldOptions, ...aggregateOptions]);
+});
+
+test('drops a field whose label collides with a series aggregate value', () => {
+ const { options } = mapStateToProps({
+ ...singleSeriesControls,
+ x_axis: 'sum',
+ groupby: ['platform'],
+ });
+ expect(options.filter(option => option.value === 'sum')).toEqual([
+ { value: SortSeriesType.Sum, label: 'Total value' },
+ ]);
+});
+
+test('offers a field named like a series aggregate when there is nothing to collide with', () => {
+ const { options } = mapStateToProps({
+ ...singleSeriesControls,
+ x_axis: 'sum',
+ });
+ expect(options.filter(option => option.value === 'sum')).toEqual([
+ { value: 'sum', label: 'sum' },
+ ]);
+});
+
+test('keeps a metric selection when a dimension is added', () => {
+ const { shouldReset } = mapStateToProps(
+ { ...singleSeriesControls, groupby: ['platform'] },
+ 'SUM(na_sales)',
+ );
+ expect(shouldReset).toBe(false);
+});
+
+test('keeps a series aggregate selection when a dimension is set', () => {
+ const { shouldReset } = mapStateToProps(
+ { ...singleSeriesControls, groupby: ['platform'] },
+ SortSeriesType.Max,
+ );
+ expect(shouldReset).toBe(false);
+});
+
+test('resets a series aggregate selection once the dimension is removed', () => {
+ const { shouldReset } = mapStateToProps(
+ singleSeriesControls,
+ SortSeriesType.Max,
+ );
+ expect(shouldReset).toBe(true);
+});
+
+test('resets a selection that is no longer offered', () => {
+ const { shouldReset } = mapStateToProps(singleSeriesControls, 'platform');
+ expect(shouldReset).toBe(true);
+});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index 276db9e6325..48a206773b6 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -55,6 +55,7 @@ import {
getOriginalSeries,
getTimeOffset,
isDerivedSeries,
+ SortSeriesType,
} from '@superset-ui/chart-controls';
import type { EChartsCoreOption } from 'echarts/core';
import type {
@@ -96,6 +97,7 @@ import {
getMinAndMaxFromBounds,
getTemporalAxisTickConfig,
resolveTemporalTickValues,
+ XAxisSortSeries,
} from '../utils/series';
import { resolveLegendLayout } from '../utils/legendLayout';
import {
@@ -460,14 +462,34 @@ export default function transformProps(
rebaseToPercentChange(forecastRebasedData, xAxisLabel || DTTM_ALIAS)
: forecastRebasedData;
const isHorizontal = orientation === OrientationType.Horizontal;
+ // With dimensions set, the pivot splits a metric into one
+ // `<metric>, <dimension values>` column per series. `label_map` lists each
+ // flattened column as `[metric, ...dimension values]`, so a metric's
+ // columns are resolved through it rather than by matching on the column
+ // name. A single-metric chart with `truncate_metric` drops the metric part
+ // from its value columns, which is fine: only sort-only metrics need
+ // resolving, and those always keep it.
+ const pivotedColumnsOf = (metricLabel: string): string[] =>
+ Object.entries(labelMap)
+ .filter(
+ ([column, parts]) =>
+ column !== metricLabel &&
+ parts.length > 1 &&
+ parts[0] === metricLabel &&
+ !derivedComparisonSeries.has(column),
+ )
+ .map(([column]) => column);
// rebasedData's keys have already been through rebaseForecastDatum, which
// renames a key to its verboseMap entry when one is configured for that
// metric. extraMetricLabels must be mapped the same way, or a sort-only
// metric with a verbose_name set would silently fail to match here (and in
- // extractSeries below, which has the same requirement).
+ // extractSeries below, which has the same requirement). The pivoted columns
+ // keep their raw names (verbose mapping only applies to an exact metric
+ // label), and must be excluded too so a sort-only metric is neither
+ // rendered as series nor counted in stacked totals.
const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData)
.map(getMetricLabel)
- .map(label => verboseMap[label] ?? label);
+ .flatMap(label => [verboseMap[label] ?? label, ...pivotedColumnsOf(label)]);
const { totalStackedValues, thresholdValues } = extractDataTotalValues(
rebasedData,
{
@@ -480,6 +502,35 @@ export default function transformProps(
);
const isMultiSeries = groupBy.length || metrics?.length > 1;
+ // `x_axis_sort` stores either a series aggregate (a `SortSeriesType`) or
+ // the label of the x-axis column or of a metric. A single-series chart is
+ // sorted by the backend sort operator; with several series the rows are
+ // ordered here instead: by axis value, by an aggregate over every series,
+ // or by the sum of the chosen metric's columns. The field is carried
+ // through as-is, so a column or metric that happens to be named like an
+ // aggregate is never guessed at.
+ const sortSeriesTypes = new Set<string>(Object.values(SortSeriesType));
+ const resolveXAxisSortSeries = (): XAxisSortSeries | undefined => {
+ if (!isMultiSeries || typeof xAxisSort !== 'string') {
+ return undefined;
+ }
+ if (sortSeriesTypes.has(xAxisSort)) {
+ return xAxisSort as SortSeriesType;
+ }
+ if (
+ xAxisSort === xAxisLabel ||
+ xAxisSort === getXAxisLabel(chartProps.rawFormData)
+ ) {
+ return SortSeriesType.Name;
+ }
+ const dataColumns = new Set(Object.keys(rebasedData[0] ?? {}));
+ const sumOfColumns = [
+ verboseMap[xAxisSort] ?? xAxisSort,
+ ...pivotedColumnsOf(xAxisSort),
+ ].filter(column => dataColumns.has(column));
+ return sumOfColumns.length ? { sumOfColumns } : undefined;
+ };
+ const xAxisSortSeries = resolveXAxisSortSeries();
const xAxisDataType = dataTypes?.[xAxisLabel] ?? dataTypes?.[xAxisOrig];
const xAxisType = getAxisType(
stack,
@@ -499,8 +550,10 @@ export default function transformProps(
isHorizontal,
sortSeriesType,
sortSeriesAscending,
- xAxisSortSeries: isMultiSeries ? xAxisSort : undefined,
- xAxisSortSeriesAscending: isMultiSeries ? xAxisSortAsc : undefined,
+ xAxisSortSeries,
+ xAxisSortSeriesAscending: isDefined(xAxisSortSeries)
+ ? xAxisSortAsc
+ : undefined,
xAxisType,
},
);
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index 6ededf12ac3..95789295a23 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -519,13 +519,39 @@ export function sortAndFilterSeries(
).map(({ name }) => name);
}
+/**
+ * Sum of a specific set of columns, used to order the rows of a multi-series
+ * chart by one metric once dimensions have pivoted that metric into a column
+ * per series.
+ */
+export type XAxisSortBySumOfColumns = { sumOfColumns: string[] };
+
+/**
+ * How the rows (one per x-axis value) of a multi-series chart are ordered:
+ * a `SortSeriesType` aggregates every numeric column in the row, while
+ * `XAxisSortBySumOfColumns` only sums the named columns.
+ */
+export type XAxisSortSeries = SortSeriesType | XAxisSortBySumOfColumns;
+
+export function isXAxisSortBySumOfColumns(
+ sort: XAxisSortSeries,
+): sort is XAxisSortBySumOfColumns {
+ return typeof sort === 'object';
+}
+
export function sortRows(
rows: DataRecord[],
totalStackedValues: number[],
xAxis: string,
- xAxisSortSeries: SortSeriesType,
+ xAxisSortSeries: XAxisSortSeries,
xAxisSortSeriesAscending: boolean,
) {
+ const sumOfColumns = isXAxisSortBySumOfColumns(xAxisSortSeries)
+ ? new Set(xAxisSortSeries.sumOfColumns)
+ : undefined;
+ const aggregation: SortSeriesType = sumOfColumns
+ ? SortSeriesType.Sum
+ : (xAxisSortSeries as SortSeriesType);
const sortedRows = rows.map((row, idx) => {
let sortKey: DataRecordValue = '';
let aggregate: number | undefined;
@@ -536,17 +562,18 @@ export function sortRows(
sortKey = value;
}
if (
- xAxisSortSeries === SortSeriesType.Name ||
- typeof value !== 'number'
+ aggregation === SortSeriesType.Name ||
+ typeof value !== 'number' ||
+ (sumOfColumns && !sumOfColumns.has(key))
) {
return;
}
- if (!(xAxisSortSeries === SortSeriesType.Avg && !isValueDefined)) {
+ if (!(aggregation === SortSeriesType.Avg && !isValueDefined)) {
entries += 1;
}
- switch (xAxisSortSeries) {
+ switch (aggregation) {
case SortSeriesType.Avg:
case SortSeriesType.Sum:
if (aggregate === undefined) {
@@ -572,7 +599,7 @@ export function sortRows(
}
});
if (
- xAxisSortSeries === SortSeriesType.Avg &&
+ aggregation === SortSeriesType.Avg &&
entries > 0 &&
aggregate !== undefined
) {
@@ -580,7 +607,7 @@ export function sortRows(
}
const value =
- xAxisSortSeries === SortSeriesType.Name
+ aggregation === SortSeriesType.Name
? typeof sortKey === 'string'
? sortKey.toLowerCase()
: sortKey
@@ -613,7 +640,7 @@ export function extractSeries(
isHorizontal?: boolean;
sortSeriesType?: SortSeriesType;
sortSeriesAscending?: boolean;
- xAxisSortSeries?: SortSeriesType;
+ xAxisSortSeries?: XAxisSortSeries;
xAxisSortSeriesAscending?: boolean;
xAxisType?: AxisType;
} = {},
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts
index 4c3cf309e95..5138a251868 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/buildQuery.test.ts
@@ -64,6 +64,49 @@ describe('Timeseries buildQuery', () => {
expect(query.metrics).toEqual(['bar', 'baz']);
});
+ test('should query the sort-only limit metric with dimensions so its pivoted columns can order the axis', () => {
+ const queryContext = buildQuery({
+ ...formData,
+ metrics: ['count'],
+ x_axis: 'genre',
+ groupby: ['platform'],
+ timeseries_limit_metric: 'na_sales',
+ x_axis_sort: 'na_sales',
+ x_axis_sort_asc: false,
+ });
+ const [query] = queryContext.queries;
+ expect(query.metrics).toEqual(['count', 'na_sales']);
+ const pivot = (query.post_processing || []).find(
+ op => op?.operation === 'pivot',
+ );
+ expect(pivot?.options).toMatchObject({
+ index: ['genre'],
+ columns: ['platform'],
+ aggregates: {
+ count: { operator: 'mean' },
+ na_sales: { operator: 'mean' },
+ },
+ });
+ // With dimensions the chart sorts the pivoted rows itself.
+ expect(
+ (query.post_processing || []).map(op => op?.operation),
+ ).not.toContain('sort');
+ });
+
+ test('should not query the limit metric with dimensions when the axis is sorted by a series aggregate', () => {
+ const queryContext = buildQuery({
+ ...formData,
+ metrics: ['count'],
+ x_axis: 'genre',
+ groupby: ['platform'],
+ timeseries_limit_metric: 'na_sales',
+ x_axis_sort: 'sum',
+ x_axis_sort_asc: false,
+ });
+ const [query] = queryContext.queries;
+ expect(query.metrics).toEqual(['count']);
+ });
+
test('should apply contribution before rename with time comparison', () => {
// rename strips the `__<offset>` suffix that contribution relies on to
// compute each time shift separately
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/xAxisSortWithDimensions.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/xAxisSortWithDimensions.test.ts
new file mode 100644
index 00000000000..ebc0c4cae50
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/xAxisSortWithDimensions.test.ts
@@ -0,0 +1,250 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ ChartDataResponseResult,
+ DataRecord,
+ QueryFormMetric,
+} from '@superset-ui/core';
+import { SortSeriesType } from '@superset-ui/chart-controls';
+import transformProps from '../../src/Timeseries/transformProps';
+import { DEFAULT_FORM_DATA } from '../../src/Timeseries/constants';
+import {
+ EchartsTimeseriesChartProps,
+ EchartsTimeseriesFormData,
+ EchartsTimeseriesSeriesType,
+} from '../../src/Timeseries/types';
+import { createEchartsTimeseriesTestChartProps } from '../helpers';
+
+// A bar chart of COUNT(*) per genre, split by platform, with the axis sorted
+// by a metric that is neither the value metric nor a dimension (#34352).
+// The rows are what the backend returns for that query: the value metric's
+// columns lose their metric label (`truncate_metric`), the sort-only metric
+// keeps it, and `label_map` records the structure behind each column.
+const naSales: QueryFormMetric = {
+ expressionType: 'SIMPLE',
+ aggregate: 'SUM',
+ column: { column_name: 'na_sales' },
+ label: 'SUM(na_sales)',
+};
+
+const rows: DataRecord[] = [
+ {
+ genre: 'Action',
+ PS4: 30,
+ XOne: 20,
+ 'SUM(na_sales), PS4': 5,
+ 'SUM(na_sales), XOne': 3,
+ },
+ {
+ genre: 'Puzzle',
+ PS4: 5,
+ XOne: 5,
+ 'SUM(na_sales), PS4': 20,
+ 'SUM(na_sales), XOne': 10,
+ },
+ {
+ genre: 'Sports',
+ PS4: 40,
+ XOne: 10,
+ 'SUM(na_sales), PS4': 1,
+ 'SUM(na_sales), XOne': 1,
+ },
+];
+
+const labelMap = {
+ genre: ['genre'],
+ PS4: ['PS4'],
+ XOne: ['XOne'],
+ 'SUM(na_sales), PS4': ['SUM(na_sales)', 'PS4'],
+ 'SUM(na_sales), XOne': ['SUM(na_sales)', 'XOne'],
+};
+
+const queriesData = [
+ {
+ annotation_data: null,
+ cache_key: null,
+ cache_timeout: null,
+ cached_dttm: null,
+ queried_dttm: null,
+ data: rows,
+ colnames: [],
+ coltypes: [],
+ error: null,
+ is_cached: false,
+ query: '',
+ rowcount: rows.length,
+ sql_rowcount: rows.length,
+ stacktrace: null,
+ status: 'success',
+ from_dttm: null,
+ to_dttm: null,
+ label_map: labelMap,
+ } as unknown as ChartDataResponseResult,
+];
+
+type Series = {
+ name: string;
+ data: [string, number][];
+ label: {
+ formatter: (params: {
+ value: [string, number];
+ dataIndex: number;
+ seriesIndex: number;
+ }) => string;
+ };
+};
+
+// transformProps reads camelCase keys from `formData` and snake_case keys
+// from `rawFormData`; the test helper hands the same object to both.
+function transform(
+ overrides: Record<string, unknown>,
+ verboseMap = {},
+ data = rows,
+) {
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsTimeseriesFormData,
+ EchartsTimeseriesChartProps
+ >({
+ defaultFormData: DEFAULT_FORM_DATA,
+ defaultVizType: 'echarts_timeseries_bar',
+ formData: {
+ colorScheme: 'bnbColors',
+ seriesType: EchartsTimeseriesSeriesType.Bar,
+ x_axis: 'genre',
+ xAxis: 'genre',
+ metrics: ['count'],
+ groupby: ['platform'],
+ timeseries_limit_metric: naSales,
+ truncate_metric: true,
+ stack: true,
+ showValue: true,
+ onlyTotal: true,
+ ...overrides,
+ } as Partial<EchartsTimeseriesFormData>,
+ queriesData:
+ data === rows
+ ? queriesData
+ : [{ ...queriesData[0], data } as ChartDataResponseResult],
+ datasource: { verboseMap },
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as Series[];
+ return {
+ series,
+ // Legend order is a separate control; only the set of series matters here.
+ seriesNames: series.map(({ name }) => name).sort(),
+ axisOrder: series[0].data.map(([genre]) => genre),
+ // With `onlyTotal` the top series' label carries the stacked total.
+ stackedTotals: series
+ .flatMap((entry, seriesIndex) =>
+ entry.data.map((value, dataIndex) =>
+ entry.label.formatter({ value, dataIndex, seriesIndex }),
+ ),
+ )
+ .filter(label => label !== ''),
+ };
+}
+
+test('sorts the axis by the sort-only metric across its pivoted columns', () => {
+ const { axisOrder, seriesNames, stackedTotals } = transform({
+ x_axis_sort: 'SUM(na_sales)',
+ xAxisSort: 'SUM(na_sales)',
+ x_axis_sort_asc: false,
+ xAxisSortAsc: false,
+ });
+ // SUM(na_sales) totals: Puzzle 30, Action 8, Sports 2. Sorting by the value
+ // metric would put Action/Sports first, sorting by name would put Puzzle
+ // second.
+ expect(axisOrder).toEqual(['Puzzle', 'Action', 'Sports']);
+ expect(seriesNames).toEqual(['PS4', 'XOne']);
+ // COUNT(*) totals only, in the sorted order.
+ expect(stackedTotals).toEqual(['10', '50', '50']);
+});
+
+test('sorts the axis by the sort-only metric ascending', () => {
+ const { axisOrder } = transform({
+ x_axis_sort: 'SUM(na_sales)',
+ xAxisSort: 'SUM(na_sales)',
+ x_axis_sort_asc: true,
+ xAxisSortAsc: true,
+ });
+ expect(axisOrder).toEqual(['Sports', 'Action', 'Puzzle']);
+});
+
+test('sorts by the sort-only metric when it has a verbose name', () => {
+ const { axisOrder, seriesNames } = transform(
+ {
+ x_axis_sort: 'SUM(na_sales)',
+ xAxisSort: 'SUM(na_sales)',
+ x_axis_sort_asc: false,
+ xAxisSortAsc: false,
+ },
+ { 'SUM(na_sales)': 'NA Sales' },
+ );
+ expect(axisOrder).toEqual(['Puzzle', 'Action', 'Sports']);
+ expect(seriesNames).toEqual(['PS4', 'XOne']);
+});
+
+test('sorts the axis by the x-axis column with dimensions set', () => {
+ expect(
+ transform({
+ x_axis_sort: 'genre',
+ xAxisSort: 'genre',
+ x_axis_sort_asc: false,
+ xAxisSortAsc: false,
+ }).axisOrder,
+ ).toEqual(['Sports', 'Puzzle', 'Action']);
+ expect(
+ transform({
+ x_axis_sort: 'genre',
+ xAxisSort: 'genre',
+ x_axis_sort_asc: true,
+ xAxisSortAsc: true,
+ }).axisOrder,
+ ).toEqual(['Action', 'Puzzle', 'Sports']);
+});
+
+test('still sorts the axis by a series aggregate with dimensions set', () => {
+ // An aggregate sort does not query the sort-only metric, so its columns
+ // are not in the response.
+ const valueRows = rows.map(({ genre, PS4, XOne }) => ({ genre, PS4, XOne }));
+ const { axisOrder, seriesNames } = transform(
+ {
+ x_axis_sort: SortSeriesType.Max,
+ xAxisSort: SortSeriesType.Max,
+ x_axis_sort_asc: true,
+ xAxisSortAsc: true,
+ },
+ {},
+ valueRows,
+ );
+ // Max per row: Puzzle 5, Action 30, Sports 40.
+ expect(axisOrder).toEqual(['Puzzle', 'Action', 'Sports']);
+ expect(seriesNames).toEqual(['PS4', 'XOne']);
+});
+
+test('leaves the query order alone when the sort field is not in the data', () => {
+ const { axisOrder } = transform({
+ x_axis_sort: 'jp_sales',
+ xAxisSort: 'jp_sales',
+ x_axis_sort_asc: false,
+ xAxisSortAsc: false,
+ });
+ expect(axisOrder).toEqual(['Action', 'Puzzle', 'Sports']);
+});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
index 78e88961a00..03af2ee7b27 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -333,6 +333,55 @@ test('sortRows by max descending', () => {
]);
});
+test('sortRows by the sum of specific columns descending', () => {
+ expect(
+ sortRows(
+ sortData,
+ totalStackedValues,
+ 'my_x_axis',
+ { sumOfColumns: ['x', 'z'] },
+ false,
+ ),
+ ).toEqual([
+ { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+ { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ ]);
+});
+
+test('sortRows by the sum of specific columns ascending ignores the other columns', () => {
+ // `y` would put `foo` last; only `x` and `z` count.
+ expect(
+ sortRows(
+ sortData,
+ totalStackedValues,
+ 'my_x_axis',
+ { sumOfColumns: ['x', 'z'] },
+ true,
+ ),
+ ).toEqual([
+ { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+ ]);
+});
+
+test('sortRows by the sum of specific columns puts rows with no value last when ascending', () => {
+ expect(
+ sortRows(
+ sortData,
+ totalStackedValues,
+ 'my_x_axis',
+ { sumOfColumns: ['x'] },
+ true,
+ ),
+ ).toEqual([
+ { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+ { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+ { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+ ]);
+});
+
test('sortAndFilterSeries by min ascending', () => {
expect(
sortAndFilterSeries(sortData, 'my_x_axis', [], SortSeriesType.Min, true),
@@ -505,6 +554,48 @@ test('extractDataTotalValues still respects legendState alongside extraMetricLab
expect(result.totalStackedValues).toEqual([32]);
});
+test("extractSeries orders rows by the sum of a metric's pivoted columns and hides those columns", () => {
+ const data = [
+ {
+ genre: 'Action',
+ PS4: 30,
+ XOne: 20,
+ 'SUM(na_sales), PS4': 5,
+ 'SUM(na_sales), XOne': 3,
+ },
+ {
+ genre: 'Puzzle',
+ PS4: 5,
+ XOne: 5,
+ 'SUM(na_sales), PS4': 20,
+ 'SUM(na_sales), XOne': 10,
+ },
+ {
+ genre: 'Sports',
+ PS4: 40,
+ XOne: 10,
+ 'SUM(na_sales), PS4': 1,
+ 'SUM(na_sales), XOne': 1,
+ },
+ ];
+ const [series, sortedTotals] = extractSeries(data, {
+ xAxis: 'genre',
+ extraMetricLabels: ['SUM(na_sales), PS4', 'SUM(na_sales), XOne'],
+ totalStackedValues: [50, 10, 50],
+ xAxisSortSeries: {
+ sumOfColumns: ['SUM(na_sales), PS4', 'SUM(na_sales), XOne'],
+ },
+ xAxisSortSeriesAscending: false,
+ });
+ expect(series.map(({ name }) => name).sort()).toEqual(['PS4', 'XOne']);
+ expect(series.find(({ name }) => name === 'PS4')?.data).toEqual([
+ ['Puzzle', 5],
+ ['Action', 30],
+ ['Sports', 40],
+ ]);
+ expect(sortedTotals).toEqual([10, 50, 50]);
+});
+
describe('extractSeries', () => {
test('should generate a valid ECharts timeseries series object', () => {
const data = [