fix(line-chart): Formula Annotations on Line Charts are broken (#20687)

apache · Jul 13, 2022 · acdb271 · acdb271
1 parent bd6037e
commit acdb271
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 18 deletions.
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -226,7 +226,14 @@ export default function transformProps(
     .forEach((layer: AnnotationLayer) => {
       if (isFormulaAnnotationLayer(layer))
         series.push(
-          transformFormulaAnnotation(layer, data1, colorScale, sliceId),
+          transformFormulaAnnotation(
+            layer,
+            data1,
+            xAxisCol,
+            xAxisType,
+            colorScale,
+            sliceId,
+          ),
         );
       else if (isIntervalAnnotationLayer(layer)) {
         series.push(

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -223,7 +223,14 @@ export default function transformProps(
     .forEach((layer: AnnotationLayer) => {
       if (isFormulaAnnotationLayer(layer))
         series.push(
-          transformFormulaAnnotation(layer, data, colorScale, sliceId),
+          transformFormulaAnnotation(
+            layer,
+            data,
+            xAxisCol,
+            xAxisType,
+            colorScale,
+            sliceId,
+          ),
         );
       else if (isIntervalAnnotationLayer(layer)) {
         series.push(

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
@@ -52,7 +52,12 @@ import {
 import { MarkLine1DDataItemOption } from 'echarts/types/src/component/marker/MarkLineModel';
 
 import { extractForecastSeriesContext } from '../utils/forecast';
-import { ForecastSeriesEnum, LegendOrientation, StackType } from '../types';
+import {
+  AxisType,
+  ForecastSeriesEnum,
+  LegendOrientation,
+  StackType,
+} from '../types';
 import { EchartsTimeseriesSeriesType } from './types';
 
 import {
@@ -250,6 +255,8 @@ export function transformSeries(
 export function transformFormulaAnnotation(
   layer: FormulaAnnotationLayer,
   data: TimeseriesDataRecord[],
+  xAxisCol: string,
+  xAxisType: AxisType,
   colorScale: CategoricalColorScale,
   sliceId?: number,
 ): SeriesOption {
@@ -267,7 +274,7 @@ export function transformFormulaAnnotation(
     },
     type: 'line',
     smooth: true,
-    data: evalFormula(layer, data),
+    data: evalFormula(layer, data, xAxisCol, xAxisType),
     symbolSize: 0,
   };
 }

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/types.ts b/superset-frontend/plugins/plugin-chart-echarts/src/types.ts
@@ -127,4 +127,6 @@ export interface EchartsTitleFormData {
 
 export type StackType = boolean | null | Partial<AreaChartExtraControlsValue>;
 
+export type AxisType = 'time' | 'value' | 'category';
+
 export * from './Timeseries/types';
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts
@@ -25,26 +25,31 @@ import {
   AnnotationLayer,
   AnnotationOpacity,
   AnnotationType,
+  DataRecord,
   evalExpression,
   FormulaAnnotationLayer,
   isRecordAnnotationResult,
   isTableAnnotationLayer,
   isTimeseriesAnnotationResult,
-  TimeseriesDataRecord,
 } from '@superset-ui/core';
-import { EchartsTimeseriesChartProps } from '../types';
+import { AxisType, EchartsTimeseriesChartProps } from '../types';
 import { EchartsMixedTimeseriesProps } from '../MixedTimeseries/types';
 
 export function evalFormula(
   formula: FormulaAnnotationLayer,
-  data: TimeseriesDataRecord[],
-): [number, number][] {
+  data: DataRecord[],
+  xAxis: string,
+  xAxisType: AxisType,
+): [any, number][] {
   const { value: expression } = formula;
 
-  return data.map(row => [
-    Number(row.__timestamp),
-    evalExpression(expression, row.__timestamp as number),
-  ]);
+  return data.map(row => {
+    let value = row[xAxis];
+    if (xAxisType === 'time') {
+      value = new Date(value as string).getTime();
+    }
+    return [value, evalExpression(expression, (value || 0) as number)];
+  });
 }
 
 export function parseAnnotationOpacity(opacity?: AnnotationOpacity): number {

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -33,7 +33,7 @@ import {
   NULL_STRING,
   TIMESERIES_CONSTANTS,
 } from '../constants';
-import { LegendOrientation, LegendType, StackType } from '../types';
+import { AxisType, LegendOrientation, LegendType, StackType } from '../types';
 import { defaultLegendPadding } from '../defaults';
 
 function isDefined<T>(value: T | undefined | null): boolean {
@@ -307,9 +307,7 @@ export const currentSeries = {
   legend: '',
 };
 
-export function getAxisType(
-  dataType?: GenericDataType,
-): 'time' | 'value' | 'category' {
+export function getAxisType(dataType?: GenericDataType): AxisType {
   if (dataType === GenericDataType.TEMPORAL) {
     return 'time';
   }

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
@@ -25,6 +25,7 @@ import {
   AnnotationType,
   FormulaAnnotationLayer,
   TimeseriesDataRecord,
+  DataRecord,
 } from '@superset-ui/core';
 import {
   evalFormula,
@@ -160,7 +161,7 @@ describe('evalFormula', () => {
       { __timestamp: 10 },
     ];
 
-    expect(evalFormula(layer, data)).toEqual([
+    expect(evalFormula(layer, data, '__timestamp', 'time')).toEqual([
       [0, 1],
       [10, 11],
     ]);
@@ -172,9 +173,27 @@ describe('evalFormula', () => {
       { __timestamp: 10 },
     ];
 
-    expect(evalFormula({ ...layer, value: 'y  = x* 2   -1' }, data)).toEqual([
+    expect(
+      evalFormula(
+        { ...layer, value: 'y  = x* 2   -1' },
+        data,
+        '__timestamp',
+        'time',
+      ),
+    ).toEqual([
       [0, -1],
       [10, 19],
     ]);
   });
+
+  it('Should evaluate a formula if axis type is category', () => {
+    const data: DataRecord[] = [{ gender: 'boy' }, { gender: 'girl' }];
+
+    expect(
+      evalFormula({ ...layer, value: 'y = 1000' }, data, 'gender', 'category'),
+    ).toEqual([
+      ['boy', 1000],
+      ['girl', 1000],
+    ]);
+  });
 });