Adds tests

apache · Sep 15, 2022 · b1028d9 · b1028d9
1 parent 49d4a66
commit b1028d9
Show file tree

Hide file tree

Showing 4 changed files with 88 additions and 79 deletions.
diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
@@ -98,6 +98,11 @@ const findSelectOption = (text: string) =>
     within(getElementByClassName('.rc-virtual-list')).getByText(text),
   );
 
+const querySelectOption = (text: string) =>
+  waitFor(() =>
+    within(getElementByClassName('.rc-virtual-list')).queryByText(text),
+  );
+
 const findAllSelectOptions = () =>
   waitFor(() => getElementsByClassName('.ant-select-item-option-content'));
 
@@ -736,6 +741,21 @@ test('renders a helper text when one is provided', async () => {
   expect(screen.queryByText(helperText)).toBeInTheDocument();
 });
 
+test('finds an element with a numeric value and does not duplicate the options', async () => {
+  const options = jest.fn(async () => ({
+    data: [
+      { label: 'a', value: 11 },
+      { label: 'b', value: 12 },
+    ],
+    totalCount: 2,
+  }));
+  render(<AsyncSelect {...defaultProps} options={options} allowNewOptions />);
+  await open();
+  await type('11');
+  expect(await findSelectOption('a')).toBeInTheDocument();
+  expect(await querySelectOption('11')).not.toBeInTheDocument();
+});
+
 /*
  TODO: Add tests that require scroll interaction. Needs further investigation.
  - Fetches more data when scrolling and more data is available

diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx
@@ -34,7 +34,7 @@ import debounce from 'lodash/debounce';
 import { isEqual } from 'lodash';
 import Icons from 'src/components/Icons';
 import { getClientErrorObject } from 'src/utils/getClientErrorObject';
-import { FAST_DEBOUNCE, SLOW_DEBOUNCE } from 'src/constants';
+import { SLOW_DEBOUNCE } from 'src/constants';
 import {
   getValue,
   hasOption,
@@ -369,53 +369,34 @@ const AsyncSelect = forwardRef(
       [fetchPage],
     );
 
-    const handleOnSearch = useCallback(
-      (search: string) => {
-        const searchValue = search.trim();
-        if (allowNewOptions && isSingleMode) {
-          const newOption = searchValue &&
-            !hasOption(searchValue, fullSelectOptions, true) && {
-              label: searchValue,
-              value: searchValue,
-              isNewOption: true,
-            };
-          const cleanSelectOptions = fullSelectOptions.filter(
-            opt => !opt.isNewOption || hasOption(opt.value, selectValue),
-          );
-          const newOptions = newOption
-            ? [newOption, ...cleanSelectOptions]
-            : cleanSelectOptions;
-          setSelectOptions(newOptions);
-        }
-        if (
-          !allValuesLoaded &&
-          loadingEnabled &&
-          !fetchedQueries.current.has(
-            getQueryCacheKey(searchValue, 0, pageSize),
-          )
-        ) {
-          // if fetch only on search but search value is empty, then should not be
-          // in loading state
-          setIsLoading(!(fetchOnlyOnSearch && !searchValue));
-        }
-        setInputValue(search);
-      },
-      [
-        allValuesLoaded,
-        allowNewOptions,
-        fetchOnlyOnSearch,
-        fullSelectOptions,
-        isSingleMode,
-        loadingEnabled,
-        pageSize,
-        selectValue,
-      ],
-    );
-
-    const debouncedOnSearch = useMemo(
-      () => debounce(handleOnSearch, FAST_DEBOUNCE),
-      [handleOnSearch],
-    );
+    const handleOnSearch = (search: string) => {
+      const searchValue = search.trim();
+      if (allowNewOptions && isSingleMode) {
+        const newOption = searchValue &&
+          !hasOption(searchValue, fullSelectOptions, true) && {
+            label: searchValue,
+            value: searchValue,
+            isNewOption: true,
+          };
+        const cleanSelectOptions = fullSelectOptions.filter(
+          opt => !opt.isNewOption || hasOption(opt.value, selectValue),
+        );
+        const newOptions = newOption
+          ? [newOption, ...cleanSelectOptions]
+          : cleanSelectOptions;
+        setSelectOptions(newOptions);
+      }
+      if (
+        !allValuesLoaded &&
+        loadingEnabled &&
+        !fetchedQueries.current.has(getQueryCacheKey(searchValue, 0, pageSize))
+      ) {
+        // if fetch only on search but search value is empty, then should not be
+        // in loading state
+        setIsLoading(!(fetchOnlyOnSearch && !searchValue));
+      }
+      setInputValue(search);
+    };
 
     const handlePagination = (e: UIEvent<HTMLElement>) => {
       const vScroll = e.currentTarget;
@@ -554,7 +535,7 @@ const AsyncSelect = forwardRef(
           onDeselect={handleOnDeselect}
           onDropdownVisibleChange={handleOnDropdownVisibleChange}
           onPopupScroll={handlePagination}
-          onSearch={showSearch ? debouncedOnSearch : undefined}
+          onSearch={showSearch ? handleOnSearch : undefined}
           onSelect={handleOnSelect}
           onClear={handleClear}
           onChange={onChange}

diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx
@@ -77,6 +77,11 @@ const findSelectOption = (text: string) =>
     within(getElementByClassName('.rc-virtual-list')).getByText(text),
   );
 
+const querySelectOption = (text: string) =>
+  waitFor(() =>
+    within(getElementByClassName('.rc-virtual-list')).queryByText(text),
+  );
+
 const findAllSelectOptions = () =>
   waitFor(() => getElementsByClassName('.ant-select-item-option-content'));
 
@@ -549,6 +554,18 @@ test('renders a helper text when one is provided', async () => {
   expect(screen.queryByText(helperText)).toBeInTheDocument();
 });
 
+test('finds an element with a numeric value and does not duplicate the options', async () => {
+  const options = [
+    { label: 'a', value: 11 },
+    { label: 'b', value: 12 },
+  ];
+  render(<Select {...defaultProps} options={options} allowNewOptions />);
+  await open();
+  await type('11');
+  expect(await findSelectOption('a')).toBeInTheDocument();
+  expect(await querySelectOption('11')).not.toBeInTheDocument();
+});
+
 /*
  TODO: Add tests that require scroll interaction. Needs further investigation.
  - Fetches more data when scrolling and more data is available

diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
@@ -27,8 +27,7 @@ import React, {
 } from 'react';
 import { ensureIsArray, t } from '@superset-ui/core';
 import { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
-import { debounce, isEqual } from 'lodash';
-import { FAST_DEBOUNCE } from 'src/constants';
+import { isEqual } from 'lodash';
 import {
   getValue,
   hasOption,
@@ -199,33 +198,25 @@ const Select = forwardRef(
       setInputValue('');
     };
 
-    const handleOnSearch = useCallback(
-      (search: string) => {
-        const searchValue = search.trim();
-        if (allowNewOptions && isSingleMode) {
-          const newOption = searchValue &&
-            !hasOption(searchValue, fullSelectOptions, true) && {
-              label: searchValue,
-              value: searchValue,
-              isNewOption: true,
-            };
-          const cleanSelectOptions = fullSelectOptions.filter(
-            opt => !opt.isNewOption || hasOption(opt.value, selectValue),
-          );
-          const newOptions = newOption
-            ? [newOption, ...cleanSelectOptions]
-            : cleanSelectOptions;
-          setSelectOptions(newOptions);
-        }
-        setInputValue(search);
-      },
-      [allowNewOptions, fullSelectOptions, isSingleMode, selectValue],
-    );
-
-    const debouncedOnSearch = useMemo(
-      () => debounce(handleOnSearch, FAST_DEBOUNCE),
-      [handleOnSearch],
-    );
+    const handleOnSearch = (search: string) => {
+      const searchValue = search.trim();
+      if (allowNewOptions && isSingleMode) {
+        const newOption = searchValue &&
+          !hasOption(searchValue, fullSelectOptions, true) && {
+            label: searchValue,
+            value: searchValue,
+            isNewOption: true,
+          };
+        const cleanSelectOptions = fullSelectOptions.filter(
+          opt => !opt.isNewOption || hasOption(opt.value, selectValue),
+        );
+        const newOptions = newOption
+          ? [newOption, ...cleanSelectOptions]
+          : cleanSelectOptions;
+        setSelectOptions(newOptions);
+      }
+      setInputValue(search);
+    };
 
     const handleFilterOption = (search: string, option: AntdLabeledValue) =>
       handleFilterOptionHelper(search, option, optionFilterProps, filterOption);
@@ -297,7 +288,7 @@ const Select = forwardRef(
           onDeselect={handleOnDeselect}
           onDropdownVisibleChange={handleOnDropdownVisibleChange}
           onPopupScroll={undefined}
-          onSearch={shouldShowSearch ? debouncedOnSearch : undefined}
+          onSearch={shouldShowSearch ? handleOnSearch : undefined}
           onSelect={handleOnSelect}
           onClear={handleClear}
           onChange={onChange}