rusackas opened a new pull request, #45555: URL: https://github.com/apache/superset/pull/45555
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY Adopts #38451 from @MelikHajlawi, which fixes `truncate_metric`/`truncate_metricB` having no effect on the Mixed Timeseries chart's series names. With a groupby set, a grouped series always rendered as `"<metric>, <dimension>"` regardless of the Truncate Metric checkbox, unlike the single-query Timeseries chart. Adds `truncateMetric`/`truncateMetricB` handling in `MixedTimeseries/transformProps.ts`: when the flag is on and there's a groupby, the series name drops the metric prefix and shows only the groupby/dimension value(s), reusing the same metric-vs-dimension split the existing `"metric, dimension"` branch already applies to the label-map tuple. This runs ahead of the shared `displayLabelMap`/`displayLabelMapB` population, so formatter lookups keyed by the truncated name still resolve correctly. This PR also resolves everything raised across the original PR's review threads: - The query-identifier suffix (`"(Query A)"`/`"(Query B)"`) is now preserved when truncating and `showQueryIdentifiers` is on, so Query A and Query B stay distinguishable when they share a groupby value (sadpandajoe/codeant, Aug 21). - `displayLabelMap`/`displayLabelMapB` population already covers the truncated name on current `master` (this was a real gap back when the original PR was opened, but `master` has since grown a shared population step that runs after either branch sets `displayName`). - The `any[]` test-assertion nit, the duplicate-debug-log and stray generated files from early revisions, and the "no groupby → blank name" case are all already clean/resolved in this rebuild. - One thread asked whether `DEFAULT_FORM_DATA`'s `truncateMetric`/`truncateMetricB` should default to `true` to match the shared control's default. Deliberately left as `false`: the sibling `renameOperator` (which implements the same setting server-side for the single-query Timeseries chart) explicitly treats an *omitted* field as off (`truncate_metric !== undefined && !!truncate_metric`), not as the control's UI default. The control's `default: true` only seeds a brand-new chart's checkbox state in Explore; defaulting the transform layer to `true` would silently start truncating every existing saved Mixed Timeseries chart with a groupby the next time it renders. Flagging this explicitly in case there's a reason to revisit it, but matching the backend's own semantics seemed safer. - One thread flagged that `inverted[entryName] || entryName` (used to resolve Query B's label-map key) could theoretically collide if a groupby value happens to equal another column's verbose label. That's pre-existing shared code the untruncated `"metric, dimension"` branch already relies on for the exact same lookup, not something this change introduces, so it's out of scope here and worth its own follow-up if it's a real-world problem. Closes #38451 ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF See #38451 for the original before/after screenshots - this PR is a straight rebuild of that same fix (plus the thread fixes above) against current `master`, with no further UI changes. ### TESTING INSTRUCTIONS 1. Create a Mixed Timeseries chart with a groupby on both queries. 2. Toggle "Truncate Metric" on for Query A and/or Query B and confirm the legend/tooltip series names drop the metric prefix for whichever query has it enabled. 3. With truncation on, also enable "Show Query Identifiers" and confirm series still show `(Query A)`/`(Query B)` so overlapping groupby values from both queries stay distinguishable. 4. `npx jest --runInBand plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts` — the new `truncateMetric / truncateMetricB` describe block covers all of the above. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [x] Has associated issue: Closes #38451 - [ ] 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]
