EnxDev commented on PR #41387: URL: https://github.com/apache/superset/pull/41387#issuecomment-4834362023
## EnxDev's Review Agent — apache/superset#41387 · HEAD cbcf755 lgtm — correct, targeted fix for the percent-metric summary row, with a real regression guard. CI green. Verified the fix closes the loop. The totals query (`columns: []`, single aggregated row) now keeps the `contribution` op, so `contribution.py` renames `revenue` → `%revenue` and the totals object gains the `%`-prefixed key the grid reads (`transformProps.ts:721` → `totals['%revenue']`, column key built at `transformProps.ts:356`). Before, `post_processing: []` left totals keyed only under `revenue`, so the grid found nothing → `0.000%`. The `.filter(operation === 'contribution')` correctly retains the rename while dropping the time-comparison operator, matching the cleared `orderby`/`order_desc`. ### 🔵 Nits - `buildQuery.ts:661` — by construction the summary always renders **100%** for the percent column (single-row contribution: `value / sum([value]) = 1.0`, see `contribution.py:95`). That's the consistent "sum of contributions" semantics and reads fine, just confirming it's intended vs. the legacy `plugin-chart-table`, whose totals query still uses `post_processing: []` (`buildQuery.ts:328`) and leaves the percent total blank. This PR is arguably the better behavior — worth aligning the two plugins eventually. - `test/buildQuery.test.ts:1383,1390` — `(op: any)` / `as any` casts; minor, and consistent with the surrounding suite. ### 🙌 Praise - The added test is a genuine red-before/green-after guard — it asserts the `contribution` op survives on the totals query and that `rename_columns` contains `%revenue`, which is exactly the dropped behavior. - `buildQuery.ts:655` — clear comment tying the change to the symptom and #104600, and noting why the time-comparison op is intentionally dropped. <!-- enxdev-review-agent:cbcf755 --> _Reviewed by EnxDev's Review Agent — @EnxDev · HEAD cbcf755._ -- 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]
