rusackas commented on code in PR #45555:
URL: https://github.com/apache/superset/pull/45555#discussion_r4343242239
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -544,7 +546,23 @@ export default function transformProps(
let displayName: string;
- if (groupby.length > 0) {
+ if (truncateMetric && groupby.length > 0) {
+ // Drop the metric part entirely and show only the groupby/dimension
+ // values. The label-map tuple is [metric, ...dimensions] only when the
+ // query has more than one metric; with a single metric the whole
+ // tuple is dimensions. Still apply the query-identifier suffix when
asked
+ // for, or Query A and Query B become indistinguishable whenever they
+ // share a groupby value (#38451).
+ const groupbyValues =
+ labelMapValues && ensureIsArray(metrics).length > 1
+ ? labelMapValues.slice(1)
+ : (labelMapValues ?? []);
+ const truncatedName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
+ displayName = showQueryIdentifiers
+ ? `${truncatedName} (Query A)`
+ : truncatedName;
+ } else if (groupby.length > 0) {
Review Comment:
With `truncateMetric` on and identifiers off, the same name on A and B is
what the user asked for; `showQueryIdentifiers` is the toggle that
disambiguates, and with it on the suffix is kept. Leaving as is.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -651,7 +669,23 @@ export default function transformProps(
let displayName: string;
- if (groupbyB.length > 0) {
+ if (truncateMetricB && groupbyB.length > 0) {
+ // Drop the metric part entirely and show only the groupby/dimension
+ // values. The label-map tuple is [metric, ...dimensions] only when the
+ // query has more than one metric; with a single metric the whole
+ // tuple is dimensions. Still apply the query-identifier suffix when
asked
+ // for, or Query A and Query B become indistinguishable whenever they
+ // share a groupby value (#38451).
+ const groupbyValues =
+ labelMapValuesB && ensureIsArray(metricsB).length > 1
+ ? labelMapValuesB.slice(1)
+ : (labelMapValuesB ?? []);
+ const truncatedName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
+ displayName = showQueryIdentifiers
+ ? `${truncatedName} (Query B)`
+ : truncatedName;
+ } else if (groupbyB.length > 0) {
Review Comment:
Same as above: `showQueryIdentifiers` keeps A and B distinct when
truncating, so no extra uniqueness logic here.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -544,7 +546,23 @@ export default function transformProps(
let displayName: string;
- if (groupby.length > 0) {
+ if (truncateMetric && groupby.length > 0) {
+ // Drop the metric part entirely and show only the groupby/dimension
+ // values. The label-map tuple is [metric, ...dimensions] only when the
+ // query has more than one metric; with a single metric the whole
+ // tuple is dimensions. Still apply the query-identifier suffix when
asked
+ // for, or Query A and Query B become indistinguishable whenever they
+ // share a groupby value (#38451).
+ const groupbyValues =
+ labelMapValues && ensureIsArray(metrics).length > 1
+ ? labelMapValues.slice(1)
+ : (labelMapValues ?? []);
+ const truncatedName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
+ displayName = showQueryIdentifiers
+ ? `${truncatedName} (Query A)`
+ : truncatedName;
+ } else if (groupby.length > 0) {
Review Comment:
Extracted `getTruncatedSeriesName` and reuse it for both queries.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -651,7 +669,23 @@ export default function transformProps(
let displayName: string;
- if (groupbyB.length > 0) {
+ if (truncateMetricB && groupbyB.length > 0) {
+ // Drop the metric part entirely and show only the groupby/dimension
+ // values. The label-map tuple is [metric, ...dimensions] only when the
+ // query has more than one metric; with a single metric the whole
+ // tuple is dimensions. Still apply the query-identifier suffix when
asked
+ // for, or Query A and Query B become indistinguishable whenever they
+ // share a groupby value (#38451).
+ const groupbyValues =
+ labelMapValuesB && ensureIsArray(metricsB).length > 1
+ ? labelMapValuesB.slice(1)
+ : (labelMapValuesB ?? []);
+ const truncatedName =
+ groupbyValues.length > 0 ? groupbyValues.join(', ') : entryName;
+ displayName = showQueryIdentifiers
+ ? `${truncatedName} (Query B)`
+ : truncatedName;
+ } else if (groupbyB.length > 0) {
Review Comment:
Extracted `getTruncatedSeriesName` and reuse it for both queries.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -544,7 +546,23 @@ export default function transformProps(
let displayName: string;
- if (groupby.length > 0) {
+ if (truncateMetric && groupby.length > 0) {
+ // Drop the metric part entirely and show only the groupby/dimension
+ // values. The label-map tuple is [metric, ...dimensions] only when the
+ // query has more than one metric; with a single metric the whole
+ // tuple is dimensions. Still apply the query-identifier suffix when
asked
+ // for, or Query A and Query B become indistinguishable whenever they
+ // share a groupby value (#38451).
+ const groupbyValues =
+ labelMapValues && ensureIsArray(metrics).length > 1
+ ? labelMapValues.slice(1)
+ : (labelMapValues ?? []);
Review Comment:
Hoisted `hasMultipleMetricsA`/`hasMultipleMetricsB` out of the loops.
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -651,7 +669,23 @@ export default function transformProps(
let displayName: string;
- if (groupbyB.length > 0) {
+ if (truncateMetricB && groupbyB.length > 0) {
+ // Drop the metric part entirely and show only the groupby/dimension
+ // values. The label-map tuple is [metric, ...dimensions] only when the
+ // query has more than one metric; with a single metric the whole
+ // tuple is dimensions. Still apply the query-identifier suffix when
asked
+ // for, or Query A and Query B become indistinguishable whenever they
+ // share a groupby value (#38451).
+ const groupbyValues =
+ labelMapValuesB && ensureIsArray(metricsB).length > 1
+ ? labelMapValuesB.slice(1)
+ : (labelMapValuesB ?? []);
Review Comment:
Hoisted `hasMultipleMetricsA`/`hasMultipleMetricsB` out of the loops.
--
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]
