From 1109fe5fb7a777e3fbb4f3d8d60283b676d1acc0 Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Thu, 7 Jul 2022 14:51:37 -0400 Subject: [PATCH] chore: Split Select component into Async and Sync components (#20466) * Created AsyncSelect Component Changed files to reference AsyncSelect if needed * modified import of AsyncSelect, removed async tests and prefixes from select tests * fixed various import and lint warnings * fixing lint errors * fixed frontend test errors * fixed alertreportmodel tests * removed accidental import * fixed lint errors * updated async select --- .../src/addSlice/AddSliceContainer.test.tsx | 4 +- .../src/addSlice/AddSliceContainer.tsx | 4 +- .../src/components/DatabaseSelector/index.tsx | 4 +- .../Datasource/DatasourceEditor.jsx | 4 +- .../components/Select/AsyncSelect.test.tsx | 705 ++++++++++++++++ .../src/components/Select/AsyncSelect.tsx | 754 ++++++++++++++++++ .../src/components/Select/Select.stories.tsx | 24 +- .../src/components/Select/Select.test.tsx | 367 +-------- superset-frontend/src/components/index.ts | 1 + .../components/PropertiesModal/index.tsx | 8 +- .../FiltersConfigForm/DatasetSelect.tsx | 4 +- .../components/PropertiesModal/index.tsx | 4 +- .../CRUD/alert/AlertReportModal.test.jsx | 14 +- .../src/views/CRUD/alert/AlertReportModal.tsx | 12 +- 14 files changed, 1519 insertions(+), 390 deletions(-) create mode 100644 superset-frontend/src/components/Select/AsyncSelect.test.tsx create mode 100644 superset-frontend/src/components/Select/AsyncSelect.tsx diff --git a/superset-frontend/src/addSlice/AddSliceContainer.test.tsx b/superset-frontend/src/addSlice/AddSliceContainer.test.tsx index 91e6b28dd..a656efb81 100644 --- a/superset-frontend/src/addSlice/AddSliceContainer.test.tsx +++ b/superset-frontend/src/addSlice/AddSliceContainer.test.tsx @@ -19,7 +19,7 @@ import React from 'react'; import { ReactWrapper } from 'enzyme'; import Button from 'src/components/Button'; -import { Select } from 'src/components'; +import { AsyncSelect } from 'src/components'; import AddSliceContainer, { AddSliceContainerProps, AddSliceContainerState, @@ -72,7 +72,7 @@ async function getWrapper(user = mockUser) { test('renders a select and a VizTypeControl', async () => { const wrapper = await getWrapper(); - expect(wrapper.find(Select)).toExist(); + expect(wrapper.find(AsyncSelect)).toExist(); expect(wrapper.find(VizTypeGallery)).toExist(); }); diff --git a/superset-frontend/src/addSlice/AddSliceContainer.tsx b/superset-frontend/src/addSlice/AddSliceContainer.tsx index d4c6bfc78..84c37b61c 100644 --- a/superset-frontend/src/addSlice/AddSliceContainer.tsx +++ b/superset-frontend/src/addSlice/AddSliceContainer.tsx @@ -23,7 +23,7 @@ import { getUrlParam } from 'src/utils/urlUtils'; import { URL_PARAMS } from 'src/constants'; import { isNullish } from 'src/utils/common'; import Button from 'src/components/Button'; -import { Select, Steps } from 'src/components'; +import { AsyncSelect, Steps } from 'src/components'; import { Tooltip } from 'src/components/Tooltip'; import VizTypeGallery, { @@ -349,7 +349,7 @@ export default class AddSliceContainer extends React.PureComponent< status={this.state.datasource?.value ? 'finish' : 'process'} description={ - ', value: null } as unknown as { value: number; }; -const loadOptions = async (search: string, page: number, pageSize: number) => { - const totalCount = OPTIONS.length; - const start = page * pageSize; - const deleteCount = - start + pageSize < totalCount ? pageSize : totalCount - start; - const data = OPTIONS.filter(option => option.label.match(search)).splice( - start, - deleteCount, - ); - return { - data, - totalCount: OPTIONS.length, - }; -}; - const defaultProps = { allowClear: true, ariaLabel: ARIA_LABEL, @@ -165,27 +149,6 @@ test('sort the options by label if no sort comparator is provided', async () => ); }); -test('sort the options using a custom sort comparator', async () => { - const sortComparator = ( - option1: typeof OPTIONS[0], - option2: typeof OPTIONS[0], - ) => option1.gender.localeCompare(option2.gender); - render( - ); const originalLabels = OPTIONS.map(option => option.label); @@ -383,7 +346,7 @@ test('clear all the values', async () => { }); test('does not add a new option if allowNewOptions is false', async () => { - render(); await open(); await type(NEW_OPTION); expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); @@ -413,18 +376,18 @@ test('adds the null option when selected in multiple mode', async () => { expect(values[1]).toHaveTextContent(NULL_OPTION.label); }); -test('static - renders the select with default props', () => { +test('renders the select with default props', () => { render(); await open(); expect(screen.getByText(NO_DATA)).toBeInTheDocument(); }); -test('static - makes a selection in single mode', async () => { +test('makes a selection in single mode', async () => { render(); await open(); const [firstOption, secondOption] = OPTIONS; @@ -443,7 +406,7 @@ test('static - multiple selections in multiple mode', async () => { expect(values[1]).toHaveTextContent(secondOption.label); }); -test('static - changes the selected item in single mode', async () => { +test('changes the selected item in single mode', async () => { const onChange = jest.fn(); render(); await open(); const [firstOption, secondOption] = OPTIONS; @@ -484,35 +447,35 @@ test('static - deselects an item in multiple mode', async () => { expect(values[0]).toHaveTextContent(secondOption.label); }); -test('static - adds a new option if none is available and allowNewOptions is true', async () => { +test('adds a new option if none is available and allowNewOptions is true', async () => { render(); await open(); await type(NEW_OPTION); expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); }); -test('static - does not show "No data" when allowNewOptions is true and a new option is entered', async () => { +test('does not show "No data" when allowNewOptions is true and a new option is entered', async () => { render(); await open(); await type(NEW_OPTION); expect(screen.queryByText(LOADING)).not.toBeInTheDocument(); }); -test('static - does not add a new option if the option already exists', async () => { +test('does not add a new option if the option already exists', async () => { render(); expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label); }); -test('static - sets a initial value in multiple mode', async () => { +test('sets a initial value in multiple mode', async () => { render( ); const search = 'Oli'; await type(search); @@ -548,303 +511,7 @@ test('static - searches for an item', async () => { expect(options[1]).toHaveTextContent('Olivia'); }); -test('async - renders the select with default props', () => { - render( ({ data: [], totalCount: 0 })} - />, - ); - await open(); - expect(await screen.findByText(/no data/i)).toBeInTheDocument(); -}); - -test('async - displays the loading indicator when opening', async () => { - render(); - const optionText = 'Emma'; - await open(); - userEvent.click(await findSelectOption(optionText)); - expect(await findSelectValue()).toHaveTextContent(optionText); -}); - -test('async - multiple selections in multiple mode', async () => { - render(, - ); - await open(); - const [firstOption, secondOption] = OPTIONS; - userEvent.click(await findSelectOption(firstOption.label)); - expect(onChange).toHaveBeenCalledWith( - expect.objectContaining({ - label: firstOption.label, - value: firstOption.value, - }), - firstOption, - ); - expect(await findSelectValue()).toHaveTextContent(firstOption.label); - userEvent.click(await findSelectOption(secondOption.label)); - expect(onChange).toHaveBeenCalledWith( - expect.objectContaining({ - label: secondOption.label, - value: secondOption.value, - }), - secondOption, - ); - expect(await findSelectValue()).toHaveTextContent(secondOption.label); -}); - -test('async - deselects an item in multiple mode', async () => { - render(); - await open(); - await type(NEW_OPTION); - expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument(); -}); - -test('async - does not add a new option if the option already exists', async () => { - render(, - ); - await open(); - await type(NEW_OPTION); - expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); -}); - -test('async - does not show "No data" when allowNewOptions is true and a new option is entered', async () => { - render(); - expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label); -}); - -test('async - sets a initial value in multiple mode', async () => { - render( - ); - await open(); - await type('and'); - - let options = await findAllSelectOptions(); - expect(options.length).toBe(1); - expect(options[0]).toHaveTextContent('Alehandro'); - - await screen.findByText('Sandro'); - options = await findAllSelectOptions(); - expect(options.length).toBe(2); - expect(options[0]).toHaveTextContent('Alehandro'); - expect(options[1]).toHaveTextContent('Sandro'); -}); - -test('async - searches for an item in a page not loaded', async () => { - const mock = jest.fn(loadOptions); - render(); - expect(loadOptions).not.toHaveBeenCalled(); -}); - -test('async - fetches data when opening', async () => { - const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); - render(); - await open(); - await waitFor(() => expect(loadOptions).not.toHaveBeenCalled()); - await type('search'); - await waitFor(() => expect(loadOptions).toHaveBeenCalled()); -}); - -test('async - displays an error message when an exception is thrown while fetching', async () => { - const error = 'Fetch error'; - const loadOptions = async () => { - throw new Error(error); - }; - render(); - await type('search'); - expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); - expect(loadOptions).toHaveBeenCalledTimes(1); - clearAll(); - await type('search'); - expect(await screen.findByText(LOADING)).toBeInTheDocument(); - expect(loadOptions).toHaveBeenCalledTimes(1); -}); - -test('async - does not fire a new request if all values have been fetched', async () => { - const mock = jest.fn(loadOptions); - const search = 'George'; - const pageSize = OPTIONS.length; - render(); - await open(); - expect(mock).toHaveBeenCalledTimes(1); - await type('or'); - - // `George` is on the first page so when it appears the API has not been called again - expect(await findSelectOption('George')).toBeInTheDocument(); - expect(mock).toHaveBeenCalledTimes(1); - - // `Igor` is on the second paged API request - expect(await findSelectOption('Igor')).toBeInTheDocument(); - expect(mock).toHaveBeenCalledTimes(2); -}); - -test('async - requests the options again after clearing the cache', async () => { - const ref: RefObject = { current: null }; - const mock = jest.fn(loadOptions); - const pageSize = OPTIONS.length; - render( - - , - ); - await open(); - expect(getPopupContainer).toHaveBeenCalled(); -}); - -test('static - triggers getPopupContainer if passed', async () => { +test('triggers getPopupContainer if passed', async () => { const getPopupContainer = jest.fn(); render( - { }; return ( - { it('renders five select elements when in report mode', () => { expect(wrapper.find(Select)).toExist(); - expect(wrapper.find(Select)).toHaveLength(5); + expect(wrapper.find(AsyncSelect)).toExist(); + expect(wrapper.find(Select)).toHaveLength(2); + expect(wrapper.find(AsyncSelect)).toHaveLength(3); }); it('renders Switch element', () => { @@ -220,7 +222,9 @@ describe('AlertReportModal', () => { it('renders five select element when in report mode', () => { expect(wrapper.find(Select)).toExist(); - expect(wrapper.find(Select)).toHaveLength(5); + expect(wrapper.find(AsyncSelect)).toExist(); + expect(wrapper.find(Select)).toHaveLength(2); + expect(wrapper.find(AsyncSelect)).toHaveLength(3); }); it('renders seven select elements when in alert mode', async () => { @@ -232,7 +236,9 @@ describe('AlertReportModal', () => { const addWrapper = await mountAndWait(props); expect(addWrapper.find(Select)).toExist(); - expect(addWrapper.find(Select)).toHaveLength(7); + expect(addWrapper.find(AsyncSelect)).toExist(); + expect(addWrapper.find(Select)).toHaveLength(3); + expect(addWrapper.find(AsyncSelect)).toHaveLength(4); }); it('renders value input element when in alert mode', async () => { diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index 04398a358..820a83b8c 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -38,11 +38,11 @@ import { Switch } from 'src/components/Switch'; import Modal from 'src/components/Modal'; import TimezoneSelector from 'src/components/TimezoneSelector'; import { Radio } from 'src/components/Radio'; -import Select, { propertyComparator } from 'src/components/Select/Select'; +import { propertyComparator } from 'src/components/Select/Select'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import withToasts from 'src/components/MessageToasts/withToasts'; import Owner from 'src/types/Owner'; -import { AntdCheckbox } from 'src/components'; +import { AntdCheckbox, AsyncSelect, Select } from 'src/components'; import TextAreaControl from 'src/explore/components/controls/TextAreaControl'; import { useCommonConf } from 'src/views/CRUD/data/database/state'; import { @@ -1098,7 +1098,7 @@ const AlertReportModal: FunctionComponent = ({ *
- = ({ {t('Dashboard')} {t('Chart')} -