sadpandajoe commented on code in PR #43116: URL: https://github.com/apache/superset/pull/43116#discussion_r3819868252
########## superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts: ########## @@ -367,17 +368,31 @@ export default function transformProps( // metric. extraMetricLabels must be mapped the same way, or a sort-only // metric with a verbose_name set would silently fail to match here (and in // extractSeries below, which has the same requirement). - const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData) - .map(getMetricLabel) - .map(label => verboseMap[label] ?? label); + const rawExtraMetricLabels = extractExtraMetrics(chartProps.rawFormData).map( + getMetricLabel, + ); + const extraMetricLabels = rawExtraMetricLabels.map( + label => verboseMap[label] ?? label, + ); + // Time comparison emits a derived column per query metric, and sort-only + // metrics are part of the query, so they get one too. These keep the *raw* + // metric label - rebaseForecastDatum only resolves the forecast context name + // through verboseMap, which never matches a `<label>__<offset>` key. Build + // the exact keys from the configured offsets rather than prefix-matching, so + // a displayed metric that merely looks like a derived one is still counted. + const extraMetricOffsetLabels = rawExtraMetricLabels.flatMap(label => Review Comment: `timeComparePivotOperator` is built from `baseQueryObject` before `extra_metrics` is appended, so `getMetricOffsetsMap` does not include this sort metric in the pivot aggregates. A real time-comparison response therefore cannot contain the derived key this block excludes, while the test injects one manually. Could the query construction include the extra metric in the pivot and cover that path through `buildQuery`? -- 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]
