From 7c966c52dc0ca74c7d926970458c579519df0935 Mon Sep 17 00:00:00 2001
From: Geido <60598000+geido@users.noreply.github.com>
Date: Thu, 7 Oct 2021 14:20:32 +0300
Subject: [PATCH] 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>
---
.../src/components/Select/Select.tsx | 1 +
.../SelectAsyncControl.test.tsx | 37 +++---
.../controls/SelectAsyncControl/index.jsx | 92 ---------------
.../controls/SelectAsyncControl/index.tsx | 106 ++++++++++++++++++
4 files changed, 123 insertions(+), 113 deletions(-)
delete mode 100644 superset-frontend/src/explore/components/controls/SelectAsyncControl/index.jsx
create mode 100644 superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx
diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
index d08b6694bad48..e1034ad9cedde 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/superset-frontend/src/components/Select/Select.tsx
@@ -93,6 +93,7 @@ export interface SelectProps extends PickedSelectProps {
pageSize?: number;
invertSelection?: boolean;
fetchOnlyOnSearch?: boolean;
+ onError?: (error: string) => void;
}
const StyledContainer = styled.div`
diff --git a/superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx b/superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx
index bc1ec8258776d..0b0b3034a6395 100644
--- a/superset-frontend/src/explore/components/controls/SelectAsyncControl/SelectAsyncControl.test.tsx
+++ b/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) => (
),
}));
+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(, { 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(, { 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(, { 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(, {
useRedux: true,
diff --git a/superset-frontend/src/explore/components/controls/SelectAsyncControl/index.jsx b/superset-frontend/src/explore/components/controls/SelectAsyncControl/index.jsx
deleted file mode 100644
index db7075513625b..0000000000000
--- a/superset-frontend/src/explore/components/controls/SelectAsyncControl/index.jsx
+++ /dev/null
@@ -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 (
-
-
-
- );
-};
-
-SelectAsyncControl.propTypes = propTypes;
-SelectAsyncControl.defaultProps = defaultProps;
-
-export default withToasts(SelectAsyncControl);
diff --git a/superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx b/superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx
new file mode 100644
index 0000000000000..85230f7b91951
--- /dev/null
+++ b/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;
+
+interface SelectAsyncControlProps extends SelectAsyncProps {
+ addDangerToast: (error: string) => void;
+ ariaLabel?: string;
+ dataEndpoint: string;
+ default?: SelectValue;
+ mutator?: (response: Record) => 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([]);
+
+ 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 (
+ }
+ mode={multi ? 'multiple' : 'single'}
+ onChange={handleOnChange}
+ options={options}
+ placeholder={placeholder}
+ />
+ );
+};
+
+export default withToasts(SelectAsyncControl);