chore: Select component refactoring - SelectAsyncControl - Iteration 5 (#16609)

* Refactor Select

* Implement onError

* Separate async request

* Lint

* Allow clear

* Improve type

* Reconcile with Select latest changes

* Fix handleOnChange

* Clean up

* Add placeholder

* Accept null values

* Update superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* Clean up

* Implement type guard

* Fix lint

* Catch error within loadOptions

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx

@@ -17,19 +17,21 @@
  * under the License.
  */
 import userEvent from '@testing-library/user-event';
+import fetchMock from 'fetch-mock';
 import React from 'react';
 import { render, screen } from 'spec/helpers/testing-library';
 import SelectAsyncControl from '.';
 
-jest.mock('src/components/AsyncSelect', () => ({
+const datasetsOwnersEndpoint = 'glob:*/api/v1/dataset/related/owners*';
+
+jest.mock('src/components/Select/Select', () => ({
   __esModule: true,
   default: (props: any) => (
     <div
       data-test="select-test"
-      data-data-endpoint={props.dataEndpoint}
       data-value={JSON.stringify(props.value)}
       data-placeholder={props.placeholder}
-      data-multi={props.multi ? 'true' : 'false'}
+      data-multi={props.mode}
     >
       <button
         type="button"
@@ -40,19 +42,19 @@ jest.mock('src/components/AsyncSelect', () => ({
       <button type="button" onClick={() => props.mutator()}>
         mutator
       </button>
-
-      <div data-test="valueRenderer">
-        {props.valueRenderer({ label: 'valueRenderer' })}
-      </div>
     </div>
   ),
 }));
 
+fetchMock.get(datasetsOwnersEndpoint, {
+  result: [],
+});
+
 const createProps = () => ({
+  ariaLabel: 'SelectAsyncControl',
   value: [],
-  dataEndpoint: 'api/v1/dataset/related/owners',
+  dataEndpoint: datasetsOwnersEndpoint,
   multi: true,
-  onAsyncErrorMessage: 'Error while fetching data',
   placeholder: 'Select ...',
   onChange: jest.fn(),
   mutator: jest.fn(),
@@ -65,17 +67,13 @@ beforeEach(() => {
 test('Should render', () => {
   const props = createProps();
   render(<SelectAsyncControl {...props} />, { useRedux: true });
-  expect(screen.getByTestId('SelectAsyncControl')).toBeInTheDocument();
+  expect(screen.getByTestId('select-test')).toBeInTheDocument();
 });
 
-test('Should send correct props to AsyncSelect component - value props', () => {
+test('Should send correct props to Select component - value props', () => {
   const props = createProps();
   render(<SelectAsyncControl {...props} />, { useRedux: true });
 
-  expect(screen.getByTestId('select-test')).toHaveAttribute(
-    'data-data-endpoint',
-    props.dataEndpoint,
-  );
   expect(screen.getByTestId('select-test')).toHaveAttribute(
     'data-value',
     JSON.stringify(props.value),
@@ -86,14 +84,11 @@ test('Should send correct props to AsyncSelect component - value props', () => {
   );
   expect(screen.getByTestId('select-test')).toHaveAttribute(
     'data-multi',
-    'true',
-  );
-  expect(screen.getByTestId('valueRenderer')).toHaveTextContent(
-    'valueRenderer',
+    'multiple',
   );
 });
 
-test('Should send correct props to AsyncSelect component - function onChange multi:true', () => {
+test('Should send correct props to Select component - function onChange multi:true', () => {
   const props = createProps();
   render(<SelectAsyncControl {...props} />, { useRedux: true });
   expect(props.onChange).toBeCalledTimes(0);
@@ -101,7 +96,7 @@ test('Should send correct props to AsyncSelect component - function onChange mul
   expect(props.onChange).toBeCalledTimes(1);
 });
 
-test('Should send correct props to AsyncSelect component - function onChange multi:false', () => {
+test('Should send correct props to Select component - function onChange multi:false', () => {
   const props = createProps();
   render(<SelectAsyncControl {...{ ...props, multi: false }} />, {
     useRedux: true,

superset-frontend/src/explore/components/controls/SelectAsyncControl/index.jsx

@@ -1,92 +0,0 @@
-/**
- * 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 React from 'react';
-import PropTypes from 'prop-types';
-import { t } from '@superset-ui/core';
-
-import Select from 'src/components/AsyncSelect';
-import ControlHeader from 'src/explore/components/ControlHeader';
-import withToasts from 'src/components/MessageToasts/withToasts';
-
-const propTypes = {
-  dataEndpoint: PropTypes.string.isRequired,
-  multi: PropTypes.bool,
-  mutator: PropTypes.func,
-  onAsyncErrorMessage: PropTypes.string,
-  onChange: PropTypes.func,
-  placeholder: PropTypes.string,
-  value: PropTypes.oneOfType([
-    PropTypes.string,
-    PropTypes.number,
-    PropTypes.arrayOf(PropTypes.string),
-    PropTypes.arrayOf(PropTypes.number),
-  ]),
-  addDangerToast: PropTypes.func.isRequired,
-};
-
-const defaultProps = {
-  multi: true,
-  onAsyncErrorMessage: t('Error while fetching data'),
-  onChange: () => {},
-  placeholder: t('Select ...'),
-};
-
-const SelectAsyncControl = props => {
-  const {
-    value,
-    onChange,
-    dataEndpoint,
-    multi,
-    mutator,
-    placeholder,
-    onAsyncErrorMessage,
-  } = props;
-  const onSelectionChange = options => {
-    let val;
-    if (multi) {
-      val = options?.map(option => option.value) ?? null;
-    } else {
-      val = options?.value ?? null;
-    }
-    onChange(val);
-  };
-
-  return (
-    <div data-test="SelectAsyncControl">
-      <ControlHeader {...props} />
-      <Select
-        dataEndpoint={dataEndpoint}
-        onChange={onSelectionChange}
-        onAsyncError={errorMsg =>
-          props.addDangerToast(`${onAsyncErrorMessage}: ${errorMsg}`)
-        }
-        mutator={mutator}
-        multi={multi}
-        value={value}
-        placeholder={placeholder}
-        valueRenderer={v => <div>{v.label}</div>}
-      />
-    </div>
-  );
-};
-
-SelectAsyncControl.propTypes = propTypes;
-SelectAsyncControl.defaultProps = defaultProps;
-
-export default withToasts(SelectAsyncControl);

superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx

@@ -0,0 +1,106 @@
+/**
+ * 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 React, { useEffect, useState } from 'react';
+import { t, SupersetClient } from '@superset-ui/core';
+import ControlHeader from 'src/explore/components/ControlHeader';
+import { Select } from 'src/components';
+import { SelectProps, OptionsType } from 'src/components/Select/Select';
+import { SelectValue, LabeledValue } from 'antd/lib/select';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { getClientErrorObject } from 'src/utils/getClientErrorObject';
+
+type SelectAsyncProps = Omit<SelectProps, 'options' | 'ariaLabel' | 'onChange'>;
+
+interface SelectAsyncControlProps extends SelectAsyncProps {
+  addDangerToast: (error: string) => void;
+  ariaLabel?: string;
+  dataEndpoint: string;
+  default?: SelectValue;
+  mutator?: (response: Record<string, any>) => OptionsType;
+  multi?: boolean;
+  onChange: (val: SelectValue) => void;
+  // ControlHeader related props
+  description?: string;
+  hovered?: boolean;
+  label?: string;
+}
+
+function isLabeledValue(arg: any): arg is LabeledValue {
+  return arg.value !== undefined;
+}
+
+const SelectAsyncControl = ({
+  addDangerToast,
+  allowClear = true,
+  ariaLabel,
+  dataEndpoint,
+  multi = true,
+  mutator,
+  onChange,
+  placeholder,
+  value,
+  ...props
+}: SelectAsyncControlProps) => {
+  const [options, setOptions] = useState<OptionsType>([]);
+
+  const handleOnChange = (val: SelectValue) => {
+    let onChangeVal = val;
+    if (Array.isArray(val)) {
+      const values = val.map(v => (isLabeledValue(v) ? v.value : v));
+      onChangeVal = values;
+    }
+    if (isLabeledValue(val)) {
+      onChangeVal = val.value;
+    }
+    onChange(onChangeVal);
+  };
+
+  useEffect(() => {
+    const onError = (response: Response) =>
+      getClientErrorObject(response).then(e => {
+        const { error } = e;
+        addDangerToast(`${t('Error while fetching data')}: ${error}`);
+      });
+    const loadOptions = () =>
+      SupersetClient.get({
+        endpoint: dataEndpoint,
+      })
+        .then(response => {
+          const data = mutator ? mutator(response.json) : response.json.result;
+          setOptions(data);
+        })
+        .catch(onError);
+    loadOptions();
+  }, [addDangerToast, dataEndpoint, mutator]);
+
+  return (
+    <Select
+      allowClear={allowClear}
+      ariaLabel={ariaLabel || t('Select ...')}
+      value={value || (props.default !== undefined ? props.default : undefined)}
+      header={<ControlHeader {...props} />}
+      mode={multi ? 'multiple' : 'single'}
+      onChange={handleOnChange}
+      options={options}
+      placeholder={placeholder}
+    />
+  );
+};
+
+export default withToasts(SelectAsyncControl);