From e986496ef4dac31079f595754bca7cfd838df894 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Fri, 15 Aug 2025 12:59:30 -0300 Subject: [PATCH] fix: Timeseries annotation layers (#34709) (cherry picked from commit fc95c4fc89effdc9c9558b35e44c0c1f5d83f54c) --- .../src/query/types/AnnotationLayer.ts | 25 +-------- .../test/query/types/AnnotationLayer.test.ts | 45 ---------------- .../src/MixedTimeseries/transformProps.ts | 2 +- .../src/Timeseries/Regular/Line/index.ts | 1 + .../src/Timeseries/transformProps.ts | 2 +- .../src/Timeseries/transformers.ts | 45 ++++++++-------- .../src/utils/annotation.ts | 52 +++++++------------ .../test/Timeseries/transformProps.test.ts | 25 +++------ .../test/utils/annotation.test.ts | 17 +----- .../test/utils/transformers.test.ts | 35 ++++--------- .../AnnotationLayer.jsx | 3 +- 11 files changed, 69 insertions(+), 183 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/AnnotationLayer.ts b/superset-frontend/packages/superset-ui-core/src/query/types/AnnotationLayer.ts index d5b29811637..24f42ba0d98 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/AnnotationLayer.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/AnnotationLayer.ts @@ -158,31 +158,10 @@ export function isTableAnnotationLayer( return layer.sourceType === AnnotationSourceType.Table; } -export type RecordAnnotationResult = { - records: DataRecord[]; +export type AnnotationResult = { + records?: DataRecord[]; }; -export type TimeseriesAnnotationResult = { - key: string; - values: { x: string | number; y?: number }[]; -}[]; - -export type AnnotationResult = - | RecordAnnotationResult - | TimeseriesAnnotationResult; - -export function isTimeseriesAnnotationResult( - result: AnnotationResult, -): result is TimeseriesAnnotationResult { - return Array.isArray(result); -} - -export function isRecordAnnotationResult( - result: any, -): result is RecordAnnotationResult { - return Array.isArray(result?.records); -} - export type AnnotationData = { [key: string]: AnnotationResult }; export type Annotation = { diff --git a/superset-frontend/packages/superset-ui-core/test/query/types/AnnotationLayer.test.ts b/superset-frontend/packages/superset-ui-core/test/query/types/AnnotationLayer.test.ts index f6a695eb91f..a2752c5cbc3 100644 --- a/superset-frontend/packages/superset-ui-core/test/query/types/AnnotationLayer.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/query/types/AnnotationLayer.test.ts @@ -27,14 +27,10 @@ import { isEventAnnotationLayer, isFormulaAnnotationLayer, isIntervalAnnotationLayer, - isRecordAnnotationResult, isTableAnnotationLayer, isTimeseriesAnnotationLayer, - isTimeseriesAnnotationResult, - RecordAnnotationResult, TableAnnotationLayer, TimeseriesAnnotationLayer, - TimeseriesAnnotationResult, } from '@superset-ui/core'; describe('AnnotationLayer type guards', () => { @@ -82,23 +78,6 @@ describe('AnnotationLayer type guards', () => { show: true, showLabel: false, }; - const timeseriesAnnotationResult: TimeseriesAnnotationResult = [ - { - key: 'My Key', - values: [ - { x: -1000, y: 0 }, - { x: 0, y: 1000 }, - { x: 1000, y: 2000 }, - ], - }, - ]; - const recordAnnotationResult: RecordAnnotationResult = { - records: [ - { a: 1, b: 2 }, - { a: 2, b: 3 }, - ], - }; - describe('isFormulaAnnotationLayer', () => { it('should return true when it is the correct type', () => { expect(isFormulaAnnotationLayer(formulaAnnotationLayer)).toEqual(true); @@ -161,28 +140,4 @@ describe('AnnotationLayer type guards', () => { expect(isTableAnnotationLayer(formulaAnnotationLayer)).toEqual(false); }); }); - - describe('isTimeseriesAnnotationResult', () => { - it('should return true when it is the correct type', () => { - expect(isTimeseriesAnnotationResult(timeseriesAnnotationResult)).toEqual( - true, - ); - }); - it('should return false otherwise', () => { - expect(isTimeseriesAnnotationResult(recordAnnotationResult)).toEqual( - false, - ); - }); - }); - - describe('isRecordAnnotationResult', () => { - it('should return true when it is the correct type', () => { - expect(isRecordAnnotationResult(recordAnnotationResult)).toEqual(true); - }); - it('should return false otherwise', () => { - expect(isRecordAnnotationResult(timeseriesAnnotationResult)).toEqual( - false, - ); - }); - }); }); diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts index 7526b820d07..6d50c68f04d 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts @@ -662,7 +662,7 @@ export default function transformProps( ForecastSeriesEnum.Observation, ) .map(entry => entry.name || '') - .concat(extractAnnotationLabels(annotationLayers, annotationData)), + .concat(extractAnnotationLabels(annotationLayers)), }, series: dedupSeries(reorderForecastSeries(series) as SeriesOption[]), toolbox: { diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Line/index.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Line/index.ts index a63168617d8..d087b41b857 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Line/index.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/Regular/Line/index.ts @@ -60,6 +60,7 @@ export default class EchartsTimeseriesLineChartPlugin extends EchartsChartPlugin 'Line chart is used to visualize measurements taken over a given category. Line chart is a type of chart which displays information as a series of data points connected by straight line segments. It is a basic type of chart common in many fields.', ), exampleGallery: [{ url: example1 }, { url: example2 }], + canBeAnnotationTypes: [AnnotationType.Timeseries], supportedAnnotationTypes: [ AnnotationType.Event, AnnotationType.Formula, 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 bb62d6f128c..33200a5413b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -476,7 +476,7 @@ export default function transformProps( ForecastSeriesEnum.Observation, ) .map(entry => entry.name || '') - .concat(extractAnnotationLabels(annotationLayers, annotationData)); + .concat(extractAnnotationLabels(annotationLayers)); let xAxis: any = { type: xAxisType, 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 0abefc95e3d..9b1f4f973f5 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts @@ -25,7 +25,6 @@ import { FilterState, FormulaAnnotationLayer, IntervalAnnotationLayer, - isTimeseriesAnnotationResult, LegendState, SupersetTheme, TimeseriesAnnotationLayer, @@ -554,26 +553,30 @@ export function transformTimeseriesAnnotation( const { hideLine, name, opacity, showMarkers, style, width, color } = layer; const result = annotationData[name]; const isHorizontal = orientation === OrientationType.Horizontal; - if (isTimeseriesAnnotationResult(result)) { - result.forEach(annotation => { - const { key, values } = annotation; - series.push({ - type: 'line', - id: key, - name: key, - data: values.map(({ x, y }) => - isHorizontal - ? ([y, x] as [number, OptionName]) - : ([x, y] as [OptionName, number]), - ), - symbolSize: showMarkers ? markerSize : 0, - lineStyle: { - opacity: parseAnnotationOpacity(opacity), - type: style as ZRLineType, - width: hideLine ? 0 : width, - color: color || colorScale(name, sliceId), - }, - }); + const { records } = result; + if (records) { + const data = records.map(record => { + const keys = Object.keys(record); + const x = keys.length > 0 ? record[keys[0]] : 0; + const y = keys.length > 1 ? record[keys[1]] : 0; + return isHorizontal + ? ([y, x] as [number, OptionName]) + : ([x, y] as [OptionName, number]); + }); + const computedStyle = { + opacity: parseAnnotationOpacity(opacity), + type: style as ZRLineType, + width: hideLine ? 0 : width, + color: color || colorScale(name, sliceId), + }; + series.push({ + type: 'line', + id: name, + name, + data, + symbolSize: showMarkers ? markerSize : 0, + itemStyle: computedStyle, + lineStyle: computedStyle, }); } return series; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts index df6515d204e..90a450c3c3a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts @@ -29,9 +29,7 @@ import { DataRecord, evalExpression, FormulaAnnotationLayer, - isRecordAnnotationResult, isTableAnnotationLayer, - isTimeseriesAnnotationResult, } from '@superset-ui/core'; import { EchartsTimeseriesChartProps } from '../types'; import { EchartsMixedTimeseriesProps } from '../MixedTimeseries/types'; @@ -79,27 +77,24 @@ export function extractRecordAnnotations( ): Annotation[] { const { name } = annotationLayer; const result = annotationData[name]; - if (isRecordAnnotationResult(result)) { - const { records } = result; - const { - descriptionColumns = [], - intervalEndColumn = '', - timeColumn = '', - titleColumn = '', - } = isTableAnnotationLayer(annotationLayer) - ? annotationLayer - : NATIVE_COLUMN_NAMES; + const records = result?.records || []; + const { + descriptionColumns = [], + intervalEndColumn = '', + timeColumn = '', + titleColumn = '', + } = isTableAnnotationLayer(annotationLayer) + ? annotationLayer + : NATIVE_COLUMN_NAMES; - return records.map(record => ({ - descriptions: descriptionColumns.map( - column => (record[column] || '') as string, - ) as string[], - intervalEnd: (record[intervalEndColumn] || '') as string, - time: (record[timeColumn] || '') as string, - title: (record[titleColumn] || '') as string, - })); - } - throw new Error('Please rerun the query.'); + return records.map(record => ({ + descriptions: descriptionColumns.map( + column => (record[column] || '') as string, + ) as string[], + intervalEnd: (record[intervalEndColumn] || '') as string, + time: (record[timeColumn] || '') as string, + title: (record[titleColumn] || '') as string, + })); } export function formatAnnotationLabel( @@ -120,23 +115,16 @@ export function formatAnnotationLabel( return labels.join('\n\n'); } -export function extractAnnotationLabels( - layers: AnnotationLayer[], - data: AnnotationData, -): string[] { +export function extractAnnotationLabels(layers: AnnotationLayer[]): string[] { const formulaAnnotationLabels = layers .filter(anno => anno.annotationType === AnnotationType.Formula && anno.show) .map(anno => anno.name); + const timeseriesAnnotationLabels = layers .filter( anno => anno.annotationType === AnnotationType.Timeseries && anno.show, ) - .flatMap(anno => { - const result = data[anno.name]; - return isTimeseriesAnnotationResult(result) - ? result.map(annoSeries => annoSeries.key) - : []; - }); + .map(anno => anno.name); return formulaAnnotationLabels.concat(timeseriesAnnotationLabels); } 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 bc2aa2bd583..f3f0d108648 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 @@ -240,21 +240,12 @@ describe('EchartsTimeseries transformProps', () => { }, ], }, - 'My Timeseries': [ - { - key: 'My Line', - values: [ - { - x: 10000, - y: 11000, - }, - { - x: 20000, - y: 21000, - }, - ], - }, - ], + 'My Timeseries': { + records: [ + { x: 10000, y: 11000 }, + { x: 20000, y: 21000 }, + ], + }, }; const chartProps = new ChartProps({ ...chartPropsConfig, @@ -274,12 +265,12 @@ describe('EchartsTimeseries transformProps', () => { expect.objectContaining({ echartOptions: expect.objectContaining({ legend: expect.objectContaining({ - data: ['San Francisco', 'New York', 'My Line'], + data: ['San Francisco', 'New York', 'My Timeseries'], }), series: expect.arrayContaining([ expect.objectContaining({ type: 'line', - id: 'My Line', + id: 'My Timeseries', }), expect.objectContaining({ type: 'line', diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/annotation.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/annotation.test.ts index fd2f4acf249..fd54fcd9e1b 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/annotation.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/annotation.test.ts @@ -17,7 +17,6 @@ * under the License. */ import { - AnnotationData, AnnotationLayer, AnnotationOpacity, AnnotationSourceType, @@ -128,21 +127,7 @@ describe('extractAnnotationLabels', () => { showLabel: true, }, ]; - const results: AnnotationData = { - 'My Interval': { - records: [{ col: 1 }], - }, - 'My Line': [ - { key: 'Line 1', values: [] }, - { key: 'Line 2', values: [] }, - ], - }; - - expect(extractAnnotationLabels(layers, results)).toEqual([ - 'My Formula', - 'Line 1', - 'Line 2', - ]); + expect(extractAnnotationLabels(layers)).toEqual(['My Formula', 'My Line']); }); }); 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 113b416f9c5..9555abfffc5 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 @@ -282,30 +282,13 @@ const mockTimeseriesAnnotationLayer: TimeseriesAnnotationLayer = { }; const mockTimeseriesAnnotationData: AnnotationData = { - 'Timeseries annotation layer': [ - { - key: 'Key 1', - values: [ - { - x: 10, - y: 12, - }, - ], - }, - { - key: 'Key 2', - values: [ - { - x: 12, - y: 15, - }, - { - x: 15, - y: 20, - }, - ], - }, - ], + 'Timeseries annotation layer': { + records: [ + { x: 10, y: 12 }, + { x: 12, y: 15 }, + { x: 15, y: 20 }, + ], + }, }; describe('transformTimeseriesAnnotation', () => { @@ -319,8 +302,8 @@ describe('transformTimeseriesAnnotation', () => { CategoricalColorNamespace.getScale(''), ).map(annotation => annotation.data), ).toEqual([ - [[10, 12]], [ + [10, 12], [12, 15], [15, 20], ], @@ -339,8 +322,8 @@ describe('transformTimeseriesAnnotation', () => { OrientationType.Horizontal, ).map(annotation => annotation.data), ).toEqual([ - [[12, 10]], [ + [12, 10], [15, 12], [20, 15], ], diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx index 905fd04f9ce..fb3103646c6 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx @@ -31,6 +31,7 @@ import { styled, getColumnLabel, withTheme, + VizType, } from '@superset-ui/core'; import SelectControl from 'src/explore/components/controls/SelectControl'; import { AsyncSelect } from 'src/components'; @@ -246,7 +247,7 @@ class AnnotationLayer extends PureComponent { chartMetadata.canBeAnnotationType(annotationType), ) .map(({ key, value: chartMetadata }) => ({ - value: key, + value: key === VizType.Line ? 'line' : key, label: chartMetadata.name, })); // Prepend native source if applicable