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 bbee0e3ab0bb..9753fa015719 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 = ( 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 = ( 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 = ( 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 ecb00812330e..7e9b185b73f2 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 c23a8a49c565..82a276d66f2d 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 @@ const buildQuery: BuildQuery = ( 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 @@ const buildQuery: BuildQuery = ( 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 @@ const buildQuery: BuildQuery = ( 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 @@ -325,7 +317,13 @@ const buildQuery: BuildQuery = ( 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 aa6e1b83001d..2fb369a9bb27 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts @@ -234,6 +234,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', () => {