fix(timeshiftcolor): fix timeshiftcolor to match the original color for echarts

Author: alexandrusoare

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

@@ -28,7 +28,9 @@ export const getTimeOffset = (
       // offset is represented as <offset>, group by list
       series.name.includes(`${timeOffset},`) ||
       // offset is represented as <metric>__<offset>
-      series.name.includes(`__${timeOffset}`),
+      series.name.includes(`__${timeOffset}`) ||
+      // offset is represented as <metric>, <offset>
+      series.name.includes(`, ${timeOffset}`),
   );
 
 export const hasTimeOffset = (
@@ -49,6 +51,8 @@ export const getOriginalSeries = (
     result = result.replace(`${compare},`, '');
     // offset is represented as <metric>__<offset>
     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

@@ -16,15 +16,79 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { getOriginalSeries } from '@superset-ui/chart-controls';
+import {
+  getOriginalSeries,
+  getTimeOffset,
+  hasTimeOffset,
+} from '@superset-ui/chart-controls';
 
-test('returns the series name when time compare is empty', () => {
+test('getOriginalSeries returns the series name when time compare is empty', () => {
   const seriesName = 'sum';
   expect(getOriginalSeries(seriesName, [])).toEqual(seriesName);
 });
 
-test('returns the original series name', () => {
+test('getOriginalSeries returns the original series name with __ pattern', () => {
   const seriesName = 'sum__1_month_ago';
   const timeCompare = ['1_month_ago'];
   expect(getOriginalSeries(seriesName, timeCompare)).toEqual('sum');
 });
+
+test('getOriginalSeries returns the original series name with <offset>, pattern', () => {
+  const seriesName = '1 year ago, groupby_value';
+  const timeCompare = ['1 year ago'];
+  expect(getOriginalSeries(seriesName, timeCompare)).toEqual('groupby_value');
+});
+
+test('getOriginalSeries returns the original series name with , <offset> pattern', () => {
+  const seriesName = 'AVG(price_each), 1 year ago';
+  const timeCompare = ['1 year ago'];
+  expect(getOriginalSeries(seriesName, timeCompare)).toEqual('AVG(price_each)');
+});
+
+test('getOriginalSeries handles multiple time compares', () => {
+  const seriesName = 'count, 1 year ago';
+  const timeCompare = ['1 month ago', '1 year ago'];
+  expect(getOriginalSeries(seriesName, timeCompare)).toEqual('count');
+});
+
+test('getTimeOffset returns undefined when no time offset pattern matches', () => {
+  const series = { name: 'count' };
+  const timeCompare = ['1 year ago'];
+  expect(getTimeOffset(series, timeCompare)).toBeUndefined();
+});
+
+test('getTimeOffset detects __ pattern', () => {
+  const series = { name: 'count__1 year ago' };
+  const timeCompare = ['1 year ago'];
+  expect(getTimeOffset(series, timeCompare)).toEqual('1 year ago');
+});
+
+test('getTimeOffset detects <offset>, pattern', () => {
+  const series = { name: '1 year ago, groupby_value' };
+  const timeCompare = ['1 year ago'];
+  expect(getTimeOffset(series, timeCompare)).toEqual('1 year ago');
+});
+
+test('getTimeOffset detects , <offset> pattern', () => {
+  const series = { name: 'AVG(price_each), 1 year ago' };
+  const timeCompare = ['1 year ago'];
+  expect(getTimeOffset(series, timeCompare)).toEqual('1 year ago');
+});
+
+test('hasTimeOffset returns false for original series', () => {
+  const series = { name: 'count' };
+  const timeCompare = ['1 year ago'];
+  expect(hasTimeOffset(series, timeCompare)).toBe(false);
+});
+
+test('hasTimeOffset returns true for derived series with , <offset> pattern', () => {
+  const series = { name: 'AVG(price_each), 1 year ago' };
+  const timeCompare = ['1 year ago'];
+  expect(hasTimeOffset(series, timeCompare)).toBe(true);
+});
+
+test('hasTimeOffset returns false when series name is not a string', () => {
+  const series = { name: 123 };
+  const timeCompare = ['1 year ago'];
+  expect(hasTimeOffset(series, timeCompare)).toBe(false);
+});