diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts index ddb3e425df0..50bb31503ad 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/operators/utils/isDerivedSeries.ts @@ -28,11 +28,16 @@ import { hasTimeOffset } from './timeOffset'; export const isDerivedSeries = ( series: JsonObject, formData: QueryFormData, + seriesName?: string, ): boolean => { const comparisonType = formData.comparison_type; if (comparisonType !== ComparisonType.Values) { return false; } const timeCompare: string[] = ensureIsArray(formData?.time_compare); - return hasTimeOffset(series, timeCompare); + // Check if series matches time offset patterns or exact match (single metric case) + return ( + hasTimeOffset(series, timeCompare) || + (seriesName !== undefined && timeCompare.includes(seriesName)) + ); }; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/isDerivedSeries.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/isDerivedSeries.test.ts index 472b980be6f..f775dea2b80 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/isDerivedSeries.test.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/test/operators/utils/isDerivedSeries.test.ts @@ -97,3 +97,24 @@ test('should be false if series name invalid', () => { }; expect(isDerivedSeries(series, formDataWithActualTypes)).toEqual(false); }); + +test('should be true for exact match when seriesName parameter is provided', () => { + const exactMatchSeries = { + id: '1 week ago', + name: '1 week ago', + data: [100], + }; + const formDataWithTimeCompare = { + ...formData, + comparison_type: ComparisonType.Values, + time_compare: ['1 week ago'], + }; + // Without seriesName parameter, exact match is not detected via hasTimeOffset + expect(isDerivedSeries(exactMatchSeries, formDataWithTimeCompare)).toEqual( + false, + ); + // With seriesName parameter, exact match is detected + expect( + isDerivedSeries(exactMatchSeries, formDataWithTimeCompare, '1 week ago'), + ).toEqual(true); +}); 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 cfb05d1602d..6ebc30fc226 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -45,6 +45,7 @@ import { GenericDataType } from '@apache-superset/core/api/core'; import { extractExtraMetrics, getOriginalSeries, + getTimeOffset, isDerivedSeries, } from '@superset-ui/chart-controls'; import type { EChartsCoreOption } from 'echarts/core'; @@ -309,7 +310,7 @@ export default function transformProps( const array = ensureIsArray(chartProps.rawFormData?.time_compare); const inverted = invert(verboseMap); - let patternIncrement = 0; + const offsetLineWidths: { [key: string]: number } = {}; // For horizontal bar charts, calculate min/max from data to avoid cutting off labels const shouldCalculateDataBounds = @@ -320,17 +321,33 @@ export default function transformProps( let dataMin: number | undefined; rawSeries.forEach(entry => { - const derivedSeries = isDerivedSeries(entry, chartProps.rawFormData); - const lineStyle: LineStyleOption = {}; - if (derivedSeries) { - patternIncrement += 1; - // use a combination of dash and dot for the line style - lineStyle.type = [(patternIncrement % 5) + 1, (patternIncrement % 3) + 1]; - lineStyle.opacity = OpacityEnum.DerivedSeries; - } - const entryName = String(entry.name || ''); const seriesName = inverted[entryName] || entryName; + // isDerivedSeries checks for time comparison series patterns: + // - "metric__1 day ago" pattern (via hasTimeOffset) + // - "1 day ago, groupby" pattern (via hasTimeOffset) + // - exact match "1 day ago" (via seriesName parameter) + const derivedSeries = isDerivedSeries( + entry, + chartProps.rawFormData, + seriesName, + ); + const lineStyle: LineStyleOption = {}; + if (derivedSeries) { + // Get the time offset for this series to assign different dash patterns + const offset = getTimeOffset(entry, array) || seriesName; + if (!offsetLineWidths[offset]) { + offsetLineWidths[offset] = Object.keys(offsetLineWidths).length + 1; + } + // Use visible dash patterns that vary by offset index + // Pattern: [dash length, gap length] - scaled to be clearly visible + const patternIndex = offsetLineWidths[offset]; + lineStyle.type = [ + (patternIndex % 5) + 4, // dash: 4-8px (visible) + (patternIndex % 3) + 3, // gap: 3-5px (visible) + ]; + lineStyle.opacity = OpacityEnum.DerivedSeries; + } // Calculate min/max from data for horizontal bar charts if (shouldCalculateDataBounds && entry.data && Array.isArray(entry.data)) { @@ -349,14 +366,17 @@ export default function transformProps( let colorScaleKey = getOriginalSeries(seriesName, array); - // If this series name exactly matches a time compare value, it's a time-shifted series - // and we need to find the corresponding original series for color matching - if (array && array.includes(seriesName)) { - // Find the original series (first non-time-compare series) - const originalSeries = rawSeries.find(s => { - const sName = inverted[String(s.name || '')] || String(s.name || ''); - return !array.includes(sName); - }); + // If series name exactly matches a time offset (single metric case), + // find the original series for color matching + if (derivedSeries && array.includes(seriesName)) { + const originalSeries = rawSeries.find( + s => + !isDerivedSeries( + s, + chartProps.rawFormData, + inverted[String(s.name || '')] || String(s.name || ''), + ), + ); if (originalSeries) { const originalSeriesName = inverted[String(originalSeries.name || '')] || diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts index 1bdaf9b5fa9..f6e7c3e770c 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts @@ -21,6 +21,7 @@ import { AnnotationStyle, AnnotationType, ChartProps, + ComparisonType, EventAnnotationLayer, FormulaAnnotationLayer, IntervalAnnotationLayer, @@ -740,6 +741,170 @@ describe('legend sorting', () => { }); }); +const timeCompareFormData: SqlaFormData = { + colorScheme: 'bnbColors', + datasource: '3__table', + granularity_sqla: 'ds', + metric: 'sum__num', + viz_type: 'my_viz', +}; + +const timeCompareChartPropsConfig = { + formData: timeCompareFormData, + width: 800, + height: 600, + theme: supersetTheme, +}; + +test('should apply dashed line style to time comparison series with single metric', () => { + const queriesDataWithTimeCompare = [ + { + data: [ + { sum__num: 100, '1 week ago': 80, __timestamp: 599616000000 }, + { sum__num: 150, '1 week ago': 120, __timestamp: 599916000000 }, + ], + }, + ]; + + const chartProps = new ChartProps({ + ...timeCompareChartPropsConfig, + formData: { + ...timeCompareFormData, + time_compare: ['1 week ago'], + comparison_type: ComparisonType.Values, + }, + queriesData: queriesDataWithTimeCompare, + }); + + const transformed = transformProps( + chartProps as unknown as EchartsTimeseriesChartProps, + ); + const series = transformed.echartOptions.series as any[]; + + const mainSeries = series.find(s => s.name === 'sum__num'); + const comparisonSeries = series.find(s => s.name === '1 week ago'); + + expect(mainSeries).toBeDefined(); + expect(comparisonSeries).toBeDefined(); + // Main series should not have a dash pattern array + expect(Array.isArray(mainSeries.lineStyle?.type)).toBe(false); + // Comparison series should have a visible dash pattern array [dash, gap] + expect(Array.isArray(comparisonSeries.lineStyle?.type)).toBe(true); + expect(comparisonSeries.lineStyle?.type[0]).toBeGreaterThanOrEqual(4); + expect(comparisonSeries.lineStyle?.type[1]).toBeGreaterThanOrEqual(3); +}); + +test('should apply dashed line style to time comparison series with metric__offset pattern', () => { + const queriesDataWithTimeCompare = [ + { + data: [ + { + sum__num: 100, + 'sum__num__1 week ago': 80, + __timestamp: 599616000000, + }, + { + sum__num: 150, + 'sum__num__1 week ago': 120, + __timestamp: 599916000000, + }, + ], + }, + ]; + + const chartProps = new ChartProps({ + ...timeCompareChartPropsConfig, + formData: { + ...timeCompareFormData, + time_compare: ['1 week ago'], + comparison_type: ComparisonType.Values, + }, + queriesData: queriesDataWithTimeCompare, + }); + + const transformed = transformProps( + chartProps as unknown as EchartsTimeseriesChartProps, + ); + const series = transformed.echartOptions.series as any[]; + + const mainSeries = series.find(s => s.name === 'sum__num'); + const comparisonSeries = series.find(s => s.name === 'sum__num__1 week ago'); + + expect(mainSeries).toBeDefined(); + expect(comparisonSeries).toBeDefined(); + // Main series should not have a dash pattern array + expect(Array.isArray(mainSeries.lineStyle?.type)).toBe(false); + // Comparison series should have a visible dash pattern array [dash, gap] + expect(Array.isArray(comparisonSeries.lineStyle?.type)).toBe(true); + expect(comparisonSeries.lineStyle?.type[0]).toBeGreaterThanOrEqual(4); + expect(comparisonSeries.lineStyle?.type[1]).toBeGreaterThanOrEqual(3); +}); + +test('should apply connectNulls to time comparison series', () => { + const queriesDataWithNulls = [ + { + data: [ + { sum__num: 100, '1 week ago': null, __timestamp: 599616000000 }, + { sum__num: 150, '1 week ago': 120, __timestamp: 599916000000 }, + { sum__num: 200, '1 week ago': null, __timestamp: 600216000000 }, + ], + }, + ]; + + const chartProps = new ChartProps({ + ...timeCompareChartPropsConfig, + formData: { + ...timeCompareFormData, + time_compare: ['1 week ago'], + comparison_type: ComparisonType.Values, + }, + queriesData: queriesDataWithNulls, + }); + + const transformed = transformProps( + chartProps as unknown as EchartsTimeseriesChartProps, + ); + const series = transformed.echartOptions.series as any[]; + + const comparisonSeries = series.find(s => s.name === '1 week ago'); + + expect(comparisonSeries).toBeDefined(); + expect(comparisonSeries.connectNulls).toBe(true); +}); + +test('should not apply dashed line style for non-Values comparison types', () => { + const queriesDataWithTimeCompare = [ + { + data: [ + { sum__num: 100, '1 week ago': 80, __timestamp: 599616000000 }, + { sum__num: 150, '1 week ago': 120, __timestamp: 599916000000 }, + ], + }, + ]; + + const chartProps = new ChartProps({ + ...timeCompareChartPropsConfig, + formData: { + ...timeCompareFormData, + time_compare: ['1 week ago'], + comparison_type: ComparisonType.Difference, + }, + queriesData: queriesDataWithTimeCompare, + }); + + const transformed = transformProps( + chartProps as unknown as EchartsTimeseriesChartProps, + ); + const series = transformed.echartOptions.series as any[]; + + const comparisonSeries = series.find(s => s.name === '1 week ago'); + + expect(comparisonSeries).toBeDefined(); + // Non-Values comparison types don't get dashed styling (isDerivedSeries returns false) + expect(Array.isArray(comparisonSeries.lineStyle?.type)).toBe(false); + expect(comparisonSeries.connectNulls).toBeFalsy(); +}); + test('EchartsTimeseries AUTO mode should detect single currency and format with $ for USD', () => { const chartProps = new ChartProps({ ...chartPropsConfig,