diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts index 852b4eb1ab8..9aae5445bc8 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/timeOffset.ts @@ -28,7 +28,9 @@ export const getTimeOffset = ( // offset is represented as , group by list series.name.includes(`${timeOffset},`) || // offset is represented as __ - series.name.includes(`__${timeOffset}`), + series.name.includes(`__${timeOffset}`) || + // offset is represented as , + series.name.includes(`, ${timeOffset}`), ); export const hasTimeOffset = ( @@ -45,10 +47,14 @@ export const getOriginalSeries = ( ): string => { let result = seriesName; timeCompare.forEach(compare => { - // offset is represented as , group by list + // offset in the middle: , , + result = result.replace(`, ${compare},`, ','); + // offset at start: , result = result.replace(`${compare},`, ''); - // offset is represented as __ + // offset with double underscore: __ result = result.replace(`__${compare}`, ''); + // offset at end: , + result = result.replace(`, ${compare}`, ''); }); return result.trim(); }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts index bee8c5bbbd2..bec42af6c9b 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/timeOffset.test.ts @@ -16,15 +16,101 @@ * 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 , 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 , 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('getOriginalSeries strips offset in the middle with dimension', () => { + const seriesName = 'SUM(sales), 28 days ago, Medium'; + const timeCompare = ['28 days ago']; + expect(getOriginalSeries(seriesName, timeCompare)).toEqual( + 'SUM(sales), Medium', + ); +}); + +test('getOriginalSeries strips offset in the middle with multiple dimensions', () => { + const seriesName = 'SUM(sales), 1 year ago, Medium, 11'; + const timeCompare = ['1 year ago']; + expect(getOriginalSeries(seriesName, timeCompare)).toEqual( + 'SUM(sales), Medium, 11', + ); +}); + +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 , 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 , pattern', () => { + const series = { name: 'AVG(price_each), 1 year ago' }; + const timeCompare = ['1 year ago']; + expect(getTimeOffset(series, timeCompare)).toEqual('1 year ago'); +}); + +test('getTimeOffset detects , , pattern (offset in middle)', () => { + const series = { name: 'SUM(sales), 28 days ago, Medium' }; + const timeCompare = ['28 days ago']; + expect(getTimeOffset(series, timeCompare)).toEqual('28 days 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 , 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); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts index 6eff01ca80b..f7d90cc3f5a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -336,6 +336,7 @@ export default function transformProps( chartProps.rawFormData, seriesName, ); + const lineStyle: LineStyleOption = {}; if (derivedSeries) { // Get the time offset for this series to assign different dash patterns @@ -370,7 +371,21 @@ export default function transformProps( let colorScaleKey = getOriginalSeries(seriesName, array); - // If series name exactly matches a time offset (single metric case), + // When there's a single metric with dimensions, the backend replaces the metric + // with the time offset in derived series (e.g., "28 days ago, Medium" instead of + // "SUM(sales), 28 days ago, Medium"). To match colors, strip the metric label + // from original series so both produce the same key (e.g., "Medium"). + if ( + groupby && + groupby.length > 0 && + array.length > 0 && + metrics?.length === 1 + ) { + const metricLabel = getMetricLabel(metrics[0]); + colorScaleKey = colorScaleKey.replace(`${metricLabel}, `, ''); + } + + // If series name exactly matches a time offset (single metric case, no dimensions), // find the original series for color matching if (derivedSeries && array.includes(seriesName)) { const originalSeries = rawSeries.find(