diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts index 01674bf29d5..f0cbdcd6d0a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts @@ -472,66 +472,85 @@ export function transformIntervalAnnotation( ): SeriesOption[] { const series: SeriesOption[] = []; const annotations = extractRecordAnnotations(layer, annotationData); + if (annotations.length === 0) { + return series; + } + + const { name, color, opacity, showLabel } = layer; + const isHorizontal = orientation === OrientationType.Horizontal; + + const intervalsByStartTime = new Map(); annotations.forEach(annotation => { - const { name, color, opacity, showLabel } = layer; - const { descriptions, intervalEnd, time, title } = annotation; + const { descriptions, time = '', title } = annotation; const label = formatAnnotationLabel(name, title, descriptions); - const isHorizontal = orientation === OrientationType.Horizontal; - const intervalData: ( - | MarkArea1DDataItemOption - | MarkArea2DDataItemOption - )[] = [ - [ - { - name: label, - ...(isHorizontal ? { yAxis: time } : { xAxis: time }), - }, - isHorizontal ? { yAxis: intervalEnd } : { xAxis: intervalEnd }, - ], + const existing = intervalsByStartTime.get(time); + if (existing) { + existing.push(label); + } else { + intervalsByStartTime.set(time, [label]); + } + }); + + const allIntervalData: ( + | MarkArea1DDataItemOption + | MarkArea2DDataItemOption + )[] = annotations.map(annotation => { + const { intervalEnd, time = '' } = annotation; + const combinedLabel = (intervalsByStartTime.get(time) || []).join('\n'); + return [ + { + name: combinedLabel, + ...(isHorizontal ? { yAxis: time } : { xAxis: time }), + }, + isHorizontal ? { yAxis: intervalEnd } : { xAxis: intervalEnd }, ]; - const intervalLabel: SeriesLabelOption = showLabel - ? { - show: true, - color: theme.colorTextLabel, + }); + + const intervalLabel: SeriesLabelOption = showLabel + ? { + show: true, + color: theme.colorTextLabel, + position: 'insideTop', + verticalAlign: 'top', + fontWeight: 'bold', + // @ts-expect-error + emphasis: { position: 'insideTop', verticalAlign: 'top', + backgroundColor: theme.colorPrimaryBgHover, + }, + } + : { + show: false, + color: theme.colorTextLabel, + emphasis: { fontWeight: 'bold', - // @ts-expect-error - emphasis: { - position: 'insideTop', - verticalAlign: 'top', - backgroundColor: theme.colorPrimaryBgHover, - }, - } - : { - show: false, - color: theme.colorTextLabel, - emphasis: { - fontWeight: 'bold', - show: true, - position: 'insideTop', - verticalAlign: 'top', - backgroundColor: theme.colorPrimaryBgHover, - }, - }; - series.push({ - id: `Interval - ${label}`, - type: 'line', - animation: false, - markArea: { - silent: false, - itemStyle: { - color: color || colorScale(name, sliceId), - opacity: parseAnnotationOpacity(opacity || AnnotationOpacity.Medium), - emphasis: { - opacity: 0.8, - }, - } as ItemStyleOption, - label: intervalLabel, - data: intervalData, - }, - }); + show: true, + position: 'insideTop', + verticalAlign: 'top', + backgroundColor: theme.colorPrimaryBgHover, + }, + }; + + // Push a single series with all intervals in the markArea data + series.push({ + id: `Interval - ${name}`, + type: 'line', + animation: false, + markArea: { + silent: false, + itemStyle: { + color: color || colorScale(name, sliceId), + opacity: parseAnnotationOpacity(opacity || AnnotationOpacity.Medium), + emphasis: { + opacity: 0.8, + }, + } as ItemStyleOption, + label: intervalLabel, + data: allIntervalData, + }, }); + return series; } @@ -546,66 +565,82 @@ export function transformEventAnnotation( ): SeriesOption[] { const series: SeriesOption[] = []; const annotations = extractRecordAnnotations(layer, annotationData); + if (annotations.length === 0) { + return series; + } + + const { name, color, opacity, style, width, showLabel } = layer; + const isHorizontal = orientation === OrientationType.Horizontal; + + const eventsByTime = new Map(); annotations.forEach(annotation => { - const { name, color, opacity, style, width, showLabel } = layer; - const { descriptions, time, title } = annotation; + const { descriptions, time = '', title } = annotation; const label = formatAnnotationLabel(name, title, descriptions); - const isHorizontal = orientation === OrientationType.Horizontal; - const eventData: MarkLine1DDataItemOption[] = [ - { - name: label, - ...(isHorizontal ? { yAxis: time } : { xAxis: time }), - }, - ]; + const existing = eventsByTime.get(time); - const lineStyle: LineStyleOption & DefaultStatesMixin['emphasis'] = { - width, - type: style as ZRLineType, - color: color || colorScale(name, sliceId), - opacity: parseAnnotationOpacity(opacity), - emphasis: { - width: width ? width + 1 : width, - opacity: 1, - }, - }; - - const eventLabel: SeriesLineLabelOption = showLabel - ? { - show: true, - color: theme.colorTextLabel, - position: 'insideEndTop', - fontWeight: 'bold', - formatter: (params: CallbackDataParams) => params.name, - // @ts-expect-error - emphasis: { - backgroundColor: theme.colorPrimaryBgHover, - }, - } - : { - show: false, - color: theme.colorTextLabel, - position: 'insideEndTop', - emphasis: { - formatter: (params: CallbackDataParams) => params.name, - fontWeight: 'bold', - show: true, - backgroundColor: theme.colorPrimaryBgHover, - }, - }; - - series.push({ - id: `Event - ${label}`, - type: 'line', - animation: false, - markLine: { - silent: false, - symbol: 'none', - lineStyle, - label: eventLabel, - data: eventData, - }, - }); + if (existing) { + existing.labels.push(label); + } else { + eventsByTime.set(time, { time, labels: [label] }); + } }); + + const allEventData: MarkLine1DDataItemOption[] = Array.from( + eventsByTime.values(), + ).map(({ time, labels }) => ({ + name: labels.join('\n'), + ...(isHorizontal ? { yAxis: time } : { xAxis: time }), + })); + + const lineStyle: LineStyleOption & DefaultStatesMixin['emphasis'] = { + width, + type: style as ZRLineType, + color: color || colorScale(name, sliceId), + opacity: parseAnnotationOpacity(opacity), + emphasis: { + width: width ? width + 1 : width, + opacity: 1, + }, + }; + + const eventLabel: SeriesLineLabelOption = showLabel + ? { + show: true, + color: theme.colorTextLabel, + position: 'insideEndTop', + fontWeight: 'bold', + formatter: (params: CallbackDataParams) => params.name, + // @ts-expect-error + emphasis: { + backgroundColor: theme.colorPrimaryBgHover, + }, + } + : { + show: false, + color: theme.colorTextLabel, + position: 'insideEndTop', + emphasis: { + formatter: (params: CallbackDataParams) => params.name, + fontWeight: 'bold', + show: true, + backgroundColor: theme.colorPrimaryBgHover, + }, + }; + + // Push a single series with all events in the markLine data + series.push({ + id: `Event - ${name}`, + type: 'line', + animation: false, + markLine: { + silent: false, + symbol: 'none', + lineStyle, + label: eventLabel, + data: allEventData, + }, + }); + return series; } diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts index 21396a08226..24685716ce0 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts @@ -127,57 +127,102 @@ const mockIntervalAnnotationData: AnnotationData = { describe('transformIntervalAnnotation', () => { test('should transform data correctly', () => { - expect( - transformIntervalAnnotation( - mockIntervalAnnotationLayer, - mockData, - mockIntervalAnnotationData, - CategoricalColorNamespace.getScale(''), - supersetTheme, - ) - .map(annotation => annotation.markArea) - .map(markArea => markArea.data), - ).toEqual([ + const result = transformIntervalAnnotation( + mockIntervalAnnotationLayer, + mockData, + mockIntervalAnnotationData, + CategoricalColorNamespace.getScale(''), + supersetTheme, + ); + + // Should return a single series with all intervals + expect(result).toHaveLength(1); + expect(result[0].markArea.data).toEqual([ [ - [ - { name: 'Interval annotation layer - Timeseries 1', xAxis: 10 }, - { xAxis: 12 }, - ], + { name: 'Interval annotation layer - Timeseries 1', xAxis: 10 }, + { xAxis: 12 }, ], [ - [ - { name: 'Interval annotation layer - Timeseries 2', xAxis: 13 }, - { xAxis: 15 }, - ], + { name: 'Interval annotation layer - Timeseries 2', xAxis: 13 }, + { xAxis: 15 }, ], ]); }); - test('should use yAxis for horizontal chart data', () => { - expect( - transformIntervalAnnotation( - mockIntervalAnnotationLayer, - mockData, - mockIntervalAnnotationData, - CategoricalColorNamespace.getScale(''), - supersetTheme, - undefined, - OrientationType.Horizontal, - ) - .map(annotation => annotation.markArea) - .map(markArea => markArea.data), - ).toEqual([ - [ - [ - { name: 'Interval annotation layer - Timeseries 1', yAxis: 10 }, - { yAxis: 12 }, + test('should combine labels for intervals with the same start date', () => { + const duplicateStartDateData: AnnotationData = { + 'Interval annotation layer': { + records: [ + { + start_dttm: 10, + end_dttm: 12, + short_descr: 'Same start event 1', + long_descr: '', + json_metadata: '', + }, + { + start_dttm: 10, + end_dttm: 15, + short_descr: 'Same start event 2', + long_descr: '', + json_metadata: '', + }, + { + start_dttm: 10, + end_dttm: 18, + short_descr: 'Same start event 3', + long_descr: '', + json_metadata: '', + }, ], + }, + }; + + const result = transformIntervalAnnotation( + mockIntervalAnnotationLayer, + mockData, + duplicateStartDateData, + CategoricalColorNamespace.getScale(''), + supersetTheme, + ); + + // Should return a single series + expect(result).toHaveLength(1); + + // The markArea data should contain all 3 intervals + expect(result[0].markArea.data).toHaveLength(3); + + // All intervals with the same start time should have the combined label + const combinedLabel = + 'Interval annotation layer - Same start event 1\nInterval annotation layer - Same start event 2\nInterval annotation layer - Same start event 3'; + expect(result[0].markArea.data).toEqual([ + [{ name: combinedLabel, xAxis: 10 }, { xAxis: 12 }], + [{ name: combinedLabel, xAxis: 10 }, { xAxis: 15 }], + [{ name: combinedLabel, xAxis: 10 }, { xAxis: 18 }], + ]); + }); + + test('should use yAxis for horizontal chart data', () => { + const result = transformIntervalAnnotation( + mockIntervalAnnotationLayer, + mockData, + mockIntervalAnnotationData, + CategoricalColorNamespace.getScale(''), + supersetTheme, + undefined, + OrientationType.Horizontal, + ); + + // Should return a single series with all intervals + expect(result).toHaveLength(1); + expect(result[0].markArea.data).toEqual([ + [ + { name: 'Interval annotation layer - Timeseries 1', yAxis: 10 }, + { yAxis: 12 }, ], [ - [ - { name: 'Interval annotation layer - Timeseries 2', yAxis: 13 }, - { yAxis: 15 }, - ], + { name: 'Interval annotation layer - Timeseries 2', yAxis: 13 }, + { yAxis: 15 }, ], ]); }); @@ -218,48 +263,96 @@ const mockEventAnnotationData: AnnotationData = { describe('transformEventAnnotation', () => { test('should transform data correctly', () => { - expect( - transformEventAnnotation( - mockEventAnnotationLayer, - mockData, - mockEventAnnotationData, - CategoricalColorNamespace.getScale(''), - supersetTheme, - ) - .map(annotation => annotation.markLine) - .map(markLine => markLine.data), - ).toEqual([ - [ - { - name: 'Event annotation layer - Test annotation', - xAxis: 10, - }, - ], - [{ name: 'Event annotation layer - Test annotation 2', xAxis: 13 }], + const result = transformEventAnnotation( + mockEventAnnotationLayer, + mockData, + mockEventAnnotationData, + CategoricalColorNamespace.getScale(''), + supersetTheme, + ); + + // Should return a single series with all events + expect(result).toHaveLength(1); + expect(result[0].markLine.data).toEqual([ + { + name: 'Event annotation layer - Test annotation', + xAxis: 10, + }, + { name: 'Event annotation layer - Test annotation 2', xAxis: 13 }, + ]); + }); + + test('should combine labels for events with the same start date', () => { + const duplicateStartDateData: AnnotationData = { + 'Event annotation layer': { + records: [ + { + start_dttm: 10, + end_dttm: 12, + short_descr: 'Same date event 1', + long_descr: '', + json_metadata: '', + }, + { + start_dttm: 10, + end_dttm: 15, + short_descr: 'Same date event 2', + long_descr: '', + json_metadata: '', + }, + { + start_dttm: 10, + end_dttm: 18, + short_descr: 'Same date event 3', + long_descr: '', + json_metadata: '', + }, + ], + }, + }; + + const result = transformEventAnnotation( + mockEventAnnotationLayer, + mockData, + duplicateStartDateData, + CategoricalColorNamespace.getScale(''), + supersetTheme, + ); + + // Should return a single series + expect(result).toHaveLength(1); + + // Events on the same date are grouped into a single entry with combined label + expect(result[0].markLine.data).toHaveLength(1); + + // The combined label should include all event names + expect(result[0].markLine.data).toEqual([ + { + name: 'Event annotation layer - Same date event 1\nEvent annotation layer - Same date event 2\nEvent annotation layer - Same date event 3', + xAxis: 10, + }, ]); }); test('should use yAxis for horizontal chart data', () => { - expect( - transformEventAnnotation( - mockEventAnnotationLayer, - mockData, - mockEventAnnotationData, - CategoricalColorNamespace.getScale(''), - supersetTheme, - undefined, - OrientationType.Horizontal, - ) - .map(annotation => annotation.markLine) - .map(markLine => markLine.data), - ).toEqual([ - [ - { - name: 'Event annotation layer - Test annotation', - yAxis: 10, - }, - ], - [{ name: 'Event annotation layer - Test annotation 2', yAxis: 13 }], + const result = transformEventAnnotation( + mockEventAnnotationLayer, + mockData, + mockEventAnnotationData, + CategoricalColorNamespace.getScale(''), + supersetTheme, + undefined, + OrientationType.Horizontal, + ); + + // Should return a single series with all events + expect(result).toHaveLength(1); + expect(result[0].markLine.data).toEqual([ + { + name: 'Event annotation layer - Test annotation', + yAxis: 10, + }, + { name: 'Event annotation layer - Test annotation 2', yAxis: 13 }, ]); }); });