sadpandajoe opened a new pull request, #45306: URL: https://github.com/apache/superset/pull/45306
### SUMMARY With non-additive metrics (for example `AVG`), the Pivot Table gets its totals and subtotals from rollup levels returned by the chart query (`grouping_sets`, see SIP-216). Which levels are requested is derived from the **Show rows total / Show rows subtotal / Show columns total / Show columns subtotal** options. Two problems followed from that: 1. **The options did not requery.** All four controls were `renderTrigger`, so toggling one re-rendered the stored result without a new query. The table drew Total rows/columns and the group collapse toggles, but the total cells were empty and no Subtotal lines appeared until a requery was forced. In that state, collapsing a group hid the whole group instead of collapsing it to its subtotal row. 2. **Row subtotals depended on "Show columns total".** With metrics applied on rows, the renderer places the metric pseudo-dimension first in the row hierarchy, so the fully collapsed row level is drawn as each metric group's "Subtotal" line. `buildGroupbyCombinations` only requested that level for "Show columns total", so with only "Show rows subtotal" the generated query was identical to the no-options query (same cache entry) and no subtotal rows were ever fetched. The mirror case is "Show columns subtotal" with metrics applied on columns, which was gated only on "Show rows total". ### ROOT CAUSE - **Requery:** `renderTrigger: true` on the four controls dates from when every total was computed client-side from leaf rows (#24960). SIP-216 (#41184) made non-additive totals database-computed and chosen per toggle, but left the flag on. #42761 hit the same class of problem for `showValuesAs` by adding that control without `renderTrigger`; the four toggles were not revisited. - **Missing level:** `rowPrefixNeeded([])` / `colPrefixNeeded([])` in `buildGroupbyCombinations` assumed the renderer's hierarchy equals `groupbyRows` / `groupbyColumns`. It does not once the metric dimension is prepended to the metric axis. ### CHANGES - `controlPanel.tsx`: remove `renderTrigger` from `rowTotals`, `rowSubTotals`, `colTotals` and `colSubTotals`. Toggling them now marks the chart as needing an update, and Run fetches the levels they need (same treatment as `showValuesAs`). Charts whose metrics are all additive also need Run after a toggle now. - `utilities.ts`: also request the collapsed metric-axis level for the metric axis's own subtotal option. It is skipped when metrics are combined (`combineMetric`), because no per-metric subtotal line exists there and a percent mode would otherwise carry the extra level through the combined-metrics filter and break rendering. - Tests: updated the "row subtotals on, everything else off" pruning expectation and added tests for the control flags, the requested levels, the generated query, and the combined-metrics percent cases (levels unchanged, chart renders). Run against the previous code, the control, level and query tests fail. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF UI QA evidence for this change follows below once recorded against this branch. **Before:** with Rows `territory`, Columns `quarter`, metrics `SUM(sales)` and `AVG(sales)` applied on rows, checking "Show rows subtotal" shows the group toggles but no Subtotal lines and does not change the query; checking "Show columns total" adds an empty Total row until the query is forced to rerun; "Show rows subtotal" alone never shows Subtotal lines. **After:** each option marks the chart as needing an update; after Run, "Show rows subtotal" alone shows a Subtotal line under each row group, the Total row is filled in, and collapsing a group leaves its subtotal row. ### TESTING INSTRUCTIONS 1. Create a Pivot Table on a dataset with a numeric column, for example Vehicle Sales: Rows `territory`, Columns `quarter`, Metrics `SUM(sales)` and `AVG(sales)`, "Apply metrics on" Rows. Run it. 2. Check **Show rows subtotal** only. The chart is marked as needing an update; click Run. Each territory group now shows a Subtotal line. 3. Uncheck it, check **Show columns total** only, and Run. The Total row is filled in. 4. Check all four options and Run; totals and subtotals are shown, and collapsing a group leaves its subtotal row. 5. Switch "Apply metrics on" to Columns and repeat with **Show columns subtotal** and **Show rows total**. 6. `cd superset-frontend && npx jest plugins/plugin-chart-pivot-table` ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] 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 -- 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]
