fix(charts): Table chart shows an error on row limit (#37218)

2026-04-23 18:14:56 +00:00 · 2026-01-30 16:45:50 -03:00
parent 570cc3e5f8
commit 9764a84402
18 changed files with 105 additions and 26 deletions
--- a/superset-frontend/packages/superset-ui-core/src/utils/index.ts
+++ b/superset-frontend/packages/superset-ui-core/src/utils/index.ts
@@ -25,6 +25,7 @@ export { default as isEqualArray } from './isEqualArray';
 export { default as makeSingleton } from './makeSingleton';
 export { default as promiseTimeout } from './promiseTimeout';
 export { default as removeDuplicates } from './removeDuplicates';
+export { default as withLabel } from './withLabel';
 export { lruCache } from './lruCache';
 export { getSelectedText } from './getSelectedText';
 export * from './featureFlags';
--- a/superset-frontend/packages/superset-ui-core/src/utils/withLabel.ts
+++ b/superset-frontend/packages/superset-ui-core/src/utils/withLabel.ts
@@ -0,0 +1,43 @@
+/**
+ * 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 type { ValidatorFunction } from '../validator';
+
+/**
+ * Wraps a validator function to prepend a label to its error message.
+ *
+ * @param validator - The validator function to wrap
+ * @param label - The label to prepend to error messages
+ * @returns A new validator function that includes the label in error messages
+ *
+ * @example
+ * validators: [
+ *   withLabel(validateInteger, t('Row limit')),
+ * ]
+ * // Returns: "Row limit is expected to be an integer"
+ */
+export default function withLabel<V = unknown, S = unknown>(
+  validator: ValidatorFunction<V, S>,
+  label: string,
+): ValidatorFunction<V, S> {
+  return (value: V, state?: S): string | false => {
+    const error = validator(value, state);
+    return error ? `${label} ${error}` : false;
+  };
+}
--- a/superset-frontend/packages/superset-ui-core/src/validator/index.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/index.ts
@@ -17,6 +17,7 @@
 * under the License.
 */

+export * from './types';
 export { default as legacyValidateInteger } from './legacyValidateInteger';
 export { default as legacyValidateNumber } from './legacyValidateNumber';
 export { default as validateInteger } from './validateInteger';
--- a/superset-frontend/packages/superset-ui-core/src/validator/legacyValidateInteger.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/legacyValidateInteger.ts
@@ -23,7 +23,7 @@ import { t } from '@apache-superset/core';
 * formerly called integer()
 * @param v
 */
-export default function legacyValidateInteger(v: unknown) {
+export default function legacyValidateInteger(v: unknown): string | false {
  if (
    v &&
    (Number.isNaN(Number(v)) || parseInt(v as string, 10) !== Number(v))
--- a/superset-frontend/packages/superset-ui-core/src/validator/legacyValidateNumber.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/legacyValidateNumber.ts
@@ -23,7 +23,7 @@ import { t } from '@apache-superset/core';
 * formerly called numeric()
 * @param v
 */
-export default function numeric(v: unknown) {
+export default function numeric(v: unknown): string | false {
  if (v && Number.isNaN(Number(v))) {
    return t('is expected to be a number');
  }
--- a/superset-frontend/packages/superset-ui-core/src/validator/types.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/types.ts
@@ -0,0 +1,27 @@
+/*
+ * 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.
+ */
+
+/**
+ * Type definition for a validator function.
+ * Returns an error message string if validation fails, or false if validation passes.
+ */
+export type ValidatorFunction<V = unknown, S = unknown> = (
+  value: V,
+  state?: S,
+) => string | false;
--- a/superset-frontend/packages/superset-ui-core/src/validator/validateInteger.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateInteger.ts
@@ -19,7 +19,7 @@

 import { t } from '@apache-superset/core';

-export default function validateInteger(v: unknown) {
+export default function validateInteger(v: unknown): string | false {
  if (
    (typeof v === 'string' &&
      v.trim().length > 0 &&
--- a/superset-frontend/packages/superset-ui-core/src/validator/validateMapboxStylesUrl.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateMapboxStylesUrl.ts
@@ -25,7 +25,7 @@ const VALIDE_OSM_URLS = ['https://tile.osm', 'https://tile.openstreetmap'];
 * Validate a [Mapbox styles URL](https://docs.mapbox.com/help/glossary/style-url/)
 * @param v
 */
-export default function validateMapboxStylesUrl(v: unknown) {
+export default function validateMapboxStylesUrl(v: unknown): string | false {
  if (typeof v === 'string') {
    const trimmed_v = v.trim();
    if (
--- a/superset-frontend/packages/superset-ui-core/src/validator/validateMaxValue.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateMaxValue.ts
@@ -18,7 +18,10 @@
 */
 import { t } from '@apache-superset/core';

-export default function validateMaxValue(v: unknown, max: number) {
+export default function validateMaxValue(
+  v: unknown,
+  max: number,
+): string | false {
  if (Number(v) > +max) {
    return t('Value cannot exceed %s', max);
  }
--- a/superset-frontend/packages/superset-ui-core/src/validator/validateNonEmpty.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateNonEmpty.ts
@@ -19,7 +19,7 @@

 import { t } from '@apache-superset/core';

-export default function validateNonEmpty(v: unknown) {
+export default function validateNonEmpty(v: unknown): string | false {
  if (
    v === null ||
    typeof v === 'undefined' ||
--- a/superset-frontend/packages/superset-ui-core/src/validator/validateNumber.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateNumber.ts
@@ -19,7 +19,7 @@

 import { t } from '@apache-superset/core';

-export default function validateInteger(v: any) {
+export default function validateNumber(v: unknown): string | false {
  if (
    (typeof v === 'string' &&
      v.trim().length > 0 &&
--- a/superset-frontend/packages/superset-ui-core/src/validator/validateServerPagination.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateServerPagination.ts
@@ -23,7 +23,7 @@ export default function validateServerPagination(
  serverPagination: boolean,
  maxValueWithoutServerPagination: number,
  maxServer: number,
-) {
+): string | false {
  if (
    Number(v) > +maxValueWithoutServerPagination &&
    Number(v) <= maxServer &&
--- a/superset-frontend/packages/superset-ui-core/src/validator/validateTimeComparisonRangeValues.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateTimeComparisonRangeValues.ts
@@ -22,13 +22,13 @@ import { t } from '@apache-superset/core';
 import { ensureIsArray } from '../utils';

 export const validateTimeComparisonRangeValues = (
-  timeRangeValue?: any,
-  controlValue?: any,
-) => {
+  timeRangeValue?: unknown,
+  controlValue?: unknown,
+): string[] => {
  const isCustomTimeRange = timeRangeValue === ComparisonTimeRangeType.Custom;
-  const isCustomControlEmpty = controlValue?.every(
-    (val: any) => ensureIsArray(val).length === 0,
-  );
+  const isCustomControlEmpty =
+    Array.isArray(controlValue) &&
+    controlValue.every((val: unknown) => ensureIsArray(val).length === 0);
  return isCustomTimeRange && isCustomControlEmpty
    ? [t('Filters for comparison must have a value')]
    : [];
--- a/superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts
+++ b/superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts
@@ -20,13 +20,13 @@
 import { validateMaxValue } from '@superset-ui/core';
 import './setup';

-test('validateInteger returns the warning message if invalid', () => {
+test('validateMaxValue returns the warning message if invalid', () => {
  expect(validateMaxValue(10.1, 10)).toBeTruthy();
  expect(validateMaxValue(1, 0)).toBeTruthy();
  expect(validateMaxValue('2', 1)).toBeTruthy();
 });

-test('validateInteger returns false if the input is valid', () => {
+test('validateMaxValue returns false if the input is valid', () => {
  expect(validateMaxValue(0, 1)).toBeFalsy();
  expect(validateMaxValue(10, 10)).toBeFalsy();
  expect(validateMaxValue(undefined, 1)).toBeFalsy();
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx
@@ -51,6 +51,7 @@ import {
  SMART_DATE_ID,
  validateMaxValue,
  validateServerPagination,
+  withLabel,
 } from '@superset-ui/core';
 import { GenericDataType } from '@apache-superset/core/api/core';
 import { isEmpty, last } from 'lodash';
@@ -384,7 +385,7 @@ const config: ControlPanelConfig = {
              description: t('Rows per page, 0 means no pagination'),
              visibility: ({ controls }: ControlPanelsContainerProps) =>
                Boolean(controls?.server_pagination?.value),
-              validators: [validateInteger],
+              validators: [withLabel(validateInteger, t('Server Page Length'))],
            },
          },
        ],
@@ -403,7 +404,7 @@ const config: ControlPanelConfig = {
                  state?.common?.conf?.SQL_MAX_ROW,
              }),
              validators: [
-                validateInteger,
+                withLabel(validateInteger, t('Row limit')),
                (v, state) =>
                  validateMaxValue(
                    v,
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Histogram/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Histogram/controlPanel.tsx
@@ -17,7 +17,11 @@
 * under the License.
 */
 import { t } from '@apache-superset/core';
-import { validateInteger, validateNonEmpty } from '@superset-ui/core';
+import {
+  validateInteger,
+  validateNonEmpty,
+  withLabel,
+} from '@superset-ui/core';
 import { GenericDataType } from '@apache-superset/core/api/core';
 import {
  ControlPanelConfig,
@@ -66,7 +70,7 @@ const config: ControlPanelConfig = {
              default: 5,
              choices: formatSelectOptionsForRange(5, 20, 5),
              description: t('The number of bins for the histogram'),
-              validators: [validateInteger],
+              validators: [withLabel(validateInteger, t('Bins'))],
            },
          },
        ],
--- a/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
+++ b/superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx
@@ -52,6 +52,7 @@ import {
  SMART_DATE_ID,
  validateMaxValue,
  validateServerPagination,
+  withLabel,
 } from '@superset-ui/core';
 import { GenericDataType } from '@apache-superset/core/api/core';
 import { isEmpty, last } from 'lodash';
@@ -407,7 +408,7 @@ const config: ControlPanelConfig = {
              description: t('Rows per page, 0 means no pagination'),
              visibility: ({ controls }: ControlPanelsContainerProps) =>
                Boolean(controls?.server_pagination?.value),
-              validators: [validateInteger],
+              validators: [withLabel(validateInteger, t('Server Page Length'))],
            },
          },
        ],
@@ -426,7 +427,7 @@ const config: ControlPanelConfig = {
                  state?.common?.conf?.SQL_MAX_ROW,
              }),
              validators: [
-                validateInteger,
+                withLabel(validateInteger, t('Row limit')),
                (v, state) =>
                  validateMaxValue(
                    v,
@@ -448,9 +449,6 @@ const config: ControlPanelConfig = {
                'Limits the number of the rows that are computed in the query that is the source of the data used for this chart.',
              ),
            },
-            override: {
-              default: 1000,
-            },
          },
        ],
        [
--- a/superset-frontend/src/explore/controlUtils/getControlState.ts
+++ b/superset-frontend/src/explore/controlUtils/getControlState.ts
@@ -122,7 +122,8 @@ export function applyMapStateToPropsToControl<T = ControlType>(
    }
  }
  // If no current value, set it as default
-  if (state.default && value === undefined) {
+  // Use loose equality to catch both null and undefined
+  if (state.default != null && value == null) {
    value = state.default;
  }
  // If a choice control went from multi=false to true, wrap value in array