rusackas opened a new pull request, #44550:
URL: https://github.com/apache/superset/pull/44550
### SUMMARY
Adopts #37712 by @YousufFFFF on a fresh branch (credited as co-author),
fixing the two halves of the "X-Axis Sort By" problem on ECharts bar charts
once dimensions are set:
- The dropdown offered a different set of choices with and without
dimensions (#34352): the x-axis column and the metrics disappeared as soon as a
dimension was added, leaving only the series aggregates (`Category name`,
`Total value`, ...).
- Even when a column or metric was selected, the grouped chart didn't sort
by it (#38249), because the multi-series render path only understood the
`SortSeriesType` aggregates.
The original PR tried to close the gap by pattern-matching the selected
value back into one of those five aggregates (`mapXAxisSortToSeriesType`),
which silently fell back to `Total value` ordering whenever the user picked an
actual column or metric. That is the case #34352 is about, so this PR takes a
different route and carries the chosen field through the grouped sort path
instead of guessing an enum:
- **Control** (`xAxisSortControl`): the x-axis column and every metric
(including the query "Sort By" metric) are offered whether or not dimensions
are set; the series aggregates are added on top for multi-series charts.
Options are deduped by value, and a field whose label collides with an
aggregate value (`sum`, `name`, ...) is dropped rather than shadowing it.
`shouldReset` semantics are unchanged, so a metric selection now survives
adding a dimension.
- **Query** (`extractExtraMetrics`): the "Sort By" metric is included in the
query when the axis is sorted by it and it is not already a value metric, with
dimensions too. The pivot then returns one `<metric>, <dimension values>`
column per series. The backend `sort` post-processing keeps handling the
no-dimension case only.
- **Render** (`transformProps` / `extractSeries` / `sortRows`): with several
series, `x_axis_sort` resolves to one of three things: a `SortSeriesType`
aggregate (as before), the x-axis column (sort by axis value), or a metric,
whose pivoted columns are looked up through the response's `label_map` (entries
whose first part is that metric) and summed per row. `sortRows` gets a typed `{
sumOfColumns }` option for that; there is no string matching on column names
anywhere. The same `label_map` lookup extends the sort-only metric exclusion,
so its pivoted columns are neither rendered as series nor counted in stacked
totals.
Sorting by a metric with dimensions therefore orders the axis exactly as the
same chart without the dimension does, and the order no longer changes when a
dimension is added or removed.
Fixes #34352
Fixes #38249
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
See the three screenshots in
https://github.com/apache/superset/pull/37712#issuecomment-3873527775 for the
"before": the third chart (dimension added) reverts to alphabetical order
despite X-Axis Sort By being set. After this change the third chart keeps the
order of the second.
### TESTING INSTRUCTIONS
Repro from @sfirke on the original PR, using the example `video_game_sales`
dataset:
1. Create a Bar Chart: X-Axis `genre`, Metric `COUNT(*)`, no dimension. Note
the alphabetical order.
2. In the query section set Sort By to `SUM(na_sales)`, then in Customize
set X-Axis Sort By to `SUM(na_sales)` (descending). The bars reorder by North
America sales.
3. Add `platform` as a Dimension. The X-Axis Sort By dropdown still lists
`genre`, `COUNT(*)` and `SUM(na_sales)` (plus the series aggregates), the
selection is kept, and the bar groups stay in the same order as step 2. Remove
the dimension again: the order does not change.
4. With the dimension set, switch X-Axis Sort By to `genre` and to `Total
value` to confirm both still sort, and toggle ascending.
5. Stacked bars with "Show Value" + "Only Total": the totals are `COUNT(*)`
only, the sort metric is not added in.
Unit tests cover the option set with/without dimensions (dedupe, verbose
labels, `shouldReset`), `extractExtraMetrics` with groupby, `buildQuery`
including the sort metric with dimensions, `sortRows`/`extractSeries` sorting
by a set of columns (asc/desc/null rows), and a Timeseries `transformProps`
test modeled on the repro above asserting row order, rendered series and
stacked totals.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [x] Has associated issue: Fixes #34352, Fixes #38249
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Known limitation carried over from the single-series path: with
"Contribution Mode" set, the backend contribution operator normalizes across
every pivoted column, sort-only metric columns included, so percentages can be
skewed when sorting by a metric that is not a value metric. That was already
the case without dimensions and is left for a follow-up.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
