You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ms...@apache.org on 2026/09/17 06:26:20 UTC
(superset) 01/01: fix(plugin-chart-table): resolve auto-detected currency on time comparison columns
This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a commit to branch agorithm/currency-time-comparison
in repository https://gitbox.apache.org/repos/asf/superset.git
commit efaeac744a08d66bbd3eb64e8fd2e58b1f2bd631
Author: Mehmet Yavuz <me...@ext.preset.io>
AuthorDate: Thu Sep 17 06:26:09 2026 +0000
fix(plugin-chart-table): resolve auto-detected currency on time comparison columns
Auto-detect currency was applied only when building the main metric columns.
getComparisonColFormatter rebuilds the formatter for the Main/#/triangle columns
whenever they carry a number or currency format, and it did so from the raw
column config, so an AUTO currency never resolved against the currency detected
by the backend. CurrencyFormatter can then only resolve AUTO from a per-row
currency code, which is present only when the currency code column is part of
the query result -- so the time comparison columns rendered plain numbers unless
that column was added to the chart.
Extract the existing AUTO resolution into a shared helper and use it when
building the comparison column formatters too. The config is still left in AUTO
mode when the currency code column is queried, so the per-row lookup keeps
winning there. The same gap and fix apply to plugin-chart-ag-grid-table.
Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
.../src/transformProps.ts | 74 ++++++++++---
.../test/transformProps.test.ts | 69 ++++++++++++
.../plugin-chart-table/src/transformProps.ts | 74 ++++++++++---
.../plugin-chart-table/test/transformProps.test.ts | 117 +++++++++++++++++++++
4 files changed, 300 insertions(+), 34 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts
index fde606dae12..5dad10a8621 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/transformProps.ts
@@ -141,12 +141,41 @@ const getComparisonColConfig = (
return comparisonColConfig;
};
+/**
+ * Resolve an AUTO currency config against the currency detected by the backend.
+ *
+ * When the currency code column is part of the query result the config is left
+ * in AUTO mode on purpose, so that CurrencyFormatter resolves it per row from
+ * the row's own currency code rather than from a single chart-wide value.
+ */
+const resolveDetectedCurrency = (
+ currency: Currency | undefined,
+ detectedCurrency: string | null | undefined,
+ currencyCodeColumn: string | undefined,
+ colnames: string[] | undefined,
+): Currency | undefined => {
+ if (
+ currency?.symbol !== 'AUTO' ||
+ !detectedCurrency ||
+ (currencyCodeColumn && colnames?.includes(currencyCodeColumn))
+ ) {
+ return currency;
+ }
+ const normalizedCurrency = normalizeCurrency(detectedCurrency);
+ return normalizedCurrency
+ ? { ...currency, symbol: normalizedCurrency }
+ : currency;
+};
+
const getComparisonColFormatter = (
label: string,
parentCol: DataColumnMeta,
columnConfig: Record<string, TableColumnConfig>,
savedFormat: string | undefined,
savedCurrency: Currency | undefined,
+ detectedCurrency: string | null | undefined,
+ currencyCodeColumn: string | undefined,
+ colnames: string[] | undefined,
) => {
const currentColConfig = getComparisonColConfig(
label,
@@ -161,7 +190,12 @@ const getComparisonColFormatter = (
if (label === '%') {
formatter = getNumberFormatter(currentColNumberFormat || PERCENT_3_POINT);
} else if (currentColNumberFormat || hasCurrency) {
- const currency = currentColConfig.currencyFormat || savedCurrency;
+ const currency = resolveDetectedCurrency(
+ currentColConfig.currencyFormat || savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
+ );
const numberFormat = currentColNumberFormat || savedFormat;
formatter = currency
? new CurrencyFormatter({
@@ -254,9 +288,12 @@ const processComparisonColumns = (
) =>
columns.flatMap(col => {
const {
- datasource: { columnFormats, currencyFormats },
+ datasource: { columnFormats, currencyFormats, currencyCodeColumn },
rawFormData: { column_config: columnConfig = {} },
+ queriesData,
} = props;
+ const { colnames, detected_currency: detectedCurrency } =
+ queriesData[0] || {};
const savedFormat = columnFormats?.[col.key];
const savedCurrency = currencyFormats?.[col.key];
const originalLabel = col.label;
@@ -279,6 +316,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
{
@@ -294,6 +334,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
{
@@ -309,6 +352,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
{
@@ -324,6 +370,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
];
@@ -477,21 +526,12 @@ const processColumns = memoizePerChart(function processColumns(
// percent metrics have a default format
formatter = getNumberFormatter(numberFormat || PERCENT_3_POINT);
} else if (isMetric || (isNumber && (numberFormat || currency))) {
- // Resolve AUTO currency when currency column isn't in query results
- let resolvedCurrency = currency;
- if (
- currency?.symbol === 'AUTO' &&
- detectedCurrency &&
- (!currencyCodeColumn || !colnames?.includes(currencyCodeColumn))
- ) {
- const normalizedCurrency = normalizeCurrency(detectedCurrency);
- if (normalizedCurrency) {
- resolvedCurrency = {
- ...currency,
- symbol: normalizedCurrency,
- };
- }
- }
+ const resolvedCurrency = resolveDetectedCurrency(
+ currency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
+ );
formatter = resolvedCurrency?.symbol
? new CurrencyFormatter({
d3Format: numberFormat,
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts
index 61c12b3bc06..cab06cddfbf 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/transformProps.test.ts
@@ -512,3 +512,72 @@ test('retains saved percentage rules with automatic bounds when server paginatio
result.columnColorFormatters.map(formatter => formatter.column),
).toEqual(['metric_a', 'metric_b']);
});
+
+const AUTO_CURRENCY = { symbol: 'AUTO', symbolPosition: 'prefix' as const };
+
+/**
+ * Time Comparison enabled on a metric whose currency is set to "Auto-detect",
+ * with the dataset's currency code column deliberately absent from the query
+ * result, so AUTO can only resolve via the backend-supplied detected_currency.
+ */
+const createAutoCurrencyComparisonProps = (
+ colnames: string[] = ['metric_1', 'metric_1__1 year ago'],
+) =>
+ createMockChartProps({
+ rawFormData: {
+ viz_type: 'ag_grid_table',
+ datasource: '1__table',
+ query_mode: QueryMode.Aggregate,
+ metrics: ['metric_1'],
+ percent_metrics: [],
+ table_timestamp_format: '',
+ time_compare: ['1 year ago'],
+ comparison_type: 'values',
+ column_config: {
+ metric_1: { currencyFormat: AUTO_CURRENCY },
+ '# metric_1': { d3NumberFormat: ',.1f' },
+ '△ metric_1': { d3NumberFormat: ',.1f' },
+ },
+ } as unknown as TableChartProps['rawFormData'],
+ queriesData: [
+ {
+ data: [{ metric_1: 100, 'metric_1__1 year ago': 80 }],
+ colnames,
+ coltypes: colnames.map(() => GenericDataType.Numeric),
+ rowcount: 1,
+ applied_filters: [],
+ rejected_filters: [],
+ detected_currency: 'GBP',
+ } as unknown as TableChartProps['queriesData'][number],
+ ],
+ datasource: {
+ columns: [],
+ metrics: [],
+ columnFormats: {},
+ currencyFormats: { metric_1: AUTO_CURRENCY },
+ currencyCodeColumn: 'currency_code',
+ verboseMap: {},
+ } as unknown as TableChartProps['datasource'],
+ });
+
+test('resolves AUTO currency on time comparison columns when the currency code column is not in the query result', () => {
+ const { columns } = transformProps(createAutoCurrencyComparisonProps());
+
+ const comparisonCol = columns.find(c => c.key === '# metric_1');
+ expect(comparisonCol).toBeDefined();
+ expect(comparisonCol!.formatter?.(20)).toContain('£');
+});
+
+test('leaves AUTO currency unresolved on time comparison columns when the currency code column is in the query result', () => {
+ const { columns } = transformProps(
+ createAutoCurrencyComparisonProps([
+ 'metric_1',
+ 'metric_1__1 year ago',
+ 'currency_code',
+ ]),
+ );
+
+ const comparisonCol = columns.find(c => c.key === '# metric_1');
+ expect(comparisonCol).toBeDefined();
+ expect(comparisonCol!.formatter?.(20)).not.toContain('£');
+});
diff --git a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
index 21442b9f0fa..fc3ebed82cb 100644
--- a/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-table/src/transformProps.ts
@@ -197,6 +197,32 @@ const processComparisonDataRecords = memoizeOne(
},
);
+/**
+ * Resolve an AUTO currency config against the currency detected by the backend.
+ *
+ * When the currency code column is part of the query result the config is left
+ * in AUTO mode on purpose, so that CurrencyFormatter resolves it per row from
+ * the row's own currency code rather than from a single chart-wide value.
+ */
+const resolveDetectedCurrency = (
+ currency: Currency | undefined,
+ detectedCurrency: string | null | undefined,
+ currencyCodeColumn: string | undefined,
+ colnames: string[] | undefined,
+): Currency | undefined => {
+ if (
+ currency?.symbol !== 'AUTO' ||
+ !detectedCurrency ||
+ (currencyCodeColumn && colnames?.includes(currencyCodeColumn))
+ ) {
+ return currency;
+ }
+ const normalizedCurrency = normalizeCurrency(detectedCurrency);
+ return normalizedCurrency
+ ? { ...currency, symbol: normalizedCurrency }
+ : currency;
+};
+
const processColumns = memoizeOne(function processColumns(
props: TableChartProps,
) {
@@ -299,21 +325,12 @@ const processColumns = memoizeOne(function processColumns(
// percent metrics have a default format
formatter = getNumberFormatter(numberFormat || PERCENT_3_POINT);
} else if (isMetric || (isNumber && (numberFormat || currency))) {
- // Resolve AUTO currency when currency column isn't in query results
- let resolvedCurrency = currency;
- if (
- currency?.symbol === 'AUTO' &&
- detectedCurrency &&
- (!currencyCodeColumn || !colnames?.includes(currencyCodeColumn))
- ) {
- const normalizedCurrency = normalizeCurrency(detectedCurrency);
- if (normalizedCurrency) {
- resolvedCurrency = {
- ...currency,
- symbol: normalizedCurrency,
- };
- }
- }
+ const resolvedCurrency = resolveDetectedCurrency(
+ currency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
+ );
formatter = resolvedCurrency?.symbol
? new CurrencyFormatter({
d3Format: numberFormat,
@@ -358,6 +375,9 @@ const getComparisonColFormatter = (
columnConfig: Record<string, TableColumnConfig>,
savedFormat: string | undefined,
savedCurrency: Currency | undefined,
+ detectedCurrency: string | null | undefined,
+ currencyCodeColumn: string | undefined,
+ colnames: string[] | undefined,
) => {
const currentColConfig = getComparisonColConfig(
label,
@@ -372,7 +392,12 @@ const getComparisonColFormatter = (
if (label === '%') {
formatter = getNumberFormatter(currentColNumberFormat || PERCENT_3_POINT);
} else if (currentColNumberFormat || hasCurrency) {
- const currency = currentColConfig.currencyFormat || savedCurrency;
+ const currency = resolveDetectedCurrency(
+ currentColConfig.currencyFormat || savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
+ );
const numberFormat = currentColNumberFormat || savedFormat;
formatter = currency
? new CurrencyFormatter({
@@ -391,9 +416,12 @@ const processComparisonColumns = (
) =>
columns.flatMap(col => {
const {
- datasource: { columnFormats, currencyFormats },
+ datasource: { columnFormats, currencyFormats, currencyCodeColumn },
rawFormData: { column_config: columnConfig = {} },
+ queriesData,
} = props;
+ const { colnames, detected_currency: detectedCurrency } =
+ queriesData[0] || {};
const savedFormat = columnFormats?.[col.key];
const savedCurrency = currencyFormats?.[col.key];
const originalLabel = col.label;
@@ -415,6 +443,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
{
@@ -429,6 +460,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
{
@@ -443,6 +477,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
{
@@ -457,6 +494,9 @@ const processComparisonColumns = (
columnConfig,
savedFormat,
savedCurrency,
+ detectedCurrency,
+ currencyCodeColumn,
+ colnames,
),
},
];
diff --git a/superset-frontend/plugins/plugin-chart-table/test/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-table/test/transformProps.test.ts
new file mode 100644
index 00000000000..db1d0218ed6
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-table/test/transformProps.test.ts
@@ -0,0 +1,117 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { GenericDataType } from '@apache-superset/core/common';
+import transformProps from '../src/transformProps';
+import { DataColumnMeta, TableChartProps } from '../src/types';
+import testData from './testData';
+
+const AUTO_CURRENCY = { symbol: 'AUTO', symbolPosition: 'prefix' as const };
+
+/**
+ * A Table chart with Time Comparison enabled and a metric whose currency is set
+ * to "Auto-detect". The currency code column is configured on the dataset but is
+ * deliberately NOT part of the query result, so the only way AUTO can resolve is
+ * via the backend-supplied `detected_currency`.
+ *
+ * `Main` carries no column_config of its own, so it reuses the parent column's
+ * already-resolved formatter. `#` and `△` carry a d3NumberFormat, which makes
+ * getComparisonColFormatter rebuild their formatter from the raw currency config.
+ */
+const buildAutoCurrencyComparisonProps = (
+ detectedCurrency: string | null = 'GBP',
+ colnames: string[] = ['metric_1', 'metric_1__1 year ago'],
+): TableChartProps => ({
+ ...testData.comparisonWithConfig,
+ rawFormData: {
+ ...testData.comparisonWithConfig.rawFormData,
+ metrics: ['metric_1'],
+ percent_metrics: [],
+ column_config: {
+ metric_1: { currencyFormat: AUTO_CURRENCY },
+ '# metric_1': { d3NumberFormat: ',.1f' },
+ '△ metric_1': { d3NumberFormat: ',.1f' },
+ },
+ },
+ datasource: {
+ ...testData.comparisonWithConfig.datasource,
+ columnFormats: {},
+ currencyFormats: { metric_1: AUTO_CURRENCY },
+ currencyCodeColumn: 'currency_code',
+ verboseMap: { metric_1: 'Metric 1' },
+ },
+ queriesData: [
+ {
+ ...testData.comparisonWithConfig.queriesData[0],
+ data: [{ metric_1: 100, 'metric_1__1 year ago': 80 }],
+ colnames,
+ coltypes: colnames.map(() => GenericDataType.Numeric),
+ detected_currency: detectedCurrency,
+ },
+ testData.comparisonWithConfig.queriesData[1],
+ ],
+});
+
+const getColumn = (props: TableChartProps, key: string): DataColumnMeta => {
+ const { columns } = transformProps(props);
+ const column = columns.find((col: DataColumnMeta) => col.key === key);
+ if (!column) {
+ throw new Error(
+ `column "${key}" not found; got: ${columns
+ .map((col: DataColumnMeta) => col.key)
+ .join(', ')}`,
+ );
+ }
+ return column;
+};
+
+test('resolves AUTO currency on time comparison columns when the currency code column is not in the query result', () => {
+ const props = buildAutoCurrencyComparisonProps();
+
+ // The parent column already resolves AUTO today - this is the behaviour the
+ // reporter sees working, and it must keep working.
+ expect(getColumn(props, 'Main metric_1').formatter?.(100)).toContain('£');
+
+ // The comparison columns rebuild their formatter and must resolve AUTO too.
+ expect(getColumn(props, '# metric_1').formatter?.(20)).toContain('£');
+ expect(getColumn(props, '△ metric_1').formatter?.(20)).toContain('£');
+});
+
+test('leaves AUTO currency unresolved on time comparison columns so per-row lookup wins when the currency code column is in the query result', () => {
+ const props = buildAutoCurrencyComparisonProps('GBP', [
+ 'metric_1',
+ 'metric_1__1 year ago',
+ 'currency_code',
+ ]);
+
+ // With the currency column queried, formatting is resolved per row at render
+ // time, so the formatter itself must stay in AUTO mode (no baked-in symbol).
+ expect(getColumn(props, '# metric_1').formatter?.(20)).not.toContain('£');
+});
+
+test('leaves the percent comparison column as a plain number formatter', () => {
+ const props = buildAutoCurrencyComparisonProps();
+
+ expect(getColumn(props, '% metric_1').formatter?.(0.2)).not.toContain('£');
+});
+
+test('does not invent a currency on time comparison columns when nothing was detected', () => {
+ const props = buildAutoCurrencyComparisonProps(null);
+
+ expect(getColumn(props, '# metric_1').formatter?.(20)).not.toContain('£');
+});