rusackas commented on code in PR #44657:
URL: https://github.com/apache/superset/pull/44657#discussion_r4249741177
##########
superset-frontend/src/explore/components/ExploreChartPanel/index.tsx:
##########
@@ -383,6 +384,9 @@ const ExploreChartPanel = ({
padding-top: ${theme.sizeUnit * 2}px;
`}
>
+ {vizType === 'pivot_table_v2' && (
+ <LegacyAggregationAlert sliceId={slice?.slice_id} />
Review Comment:
Added a test in ExploreChartPanel.test.tsx rendering a saved pivot_table_v2
chart (slice 318) whose tags response includes the migration tag, asserting the
notice shows. Not run locally, so CI is the first real run.
##########
UPDATING.md:
##########
@@ -1806,9 +1804,28 @@ Fraction of ..." variants are **not** migrated: they
divided a record count,
while the new control divides the metric's own value, so translating them
automatically would silently change what the chart displays rather than
restore it; those charts need to be manually reconfigured if the value-based
-percentage is what's wanted. Charts that used any other non-fraction
-`aggregateFunction` value (Sum, Average, Count, ...) are unaffected, since
-that specific behavior remains unavailable per the above.
+percentage is what's wanted.
+
+The **"Aggregation function"** control itself (form_data field
+`aggregateFunction`) is also back, as a second aggregation pass over a
+metric's own grouped results -- e.g. the median of a set of per-store
+`SUM(sales)` values -- computed correctly this time: every scope (subtotal,
+row/column total, grand total) reduces its own original contributing query
+results, never another scope's already-displayed value, so the previous
+non-additive-metric bug this section originally removed the control for does
+not return. It reuses the exact same field name and value spellings as
+before, so a chart that still had e.g. `aggregateFunction: "Median"` sitting
+unused in its saved `params` starts computing totals with that aggregation
+again automatically, with no action required and no value to reconfigure. A
+one-time migration tags every such chart with a
`legacy-pivot-aggregation-restored`
+custom tag; opening a tagged chart in Explore shows a notice that its totals
+may now look different, which clears once you review and accept it (or save
+the chart). Accepting only removes the tag; it does not rebuild the saved
+`query_context` that alerts, reports and cache warm-up keep using. If you have
+alerts/reports built on one of these charts, re-save each affected chart in
+Explore first, then validate its scheduled outputs, since a scheduled report
+render does not pass through that notice and Accept alone leaves the stale
+`query_context` in place.
Review Comment:
Applied, the section now says exports and reports do not apply the restored
aggregation yet and links #45215.
--
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]
