fix: Reordering echart props to fix confidence interval in Mixed Char…

…ts (apache#30716) Co-authored-by: Vedant Prajapati <[email protected]>
preset-io · Jan 28, 2025 · f4efce3 · f4efce3
1 parent 23d9f46
commit f4efce3
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 3 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
@@ -78,6 +78,7 @@ import {
   extractForecastValuesFromTooltipParams,
   formatForecastTooltipSeries,
   rebaseForecastDatum,
+  reorderForecastSeries,
 } from '../utils/forecast';
 import { convertInteger } from '../utils/convertInteger';
 import { defaultGrid, defaultYAxis } from '../defaults';
@@ -661,7 +662,7 @@ export default function transformProps(
         .map(entry => entry.name || '')
         .concat(extractAnnotationLabels(annotationLayers, annotationData)),
     },
-    series: dedupSeries(series),
+    series: dedupSeries(reorderForecastSeries(series) as SeriesOption[]),
     toolbox: {
       show: zoomable,
       top: TIMESERIES_CONSTANTS.toolboxTop,

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -80,6 +80,7 @@ import {
   extractForecastValuesFromTooltipParams,
   formatForecastTooltipSeries,
   rebaseForecastDatum,
+  reorderForecastSeries,
 } from '../utils/forecast';
 import { convertInteger } from '../utils/convertInteger';
 import { defaultGrid, defaultYAxis } from '../defaults';
@@ -624,7 +625,7 @@ export default function transformProps(
       ),
       data: legendData as string[],
     },
-    series: dedupSeries(series),
+    series: dedupSeries(reorderForecastSeries(series) as SeriesOption[]),
     toolbox: {
       show: zoomable,
       top: TIMESERIES_CONSTANTS.toolboxTop,

diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts b/superset-frontend/plugins/plugin-chart-echarts/src/utils/forecast.ts
@@ -17,7 +17,7 @@
  * under the License.
  */
 import { DataRecord, DTTM_ALIAS, ValueFormatter } from '@superset-ui/core';
-import type { OptionName } from 'echarts/types/src/util/types';
+import type { OptionName, SeriesOption } from 'echarts/types/src/util/types';
 import type { TooltipMarker } from 'echarts/types/src/util/format';
 import {
   ForecastSeriesContext,
@@ -149,3 +149,34 @@ export function rebaseForecastDatum(
     return newRow;
   });
 }
+
+// For Confidence Bands, forecast series on mixed charts require the series sent in the following sortOrder:
+export function reorderForecastSeries(row: SeriesOption[]): SeriesOption[] {
+  const sortOrder = {
+    [ForecastSeriesEnum.ForecastLower]: 1,
+    [ForecastSeriesEnum.ForecastUpper]: 2,
+    [ForecastSeriesEnum.ForecastTrend]: 3,
+    [ForecastSeriesEnum.Observation]: 4,
+  };
+
+  // Check if any item needs reordering
+  if (
+    !row.some(
+      item =>
+        item.id &&
+        sortOrder.hasOwnProperty(extractForecastSeriesContext(item.id).type),
+    )
+  ) {
+    return row;
+  }
+
+  return row.sort((a, b) => {
+    const aOrder =
+      sortOrder[extractForecastSeriesContext(a.id ?? '').type] ??
+      Number.MAX_SAFE_INTEGER;
+    const bOrder =
+      sortOrder[extractForecastSeriesContext(b.id ?? '').type] ??
+      Number.MAX_SAFE_INTEGER;
+    return aOrder - bOrder;
+  });
+}
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/forecast.test.ts
@@ -17,11 +17,13 @@
  * under the License.
  */
 import { getNumberFormatter, NumberFormats } from '@superset-ui/core';
+import { SeriesOption } from 'echarts';
 import {
   extractForecastSeriesContext,
   extractForecastValuesFromTooltipParams,
   formatForecastTooltipSeries,
   rebaseForecastDatum,
+  reorderForecastSeries,
 } from '../../src/utils/forecast';
 import { ForecastSeriesEnum } from '../../src/types';
 
@@ -46,6 +48,47 @@ describe('extractForecastSeriesContext', () => {
   });
 });
 
+describe('reorderForecastSeries', () => {
+  it('should reorder the forecast series and preserve values', () => {
+    const input: SeriesOption[] = [
+      { id: `series${ForecastSeriesEnum.Observation}`, data: [10, 20, 30] },
+      { id: `series${ForecastSeriesEnum.ForecastTrend}`, data: [15, 25, 35] },
+      { id: `series${ForecastSeriesEnum.ForecastLower}`, data: [5, 15, 25] },
+      { id: `series${ForecastSeriesEnum.ForecastUpper}`, data: [25, 35, 45] },
+    ];
+    const expectedOutput: SeriesOption[] = [
+      { id: `series${ForecastSeriesEnum.ForecastLower}`, data: [5, 15, 25] },
+      { id: `series${ForecastSeriesEnum.ForecastUpper}`, data: [25, 35, 45] },
+      { id: `series${ForecastSeriesEnum.ForecastTrend}`, data: [15, 25, 35] },
+      { id: `series${ForecastSeriesEnum.Observation}`, data: [10, 20, 30] },
+    ];
+    expect(reorderForecastSeries(input)).toEqual(expectedOutput);
+  });
+
+  it('should handle an empty array', () => {
+    expect(reorderForecastSeries([])).toEqual([]);
+  });
+
+  it('should not reorder if no relevant series are present', () => {
+    const input: SeriesOption[] = [{ id: 'some-other-series' }];
+    expect(reorderForecastSeries(input)).toEqual(input);
+  });
+
+  it('should handle undefined ids', () => {
+    const input: SeriesOption[] = [
+      { id: `series${ForecastSeriesEnum.ForecastLower}` },
+      { id: undefined },
+      { id: `series${ForecastSeriesEnum.ForecastTrend}` },
+    ];
+    const expectedOutput: SeriesOption[] = [
+      { id: `series${ForecastSeriesEnum.ForecastLower}` },
+      { id: `series${ForecastSeriesEnum.ForecastTrend}` },
+      { id: undefined },
+    ];
+    expect(reorderForecastSeries(input)).toEqual(expectedOutput);
+  });
+});
+
 describe('rebaseForecastDatum', () => {
   it('should subtract lower confidence level from upper value', () => {
     expect(