rusackas opened a new pull request, #42810:
URL: https://github.com/apache/superset/pull/42810

   ### SUMMARY
   Follow-up to #42761 (pivot table "Show values as" percent display), fixing 
two edge cases flagged in review that weren't small enough to fold into that PR 
without risking a rushed change to already-reviewed code:
   
   - **Cross-metric total collision.** `processRecord`'s "Metric-collapse 
totals" mirrors each metric's grand-total record into a single shared 
`allTotal`/`rowTotals`/`colTotals` slot when no real dimension is left to key 
on (documented in-code as deliberately deferred "future work" for actual-values 
mode). In fraction mode this is worse than a stale display: `metricAxis` locks 
onto the first metric pushed into that slot, but the underlying value is always 
the *last* metric pushed, so a multi-metric table's grand-total corner cell 
could divide one metric's value by a different metric's total and render a 
wrong-but-plausible percentage (concretely: with metric A's grand total of 30 
and metric B's of 300, the corner cell showed `1,000.0%`). This PR detects the 
collision on push and blanks the cell instead of showing a misleading number. 
Actual-values mode is unchanged and still has the pre-existing "future work" 
limitation — this only prevents fraction mode from making it worse.
   - **NULL metric value renders as "0.0%".** A DB-computed value can be a 
genuine SQL NULL (e.g. `AVG` over an empty group), which renders blank in 
"Actual values" mode. In fraction mode, `null / acc` coerces to `0` in JS, so 
the same cell rendered a measured "0.0%" instead of staying blank. Now checked 
explicitly.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A — covered by the new regression tests below; both were confirmed to fail 
against the pre-fix code (corner cell showed `1,000.0%`, null cell showed 
`0.0%`) before being fixed.
   
   ### TESTING INSTRUCTIONS
   `npx jest 
plugins/plugin-chart-pivot-table/test/react-pivottable/tableRenders.test.tsx 
--runInBand` — two new tests: `blanks the grand-total corner cell when it mixes 
multiple metrics in fraction mode` and `keeps a null metric value blank in 
fraction mode instead of showing 0.0%`.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   Base is `feat/pivot-table-fraction-of` (not `master`) since this fixes code 
introduced there and hasn't landed yet — will retarget to `master` once #42761 
merges, or this can be merged into that branch directly if easier.
   
   A third item from the same review round — CSV/XLSX exports and scheduled 
reports not respecting `showValuesAs` — needs a real design decision in the 
export/report query path rather than a quick fix, so it's tracked separately as 
#42809 instead of bundled here.


-- 
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]

Reply via email to