vjymisal0 opened a new pull request, #42881: URL: https://github.com/apache/superset/pull/42881
### SUMMARY On a stacked Timeseries Bar chart (`echarts_timeseries_bar`) with "Show Total"/"Only Total" enabled, the displayed stacked total was silently inflated when the chart also used a `timeseries_limit_metric` (the "Sort by" metric used to order/limit the x-axis) that isn't one of the chart's displayed metrics. `extraMetricLabels` — which correctly excludes series-limit/sort-only metrics when building the actual rendered series via `extractSeries` — was computed in `Timeseries/transformProps.ts` *after* `extractDataTotalValues` ran, and was never passed into it. `extractDataTotalValues` summed every remaining numeric key in each row except the x-axis column, so the sort-only metric's value leaked into the total even though it never appeared in the legend. Fix: - `extractDataTotalValues` (`utils/series.ts`) now accepts an optional `extraMetricLabels` list and excludes those columns the same way it already excludes the x-axis column. - `transformProps.ts` now computes `extraMetricLabels` once, above both call sites that need it (it was already being computed below, for `extractSeries`), instead of after `extractDataTotalValues` had already run without it. This is easy to hit in practice: `timeseries_limit_metric` is the standard way to control x-axis category ordering when the natural order isn't alphabetical (e.g. binned/bucketed labels like `1-3d`, `4-6d`), which is precisely the situation where the sort metric is likely to differ from the displayed metrics. Closes #42701 ### TESTING INSTRUCTIONS Added two unit tests to `test/utils/series.test.ts`: - `extractDataTotalValues excludes extraMetricLabels from the stacked total (#42701)` — reproduces the bug on the unfixed path (verifies the pre-fix behavior with a comment) and asserts the fixed behavior with `extraMetricLabels` passed. - `extractDataTotalValues still respects legendState alongside extraMetricLabels` — confirms the existing legend-toggle exclusion still works together with the new one. Ran locally: ``` npx jest plugins/plugin-chart-echarts/test/utils/series.test.ts npx jest plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts npx oxlint --config oxlint.json --quiet <changed files> ``` All pass (102 + 112 tests, oxlint clean). `tsc --noEmit` on the package reports pre-existing errors from unbuilt monorepo package type declarations and other unrelated files/lines (confirmed identical patterns exist elsewhere in the same untouched file); none are on the lines this PR changes. Manual repro steps are in the linked issue (stacked bar chart, 2+ metrics, `timeseries_limit_metric` set to a non-displayed metric, "Only Total" enabled). ### ADDITIONAL INFORMATION - [x] Has associated issue: #42701 - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
