You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2023/12/27 02:32:17 UTC

(superset) branch master updated: fix(chart): Set max row limit + removed the option to use an empty row limit value (#26151)

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

rusackas 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 863f8bbbcd fix(chart): Set max row limit + removed the option to use an empty row limit value (#26151)
863f8bbbcd is described below

commit 863f8bbbcdd078814973d444368c12e06ad0c0c0
Author: Corbin Bullard <co...@gmail.com>
AuthorDate: Tue Dec 26 21:32:11 2023 -0500

    fix(chart): Set max row limit + removed the option to use an empty row limit value (#26151)
    
    Co-authored-by: Lily Kuang <li...@preset.io>
    Co-authored-by: Michael S. Molina <70...@users.noreply.github.com>
---
 .../superset-ui-chart-controls/src/constants.ts    |  2 ++
 .../src/shared-controls/sharedControls.tsx         | 10 +++++++--
 .../superset-ui-core/src/validator/index.ts        |  1 +
 .../src/validator/validateMaxValue.ts              |  8 ++++++++
 .../validator/validateMaxValue.test.ts}            | 24 ++++++++++++++++------
 5 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts b/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts
index cbde46b0ef..ae6dd8f894 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/constants.ts
@@ -26,6 +26,8 @@ import {
 } from '@superset-ui/core';
 import { ColumnMeta, SortSeriesData, SortSeriesType } from './types';
 
+export const DEFAULT_MAX_ROW = 100000;
+
 // eslint-disable-next-line import/prefer-default-export
 export const TIME_FILTER_LABELS = {
   time_range: t('Time Range'),
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx
index 57419b4918..341aaa0729 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx
@@ -47,6 +47,7 @@ import {
   isDefined,
   hasGenericChartAxes,
   NO_TIME_RANGE,
+  validateMaxValue,
 } from '@superset-ui/core';
 
 import {
@@ -58,7 +59,7 @@ import {
   DEFAULT_TIME_FORMAT,
   DEFAULT_NUMBER_FORMAT,
 } from '../utils';
-import { TIME_FILTER_LABELS } from '../constants';
+import { DEFAULT_MAX_ROW, TIME_FILTER_LABELS } from '../constants';
 import {
   SharedControlConfig,
   Dataset,
@@ -243,7 +244,12 @@ const row_limit: SharedControlConfig<'SelectControl'> = {
   type: 'SelectControl',
   freeForm: true,
   label: t('Row limit'),
-  validators: [legacyValidateInteger],
+  clearable: false,
+  validators: [
+    legacyValidateInteger,
+    (v, state) =>
+      validateMaxValue(v, state?.common?.conf?.SQL_MAX_ROW || DEFAULT_MAX_ROW),
+  ],
   default: 10000,
   choices: formatSelectOptions(ROW_LIMIT_OPTIONS),
   description: t(
diff --git a/superset-frontend/packages/superset-ui-core/src/validator/index.ts b/superset-frontend/packages/superset-ui-core/src/validator/index.ts
index 169675b682..6294bddec7 100644
--- a/superset-frontend/packages/superset-ui-core/src/validator/index.ts
+++ b/superset-frontend/packages/superset-ui-core/src/validator/index.ts
@@ -22,4 +22,5 @@ export { default as legacyValidateNumber } from './legacyValidateNumber';
 export { default as validateInteger } from './validateInteger';
 export { default as validateNumber } from './validateNumber';
 export { default as validateNonEmpty } from './validateNonEmpty';
+export { default as validateMaxValue } from './validateMaxValue';
 export { default as validateMapboxStylesUrl } from './validateMapboxStylesUrl';
diff --git a/superset-frontend/packages/superset-ui-core/src/validator/validateMaxValue.ts b/superset-frontend/packages/superset-ui-core/src/validator/validateMaxValue.ts
new file mode 100644
index 0000000000..24c1da1c79
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-core/src/validator/validateMaxValue.ts
@@ -0,0 +1,8 @@
+import { t } from '../translation';
+
+export default function validateMaxValue(v: unknown, max: Number) {
+  if (Number(v) > +max) {
+    return t('Value cannot exceed %s', max);
+  }
+  return false;
+}
diff --git a/superset-frontend/packages/superset-ui-core/src/validator/index.ts b/superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts
similarity index 53%
copy from superset-frontend/packages/superset-ui-core/src/validator/index.ts
copy to superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts
index 169675b682..6a8ed1642e 100644
--- a/superset-frontend/packages/superset-ui-core/src/validator/index.ts
+++ b/superset-frontend/packages/superset-ui-core/test/validator/validateMaxValue.test.ts
@@ -17,9 +17,21 @@
  * under the License.
  */
 
-export { default as legacyValidateInteger } from './legacyValidateInteger';
-export { default as legacyValidateNumber } from './legacyValidateNumber';
-export { default as validateInteger } from './validateInteger';
-export { default as validateNumber } from './validateNumber';
-export { default as validateNonEmpty } from './validateNonEmpty';
-export { default as validateMapboxStylesUrl } from './validateMapboxStylesUrl';
+import { validateMaxValue } from '@superset-ui/core';
+import './setup';
+
+test('validateInteger 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', () => {
+  expect(validateMaxValue(0, 1)).toBeFalsy();
+  expect(validateMaxValue(10, 10)).toBeFalsy();
+  expect(validateMaxValue(undefined, 1)).toBeFalsy();
+  expect(validateMaxValue(NaN, NaN)).toBeFalsy();
+  expect(validateMaxValue(null, 1)).toBeFalsy();
+  expect(validateMaxValue('1', 1)).toBeFalsy();
+  expect(validateMaxValue('a', 1)).toBeFalsy();
+});