(superset) 01/01: fix(datasets): warn when a percent D3 format is set on a count metric

[en...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

EnxDev pushed a commit to branch enxdev/fix/explore-count-metric
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 7ad57267178f6995535cd3cf4b53a3b663b8e6b5
Author: Enzo Martellucci <en...@gmail.com>
AuthorDate: Mon Aug 24 11:54:02 2026 +0200

    fix(datasets): warn when a percent D3 format is set on a count metric
---
 .../DatasourceEditor/DatasourceEditor.tsx          | 31 +++++++-
 .../DatasourceEditorMetricFormatWarning.test.tsx   | 91 ++++++++++++++++++++++
 .../Datasource/components/Fieldset/index.tsx       |  3 +
 3 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
index 847bc71f52e..066a16d6740 100644
--- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
@@ -779,6 +779,35 @@ function EditorsSelector({
 const ResultTable =
   extensionsRegistry.get('sqleditor.extension.resultTable') ?? FilterableTable;
 
+// D3's '%' type is a valid spec that multiplies by 100, so it never trips
+// the "Invalid format" fallback even when applied to a raw count.
+const isPercentD3Format = (d3format?: string): boolean =>
+  !!d3format && d3format.trim().endsWith('%');
+
+const isCountExpression = (expression?: string): boolean =>
+  !!expression && /^\s*count\s*\(/i.test(expression);
+
+function renderMetricFormatWarning(item: Record<string, any>): ReactNode {
+  if (
+    !isCountExpression(item.expression) ||
+    !isPercentD3Format(item.d3format)
+  ) {
+    return null;
+  }
+  return (
+    <Alert
+      css={themeParam => ({ marginBottom: themeParam.sizeUnit * 4 })}
+      type="warning"
+      showIcon
+      message={t(
+        'This metric is a count, but its D3 format is a percentage. ' +
+          'Percent formats multiply the value by 100, which will make a ' +
+          'raw count render as a misleadingly large number.',
+      )}
+    />
+  );
+}
+
 // Redux connector types
 interface QueryPayload {
   client_id?: string;
@@ -2140,7 +2169,7 @@ function DatasourceEditor({
           }}
           expandFieldset={
             <FormContainer>
-              <Fieldset compact>
+              <Fieldset compact renderWarning={renderMetricFormatWarning}>
                 <Field
                   fieldKey="expression"
                   label={t('SQL expression')}
diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorMetricFormatWarning.test.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorMetricFormatWarning.test.tsx
new file mode 100644
index 00000000000..cf05483c73b
--- /dev/null
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorMetricFormatWarning.test.tsx
@@ -0,0 +1,91 @@
+/**
+ * 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 fetchMock from 'fetch-mock';
+import { screen, userEvent, waitFor } from 'spec/helpers/testing-library';
+import {
+  createProps,
+  DATASOURCE_ENDPOINT,
+  setupDatasourceEditorMocks,
+  cleanupAsyncOperations,
+  fastRender,
+  dismissDatasourceWarning,
+} from './DatasourceEditor.test.utils';
+
+beforeEach(() => {
+  fetchMock.get(DATASOURCE_ENDPOINT, [], { name: DATASOURCE_ENDPOINT });
+  setupDatasourceEditorMocks();
+});
+
+afterEach(async () => {
+  await cleanupAsyncOperations();
+  fetchMock.clearHistory().removeRoutes();
+});
+
+const WARNING_TEXT = /D3 format is a percentage/i;
+
+// A '%' format is valid syntax, so it never hits the "Invalid format" fallback.
+test('warns when a percent D3 format is set on a COUNT metric', async () => {
+  const testProps = createProps();
+  fastRender(testProps);
+  await dismissDatasourceWarning();
+
+  await userEvent.click(await screen.findByTestId('collection-tab-Metrics'));
+  const expandToggles = await screen.findAllByLabelText(/expand row/i);
+  // Rows sort by metric id descending, so `COUNT(*)` (id 7) is first.
+  await userEvent.click(expandToggles[0]);
+
+  expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument();
+
+  await userEvent.type(await screen.findByPlaceholderText('%y/%m/%d'), '.0%');
+
+  expect(await screen.findByText(WARNING_TEXT)).toBeInTheDocument();
+});
+
+test('does not warn for a non-percent format on a COUNT metric', async () => {
+  const testProps = createProps();
+  fastRender(testProps);
+  await dismissDatasourceWarning();
+
+  await userEvent.click(await screen.findByTestId('collection-tab-Metrics'));
+  const expandToggles = await screen.findAllByLabelText(/expand row/i);
+  await userEvent.click(expandToggles[0]);
+
+  await userEvent.type(await screen.findByPlaceholderText('%y/%m/%d'), ',.0f');
+
+  await waitFor(() =>
+    expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument(),
+  );
+});
+
+test('does not warn for a percent format on a non-COUNT metric', async () => {
+  const testProps = createProps();
+  fastRender(testProps);
+  await dismissDatasourceWarning();
+
+  await userEvent.click(await screen.findByTestId('collection-tab-Metrics'));
+  const expandToggles = await screen.findAllByLabelText(/expand row/i);
+  // Rows sort by metric id descending, so id 1 (`SUM(...)`) sorts last.
+  await userEvent.click(expandToggles[6]);
+
+  await userEvent.type(await screen.findByPlaceholderText('%y/%m/%d'), '.0%');
+
+  await waitFor(() =>
+    expect(screen.queryByText(WARNING_TEXT)).not.toBeInTheDocument(),
+  );
+});
diff --git a/superset-frontend/src/components/Datasource/components/Fieldset/index.tsx b/superset-frontend/src/components/Datasource/components/Fieldset/index.tsx
index e74d25f6965..e2f77be2c47 100644
--- a/superset-frontend/src/components/Datasource/components/Fieldset/index.tsx
+++ b/superset-frontend/src/components/Datasource/components/Fieldset/index.tsx
@@ -28,6 +28,7 @@ export interface FieldsetProps {
   item?: Record<string, any>;
   title?: ReactNode;
   compact?: boolean;
+  renderWarning?: (item: Record<string, any>) => ReactNode;
 }
 
 type fieldKeyType = string | number;
@@ -38,6 +39,7 @@ export default function Fieldset({
   item = {},
   title = null,
   compact = false,
+  renderWarning,
 }: FieldsetProps) {
   // Controls report their edits asynchronously - TextControl debounces by
   // FAST_DEBOUNCE - so the callback that eventually fires was built during an
@@ -78,6 +80,7 @@ export default function Fieldset({
         </Typography.Title>
       )}
 
+      {renderWarning?.(item)}
       {recurseReactClone(children, Field, propExtender)}
     </Form>
   );