fix: Dataset currency (#33682)

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]']

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,

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' },
+            },
+          }),
+        }),
+      }),
+    }),
+  );
+});

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,

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);
+  });
+});

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`);

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)

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

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