Copilot commented on code in PR #38120:
URL: https://github.com/apache/superset/pull/38120#discussion_r2897394326
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.tsx:
##########
@@ -129,7 +129,10 @@ export default function EchartsTimeseries({
values.length === 0
? []
: groupby.map((col, idx) => {
- const val = groupbyValues.map(v => v[idx]);
+ const val = groupbyValues.map(v => {
+ const metricsCount = v.length - groupby.length;
+ return v[metricsCount + idx];
+ });
if (val === null || val === undefined)
Review Comment:
The offset logic for multi-metric labelMap entries is updated here, but
there doesn’t appear to be any unit test coverage exercising
`EchartsTimeseries` cross-filter click behavior (especially for stacked
multi-metric series). Consider adding a focused test (or extracting this
indexing into a shared helper that can be unit-tested) to prevent regressions
for the original bug scenario.
```suggestion
if (val.every(vv => vv == null))
```
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/EchartsMixedTimeseries.tsx:
##########
@@ -80,7 +80,11 @@ export default function EchartsMixedTimeseries({
? []
: currentGroupBy.map((col, idx) => {
const val: DataRecordValue[] = groupbyValues.map(
- v => v[idx],
+ v => {
+ const metricsCount =
+ v.length - currentGroupBy.length;
+ return v[metricsCount + idx];
+ },
);
Review Comment:
The cross-filter labelMap indexing logic changed here, but there are no
tests covering `EchartsMixedTimeseries` click-to-cross-filter behavior for
multi-metric stacked series (including multiple groupby columns). Adding a
small unit test (or factoring the indexing into a shared, tested helper) would
help ensure this fix doesn’t regress.
--
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]
