This is an automated email from the ASF dual-hosted git repository.
vavila 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 86e7139245 fix: Dataset currency (#33682)
86e7139245 is described below
commit 86e7139245e7444e749b5e8ad1d5167b4df53ce9
Author: Vitor Avila <[email protected]>
AuthorDate: Mon Jun 9 22:47:14 2025 -0300
fix: Dataset currency (#33682)
---
.../superset-ui-chart-controls/src/types.ts | 1 +
.../src/components/Datasource/DatasourceModal.tsx | 5 +-
.../src/explore/actions/hydrateExplore.test.ts | 48 ++++++++++++
.../src/explore/actions/hydrateExplore.ts | 9 +++
.../src/hooks/apiResources/dashboards.test.ts | 89 ++++++++++++++++++++++
.../src/hooks/apiResources/dashboards.ts | 17 ++++-
superset/datasets/schemas.py | 7 +-
...2_convert_metric_currencies_from_str_to_json.py | 84 ++++++++++++++++++++
tests/integration_tests/datasets/api_tests.py | 23 ++++++
9 files changed, 277 insertions(+), 6 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
index 71517884da..418efe55fc 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts
@@ -69,6 +69,7 @@ export interface Dataset {
columns: ColumnMeta[];
metrics: Metric[];
column_formats: Record<string, string>;
+ currency_formats?: Record<string, Currency>;
verbose_map: Record<string, string>;
main_dttm_col: string;
// eg. ['["ds", true]', 'ds [asc]']
diff --git a/superset-frontend/src/components/Datasource/DatasourceModal.tsx
b/superset-frontend/src/components/Datasource/DatasourceModal.tsx
index c89f588a0c..e5c3e5c6cd 100644
--- a/superset-frontend/src/components/Datasource/DatasourceModal.tsx
+++ b/superset-frontend/src/components/Datasource/DatasourceModal.tsx
@@ -21,7 +21,6 @@ import { useSelector } from 'react-redux';
import Alert from 'src/components/Alert';
import Button from 'src/components/Button';
import {
- isDefined,
styled,
SupersetClient,
getClientErrorObject,
@@ -141,9 +140,7 @@ const DatasourceModal:
FunctionComponent<DatasourceModalProps> = ({
metric_name: metric.metric_name,
metric_type: metric.metric_type,
d3format: metric.d3format || null,
- currency: !isDefined(metric.currency)
- ? null
- : JSON.stringify(metric.currency),
+ currency: metric.currency,
verbose_name: metric.verbose_name,
warning_text: metric.warning_text,
uuid: metric.uuid,
diff --git a/superset-frontend/src/explore/actions/hydrateExplore.test.ts
b/superset-frontend/src/explore/actions/hydrateExplore.test.ts
index cdbca5b94d..f070369914 100644
--- a/superset-frontend/src/explore/actions/hydrateExplore.test.ts
+++ b/superset-frontend/src/explore/actions/hydrateExplore.test.ts
@@ -213,3 +213,51 @@ test('uses configured default time range if not set', ()
=> {
}),
);
});
+
+test('extracts currency formats from metrics in dataset', () => {
+ const dispatch = jest.fn();
+ const getState = jest.fn(() => ({
+ user: {},
+ charts: {},
+ datasources: {},
+ common: {},
+ explore: {},
+ }));
+
+ const datasetWithMetrics = {
+ ...exploreInitialData.dataset,
+ metrics: [
+ {
+ metric_name: 'count',
+ currency: { symbol: 'GBP', symbolPosition: 'prefix' },
+ },
+ {
+ metric_name: 'revenue',
+ currency: { symbol: 'USD', symbolPosition: 'suffix' },
+ },
+ { metric_name: 'no_currency' },
+ ],
+ };
+
+ // @ts-ignore
+ hydrateExplore({ ...exploreInitialData, dataset: datasetWithMetrics })(
+ dispatch,
+ // @ts-ignore
+ getState,
+ );
+
+ expect(dispatch).toHaveBeenCalledWith(
+ expect.objectContaining({
+ data: expect.objectContaining({
+ datasources: expect.objectContaining({
+ '8__table': expect.objectContaining({
+ currency_formats: {
+ count: { symbol: 'GBP', symbolPosition: 'prefix' },
+ revenue: { symbol: 'USD', symbolPosition: 'suffix' },
+ },
+ }),
+ }),
+ }),
+ }),
+ );
+});
diff --git a/superset-frontend/src/explore/actions/hydrateExplore.ts
b/superset-frontend/src/explore/actions/hydrateExplore.ts
index 2ee46c1e93..b8e0375a64 100644
--- a/superset-frontend/src/explore/actions/hydrateExplore.ts
+++ b/superset-frontend/src/explore/actions/hydrateExplore.ts
@@ -27,6 +27,7 @@ import { getChartKey } from 'src/explore/exploreUtils';
import { getControlsState } from 'src/explore/store';
import { Dispatch } from 'redux';
import {
+ Currency,
ensureIsArray,
getCategoricalSchemeRegistry,
getColumnLabel,
@@ -97,6 +98,14 @@ export const hydrateExplore =
}
const initialDatasource = dataset;
+ initialDatasource.currency_formats = Object.fromEntries(
+ (initialDatasource.metrics ?? [])
+ .filter(metric => !!metric.currency)
+ .map((metric): [string, Currency] => [
+ metric.metric_name,
+ metric.currency!,
+ ]),
+ );
const initialExploreState = {
form_data: initialFormData,
diff --git a/superset-frontend/src/hooks/apiResources/dashboards.test.ts
b/superset-frontend/src/hooks/apiResources/dashboards.test.ts
new file mode 100644
index 0000000000..3a6265163e
--- /dev/null
+++ b/superset-frontend/src/hooks/apiResources/dashboards.test.ts
@@ -0,0 +1,89 @@
+/**
+ * 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 { renderHook } from '@testing-library/react-hooks';
+import fetchMock from 'fetch-mock';
+import { useDashboardDatasets } from './dashboards';
+
+describe('useDashboardDatasets', () => {
+ const mockDatasets = [
+ {
+ id: 1,
+ metrics: [
+ {
+ metric_name: 'count',
+ currency: { symbol: 'GBP', symbolPosition: 'prefix' },
+ },
+ {
+ metric_name: 'revenue',
+ currency: { symbol: 'USD', symbolPosition: 'suffix' },
+ },
+ { metric_name: 'no_currency' },
+ ],
+ },
+ {
+ id: 2,
+ metrics: [{ metric_name: 'no_currency' }],
+ },
+ {
+ id: 3,
+ metrics: [
+ {
+ metric_name: 'other_currency',
+ currency: { symbol: 'CNY', symbolPosition: 'suffix' },
+ },
+ ],
+ },
+ ];
+
+ beforeEach(() => {
+ fetchMock.reset();
+ });
+
+ it('adds currencyFormats to datasets', async () => {
+ fetchMock.get('glob:*/api/v1/dashboard/*/datasets', {
+ result: mockDatasets,
+ });
+
+ const { result, waitForNextUpdate } = renderHook(() =>
+ useDashboardDatasets(1),
+ );
+ await waitForNextUpdate();
+
+ const expectedContent = [
+ {
+ ...mockDatasets[0],
+ currencyFormats: {
+ count: { symbol: 'GBP', symbolPosition: 'prefix' },
+ revenue: { symbol: 'USD', symbolPosition: 'suffix' },
+ },
+ },
+ {
+ ...mockDatasets[1],
+ currencyFormats: {},
+ },
+ {
+ ...mockDatasets[2],
+ currencyFormats: {
+ other_currency: { symbol: 'CNY', symbolPosition: 'suffix' },
+ },
+ },
+ ];
+ expect(result.current.result).toEqual(expectedContent);
+ });
+});
diff --git a/superset-frontend/src/hooks/apiResources/dashboards.ts
b/superset-frontend/src/hooks/apiResources/dashboards.ts
index 61896ba130..65e4b1c0bd 100644
--- a/superset-frontend/src/hooks/apiResources/dashboards.ts
+++ b/superset-frontend/src/hooks/apiResources/dashboards.ts
@@ -19,6 +19,7 @@
import { Dashboard, Datasource, EmbeddedDashboard } from 'src/dashboard/types';
import { Chart } from 'src/types/Chart';
+import { Currency } from '@superset-ui/core';
import { useApiV1Resource, useTransformedResource } from './apiResources';
export const useDashboard = (idOrSlug: string | number) =>
@@ -43,7 +44,21 @@ export const useDashboardCharts = (idOrSlug: string |
number) =>
// important: this endpoint only returns the fields in the dataset
// that are necessary for rendering the given dashboard
export const useDashboardDatasets = (idOrSlug: string | number) =>
- useApiV1Resource<Datasource[]>(`/api/v1/dashboard/${idOrSlug}/datasets`);
+ useTransformedResource(
+ useApiV1Resource<Datasource[]>(`/api/v1/dashboard/${idOrSlug}/datasets`),
+ datasets =>
+ datasets.map(dataset => ({
+ ...dataset,
+ currencyFormats: Object.fromEntries(
+ (dataset.metrics ?? [])
+ .filter(metric => !!metric.currency)
+ .map((metric): [string, Currency] => [
+ metric.metric_name,
+ metric.currency!,
+ ]),
+ ),
+ })),
+ );
export const useEmbeddedDashboard = (idOrSlug: string | number) =>
useApiV1Resource<EmbeddedDashboard>(`/api/v1/dashboard/${idOrSlug}/embedded`);
diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index 54ce448173..405d0bab3d 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -74,6 +74,11 @@ class DatasetColumnsPutSchema(Schema):
uuid = fields.UUID(allow_none=True)
+class DatasetMetricCurrencyPutSchema(Schema):
+ symbol = fields.String(validate=Length(1, 128))
+ symbolPosition = fields.String(validate=Length(1, 128)) # noqa: N815
+
+
class DatasetMetricsPutSchema(Schema):
id = fields.Integer()
expression = fields.String(required=True)
@@ -82,7 +87,7 @@ class DatasetMetricsPutSchema(Schema):
metric_name = fields.String(required=True, validate=Length(1, 255))
metric_type = fields.String(allow_none=True, validate=Length(1, 32))
d3format = fields.String(allow_none=True, validate=Length(1, 128))
- currency = fields.String(allow_none=True, required=False,
validate=Length(1, 128))
+ currency = fields.Nested(DatasetMetricCurrencyPutSchema, allow_none=True)
verbose_name = fields.String(allow_none=True, metadata={Length: (1, 1024)})
warning_text = fields.String(allow_none=True)
uuid = fields.UUID(allow_none=True)
diff --git
a/superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py
b/superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py
new file mode 100644
index 0000000000..708e7c52da
--- /dev/null
+++
b/superset/migrations/versions/2025-06-06_00-39_363a9b1e8992_convert_metric_currencies_from_str_to_json.py
@@ -0,0 +1,84 @@
+# 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.
+"""convert_metric_currencies_from_str_to_json
+
+Revision ID: 363a9b1e8992
+Revises: f1edd4a4d4f2
+Create Date: 2025-06-06 00:39:00.107746
+
+"""
+
+import json
+import logging
+
+from alembic import op
+from sqlalchemy import Column, Integer, JSON, String
+from sqlalchemy.ext.declarative import declarative_base
+
+from superset import db
+from superset.migrations.shared.utils import paginated_update
+
+logger = logging.getLogger("alembic")
+logger.setLevel(logging.INFO)
+
+# revision identifiers, used by Alembic.
+revision = "363a9b1e8992"
+down_revision = "f1edd4a4d4f2"
+
+Base = declarative_base()
+
+
+class SqlMetric(Base):
+ __tablename__ = "sql_metrics"
+
+ id = Column(Integer, primary_key=True)
+ metric_name = Column(String(512))
+ currency = Column(JSON)
+
+
+def upgrade():
+ bind = op.get_bind()
+ session = db.Session(bind=bind)
+ currency_configs =
session.query(SqlMetric).filter(SqlMetric.currency.isnot(None))
+ for metric in paginated_update(
+ currency_configs,
+ lambda current, total: logger.info((f"Upgrading {current}/{total}
metrics")),
+ ):
+ while True:
+ if isinstance(metric.currency, str):
+ try:
+ metric.currency = json.loads(metric.currency)
+ except Exception as e:
+ logger.error(
+ f"Error loading metric {metric.metric_name} as json:
{e}"
+ )
+ metric.currency = {}
+ break
+ else:
+ break
+
+
+def downgrade():
+ """
+ No op downgrade.
+
+ The downgrade could just do `metric.currency =
json.dumps(metric.currency)`. However
+ this is happening after `f1edd4a4d4f2` which already converted the
currency column
+ to JSON at the DB level, so we shouldn't have currencies in str anymore.
It was the
+ case because the client was still stringifying it.
+ """
+ pass
diff --git a/tests/integration_tests/datasets/api_tests.py
b/tests/integration_tests/datasets/api_tests.py
index ab645d300e..9d226f793d 100644
--- a/tests/integration_tests/datasets/api_tests.py
+++ b/tests/integration_tests/datasets/api_tests.py
@@ -1462,6 +1462,29 @@ class TestDatasetApi(SupersetTestCase):
assert data == expected_result
self.items_to_delete = [dataset]
+ def test_update_dataset_update_metric_invalid_currency(self):
+ """
+ Dataset API: Test update dataset metric with an invalid currency config
+ """
+
+ dataset = self.insert_default_dataset()
+
+ self.login(ADMIN_USERNAME)
+ uri = f"api/v1/dataset/{dataset.id}"
+ data = {
+ "metrics": [
+ {
+ "metric_name": "test",
+ "expression": "COUNT(*)",
+ "currency": '{"symbol": "USD", "symbolPosition":
"suffix"}',
+ },
+ ]
+ }
+ rv = self.put_assert_metric(uri, data, "put")
+ assert rv.status_code == 422
+
+ self.items_to_delete = [dataset]
+
def test_update_dataset_item_gamma(self):
"""
Dataset API: Test update dataset item gamma
