fix: annotation broken (apache#20651)

* fix: annotation broken * fix UT * add annotation data to mixed timeseries chart (cherry picked from commit 7f918a4)
airbnb · Jul 12, 2022 · f9544e0 · f9544e0
1 parent 5a78f30
commit f9544e0
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 51 deletions.
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/annotationsAndLayers.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/annotationsAndLayers.tsx
@@ -34,7 +34,7 @@ export const annotationsAndLayersControls: ControlPanelSectionConfig = {
           label: '',
           default: annotationLayers,
           description: t('Annotation Layers'),
-          renderTrigger: true,
+          renderTrigger: false,
         },
       },
     ],

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
@@ -178,9 +178,9 @@ export function isTimeseriesAnnotationResult(
 }
 
 export function isRecordAnnotationResult(
-  result: AnnotationResult,
+  result: any,
 ): result is RecordAnnotationResult {
-  return 'columns' in result && 'records' in result;
+  return Array.isArray(result?.columns) && Array.isArray(result?.records);
 }
 
 export type AnnotationData = { [key: string]: AnnotationResult };

diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts b/superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts
@@ -47,7 +47,7 @@ export interface ChartDataResponseResult {
   /**
    * Data for the annotation layer.
    */
-  annotation_data: AnnotationData[] | null;
+  annotation_data: AnnotationData | null;
   cache_key: string | null;
   cache_timeout: number | null;
   cached_dttm: string | null;

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -48,7 +48,10 @@ import {
   getColtypesMapping,
   getLegendProps,
 } from '../utils/series';
-import { extractAnnotationLabels } from '../utils/annotation';
+import {
+  extractAnnotationLabels,
+  getAnnotationData,
+} from '../utils/annotation';
 import {
   extractForecastSeriesContext,
   extractForecastValuesFromTooltipParams,
@@ -81,11 +84,11 @@ export default function transformProps(
     filterState,
     datasource,
     theme,
-    annotationData = {},
   } = chartProps;
   const { verboseMap = {} } = datasource;
   const data1 = (queriesData[0].data || []) as TimeseriesDataRecord[];
   const data2 = (queriesData[1].data || []) as TimeseriesDataRecord[];
+  const annotationData = getAnnotationData(chartProps);
 
   const {
     area,

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -55,7 +55,10 @@ import {
   extractDataTotalValues,
   extractShowValueIndexes,
 } from '../utils/series';
-import { extractAnnotationLabels } from '../utils/annotation';
+import {
+  extractAnnotationLabels,
+  getAnnotationData,
+} from '../utils/annotation';
 import {
   extractForecastSeriesContext,
   extractForecastSeriesContexts,
@@ -93,12 +96,12 @@ export default function transformProps(
     queriesData,
     datasource,
     theme,
-    annotationData = {},
   } = chartProps;
   const { verboseMap = {} } = datasource;
   const [queryData] = queriesData;
   const { data = [] } = queryData as TimeseriesChartDataResponseResult;
   const dataTypes = getColtypesMapping(queryData);
+  const annotationData = getAnnotationData(chartProps);
 
   const {
     area,

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/annotation.ts
@@ -17,6 +17,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { isEmpty } from 'lodash';
+
 import {
   Annotation,
   AnnotationData,
@@ -30,6 +32,8 @@ import {
   isTimeseriesAnnotationResult,
   TimeseriesDataRecord,
 } from '@superset-ui/core';
+import { EchartsTimeseriesChartProps } from '../types';
+import { EchartsMixedTimeseriesProps } from '../MixedTimeseries/types';
 
 export function evalFormula(
   formula: FormulaAnnotationLayer,
@@ -130,3 +134,13 @@ export function extractAnnotationLabels(
 
   return formulaAnnotationLabels.concat(timeseriesAnnotationLabels);
 }
+
+export function getAnnotationData(
+  chartProps: EchartsTimeseriesChartProps | EchartsMixedTimeseriesProps,
+): AnnotationData {
+  const data = chartProps?.queriesData[0]?.annotation_data as AnnotationData;
+  if (!isEmpty(data)) {
+    return data;
+  }
+  return {};
+}
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
@@ -174,60 +174,62 @@ describe('EchartsTimeseries transformProps', () => {
       titleColumn: '',
       value: 3,
     };
-    const chartProps = new ChartProps({
-      ...chartPropsConfig,
-      formData: {
-        ...formData,
-        annotationLayers: [event, interval, timeseries],
+    const annotationData = {
+      'My Event': {
+        columns: [
+          'start_dttm',
+          'end_dttm',
+          'short_descr',
+          'long_descr',
+          'json_metadata',
+        ],
+        records: [
+          {
+            start_dttm: 0,
+            end_dttm: 1000,
+            short_descr: '',
+            long_descr: '',
+            json_metadata: null,
+          },
+        ],
       },
-      annotationData: {
-        'My Event': {
-          columns: [
-            'start_dttm',
-            'end_dttm',
-            'short_descr',
-            'long_descr',
-            'json_metadata',
-          ],
-          records: [
+      'My Interval': {
+        columns: ['start', 'end', 'title'],
+        records: [
+          {
+            start: 2000,
+            end: 3000,
+            title: 'My Title',
+          },
+        ],
+      },
+      'My Timeseries': [
+        {
+          key: 'My Line',
+          values: [
             {
-              start_dttm: 0,
-              end_dttm: 1000,
-              short_descr: '',
-              long_descr: '',
-              json_metadata: null,
+              x: 10000,
+              y: 11000,
             },
-          ],
-        },
-        'My Interval': {
-          columns: ['start', 'end', 'title'],
-          records: [
             {
-              start: 2000,
-              end: 3000,
-              title: 'My Title',
+              x: 20000,
+              y: 21000,
             },
           ],
         },
-        'My Timeseries': [
-          {
-            key: 'My Line',
-            values: [
-              {
-                x: 10000,
-                y: 11000,
-              },
-              {
-                x: 20000,
-                y: 21000,
-              },
-            ],
-          },
-        ],
+      ],
+    };
+    const chartProps = new ChartProps({
+      ...chartPropsConfig,
+      formData: {
+        ...formData,
+        annotationLayers: [event, interval, timeseries],
       },
+      annotationData,
       queriesData: [
         {
           ...queriesData[0],
+          annotation_data: annotationData,
         },
       ],
     });