From 2816a70af3ae0675110c8738246e97ce99c6f9be Mon Sep 17 00:00:00 2001 From: Damian Pendrak Date: Thu, 5 Dec 2024 22:20:22 +0100 Subject: [PATCH] fix: annotations on horizontal bar chart (#31308) --- .../src/Timeseries/transformProps.ts | 4 + .../src/Timeseries/transformers.ts | 28 +- .../test/utils/transformers.test.ts | 349 ++++++++++++++++++ 3 files changed, 374 insertions(+), 7 deletions(-) create mode 100644 superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts 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 c62b7284b29b1..5eb0bdd0a2bd2 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts @@ -384,6 +384,7 @@ export default function transformProps( xAxisType, colorScale, sliceId, + orientation, ), ); else if (isIntervalAnnotationLayer(layer)) { @@ -395,6 +396,7 @@ export default function transformProps( colorScale, theme, sliceId, + orientation, ), ); } else if (isEventAnnotationLayer(layer)) { @@ -406,6 +408,7 @@ export default function transformProps( colorScale, theme, sliceId, + orientation, ), ); } else if (isTimeseriesAnnotationLayer(layer)) { @@ -417,6 +420,7 @@ export default function transformProps( annotationData, colorScale, sliceId, + orientation, ), ); } 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 df4817a018c86..30d2509e45067 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts @@ -53,6 +53,7 @@ import { EchartsTimeseriesSeriesType, ForecastSeriesEnum, LegendOrientation, + OrientationType, StackType, } from '../types'; @@ -364,8 +365,11 @@ export function transformFormulaAnnotation( xAxisType: AxisType, colorScale: CategoricalColorScale, sliceId?: number, + orientation?: OrientationType, ): SeriesOption { const { name, color, opacity, width, style } = layer; + const isHorizontal = orientation === OrientationType.Horizontal; + return { name, id: name, @@ -379,7 +383,9 @@ export function transformFormulaAnnotation( }, type: 'line', smooth: true, - data: evalFormula(layer, data, xAxisCol, xAxisType), + data: evalFormula(layer, data, xAxisCol, xAxisType).map(([x, y]) => + isHorizontal ? [y, x] : [x, y], + ), symbolSize: 0, }; } @@ -391,6 +397,7 @@ export function transformIntervalAnnotation( colorScale: CategoricalColorScale, theme: SupersetTheme, sliceId?: number, + orientation?: OrientationType, ): SeriesOption[] { const series: SeriesOption[] = []; const annotations = extractRecordAnnotations(layer, annotationData); @@ -398,6 +405,7 @@ export function transformIntervalAnnotation( const { name, color, opacity, showLabel } = layer; const { descriptions, intervalEnd, time, title } = annotation; const label = formatAnnotationLabel(name, title, descriptions); + const isHorizontal = orientation === OrientationType.Horizontal; const intervalData: ( | MarkArea1DDataItemOption | MarkArea2DDataItemOption @@ -405,11 +413,9 @@ export function transformIntervalAnnotation( [ { name: label, - xAxis: time, - }, - { - xAxis: intervalEnd, + ...(isHorizontal ? { yAxis: time } : { xAxis: time }), }, + isHorizontal ? { yAxis: intervalEnd } : { xAxis: intervalEnd }, ], ]; const intervalLabel: SeriesLabelOption = showLabel @@ -466,6 +472,7 @@ export function transformEventAnnotation( colorScale: CategoricalColorScale, theme: SupersetTheme, sliceId?: number, + orientation?: OrientationType, ): SeriesOption[] { const series: SeriesOption[] = []; const annotations = extractRecordAnnotations(layer, annotationData); @@ -473,10 +480,11 @@ export function transformEventAnnotation( const { name, color, opacity, style, width, showLabel } = layer; const { descriptions, time, title } = annotation; const label = formatAnnotationLabel(name, title, descriptions); + const isHorizontal = orientation === OrientationType.Horizontal; const eventData: MarkLine1DDataItemOption[] = [ { name: label, - xAxis: time, + ...(isHorizontal ? { yAxis: time } : { xAxis: time }), }, ]; @@ -539,10 +547,12 @@ export function transformTimeseriesAnnotation( annotationData: AnnotationData, colorScale: CategoricalColorScale, sliceId?: number, + orientation?: OrientationType, ): SeriesOption[] { const series: SeriesOption[] = []; 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; @@ -550,7 +560,11 @@ export function transformTimeseriesAnnotation( type: 'line', id: key, name: key, - data: values.map(row => [row.x, row.y] as [OptionName, number]), + data: values.map(({ x, y }) => + isHorizontal + ? ([y, x] as [number, OptionName]) + : ([x, y] as [OptionName, number]), + ), symbolSize: showMarkers ? markerSize : 0, lineStyle: { opacity: parseAnnotationOpacity(opacity), 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 new file mode 100644 index 0000000000000..113b416f9c5b9 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/transformers.test.ts @@ -0,0 +1,349 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { + AnnotationData, + AnnotationSourceType, + AnnotationStyle, + AnnotationType, + AxisType, + CategoricalColorNamespace, + EventAnnotationLayer, + FormulaAnnotationLayer, + IntervalAnnotationLayer, + supersetTheme, + TimeseriesAnnotationLayer, + TimeseriesDataRecord, +} from '@superset-ui/core'; +import { OrientationType } from '@superset-ui/plugin-chart-echarts'; +import { + transformEventAnnotation, + transformFormulaAnnotation, + transformIntervalAnnotation, + transformTimeseriesAnnotation, +} from '../../src/Timeseries/transformers'; + +const mockData: TimeseriesDataRecord[] = [ + { + __timestamp: 10, + }, + { + __timestamp: 20, + }, +]; + +const mockFormulaAnnotationLayer: FormulaAnnotationLayer = { + annotationType: AnnotationType.Formula as const, + name: 'My Formula', + show: true, + style: AnnotationStyle.Solid, + value: '50', + showLabel: true, +}; + +describe('transformFormulaAnnotation', () => { + it('should transform data correctly', () => { + expect( + transformFormulaAnnotation( + mockFormulaAnnotationLayer, + mockData, + '__timestamp', + AxisType.Value, + CategoricalColorNamespace.getScale(''), + undefined, + ).data, + ).toEqual([ + [10, 50], + [20, 50], + ]); + }); + + it('should swap x and y for horizontal chart', () => { + expect( + transformFormulaAnnotation( + mockFormulaAnnotationLayer, + mockData, + '__timestamp', + AxisType.Value, + CategoricalColorNamespace.getScale(''), + undefined, + OrientationType.Horizontal, + ).data, + ).toEqual([ + [50, 10], + [50, 20], + ]); + }); +}); + +const mockIntervalAnnotationLayer: IntervalAnnotationLayer = { + name: 'Interval annotation layer', + annotationType: AnnotationType.Interval as const, + sourceType: AnnotationSourceType.Native as const, + color: null, + style: AnnotationStyle.Solid, + width: 1, + show: true, + showLabel: false, + value: 1, +}; + +const mockIntervalAnnotationData: AnnotationData = { + 'Interval annotation layer': { + records: [ + { + start_dttm: 10, + end_dttm: 12, + short_descr: 'Timeseries 1', + long_descr: '', + json_metadata: '', + }, + { + start_dttm: 13, + end_dttm: 15, + short_descr: 'Timeseries 2', + long_descr: '', + json_metadata: '', + }, + ], + }, +}; + +describe('transformIntervalAnnotation', () => { + it('should transform data correctly', () => { + expect( + transformIntervalAnnotation( + mockIntervalAnnotationLayer, + mockData, + mockIntervalAnnotationData, + CategoricalColorNamespace.getScale(''), + supersetTheme, + ) + .map(annotation => annotation.markArea) + .map(markArea => markArea.data), + ).toEqual([ + [ + [ + { name: 'Interval annotation layer - Timeseries 1', xAxis: 10 }, + { xAxis: 12 }, + ], + ], + [ + [ + { name: 'Interval annotation layer - Timeseries 2', xAxis: 13 }, + { xAxis: 15 }, + ], + ], + ]); + }); + + it('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 }, + ], + ], + [ + [ + { name: 'Interval annotation layer - Timeseries 2', yAxis: 13 }, + { yAxis: 15 }, + ], + ], + ]); + }); +}); + +const mockEventAnnotationLayer: EventAnnotationLayer = { + annotationType: AnnotationType.Event, + color: null, + name: 'Event annotation layer', + show: true, + showLabel: false, + sourceType: AnnotationSourceType.Native, + style: AnnotationStyle.Solid, + value: 1, + width: 1, +}; + +const mockEventAnnotationData: AnnotationData = { + 'Event annotation layer': { + records: [ + { + start_dttm: 10, + end_dttm: 12, + short_descr: 'Test annotation', + long_descr: '', + json_metadata: '', + }, + { + start_dttm: 13, + end_dttm: 15, + short_descr: 'Test annotation 2', + long_descr: '', + json_metadata: '', + }, + ], + }, +}; + +describe('transformEventAnnotation', () => { + it('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 }], + ]); + }); + + it('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 mockTimeseriesAnnotationLayer: TimeseriesAnnotationLayer = { + annotationType: AnnotationType.Timeseries, + color: null, + hideLine: false, + name: 'Timeseries annotation layer', + overrides: { + time_range: null, + }, + show: true, + showLabel: false, + showMarkers: false, + sourceType: AnnotationSourceType.Line, + style: AnnotationStyle.Solid, + value: 1, + width: 1, +}; + +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, + }, + ], + }, + ], +}; + +describe('transformTimeseriesAnnotation', () => { + it('should transform data correctly', () => { + expect( + transformTimeseriesAnnotation( + mockTimeseriesAnnotationLayer, + 1, + mockData, + mockTimeseriesAnnotationData, + CategoricalColorNamespace.getScale(''), + ).map(annotation => annotation.data), + ).toEqual([ + [[10, 12]], + [ + [12, 15], + [15, 20], + ], + ]); + }); + + it('should swap x and y for horizontal chart', () => { + expect( + transformTimeseriesAnnotation( + mockTimeseriesAnnotationLayer, + 1, + mockData, + mockTimeseriesAnnotationData, + CategoricalColorNamespace.getScale(''), + undefined, + OrientationType.Horizontal, + ).map(annotation => annotation.data), + ).toEqual([ + [[12, 10]], + [ + [15, 12], + [20, 15], + ], + ]); + }); +});