feat: Make time shifted series colors match the original series (#24048)

superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/index.ts

@@ -21,4 +21,5 @@ export { getMetricOffsetsMap } from './getMetricOffsetsMap';
 export { isTimeComparison } from './isTimeComparison';
 export { isDerivedSeries } from './isDerivedSeries';
 export { extractExtraMetrics } from './extractExtraMetrics';
+export { getOriginalSeries, hasTimeOffset } from './timeOffset';
 export { TIME_COMPARISON_SEPARATOR } from './constants';

superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts

@@ -23,7 +23,7 @@ import {
   QueryFormData,
   ComparisonType,
 } from '@superset-ui/core';
-import { isString } from 'lodash';
+import { hasTimeOffset } from './timeOffset';
 
 export const isDerivedSeries = (
   series: JsonObject,
@@ -33,9 +33,6 @@ export const isDerivedSeries = (
   if (comparisonType !== ComparisonType.Values) {
     return false;
   }
-
   const timeCompare: string[] = ensureIsArray(formData?.time_compare);
-  return isString(series.name)
-    ? !!timeCompare.find(timeOffset => series.name.endsWith(timeOffset))
-    : false;
+  return hasTimeOffset(series, timeCompare);
 };

superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts

@@ -0,0 +1,49 @@
+/* eslint-disable camelcase */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitationsxw
+ * under the License.
+ */
+import { JsonObject } from '@superset-ui/core';
+import { isString } from 'lodash';
+
+export const hasTimeOffset = (
+  series: JsonObject,
+  timeCompare: string[],
+): boolean =>
+  isString(series.name)
+    ? !!timeCompare.find(
+        timeOffset =>
+          // offset is represented as <offset>, group by list
+          series.name.includes(`${timeOffset},`) ||
+          // offset is represented as <metric>__<offset>
+          series.name.includes(`__${timeOffset}`),
+      )
+    : false;
+
+export const getOriginalSeries = (
+  seriesName: string,
+  timeCompare: string[],
+): string => {
+  let result = seriesName;
+  timeCompare.forEach(compare => {
+    // offset is represented as <offset>, group by list
+    result = result.replace(`${compare},`, '');
+    // offset is represented as <metric>__<offset>
+    result = result.replace(`__${compare}`, '');
+  });
+  return result.trim();
+};

superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts

@@ -0,0 +1,30 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { getOriginalSeries } from '@superset-ui/chart-controls';
+
+test('returns the series name when time compare is empty', () => {
+  const seriesName = 'sum';
+  expect(getOriginalSeries(seriesName, [])).toEqual(seriesName);
+});
+
+test('returns the original series name', () => {
+  const seriesName = 'sum__1_month_ago';
+  const timeCompare = ['1_month_ago'];
+  expect(getOriginalSeries(seriesName, timeCompare)).toEqual('sum');
+});

superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 /* eslint-disable camelcase */
+import { invert } from 'lodash';
 import {
   AnnotationLayer,
   CategoricalColorNamespace,
@@ -32,7 +33,9 @@ import {
   getXAxisLabel,
   isPhysicalColumn,
   isDefined,
+  ensureIsArray,
 } from '@superset-ui/core';
+import { getOriginalSeries } from '@superset-ui/chart-controls';
 import { EChartsCoreOption, SeriesOption } from 'echarts';
 import {
   DEFAULT_FORM_DATA,
@@ -296,55 +299,76 @@ export default function transformProps(
     formatter: formatterSecondary,
   });
 
+  const array = ensureIsArray(chartProps.rawFormData?.time_compare);
+  const inverted = invert(verboseMap);
+
   rawSeriesA.forEach(entry => {
-    const transformedSeries = transformSeries(entry, colorScale, {
-      area,
-      markerEnabled,
-      markerSize,
-      areaOpacity: opacity,
-      seriesType,
-      showValue,
-      stack: Boolean(stack),
-      yAxisIndex,
-      filterState,
-      seriesKey: entry.name,
-      sliceId,
-      queryIndex: 0,
-      formatter:
-        seriesType === EchartsTimeseriesSeriesType.Bar
-          ? maxLabelFormatter
-          : formatter,
-      showValueIndexes: showValueIndexesA,
-      totalStackedValues,
-      thresholdValues,
-    });
+    const entryName = String(entry.name || '');
+    const seriesName = inverted[entryName] || entryName;
+    const colorScaleKey = getOriginalSeries(seriesName, array);
+
+    const transformedSeries = transformSeries(
+      entry,
+      colorScale,
+      colorScaleKey,
+      {
+        area,
+        markerEnabled,
+        markerSize,
+        areaOpacity: opacity,
+        seriesType,
+        showValue,
+        stack: Boolean(stack),
+        yAxisIndex,
+        filterState,
+        seriesKey: entry.name,
+        sliceId,
+        queryIndex: 0,
+        formatter:
+          seriesType === EchartsTimeseriesSeriesType.Bar
+            ? maxLabelFormatter
+            : formatter,
+        showValueIndexes: showValueIndexesA,
+        totalStackedValues,
+        thresholdValues,
+      },
+    );
     if (transformedSeries) series.push(transformedSeries);
   });
 
   rawSeriesB.forEach(entry => {
-    const transformedSeries = transformSeries(entry, colorScale, {
-      area: areaB,
-      markerEnabled: markerEnabledB,
-      markerSize: markerSizeB,
-      areaOpacity: opacityB,
-      seriesType: seriesTypeB,
-      showValue: showValueB,
-      stack: Boolean(stackB),
-      yAxisIndex: yAxisIndexB,
-      filterState,
-      seriesKey: primarySeries.has(entry.name as string)
-        ? `${entry.name} (1)`
-        : entry.name,
-      sliceId,
-      queryIndex: 1,
-      formatter:
-        seriesTypeB === EchartsTimeseriesSeriesType.Bar
-          ? maxLabelFormatterSecondary
-          : formatterSecondary,
-      showValueIndexes: showValueIndexesB,
-      totalStackedValues: totalStackedValuesB,
-      thresholdValues: thresholdValuesB,
-    });
+    const entryName = String(entry.name || '');
+    const seriesName = `${inverted[entryName] || entryName} (1)`;
+    const colorScaleKey = getOriginalSeries(seriesName, array);
+
+    const transformedSeries = transformSeries(
+      entry,
+      colorScale,
+      colorScaleKey,
+      {
+        area: areaB,
+        markerEnabled: markerEnabledB,
+        markerSize: markerSizeB,
+        areaOpacity: opacityB,
+        seriesType: seriesTypeB,
+        showValue: showValueB,
+        stack: Boolean(stackB),
+        yAxisIndex: yAxisIndexB,
+        filterState,
+        seriesKey: primarySeries.has(entry.name as string)
+          ? `${entry.name} (1)`
+          : entry.name,
+        sliceId,
+        queryIndex: 1,
+        formatter:
+          seriesTypeB === EchartsTimeseriesSeriesType.Bar
+            ? maxLabelFormatterSecondary
+            : formatterSecondary,
+        showValueIndexes: showValueIndexesB,
+        totalStackedValues: totalStackedValuesB,
+        thresholdValues: thresholdValuesB,
+      },
+    );
     if (transformedSeries) series.push(transformedSeries);
   });
 

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts

@@ -17,10 +17,12 @@
  * under the License.
  */
 /* eslint-disable camelcase */
+import { invert } from 'lodash';
 import {
   AnnotationLayer,
   AxisType,
   CategoricalColorNamespace,
+  ensureIsArray,
   GenericDataType,
   getMetricLabel,
   getNumberFormatter,
@@ -36,6 +38,7 @@ import {
 } from '@superset-ui/core';
 import {
   extractExtraMetrics,
+  getOriginalSeries,
   isDerivedSeries,
 } from '@superset-ui/chart-controls';
 import { EChartsCoreOption, SeriesOption } from 'echarts';
@@ -227,30 +230,43 @@ export default function transformProps(
     contributionMode || isAreaExpand ? ',.0%' : yAxisFormat,
   );
 
+  const array = ensureIsArray(chartProps.rawFormData?.time_compare);
+  const inverted = invert(verboseMap);
+
   rawSeries.forEach(entry => {
     const lineStyle = isDerivedSeries(entry, chartProps.rawFormData)
       ? { type: 'dashed' as ZRLineType }
       : {};
-    const transformedSeries = transformSeries(entry, colorScale, {
-      area,
-      filterState,
-      seriesContexts,
-      markerEnabled,
-      markerSize,
-      areaOpacity: opacity,
-      seriesType,
-      stack,
-      formatter,
-      showValue,
-      onlyTotal,
-      totalStackedValues: sortedTotalValues,
-      showValueIndexes,
-      thresholdValues,
-      richTooltip,
-      sliceId,
-      isHorizontal,
-      lineStyle,
-    });
+
+    const entryName = String(entry.name || '');
+    const seriesName = inverted[entryName] || entryName;
+    const colorScaleKey = getOriginalSeries(seriesName, array);
+
+    const transformedSeries = transformSeries(
+      entry,
+      colorScale,
+      colorScaleKey,
+      {
+        area,
+        filterState,
+        seriesContexts,
+        markerEnabled,
+        markerSize,
+        areaOpacity: opacity,
+        seriesType,
+        stack,
+        formatter,
+        showValue,
+        onlyTotal,
+        totalStackedValues: sortedTotalValues,
+        showValueIndexes,
+        thresholdValues,
+        richTooltip,
+        sliceId,
+        isHorizontal,
+        lineStyle,
+      },
+    );
     if (transformedSeries) {
       if (stack === StackControlsValue.Stream) {
         // bug in Echarts - `stackStrategy: 'all'` doesn't work with nulls, so we cast them to 0

superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts

@@ -51,7 +51,6 @@ import {
   MarkArea2DDataItemOption,
 } from 'echarts/types/src/component/marker/MarkAreaModel';
 import { MarkLine1DDataItemOption } from 'echarts/types/src/component/marker/MarkLineModel';
-
 import { extractForecastSeriesContext } from '../utils/forecast';
 import {
   EchartsTimeseriesSeriesType,
@@ -144,6 +143,7 @@ export const getBaselineSeriesForStream = (
 export function transformSeries(
   series: SeriesOption,
   colorScale: CategoricalColorScale,
+  colorScaleKey: string,
   opts: {
     area?: boolean;
     filterState?: FilterState;
@@ -186,7 +186,6 @@ export function transformSeries(
     showValueIndexes = [],
     thresholdValues = [],
     richTooltip,
-    seriesKey,
     sliceId,
     isHorizontal = false,
     queryIndex = 0,
@@ -236,7 +235,7 @@ export function transformSeries(
   }
   // forcing the colorScale to return a different color for same metrics across different queries
   const itemStyle = {
-    color: colorScale(seriesKey || forecastSeries.name, sliceId),
+    color: colorScale(colorScaleKey, sliceId),
     opacity,
   };
   let emphasis = {};