rusackas opened a new pull request, #40146:
URL: https://github.com/apache/superset/pull/40146
### SUMMARY
Adds a failing unit test that pins down the residual mixed-chart bug
reported in #37921 ("Residual" follow-up to #37055). **Intentionally lands as a
failing test** — it'll go red in CI and stay red until the duplication is
fixed; that gives us clear regression coverage and a single place to flip
"should be" assertions to "verified" once a fix lands.
### The bug
In
`superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts`,
`MetricDisplayNameA` is hard-coded to `metrics[0]` (line 254). When Query A
has multiple metrics + at least one Group By dimension, the per-series
display-name builder (~line 441) prepends that first-metric label to every
series whose `extractSeries` name doesn't already contain it:
```ts
const metricPart: string = showQueryIdentifiers
? `${MetricDisplayNameA} (Query A)`
: MetricDisplayNameA;
displayName = entryName.includes(metricPart)
? entryName
: `${metricPart}, ${entryName}`;
```
For series belonging to `metrics[1+]` (e.g. `score_two`), the `includes`
check is false, so they end up named `score_one, score_two, A` instead of
`score_two, A`. That is the "first metric duplicated in legend/tooltip" symptom
the OP reported.
A correct fix would derive each series' metric from `labelMap` (which
already maps each output column to its `[metric, dim_value]` tuple) rather than
always falling back to `metrics[0]`.
### What this PR adds
A single test in `transformProps.test.ts` titled `regression #37921:
multi-metric Query A with groupby does not duplicate first metric in series
names`. It:
- Builds a Mixed Chart with `metrics: ['score_one', 'score_two']` +
`groupby: ['category']` on Query A
- Provides 4-column query data (`score_one, A`, `score_one, B`, `score_two,
A`, `score_two, B`) with a matching `label_map`
- Asserts each `(metric, dim_value)` combo appears exactly once with the
**correct** metric prefix
- Adds a smoking-gun assertion that no series name contains both metric
labels concatenated (`/score_one,\s+score_two/`)
### Why a unit test rather than e2e
The bug lives in a pure transformation (`formData` → echarts series config).
A unit test reproduces it in milliseconds, asserts string-level invariants
directly, and pins the contract regardless of how the surrounding rendering
layer evolves. Playwright would be slower, brittler, and the bug isn't
UI-interaction-shaped.
### Companion / related work
- #37921 is the residual issue this test pins down.
- #37055 was the original report — fixed by #37217 (`prevent duplicate
legend entries`), which the OP correctly noted didn't cover the multi-metric +
group-by case.
- #38451 is in the same file but addresses a different bug (truncate_metric
setting). It might or might not incidentally fix this one — verifying that
becomes trivial once this test lands: just rebase #38451 and see if the test
goes green.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A.
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npm run test --
plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts -t
"regression #37921"
```
Expected: test fails on this commit. Specifically, the
`toContain('score_two, A')` and `toContain('score_two, B')` assertions fail
(the actual names are `'score_one, score_two, A'` and `'score_one, score_two,
B'`), and the `not.toMatch(/score_one,\s+score_two/)` assertion fails for both
score_two entries.
### ADDITIONAL INFORMATION
- [ ] Has associated issue: #37921
- [ ] Required feature flags:
- [ ] 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]
