fix(plugin-chart-echarts): reorder totals and support multimetric sort (#23675)

2024-09-06 13:57:40 -04:00 · 2023-04-14 20:43:15 +03:00 · 2023-04-14 20:43:15 +03:00 · cbbcc8d2e1
commit cbbcc8d2e1
parent 47fd73255e
6 changed files with 244 additions and 120 deletions
--- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/customControls.tsx
@ -54,27 +54,29 @@ export const contributionModeControl = {
  },
 };

+function isTemporal(controls: ControlStateMapping): boolean {
+  return !(
+    isDefined(controls?.x_axis?.value) &&
+    !isTemporalColumn(
+      getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
+      controls?.datasource?.datasource,
+    )
+  );
+}
+
 const xAxisSortVisibility = ({ controls }: { controls: ControlStateMapping }) =>
-  isDefined(controls?.x_axis?.value) &&
-  !isTemporalColumn(
-    getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
-    controls?.datasource?.datasource,
-  ) &&
-  Array.isArray(controls?.groupby?.value) &&
-  controls.groupby.value.length === 0;
+  !isTemporal(controls) &&
+  ensureIsArray(controls?.groupby?.value).length === 0 &&
+  ensureIsArray(controls?.metrics?.value).length === 1;

 const xAxisMultiSortVisibility = ({
  controls,
 }: {
  controls: ControlStateMapping;
 }) =>
-  isDefined(controls?.x_axis?.value) &&
-  !isTemporalColumn(
-    getColumnLabel(controls?.x_axis?.value as QueryFormColumn),
-    controls?.datasource?.datasource,
-  ) &&
-  Array.isArray(controls?.groupby?.value) &&
-  !!controls.groupby.value.length;
+  !isTemporal(controls) &&
+  (!!ensureIsArray(controls?.groupby?.value).length ||
+    ensureIsArray(controls?.metrics?.value).length > 1);

 export const xAxisSortControl = {
  name: 'x_axis_sort',
--- a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@ -170,12 +170,12 @@ export default function transformProps(
  }

  const rebasedDataA = rebaseForecastDatum(data1, verboseMap);
-  const rawSeriesA = extractSeries(rebasedDataA, {
+  const [rawSeriesA] = extractSeries(rebasedDataA, {
    fillNeighborValue: stack ? 0 : undefined,
    xAxis: xAxisLabel,
  });
  const rebasedDataB = rebaseForecastDatum(data2, verboseMap);
-  const rawSeriesB = extractSeries(rebasedDataB, {
+  const [rawSeriesB] = extractSeries(rebasedDataB, {
    fillNeighborValue: stackB ? 0 : undefined,
    xAxis: xAxisLabel,
  });
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@ -126,6 +126,7 @@ export default function transformProps(
    logAxis,
    markerEnabled,
    markerSize,
+    metrics,
    minorSplitLine,
    onlyTotal,
    opacity,
@ -193,7 +194,9 @@ export default function transformProps(
    getMetricLabel,
  );

-  const rawSeries = extractSeries(rebasedData, {
+  const isMultiSeries = groupby.length || metrics.length > 1;
+
+  const [rawSeries, sortedTotalValues] = extractSeries(rebasedData, {
    fillNeighborValue: stack && !forecastEnabled ? 0 : undefined,
    xAxis: xAxisLabel,
    extraMetricLabels,
@ -202,8 +205,8 @@ export default function transformProps(
    isHorizontal,
    sortSeriesType,
    sortSeriesAscending,
-    xAxisSortSeries: groupby.length ? xAxisSortSeries : undefined,
-    xAxisSortSeriesAscending: groupby.length
+    xAxisSortSeries: isMultiSeries ? xAxisSortSeries : undefined,
+    xAxisSortSeriesAscending: isMultiSeries
      ? xAxisSortSeriesAscending
      : undefined,
  });
@ -240,7 +243,7 @@ export default function transformProps(
      formatter,
      showValue,
      onlyTotal,
-      totalStackedValues,
+      totalStackedValues: sortedTotalValues,
      showValueIndexes,
      thresholdValues,
      richTooltip,
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts
@ -23,6 +23,7 @@ import {
  ContributionType,
  QueryFormColumn,
  QueryFormData,
+  QueryFormMetric,
  TimeFormatter,
  TimeGranularity,
 } from '@superset-ui/core';
@ -65,6 +66,7 @@ export type EchartsTimeseriesFormData = QueryFormData & {
  logAxis: boolean;
  markerEnabled: boolean;
  markerSize: number;
+  metrics: QueryFormMetric[];
  minorSplitLine: boolean;
  opacity: number;
  orderDesc: boolean;
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@ -153,11 +153,12 @@ export function sortAndFilterSeries(

 export function sortRows(
  rows: DataRecord[],
+  totalStackedValues: number[],
  xAxis: string,
  xAxisSortSeries: SortSeriesType,
  xAxisSortSeriesAscending: boolean,
 ) {
-  const sortedRows = rows.map(row => {
+  const sortedRows = rows.map((row, idx) => {
    let sortKey: DataRecordValue = '';
    let aggregate: number | undefined;
    let entries = 0;
@ -219,6 +220,7 @@ export function sortRows(
      key: sortKey,
      value,
      row,
+      totalStackedValue: totalStackedValues[idx],
    };
  });

@ -226,7 +228,7 @@ export function sortRows(
    sortedRows,
    ['value'],
    [xAxisSortSeriesAscending ? 'asc' : 'desc'],
-  ).map(({ row }) => row);
+  ).map(({ row, totalStackedValue }) => ({ row, totalStackedValue }));
 }

 export function extractSeries(
@ -244,7 +246,7 @@ export function extractSeries(
    xAxisSortSeries?: SortSeriesType;
    xAxisSortSeriesAscending?: boolean;
  } = {},
-): SeriesOption[] {
+): [SeriesOption[], number[]] {
  const {
    fillNeighborValue,
    xAxis = DTTM_ALIAS,
@ -258,7 +260,7 @@ export function extractSeries(
    xAxisSortSeries,
    xAxisSortSeriesAscending,
  } = opts;
-  if (data.length === 0) return [];
+  if (data.length === 0) return [[], []];
  const rows: DataRecord[] = data.map(datum => ({
    ...datum,
    [xAxis]: datum[xAxis],
@ -272,14 +274,23 @@ export function extractSeries(
  );
  const sortedRows =
    isDefined(xAxisSortSeries) && isDefined(xAxisSortSeriesAscending)
-      ? sortRows(rows, xAxis, xAxisSortSeries!, xAxisSortSeriesAscending!)
-      : rows;
+      ? sortRows(
+          rows,
+          totalStackedValues,
+          xAxis,
+          xAxisSortSeries!,
+          xAxisSortSeriesAscending!,
+        )
+      : rows.map((row, idx) => ({
+          row,
+          totalStackedValue: totalStackedValues[idx],
+        }));

-  return sortedSeries.map(name => ({
+  const finalSeries = sortedSeries.map(name => ({
    id: name,
    name,
    data: sortedRows
-      .map((row, idx) => {
+      .map(({ row, totalStackedValue }, idx) => {
        const isNextToDefinedValue =
          isDefined(rows[idx - 1]?.[name]) || isDefined(rows[idx + 1]?.[name]);
        const isFillNeighborValue =
@ -291,15 +302,19 @@ export function extractSeries(
          value = fillNeighborValue;
        } else if (
          stack === StackControlsValue.Expand &&
-          totalStackedValues.length > 0
+          totalStackedValue !== undefined
        ) {
-          value = ((value || 0) as number) / totalStackedValues[idx];
+          value = ((value || 0) as number) / totalStackedValue;
        }
        return [row[xAxis], value];
      })
      .filter(obs => !removeNulls || (obs[0] !== null && obs[1] !== null))
      .map(obs => (isHorizontal ? [obs[1], obs[0]] : obs)),
  }));
+  return [
+    finalSeries,
+    sortedRows.map(({ totalStackedValue }) => totalStackedValue),
+  ];
 }

 export function formatSeriesName(
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@ -56,83 +56,165 @@ const sortData: DataRecord[] = [
  { my_x_axis: null, x: 4, y: 3, z: 7 },
 ];

+const totalStackedValues = [3, 15, 14];
+
 test('sortRows by name ascending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Name, true)).toEqual([
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Name,
+      true,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
  ]);
 });

 test('sortRows by name descending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Name, false)).toEqual([
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Name,
+      false,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
  ]);
 });

 test('sortRows by sum ascending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Sum, true)).toEqual([
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Sum,
+      true,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
  ]);
 });

 test('sortRows by sum descending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Sum, false)).toEqual([
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Sum,
+      false,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
  ]);
 });

 test('sortRows by avg ascending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Avg, true)).toEqual([
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Avg,
+      true,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
  ]);
 });

 test('sortRows by avg descending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Avg, false)).toEqual([
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Avg,
+      false,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
  ]);
 });

 test('sortRows by min ascending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Min, true)).toEqual([
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Min,
+      true,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
  ]);
 });

 test('sortRows by min descending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Min, false)).toEqual([
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Min,
+      false,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
  ]);
 });

 test('sortRows by max ascending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Min, true)).toEqual([
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Min,
+      true,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
  ]);
 });

 test('sortRows by max descending', () => {
-  expect(sortRows(sortData, 'my_x_axis', SortSeriesType.Min, false)).toEqual([
-    { my_x_axis: 'foo', x: null, y: 10, z: 5 },
-    { my_x_axis: null, x: 4, y: 3, z: 7 },
-    { my_x_axis: 'abc', x: 1, y: 0, z: 2 },
+  expect(
+    sortRows(
+      sortData,
+      totalStackedValues,
+      'my_x_axis',
+      SortSeriesType.Min,
+      false,
+    ),
+  ).toEqual([
+    { row: { my_x_axis: 'foo', x: null, y: 10, z: 5 }, totalStackedValue: 15 },
+    { row: { my_x_axis: null, x: 4, y: 3, z: 7 }, totalStackedValue: 14 },
+    { row: { my_x_axis: 'abc', x: 1, y: 0, z: 2 }, totalStackedValue: 3 },
  ]);
 });

@ -215,25 +297,29 @@ describe('extractSeries', () => {
        abc: 5,
      },
    ];
-    expect(extractSeries(data)).toEqual([
-      {
-        id: 'Hulk',
-        name: 'Hulk',
-        data: [
-          ['2000-01-01', null],
-          ['2000-02-01', 2],
-          ['2000-03-01', 1],
-        ],
-      },
-      {
-        id: 'abc',
-        name: 'abc',
-        data: [
-          ['2000-01-01', 2],
-          ['2000-02-01', 10],
-          ['2000-03-01', 5],
-        ],
-      },
+    const totalStackedValues = [2, 12, 6];
+    expect(extractSeries(data, { totalStackedValues })).toEqual([
+      [
+        {
+          id: 'Hulk',
+          name: 'Hulk',
+          data: [
+            ['2000-01-01', null],
+            ['2000-02-01', 2],
+            ['2000-03-01', 1],
+          ],
+        },
+        {
+          id: 'abc',
+          name: 'abc',
+          data: [
+            ['2000-01-01', 2],
+            ['2000-02-01', 10],
+            ['2000-03-01', 5],
+          ],
+        },
+      ],
+      totalStackedValues,
    ]);
  });

@ -255,20 +341,30 @@ describe('extractSeries', () => {
        abc: 5,
      },
    ];
-    expect(extractSeries(data, { xAxis: 'x', removeNulls: true })).toEqual([
-      {
-        id: 'Hulk',
-        name: 'Hulk',
-        data: [[2, 1]],
-      },
-      {
-        id: 'abc',
-        name: 'abc',
-        data: [
-          [1, 2],
-          [2, 5],
-        ],
-      },
+    const totalStackedValues = [3, 12, 8];
+    expect(
+      extractSeries(data, {
+        totalStackedValues,
+        xAxis: 'x',
+        removeNulls: true,
+      }),
+    ).toEqual([
+      [
+        {
+          id: 'Hulk',
+          name: 'Hulk',
+          data: [[2, 1]],
+        },
+        {
+          id: 'abc',
+          name: 'abc',
+          data: [
+            [1, 2],
+            [2, 5],
+          ],
+        },
+      ],
+      totalStackedValues,
    ]);
  });

@ -315,23 +411,29 @@ describe('extractSeries', () => {
        abc: null,
      },
    ];
-    expect(extractSeries(data, { fillNeighborValue: 0 })).toEqual([
-      {
-        id: 'abc',
-        name: 'abc',
-        data: [
-          ['2000-01-01', null],
-          ['2000-02-01', 0],
-          ['2000-03-01', 1],
-          ['2000-04-01', 0],
-          ['2000-05-01', null],
-          ['2000-06-01', 0],
-          ['2000-07-01', 2],
-          ['2000-08-01', 3],
-          ['2000-09-01', 0],
-          ['2000-10-01', null],
-        ],
-      },
+    const totalStackedValues = [0, 0, 1, 0, 0, 0, 2, 3, 0, 0];
+    expect(
+      extractSeries(data, { totalStackedValues, fillNeighborValue: 0 }),
+    ).toEqual([
+      [
+        {
+          id: 'abc',
+          name: 'abc',
+          data: [
+            ['2000-01-01', null],
+            ['2000-02-01', 0],
+            ['2000-03-01', 1],
+            ['2000-04-01', 0],
+            ['2000-05-01', null],
+            ['2000-06-01', 0],
+            ['2000-07-01', 2],
+            ['2000-08-01', 3],
+            ['2000-09-01', 0],
+            ['2000-10-01', null],
+          ],
+        },
+      ],
+      totalStackedValues,
    ]);
  });
 });