This is an automated email from the ASF dual-hosted git repository.
jli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 422a07b382 fix: revert "fix: remove sort values on stacked totals
(#31333)" (#32337)
422a07b382 is described below
commit 422a07b3825bd675deb540f652dd3aa3a8737eac
Author: Elizabeth Thompson <[email protected]>
AuthorDate: Fri Feb 21 10:35:06 2025 -0800
fix: revert "fix: remove sort values on stacked totals (#31333)" (#32337)
---
.../superset-ui-core/src/query/getMetricLabel.ts | 24 ++---
.../src/MixedTimeseries/transformProps.ts | 13 +--
.../src/Timeseries/transformProps.ts | 7 +-
.../plugin-chart-echarts/src/utils/series.ts | 6 +-
.../test/Timeseries/transformProps.test.ts | 34 ++----
.../plugin-chart-echarts/test/utils/series.test.ts | 118 ---------------------
6 files changed, 27 insertions(+), 175 deletions(-)
diff --git
a/superset-frontend/packages/superset-ui-core/src/query/getMetricLabel.ts
b/superset-frontend/packages/superset-ui-core/src/query/getMetricLabel.ts
index 246bf99432..7ac7930c6a 100644
--- a/superset-frontend/packages/superset-ui-core/src/query/getMetricLabel.ts
+++ b/superset-frontend/packages/superset-ui-core/src/query/getMetricLabel.ts
@@ -19,23 +19,17 @@
import { QueryFormMetric, isSavedMetric, isAdhocMetricSimple } from './types';
-export default function getMetricLabel(
- metric: QueryFormMetric,
- index?: number,
- queryFormMetrics?: QueryFormMetric[],
- verboseMap?: Record<string, string>,
-): string {
- let label = '';
+export default function getMetricLabel(metric: QueryFormMetric): string {
if (isSavedMetric(metric)) {
- label = metric;
- } else if (metric.label) {
- ({ label } = metric);
- } else if (isAdhocMetricSimple(metric)) {
- label = `${metric.aggregate}(${
+ return metric;
+ }
+ if (metric.label) {
+ return metric.label;
+ }
+ if (isAdhocMetricSimple(metric)) {
+ return `${metric.aggregate}(${
metric.column.columnName || metric.column.column_name
})`;
- } else {
- label = metric.sqlExpression;
}
- return verboseMap?.[label] || label;
+ return metric.sqlExpression;
}
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
index 63582a860c..7526b820d0 100644
---
a/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
+++
b/superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts
@@ -27,7 +27,6 @@ import {
ensureIsArray,
GenericDataType,
getCustomFormatter,
- getMetricLabel,
getNumberFormatter,
getXAxisLabel,
isDefined,
@@ -292,20 +291,12 @@ export default function transformProps(
const showValueIndexesB = extractShowValueIndexes(rawSeriesB, {
stack,
});
-
- const metricsLabels = metrics
- .map(metric => getMetricLabel(metric, undefined, undefined, verboseMap))
- .filter((label): label is string => label !== undefined);
- const metricsLabelsB = metricsB.map((metric: QueryFormMetric) =>
- getMetricLabel(metric, undefined, undefined, verboseMap),
- );
-
const { totalStackedValues, thresholdValues } = extractDataTotalValues(
rebasedDataA,
{
stack,
percentageThreshold,
- metricsLabels,
+ xAxisCol: xAxisLabel,
},
);
const {
@@ -314,7 +305,7 @@ export default function transformProps(
} = extractDataTotalValues(rebasedDataB, {
stack: Boolean(stackB),
percentageThreshold,
- metricsLabels: metricsLabelsB,
+ xAxisCol: xAxisLabel,
});
annotationLayers
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
index d5bfd88095..0fb392a12d 100644
---
a/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
+++
b/superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -215,18 +215,14 @@ export default function transformProps(
) {
xAxisLabel = verboseMap[xAxisLabel];
}
- const metricsLabels = metrics
- .map(metric => getMetricLabel(metric, undefined, undefined, verboseMap))
- .filter((label): label is string => label !== undefined);
const isHorizontal = orientation === OrientationType.Horizontal;
-
const { totalStackedValues, thresholdValues } = extractDataTotalValues(
rebasedData,
{
stack,
percentageThreshold,
+ xAxisCol: xAxisLabel,
legendState,
- metricsLabels,
},
);
const extraMetricLabels = extractExtraMetrics(chartProps.rawFormData).map(
@@ -300,6 +296,7 @@ export default function transformProps(
const entryName = String(entry.name || '');
const seriesName = inverted[entryName] || entryName;
const colorScaleKey = getOriginalSeries(seriesName, array);
+
const transformedSeries = transformSeries(
entry,
colorScale,
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
index 3fe8ef3310..bbf222a2a5 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -60,8 +60,8 @@ export function extractDataTotalValues(
opts: {
stack: StackType;
percentageThreshold: number;
+ xAxisCol: string;
legendState?: LegendState;
- metricsLabels: string[];
},
): {
totalStackedValues: number[];
@@ -69,11 +69,11 @@ export function extractDataTotalValues(
} {
const totalStackedValues: number[] = [];
const thresholdValues: number[] = [];
- const { stack, percentageThreshold, legendState, metricsLabels } = opts;
+ const { stack, percentageThreshold, xAxisCol, legendState } = opts;
if (stack) {
data.forEach(datum => {
const values = Object.keys(datum).reduce((prev, curr) => {
- if (!metricsLabels.includes(curr)) {
+ if (curr === xAxisCol) {
return prev;
}
if (legendState && !legendState[curr]) {
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
index 5601158675..bc2aa2bd58 100644
---
a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
+++
b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts
@@ -36,25 +36,15 @@ describe('EchartsTimeseries transformProps', () => {
colorScheme: 'bnbColors',
datasource: '3__table',
granularity_sqla: 'ds',
- metrics: ['sum__num'],
+ metric: 'sum__num',
groupby: ['foo', 'bar'],
viz_type: 'my_viz',
};
const queriesData = [
{
data: [
- {
- 'San Francisco': 1,
- 'New York': 2,
- __timestamp: 599616000000,
- sum__num: 4,
- },
- {
- 'San Francisco': 3,
- 'New York': 4,
- __timestamp: 599916000000,
- sum__num: 8,
- },
+ { 'San Francisco': 1, 'New York': 2, __timestamp: 599616000000 },
+ { 'San Francisco': 3, 'New York': 4, __timestamp: 599916000000 },
],
},
];
@@ -74,7 +64,7 @@ describe('EchartsTimeseries transformProps', () => {
height: 600,
echartOptions: expect.objectContaining({
legend: expect.objectContaining({
- data: ['sum__num', 'San Francisco', 'New York'],
+ data: ['San Francisco', 'New York'],
}),
series: expect.arrayContaining([
expect.objectContaining({
@@ -111,7 +101,7 @@ describe('EchartsTimeseries transformProps', () => {
height: 600,
echartOptions: expect.objectContaining({
legend: expect.objectContaining({
- data: ['sum__num', 'San Francisco', 'New York'],
+ data: ['San Francisco', 'New York'],
}),
series: expect.arrayContaining([
expect.objectContaining({
@@ -156,7 +146,7 @@ describe('EchartsTimeseries transformProps', () => {
height: 600,
echartOptions: expect.objectContaining({
legend: expect.objectContaining({
- data: ['sum__num', 'San Francisco', 'New York', 'My Formula'],
+ data: ['San Francisco', 'New York', 'My Formula'],
}),
series: expect.arrayContaining([
expect.objectContaining({
@@ -284,7 +274,7 @@ describe('EchartsTimeseries transformProps', () => {
expect.objectContaining({
echartOptions: expect.objectContaining({
legend: expect.objectContaining({
- data: ['sum__num', 'San Francisco', 'New York', 'My Line'],
+ data: ['San Francisco', 'New York', 'My Line'],
}),
series: expect.arrayContaining([
expect.objectContaining({
@@ -430,7 +420,7 @@ describe('Does transformProps transform series correctly',
() => {
colorScheme: 'bnbColors',
datasource: '3__table',
granularity_sqla: 'ds',
- metrics: ['sum__num'],
+ metric: 'sum__num',
groupby: ['foo', 'bar'],
showValue: true,
stack: true,
@@ -445,28 +435,24 @@ describe('Does transformProps transform series
correctly', () => {
'New York': 2,
Boston: 1,
__timestamp: 599616000000,
- sum__num: 4,
},
{
'San Francisco': 3,
'New York': 4,
Boston: 1,
__timestamp: 599916000000,
- sum__num: 8,
},
{
'San Francisco': 5,
'New York': 8,
Boston: 6,
__timestamp: 600216000000,
- sum__num: 19,
},
{
'San Francisco': 2,
'New York': 7,
Boston: 2,
__timestamp: 600516000000,
- sum__num: 11,
},
],
},
@@ -482,7 +468,7 @@ describe('Does transformProps transform series correctly',
() => {
const totalStackedValues = queriesData[0].data.reduce(
(totals, currentStack) => {
const total = Object.keys(currentStack).reduce((stackSum, key) => {
- if (key === '__timestamp' || key === 'sum__num') return stackSum;
+ if (key === '__timestamp') return stackSum;
return stackSum + currentStack[key as keyof typeof currentStack];
}, 0);
totals.push(total);
@@ -575,6 +561,7 @@ describe('Does transformProps transform series correctly',
() => {
const expectedThresholds = totalStackedValues.map(
total => ((formData.percentageThreshold || 0) / 100) * total,
);
+
transformedSeries.forEach((series, seriesIndex) => {
expect(series.label.show).toBe(true);
series.data.forEach((value, dataIndex) => {
@@ -589,6 +576,7 @@ describe('Does transformProps transform series correctly',
() => {
});
});
});
+
it('should not apply percentage threshold when showValue is true and stack
is false', () => {
const updatedChartPropsConfig = {
...chartPropsConfig,
diff --git
a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
index afac03096b..7054f6019a 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -29,7 +29,6 @@ import {
calculateLowerLogTick,
dedupSeries,
extractGroupbyLabel,
- extractDataTotalValues,
extractSeries,
extractShowValueIndexes,
extractTooltipKeys,
@@ -1086,123 +1085,6 @@ const forecastValue = [
},
];
-describe('extractDataTotalValues', () => {
- it('test_extractDataTotalValues_withStack', () => {
- const data: DataRecord[] = [
- { metric1: 10, metric2: 20, xAxisCol: '2021-01-01' },
- { metric1: 15, metric2: 25, xAxisCol: '2021-01-02' },
- ];
- const metricsLabels = ['metric1', 'metric2'];
- const opts = {
- stack: true,
- percentageThreshold: 10,
- metricsLabels,
- };
- const result = extractDataTotalValues(data, opts);
- expect(result.totalStackedValues).toEqual([30, 40]);
- expect(result.thresholdValues).toEqual([3, 4]);
- });
-
- it('should calculate total and threshold values with stack option enabled',
() => {
- const data: DataRecord[] = [
- { metric1: 10, metric2: 20, xAxisCol: '2021-01-01' },
- { metric1: 15, metric2: 25, xAxisCol: '2021-01-02' },
- ];
- const metricsLabels = ['metric1', 'metric2'];
- const opts = {
- stack: true,
- percentageThreshold: 10,
- metricsLabels,
- };
- const result = extractDataTotalValues(data, opts);
- expect(result.totalStackedValues).toEqual([30, 40]);
- expect(result.thresholdValues).toEqual([3, 4]);
- });
-
- it('should handle empty data array', () => {
- const data: DataRecord[] = [];
- const metricsLabels: string[] = [];
- const opts = {
- stack: true,
- percentageThreshold: 10,
- metricsLabels,
- };
- const result = extractDataTotalValues(data, opts);
- expect(result.totalStackedValues).toEqual([]);
- expect(result.thresholdValues).toEqual([]);
- });
-
- it('should calculate total and threshold values with stack option disabled',
() => {
- const data: DataRecord[] = [
- { metric1: 10, metric2: 20, xAxisCol: '2021-01-01' },
- { metric1: 15, metric2: 25, xAxisCol: '2021-01-02' },
- ];
- const metricsLabels = ['metric1', 'metric2'];
- const opts = {
- stack: false,
- percentageThreshold: 10,
- metricsLabels,
- };
- const result = extractDataTotalValues(data, opts);
- expect(result.totalStackedValues).toEqual([]);
- expect(result.thresholdValues).toEqual([]);
- });
-
- it('should handle data with null or undefined values', () => {
- const data: DataRecord[] = [
- { 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 },
- ];
- const metricsLabels = ['x', 'y', 'z'];
- const opts = {
- stack: true,
- percentageThreshold: 10,
- metricsLabels,
- };
- const result = extractDataTotalValues(data, opts);
- expect(result.totalStackedValues).toEqual([3, 15, 14]);
- expect(result.thresholdValues).toEqual([
- 0.30000000000000004, 1.5, 1.4000000000000001,
- ]);
- });
-
- it('should handle different percentage thresholds', () => {
- const data: DataRecord[] = [
- { metric1: 10, metric2: 20, xAxisCol: '2021-01-01' },
- { metric1: 15, metric2: 25, xAxisCol: '2021-01-02' },
- ];
- const metricsLabels = ['metric1', 'metric2'];
- const opts = {
- stack: true,
- percentageThreshold: 50,
- metricsLabels,
- };
- const result = extractDataTotalValues(data, opts);
- expect(result.totalStackedValues).toEqual([30, 40]);
- expect(result.thresholdValues).toEqual([15, 20]);
- });
- it('should not add datum not in metrics to the total value when stacked', ()
=> {
- const data: DataRecord[] = [
- { xAxisCol: 'foo', xAxisSort: 10, val: 345 },
- { xAxisCol: 'bar', xAxisSort: 20, val: 2432 },
- { xAxisCol: 'baz', xAxisSort: 30, val: 4543 },
- ];
- const metricsLabels = ['val'];
- const opts = {
- stack: true,
- percentageThreshold: 50,
- metricsLabels,
- };
-
- const result = extractDataTotalValues(data, opts);
-
- // Assuming extractDataTotalValues returns the total value
- // without including the 'xAxisCol' category
- expect(result.totalStackedValues).toEqual([345, 2432, 4543]); // 10 + 20,
excluding the 'xAxisCol' category
- });
-});
-
test('extractTooltipKeys with rich tooltip', () => {
const result = extractTooltipKeys(forecastValue, 1, true, false);
expect(result).toEqual(['foo', 'bar']);
