You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2026/06/24 17:21:08 UTC
(superset) branch master updated: fix(plugin-chart-ag-grid-table): show correct percent-metric totals in summary row (#41247)
This is an automated email from the ASF dual-hosted git repository.
kgabryje 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 2d78a8733cd fix(plugin-chart-ag-grid-table): show correct percent-metric totals in summary row (#41247)
2d78a8733cd is described below
commit 2d78a8733cde68271e2f1a610ec62ddc26c3f1ac
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Wed Jun 24 19:21:00 2026 +0200
fix(plugin-chart-ag-grid-table): show correct percent-metric totals in summary row (#41247)
Signed-off-by: Kamil Gabryjelski <ka...@gmail.com>
---
.../plugin-chart-ag-grid-table/src/buildQuery.ts | 30 ++++++---
.../test/buildQuery.test.ts | 69 +++++++++++++++++++
.../plugins/plugin-chart-table/src/buildQuery.ts | 46 +++++++------
.../plugin-chart-table/test/buildQuery.test.ts | 77 ++++++++++++++++++++++
4 files changed, 189 insertions(+), 33 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts
index bbee0e3ab0b..9753fa01571 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts
@@ -90,6 +90,13 @@ const buildQuery: BuildQuery<TableChartFormData> = (
let { metrics, orderby = [], columns = [] } = baseQueryObject;
const { extras = {} } = baseQueryObject;
let postProcessing: PostProcessingRule[] = [];
+ // Capture the percent-metric `contribution` rule so it can be reused for
+ // the totals query below. The totals query must rename percent-metric
+ // columns the same way (`metric` -> `%metric`) so the footer can look them
+ // up; without it the totals row renders 0.000%. We deliberately reuse only
+ // this rule and not the full `postProcessing` array, which may also contain
+ // a time-comparison operator that must not run on the single totals row.
+ let contributionPostProcessing: PostProcessingRule | undefined;
const nonCustomNorInheritShifts = ensureIsArray(
formData.time_compare,
).filter((shift: string) => shift !== 'custom' && shift !== 'inherit');
@@ -157,15 +164,14 @@ const buildQuery: BuildQuery<TableChartFormData> = (
metrics.concat(percentMetrics),
getMetricLabel,
);
- postProcessing = [
- {
- operation: 'contribution',
- options: {
- columns: percentMetricLabels,
- rename_columns: percentMetricLabels.map(x => `%${x}`),
- },
+ contributionPostProcessing = {
+ operation: 'contribution',
+ options: {
+ columns: percentMetricLabels,
+ rename_columns: percentMetricLabels.map(x => `%${x}`),
},
- ];
+ };
+ postProcessing = [contributionPostProcessing];
}
// Add the operator for the time comparison if some is selected
if (!isEmpty(timeOffsets)) {
@@ -658,7 +664,13 @@ const buildQuery: BuildQuery<TableChartFormData> = (
extras: totalsExtras, // Use extras with AG Grid WHERE removed
row_limit: 0,
row_offset: 0,
- post_processing: [],
+ // Reapply only the percent-metric contribution rule so the totals row
+ // exposes `%metric` keys (value/value = 100% on the single aggregated
+ // row). The time-comparison operator from the main query is omitted on
+ // purpose; it must not run against the single-row totals query.
+ post_processing: contributionPostProcessing
+ ? [contributionPostProcessing]
+ : [],
order_desc: undefined, // we don't need orderby stuff here,
orderby: undefined, // because this query will be used for get total aggregation.
});
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts
index ecb00812330..7e9b185b73f 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts
@@ -852,6 +852,75 @@ describe('plugin-chart-ag-grid-table', () => {
expect(totalsQuery.columns).toEqual([]);
expect(totalsQuery.row_limit).toBe(0);
});
+
+ test('should reapply percent-metric contribution op to totals query', () => {
+ // Regression test for #37627: when a percent metric is configured and
+ // Show Summary (show_totals) is enabled, the totals query must rename
+ // percent-metric columns (`metric` -> `%metric`) so the footer can
+ // look them up. Otherwise the totals row renders 0.000%.
+ const { queries } = buildQuery({
+ ...basicFormData,
+ metrics: ['count'],
+ percent_metrics: ['count'],
+ show_totals: true,
+ query_mode: QueryMode.Aggregate,
+ });
+
+ // No server pagination -> queries[1] is the totals query.
+ const totalsQuery = queries[1];
+ const contributionRule = {
+ operation: 'contribution',
+ options: {
+ columns: ['count'],
+ rename_columns: ['%count'],
+ },
+ };
+
+ expect(queries[0].post_processing).toContainEqual(contributionRule);
+ expect(totalsQuery.post_processing).toEqual([contributionRule]);
+ });
+
+ test('should omit time-comparison op from totals post_processing', () => {
+ // The totals query must reuse ONLY the contribution rule; the
+ // time-comparison operator from the main query must not run against
+ // the single-row totals query.
+ const { queries } = buildQuery({
+ ...basicFormData,
+ metrics: ['count'],
+ percent_metrics: ['count'],
+ show_totals: true,
+ query_mode: QueryMode.Aggregate,
+ time_compare: ['1 year ago'],
+ comparison_type: 'values',
+ });
+
+ const totalsQuery = queries[1];
+
+ // Exactly one op (contribution) — the time-comparison operator from the
+ // main query must not be carried over to the single-row totals query.
+ expect(totalsQuery.post_processing).toHaveLength(1);
+ expect(totalsQuery.post_processing?.[0]).toMatchObject({
+ operation: 'contribution',
+ });
+ // The reused rule matches the main query's contribution rule verbatim.
+ expect(totalsQuery.post_processing?.[0]).toEqual(
+ queries[0].post_processing?.find(
+ op => op?.operation === 'contribution',
+ ),
+ );
+ });
+
+ test('should leave totals post_processing empty without percent metrics', () => {
+ const { queries } = buildQuery({
+ ...basicFormData,
+ metrics: ['count'],
+ show_totals: true,
+ query_mode: QueryMode.Aggregate,
+ });
+
+ const totalsQuery = queries[1];
+ expect(totalsQuery.post_processing).toEqual([]);
+ });
});
describe('Integration - all filter types together', () => {
diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
index fe8278f1c0a..37687c0cf96 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
+++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts
@@ -86,6 +86,13 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
let { metrics, orderby = [], columns = [] } = baseQueryObject;
const { extras = {} } = baseQueryObject;
const postProcessing: PostProcessingRule[] = [];
+ // Capture the percent-metric `contribution` rule so it can be reused for
+ // the totals query below. Without it the totals row's percent-metric
+ // columns are keyed `metric` instead of `%metric`, so the footer renders
+ // 0.000%. We reuse only this rule and not the full `postProcessing` array,
+ // which may also contain a time-comparison operator that must not run on
+ // the single totals row.
+ let contributionPostProcessing: PostProcessingRule | undefined;
const nonCustomNorInheritShifts = ensureIsArray(
formData.time_compare,
).filter((shift: string) => shift !== 'custom' && shift !== 'inherit');
@@ -137,12 +144,6 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
orderby = [[metrics[0], false]];
}
// add postprocessing for percent metrics only when in aggregation mode
- type PercentMetricCalculationMode = 'row_limit' | 'all_records';
-
- const calculationMode: PercentMetricCalculationMode =
- (formData.percent_metric_calculation as PercentMetricCalculationMode) ||
- 'row_limit';
-
if (percentMetrics && percentMetrics.length > 0) {
const percentMetricsLabelsWithTimeComparison = isTimeComparison(
formData,
@@ -162,23 +163,14 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
getMetricLabel,
);
- if (calculationMode === 'all_records') {
- postProcessing.push({
- operation: 'contribution',
- options: {
- columns: percentMetricLabels,
- rename_columns: percentMetricLabels.map(m => `%${m}`),
- },
- });
- } else {
- postProcessing.push({
- operation: 'contribution',
- options: {
- columns: percentMetricLabels,
- rename_columns: percentMetricLabels.map(m => `%${m}`),
- },
- });
- }
+ contributionPostProcessing = {
+ operation: 'contribution',
+ options: {
+ columns: percentMetricLabels,
+ rename_columns: percentMetricLabels.map(m => `%${m}`),
+ },
+ };
+ postProcessing.push(contributionPostProcessing);
}
// Add the operator for the time comparison if some is selected
@@ -357,7 +349,13 @@ export const buildQuery: BuildQuery<TableChartFormData> = (
columns: [],
row_limit: 0,
row_offset: 0,
- post_processing: [],
+ // Reapply only the percent-metric contribution rule so the totals row
+ // exposes `%metric` keys (value/value = 100% on the single aggregated
+ // row). The time-comparison operator from the main query is omitted on
+ // purpose; it must not run against the single-row totals query.
+ post_processing: contributionPostProcessing
+ ? [contributionPostProcessing]
+ : [],
order_desc: undefined,
orderby: undefined,
});
diff --git a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts
index bbea811f594..3b2f095e966 100644
--- a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts
+++ b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts
@@ -236,6 +236,83 @@ describe('plugin-chart-table', () => {
expect(queries).toHaveLength(1);
expect(queries[0].post_processing).toEqual([]);
});
+
+ test('should reapply contribution op to totals query in row_limit mode', () => {
+ // Regression test for #37627: with a percent metric and Show Summary
+ // (show_totals) enabled, the totals query must rename percent-metric
+ // columns (`metric` -> `%metric`) so the footer can look them up.
+ // Otherwise the totals row renders 0.000%.
+ const formData = {
+ ...baseFormDataWithPercents,
+ show_totals: true,
+ };
+
+ const { queries } = buildQuery(formData);
+
+ // row_limit mode + show_totals -> [main, totals].
+ expect(queries).toHaveLength(2);
+
+ const contributionRule = {
+ operation: 'contribution',
+ options: {
+ columns: ['sum_sales'],
+ rename_columns: ['%sum_sales'],
+ },
+ };
+
+ expect(queries[1]).toMatchObject({
+ columns: [],
+ post_processing: [contributionRule],
+ });
+ });
+
+ test('should omit time-comparison op from totals post_processing', () => {
+ // The totals query must reuse ONLY the contribution rule; the
+ // time-comparison operator from the main query must not run against
+ // the single-row totals query.
+ const formData = {
+ ...baseFormDataWithPercents,
+ show_totals: true,
+ time_compare: ['1 year ago'],
+ comparison_type: 'values',
+ };
+
+ const { queries } = buildQuery(formData);
+
+ // row_limit mode + show_totals -> [main, totals].
+ expect(queries).toHaveLength(2);
+
+ const totalsQuery = queries[1];
+
+ // Exactly one op (contribution) — the time-comparison operator from the
+ // main query must not be carried over to the single-row totals query.
+ expect(totalsQuery.post_processing).toHaveLength(1);
+ expect(totalsQuery.post_processing?.[0]).toMatchObject({
+ operation: 'contribution',
+ });
+ // The reused rule matches the main query's contribution rule verbatim.
+ expect(totalsQuery.post_processing?.[0]).toEqual(
+ queries[0].post_processing?.find(
+ op => op?.operation === 'contribution',
+ ),
+ );
+ });
+
+ test('should leave totals post_processing empty without percent metrics', () => {
+ const formData = {
+ ...basicFormData,
+ query_mode: QueryMode.Aggregate,
+ metrics: ['count'],
+ percent_metrics: [],
+ groupby: ['category'],
+ show_totals: true,
+ };
+
+ const { queries } = buildQuery(formData);
+
+ expect(queries).toHaveLength(2);
+ expect(queries[1].post_processing).toEqual([]);
+ });
});
describe('Testing for server pagination with search filter', () => {