fix(echarts): formula annotations not rendering with dataset-level columns label (#37522)

2026-04-07 10:31:50 +00:00 · 2026-02-13 06:37:19 -03:00
parent 142b2cc425
commit 080f629ea2
5 changed files with 727 additions and 373 deletions
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -360,7 +360,7 @@ export default function transformProps(
        series.push(
          transformFormulaAnnotation(
            layer,
-            data1,
+            rebasedDataA as TimeseriesDataRecord[],
            xAxisLabel,
            xAxisType,
            colorScale,
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -39,6 +39,7 @@ import {
  isTimeseriesAnnotationLayer,
  resolveAutoCurrency,
  TimeseriesChartDataResponseResult,
+  TimeseriesDataRecord,
  NumberFormats,
 } from '@superset-ui/core';
 import { GenericDataType } from '@apache-superset/core/api/core';
@@ -463,7 +464,7 @@ export default function transformProps(
        series.push(
          transformFormulaAnnotation(
            layer,
-            data,
+            rebasedData as TimeseriesDataRecord[],
            xAxisLabel,
            xAxisType,
            colorScale,
--- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts
@@ -16,8 +16,14 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import { ChartProps, VizType } from '@superset-ui/core';
-import { supersetTheme } from '@apache-superset/core/ui';
+import {
+  AnnotationStyle,
+  AnnotationType,
+  DataRecord,
+  FormulaAnnotationLayer,
+  VizType,
+  ChartDataResponseResult,
+} from '@superset-ui/core';
 import {
  LegendOrientation,
  LegendType,
@@ -28,6 +34,48 @@ import {
  EchartsMixedTimeseriesFormData,
  EchartsMixedTimeseriesProps,
 } from '../../src/MixedTimeseries/types';
+import { DEFAULT_FORM_DATA } from '../../src/MixedTimeseries/types';
+import { createEchartsTimeseriesTestChartProps } from '../helpers';
+import type { SeriesOption } from 'echarts';
+
+/**
+ * Creates a partial ChartDataResponseResult for testing.
+ * Only includes the fields needed for tests, with sensible defaults for required fields.
+ */
+function createTestQueryData(
+  data: unknown[],
+  overrides?: Partial<ChartDataResponseResult> & {
+    label_map?: Record<string, string[]>;
+  },
+): ChartDataResponseResult {
+  return {
+    annotation_data: null,
+    cache_key: null,
+    cache_timeout: null,
+    cached_dttm: null,
+    queried_dttm: null,
+    data: data as DataRecord[],
+    colnames: [],
+    coltypes: [],
+    error: null,
+    is_cached: false,
+    query: '',
+    rowcount: data.length,
+    sql_rowcount: data.length,
+    stacktrace: null,
+    status: 'success',
+    from_dttm: null,
+    to_dttm: null,
+    label_map: {},
+    ...overrides,
+  } as ChartDataResponseResult & { label_map?: Record<string, string[]> };
+}
+
+/** Defaults for createEchartsTimeseriesTestChartProps in Mixed Timeseries tests. */
+const MIXED_TIMESERIES_CHART_PROPS_DEFAULTS = {
+  defaultFormData: DEFAULT_FORM_DATA,
+  defaultVizType: 'mixed_timeseries' as const,
+};

 const formData: EchartsMixedTimeseriesFormData = {
  annotationLayers: [],
@@ -85,49 +133,28 @@ const formData: EchartsMixedTimeseriesFormData = {
  legendSort: null,
 };

-const queriesData = [
-  {
-    data: [
-      { boy: 1, girl: 2, ds: 599616000000 },
-      { boy: 3, girl: 4, ds: 599916000000 },
-    ],
-    label_map: {
-      ds: ['ds'],
-      boy: ['boy'],
-      girl: ['girl'],
-    },
-  },
-  {
-    data: [
-      { boy: 1, girl: 2, ds: 599616000000 },
-      { boy: 3, girl: 4, ds: 599916000000 },
-    ],
-    label_map: {
-      ds: ['ds'],
-      boy: ['boy'],
-      girl: ['girl'],
-    },
-  },
+const defaultQueryRows = [
+  { boy: 1, girl: 2, ds: 599616000000 },
+  { boy: 3, girl: 4, ds: 599916000000 },
+];
+const defaultLabelMap = { ds: ['ds'], boy: ['boy'], girl: ['girl'] };
+
+const queriesData: ChartDataResponseResult[] = [
+  createTestQueryData(defaultQueryRows, { label_map: defaultLabelMap }),
+  createTestQueryData(defaultQueryRows, { label_map: defaultLabelMap }),
 ];

-const chartPropsConfig = {
-  formData,
-  width: 800,
-  height: 600,
-  queriesData,
-  theme: supersetTheme,
-};
-
 test('should transform chart props for viz with showQueryIdentifiers=false', () => {
-  const chartPropsConfigWithoutIdentifiers = {
-    ...chartPropsConfig,
-    formData: {
-      ...formData,
-      showQueryIdentifiers: false,
-    },
-  };
-  const chartProps = new ChartProps(chartPropsConfigWithoutIdentifiers);
-  const transformed = transformProps(chartProps as EchartsMixedTimeseriesProps);
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    defaultQueriesData: queriesData,
+    formData: { ...formData, showQueryIdentifiers: false },
+    queriesData,
+  });
+  const transformed = transformProps(chartProps);

  // Check that series IDs don't include query identifiers
  const seriesIds = (transformed.echartOptions.series as any[]).map(
@@ -160,15 +187,16 @@ test('should transform chart props for viz with showQueryIdentifiers=false', ()
 });

 test('should transform chart props for viz with showQueryIdentifiers=true', () => {
-  const chartPropsConfigWithIdentifiers = {
-    ...chartPropsConfig,
-    formData: {
-      ...formData,
-      showQueryIdentifiers: true,
-    },
-  };
-  const chartProps = new ChartProps(chartPropsConfigWithIdentifiers);
-  const transformed = transformProps(chartProps as EchartsMixedTimeseriesProps);
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    defaultQueriesData: queriesData,
+    formData: { ...formData, showQueryIdentifiers: true },
+    queriesData,
+  });
+  const transformed = transformProps(chartProps);

  // Check that series IDs include query identifiers
  const seriesIds = (transformed.echartOptions.series as any[]).map(
@@ -202,22 +230,25 @@ test('should transform chart props for viz with showQueryIdentifiers=true', () =

 describe('legend sorting', () => {
  const getChartProps = (overrides = {}) =>
-    new ChartProps({
-      ...chartPropsConfig,
+    createEchartsTimeseriesTestChartProps<
+      EchartsMixedTimeseriesFormData,
+      EchartsMixedTimeseriesProps
+    >({
+      ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+      defaultQueriesData: queriesData,
      formData: {
        ...formData,
        ...overrides,
        showQueryIdentifiers: true,
      },
+      queriesData,
    });

  test('sort legend by data', () => {
    const chartProps = getChartProps({
      legendSort: null,
    });
-    const transformed = transformProps(
-      chartProps as EchartsMixedTimeseriesProps,
-    );
+    const transformed = transformProps(chartProps);

    expect((transformed.echartOptions.legend as any).data).toEqual([
      'sum__num (Query A), girl',
@@ -231,9 +262,7 @@ describe('legend sorting', () => {
    const chartProps = getChartProps({
      legendSort: 'asc',
    });
-    const transformed = transformProps(
-      chartProps as EchartsMixedTimeseriesProps,
-    );
+    const transformed = transformProps(chartProps);

    expect((transformed.echartOptions.legend as any).data).toEqual([
      'sum__num (Query A), boy',
@@ -247,9 +276,7 @@ describe('legend sorting', () => {
    const chartProps = getChartProps({
      legendSort: 'desc',
    });
-    const transformed = transformProps(
-      chartProps as EchartsMixedTimeseriesProps,
-    );
+    const transformed = transformProps(chartProps);

    expect((transformed.echartOptions.legend as any).data).toEqual([
      'sum__num (Query B), girl',
@@ -261,64 +288,148 @@ describe('legend sorting', () => {
 });

 test('legend margin: top orientation sets grid.top correctly', () => {
-  const chartPropsConfigWithoutIdentifiers = {
-    ...chartPropsConfig,
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    defaultQueriesData: queriesData,
    formData: {
      ...formData,
      legendMargin: 250,
      showLegend: true,
    },
-  };
-  const chartProps = new ChartProps(chartPropsConfigWithoutIdentifiers);
-  const transformed = transformProps(chartProps as EchartsMixedTimeseriesProps);
+    queriesData,
+  });
+  const transformed = transformProps(chartProps);

  expect((transformed.echartOptions.grid as any).top).toEqual(270);
 });

 test('legend margin: bottom orientation sets grid.bottom correctly', () => {
-  const chartPropsConfigWithoutIdentifiers = {
-    ...chartPropsConfig,
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    defaultQueriesData: queriesData,
    formData: {
      ...formData,
      legendMargin: 250,
      showLegend: true,
      legendOrientation: LegendOrientation.Bottom,
    },
-  };
-  const chartProps = new ChartProps(chartPropsConfigWithoutIdentifiers);
-  const transformed = transformProps(chartProps as EchartsMixedTimeseriesProps);
+    queriesData,
+  });
+  const transformed = transformProps(chartProps);

  expect((transformed.echartOptions.grid as any).bottom).toEqual(270);
 });

 test('legend margin: left orientation sets grid.left correctly', () => {
-  const chartPropsConfigWithoutIdentifiers = {
-    ...chartPropsConfig,
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    defaultQueriesData: queriesData,
    formData: {
      ...formData,
      legendMargin: 250,
      showLegend: true,
      legendOrientation: LegendOrientation.Left,
    },
-  };
-  const chartProps = new ChartProps(chartPropsConfigWithoutIdentifiers);
-  const transformed = transformProps(chartProps as EchartsMixedTimeseriesProps);
+    queriesData,
+  });
+  const transformed = transformProps(chartProps);

  expect((transformed.echartOptions.grid as any).left).toEqual(270);
 });

 test('legend margin: right orientation sets grid.right correctly', () => {
-  const chartPropsConfigWithoutIdentifiers = {
-    ...chartPropsConfig,
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    defaultQueriesData: queriesData,
    formData: {
      ...formData,
      legendMargin: 270,
      showLegend: true,
      legendOrientation: LegendOrientation.Right,
    },
-  };
-  const chartProps = new ChartProps(chartPropsConfigWithoutIdentifiers);
-  const transformed = transformProps(chartProps as EchartsMixedTimeseriesProps);
+    queriesData,
+  });
+  const transformed = transformProps(chartProps);

  expect((transformed.echartOptions.grid as any).right).toEqual(270);
 });
+
+test('should add a formula annotation when X-axis column has dataset-level label', () => {
+  const formula: FormulaAnnotationLayer = {
+    name: 'My Formula',
+    annotationType: AnnotationType.Formula,
+    value: 'x*2',
+    style: AnnotationStyle.Solid,
+    show: true,
+    showLabel: true,
+  };
+  const timeColumnName = 'ds';
+  const timeColumnLabel = 'Time Label';
+  const testData = [
+    {
+      [timeColumnLabel]: 599616000000,
+      boy: 1,
+      girl: 2,
+    },
+    {
+      [timeColumnLabel]: 599916000000,
+      boy: 3,
+      girl: 4,
+    },
+  ];
+  const chartProps = createEchartsTimeseriesTestChartProps<
+    EchartsMixedTimeseriesFormData,
+    EchartsMixedTimeseriesProps
+  >({
+    ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+    defaultQueriesData: [],
+    formData: {
+      ...formData,
+      x_axis: timeColumnName,
+      annotationLayers: [formula],
+    },
+    queriesData: [
+      createTestQueryData(testData, {
+        label_map: {
+          [timeColumnName]: [timeColumnLabel],
+          boy: ['boy'],
+          girl: ['girl'],
+        },
+      }),
+      createTestQueryData(testData, {
+        label_map: {
+          [timeColumnName]: [timeColumnLabel],
+          boy: ['boy'],
+          girl: ['girl'],
+        },
+      }),
+    ],
+    datasource: {
+      verboseMap: {
+        [timeColumnName]: timeColumnLabel,
+      },
+      columnFormats: {},
+      currencyFormats: {},
+    },
+  });
+  const result = transformProps(chartProps);
+  const formulaSeries = (
+    result.echartOptions.series as SeriesOption[] | undefined
+  )?.find((s: SeriesOption) => s.name === 'My Formula');
+  expect(formulaSeries).toBeDefined();
+  expect(formulaSeries?.data).toBeDefined();
+  expect(Array.isArray(formulaSeries?.data)).toBe(true);
+  expect((formulaSeries?.data as unknown[]).length).toBeGreaterThan(0);
+});
--- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
--- a/superset-frontend/plugins/plugin-chart-echarts/test/helpers.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/helpers.ts
@@ -0,0 +1,110 @@
+/**
+ * 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 { ChartProps, ChartDataResponseResult } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/ui';
+
+/**
+ * Datasource shape used by Echarts Timeseries and Mixed Timeseries chart props.
+ */
+export interface EchartsTimeseriesTestDatasource {
+  verboseMap?: Record<string, string>;
+  columnFormats?: Record<string, string>;
+  currencyFormats?: Record<string, { symbol: string; symbolPosition: string }>;
+  currencyCodeColumn?: string;
+}
+
+const DEFAULT_DATASOURCE: EchartsTimeseriesTestDatasource = {
+  verboseMap: {},
+  columnFormats: {},
+  currencyFormats: {},
+};
+
+/**
+ * Form data shape that at minimum has datasource and viz_type (used for merging).
+ */
+export interface EchartsTimeseriesTestFormDataBase {
+  datasource?: string;
+  viz_type?: string;
+  [key: string]: unknown;
+}
+
+/**
+ * Config for creating Echarts Timeseries-style chart props in tests.
+ * Shared by Timeseries and Mixed Timeseries transformProps tests.
+ */
+export interface CreateEchartsTimeseriesTestChartPropsConfig<TFormData> {
+  defaultFormData: TFormData;
+  defaultVizType: string;
+  defaultQueriesData?: ChartDataResponseResult[];
+  formData?: Partial<TFormData>;
+  queriesData?: ChartDataResponseResult[];
+  datasource?: EchartsTimeseriesTestDatasource;
+  annotationData?: Record<string, unknown>;
+  width?: number;
+  height?: number;
+}
+
+/**
+ * Creates chart props for Echarts Timeseries-style plugins in tests.
+ * Merges partial formData with defaultFormData and builds a ChartProps-like object.
+ * Use this to avoid duplicating createTestChartProps in Timeseries and Mixed Timeseries tests.
+ *
+ * @param config - defaultFormData, defaultVizType, defaultQueriesData, and optional overrides
+ * @returns Chart props object typed as TProps (e.g. EchartsTimeseriesChartProps)
+ */
+export function createEchartsTimeseriesTestChartProps<
+  TFormData extends EchartsTimeseriesTestFormDataBase,
+  TProps,
+>(config: CreateEchartsTimeseriesTestChartPropsConfig<TFormData>): TProps {
+  const {
+    defaultFormData,
+    defaultVizType,
+    defaultQueriesData = [],
+    formData: partialFormData = {},
+    queriesData: customQueriesData,
+    datasource: customDatasource,
+    annotationData,
+    width = 800,
+    height = 600,
+  } = config;
+
+  const partial = partialFormData as Partial<EchartsTimeseriesTestFormDataBase>;
+  const fullFormData = {
+    ...defaultFormData,
+    ...partialFormData,
+    datasource: partial.datasource ?? '3__table',
+    viz_type: partial.viz_type ?? defaultVizType,
+  } as TFormData;
+
+  const chartProps = new ChartProps({
+    formData: fullFormData,
+    width,
+    height,
+    queriesData: customQueriesData ?? defaultQueriesData,
+    theme: supersetTheme,
+    datasource: customDatasource ?? { ...DEFAULT_DATASOURCE },
+    ...(annotationData !== undefined && { annotationData }),
+  });
+
+  return {
+    ...chartProps,
+    formData: fullFormData,
+    queriesData: customQueriesData ?? defaultQueriesData,
+  } as TProps;
+}