fix(table): pick totals query positionally so all_records percent metrics don't shift it#42428
fix(table): pick totals query positionally so all_records percent metrics don't shift it#42428rusackas wants to merge 2 commits into
Conversation
…rics don't shift it `buildQuery` inserts an extra query ahead of the totals query in `queriesData` when `percent_metric_calculation` is set to "all_records", so the fixed-index destructuring in `transformProps` grabbed that query instead of the actual totals query, leaving the summary row with the wrong (and unformatted) values. Fixes #39587
dosubot
Bot
added
change:frontend
Code Review Agent Run #365a18Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
…memoized state The added test called transformProps before the pre-existing 'render basic data' test in the same file, evicting the module-level memoizeOne cache that test relies on and flipping its timestamp column formatting. Relocating the test to the end of the suite keeps it from disturbing earlier tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #42428 +/- ##
=======================================
Coverage 65.24% 65.24%
=======================================
Files 2795 2795
Lines 157643 157649 +6
Branches 36061 36066 +5
=======================================
+ Hits 102853 102859 +6
Misses 52814 52814
Partials 1976 1976
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Code Review Agent Run #ca1589Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
1 participant
SUMMARY
When "Percentage metric calculation" is set to "Use all records" and "Show totals" is enabled,
buildQueryinserts an extra query (used to compute percent metrics against the full result set) ahead of the totals query insidequeriesData.transformPropspicked the totals query by a fixed array index, so in this configuration it grabbed the percent-metrics-only query instead, and the summary/totals row ended up with the wrong values (and, since those values didn't line up with the columns' configuredd3NumberFormat, effectively unformatted-looking numbers).This locates the totals query positionally (it's always the last entry in
queriesDatawhen present) instead of assuming a fixed index, so it's no longer displaced by the extra percent-metrics query.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable — logic fix in query result wiring, not a visual/styling change.
TESTING INSTRUCTIONS
Also covered by a new unit test in
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsxthat pins down the correcttotalsquery is picked when the "all records" percent-metric query precedes it inqueriesData.Note: I wasn't able to run the frontend test suite in this environment (broken/hanging local
node_modules), so please treat CI as the source of truth here.ADDITIONAL INFORMATION