codeant-ai-for-open-source[bot] commented on code in PR #38451:
URL: https://github.com/apache/superset/pull/38451#discussion_r3831182326
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -422,17 +424,23 @@ export default function transformProps(
let displayName: string;
- if (groupby.length > 0) {
- // When we have groupby, format as "metric, dimension"
- const metricPart: string = showQueryIdentifiers
- ? `${MetricDisplayNameA} (Query A)`
- : MetricDisplayNameA;
- displayName = entryName.includes(metricPart)
- ? entryName
- : `${metricPart}, ${entryName}`;
+ if (truncateMetric && groupby.length > 0) {
+ const groupbyValues = labelMap?.[seriesName] || [];
+ displayName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
Review Comment:
Yes. The truncated branch should append the query identifier when
`showQueryIdentifiers` is enabled, while still omitting the metric:
```typescript
if (truncateMetric && groupby.length > 0) {
const groupbyValues = labelMap?.[seriesName] || [];
const truncatedName =
groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
displayName = showQueryIdentifiers
? `${truncatedName} (Query A)`
: truncatedName;
}
```
Apply the equivalent logic for Query B:
```typescript
if (truncateMetricB && groupbyB.length > 0) {
const groupbyValues =
labelMapB?.[seriesEntry] || labelMapB?.[entryName] || [];
const truncatedName =
groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
displayName = showQueryIdentifiers
? `${truncatedName} (Query B)`
: truncatedName;
}
```
Add coverage for `truncateMetric` and `truncateMetricB` with
`showQueryIdentifiers: true`, expecting names such as `boy (Query A)` and `boy
(Query B)`. This preserves the truncation behavior while ensuring legends and
tooltips remain distinguishable between queries.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -495,17 +503,24 @@ export default function transformProps(
let displayName: string;
- if (groupbyB.length > 0) {
- // When we have groupby, format as "metric, dimension"
- const metricPart: string = showQueryIdentifiers
- ? `${MetricDisplayNameB} (Query B)`
- : MetricDisplayNameB;
- displayName = entryName.includes(metricPart)
- ? entryName
- : `${metricPart}, ${entryName}`;
+ if (truncateMetricB && groupbyB.length > 0) {
+ const groupbyValues =
+ labelMapB?.[seriesEntry] || labelMapB?.[entryName] || [];
+ displayName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
Review Comment:
Yes, this is a valid issue. Query B already computes the suffixed key as
`seriesName`, and the truncation lookup should use the same key as the
formatter lookup.
Update it to prefer `seriesName`, while retaining the existing fallbacks for
compatibility:
```typescript
const groupbyValues =
labelMapB?.[seriesName] ||
labelMapB?.[seriesEntry] ||
labelMapB?.[entryName] ||
[];
```
This ensures `truncateMetricB` works when `labelMapB` is keyed by the
secondary series name (`${seriesEntry} (1)`), while preserving support for
unsuffixed response keys.
--
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]
