You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by yj...@apache.org on 2020/11/02 23:06:56 UTC
[incubator-superset] branch master updated: refactor: rewrite and
enhance chart control withVerification (#11435)
This is an automated email from the ASF dual-hosted git repository.
yjc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new fac29f9 refactor: rewrite and enhance chart control withVerification (#11435)
fac29f9 is described below
commit fac29f9dff8ca504006b20e1b81f094d4d9761b4
Author: Jesse Yang <je...@airbnb.com>
AuthorDate: Mon Nov 2 15:06:20 2020 -0800
refactor: rewrite and enhance chart control withVerification (#11435)
* refactor: rewrite and enhance chart control withVerification
* Add toasts for failed messages; fixes popover render
---
.../components/withAsyncVerification_spec.tsx | 142 +++++++++++++
.../explore/components/withVerification_spec.jsx | 127 ------------
.../src/explore/actions/exploreActions.ts | 3 +-
.../src/explore/components/AdhocFilterOption.jsx | 19 +-
.../src/explore/components/AdhocMetricOption.jsx | 19 +-
.../src/explore/components/Control.tsx | 2 +-
.../components/controls/AdhocFilterControl.jsx | 2 +
.../explore/components/controls/MetricsControl.jsx | 2 +
.../src/explore/components/controls/index.js | 16 --
.../components/controls/withAsyncVerification.tsx | 224 +++++++++++++++++++++
.../components/controls/withVerification.jsx | 92 ---------
.../src/messageToasts/actions/index.ts | 47 ++++-
.../src/messageToasts/reducers/index.js | 6 +-
superset-frontend/src/messageToasts/types.ts | 3 +
14 files changed, 436 insertions(+), 268 deletions(-)
diff --git a/superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx b/superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx
new file mode 100644
index 0000000..73805c7
--- /dev/null
+++ b/superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx
@@ -0,0 +1,142 @@
+/**
+ * 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 { mount, ReactWrapper } from 'enzyme';
+import { act } from 'react-dom/test-utils';
+
+import MetricsControl from 'src/explore/components/controls/MetricsControl';
+import withAsyncVerification, {
+ ControlPropsWithExtras,
+ WithAsyncVerificationOptions,
+} from 'src/explore/components/controls/withAsyncVerification';
+import { ExtraControlProps } from '@superset-ui/chart-controls';
+
+const VALID_METRIC = {
+ metric_name: 'sum__value',
+ expression: 'SUM(energy_usage.value)',
+};
+
+const mockSetControlValue = jest.fn();
+
+const defaultProps = {
+ name: 'metrics',
+ label: 'Metrics',
+ value: undefined,
+ multi: true,
+ needAsyncVerification: true,
+ actions: { setControlValue: mockSetControlValue },
+ onChange: () => {},
+ columns: [
+ { type: 'VARCHAR(255)', column_name: 'source' },
+ { type: 'VARCHAR(255)', column_name: 'target' },
+ { type: 'DOUBLE', column_name: 'value' },
+ ],
+ savedMetrics: [
+ VALID_METRIC,
+ { metric_name: 'avg__value', expression: 'AVG(energy_usage.value)' },
+ ],
+ datasourceType: 'sqla',
+};
+
+function verify(sourceProp: string) {
+ const mock = jest.fn();
+ mock.mockImplementation(async (props: ControlPropsWithExtras) => {
+ return { [sourceProp]: props.validMetrics || [VALID_METRIC] };
+ });
+ return mock;
+}
+
+async function setup({
+ extraProps,
+ baseControl = MetricsControl,
+ onChange,
+}: Partial<WithAsyncVerificationOptions> & {
+ extraProps?: ExtraControlProps;
+} = {}) {
+ const props = {
+ ...defaultProps,
+ ...extraProps,
+ };
+ const verifier = verify('savedMetrics');
+ const VerifiedControl = withAsyncVerification({
+ baseControl,
+ verify: verifier,
+ onChange,
+ });
+ type Wrapper = ReactWrapper<typeof props & ExtraControlProps>;
+ let wrapper: Wrapper | undefined;
+ await act(async () => {
+ wrapper = mount(<VerifiedControl {...props} />);
+ });
+ return { props, wrapper: wrapper as Wrapper, onChange, verifier };
+}
+
+describe('VerifiedMetricsControl', () => {
+ it('should calls verify correctly', async () => {
+ expect.assertions(5);
+ const { wrapper, verifier, props } = await setup();
+
+ expect(wrapper.find(MetricsControl).length).toBe(1);
+
+ expect(verifier).toBeCalledTimes(1);
+ expect(verifier).toBeCalledWith(
+ expect.objectContaining({ savedMetrics: props.savedMetrics }),
+ );
+
+ // should call verifier with new props when props are updated.
+ await act(async () => {
+ wrapper.setProps({ validMetric: ['abc'] });
+ });
+
+ expect(verifier).toBeCalledTimes(2);
+ expect(verifier).toBeCalledWith(
+ expect.objectContaining({ validMetric: ['abc'] }),
+ );
+ });
+
+ it('should trigger onChange event', async () => {
+ expect.assertions(3);
+ const mockOnChange = jest.fn();
+ const { wrapper } = await setup({
+ // should allow specify baseControl with control component name
+ baseControl: 'MetricsControl',
+ onChange: mockOnChange,
+ });
+
+ const child = wrapper.find(MetricsControl);
+ child.props().onChange(['abc']);
+
+ expect(child.length).toBe(1);
+ expect(mockOnChange).toBeCalledTimes(1);
+ expect(mockOnChange).toBeCalledWith(['abc'], {
+ actions: defaultProps.actions,
+ columns: defaultProps.columns,
+ datasourceType: defaultProps.datasourceType,
+ label: defaultProps.label,
+ multi: defaultProps.multi,
+ name: defaultProps.name,
+ // in real life, `onChange` should have been called with the updated
+ // props (both savedMetrics and value should have beend updated), but
+ // because of the limitation of enzyme (it cannot get props updated from
+ // useEffect hooks), we are not able to check that here.
+ savedMetrics: defaultProps.savedMetrics,
+ value: undefined,
+ });
+ });
+});
diff --git a/superset-frontend/spec/javascripts/explore/components/withVerification_spec.jsx b/superset-frontend/spec/javascripts/explore/components/withVerification_spec.jsx
deleted file mode 100644
index ed94572..0000000
--- a/superset-frontend/spec/javascripts/explore/components/withVerification_spec.jsx
+++ /dev/null
@@ -1,127 +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 sinon from 'sinon';
-import { shallow } from 'enzyme';
-import fetchMock from 'fetch-mock';
-
-import MetricsControl from 'src/explore/components/controls/MetricsControl';
-import withVerification from 'src/explore/components/controls/withVerification';
-
-const defaultProps = {
- name: 'metrics',
- label: 'Metrics',
- value: undefined,
- multi: true,
- columns: [
- { type: 'VARCHAR(255)', column_name: 'source' },
- { type: 'VARCHAR(255)', column_name: 'target' },
- { type: 'DOUBLE', column_name: 'value' },
- ],
- savedMetrics: [
- { metric_name: 'sum__value', expression: 'SUM(energy_usage.value)' },
- { metric_name: 'avg__value', expression: 'AVG(energy_usage.value)' },
- ],
- datasourceType: 'sqla',
- getEndpoint: controlValues => `valid_metrics?data=${controlValues}`,
-};
-
-const VALID_METRIC = {
- metric_name: 'sum__value',
- expression: 'SUM(energy_usage.value)',
-};
-
-function setup(overrides, validMetric) {
- const onChange = sinon.spy();
- const props = {
- onChange,
- ...defaultProps,
- ...overrides,
- };
- const VerifiedControl = withVerification(
- MetricsControl,
- 'metric_name',
- 'savedMetrics',
- );
- const wrapper = shallow(<VerifiedControl {...props} />);
- fetchMock.mock(
- 'glob:*/valid_metrics*',
- validMetric || `["${VALID_METRIC.metric_name}"]`,
- );
- return { props, wrapper, onChange };
-}
-
-afterEach(fetchMock.restore);
-
-describe('VerifiedMetricsControl', () => {
- it('Gets valid options', () => {
- const { wrapper } = setup();
- setTimeout(() => {
- expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1);
- expect(wrapper.state('validOptions')).toEqual([VALID_METRIC]);
- fetchMock.reset();
- }, 0);
- });
-
- it('Returns verified options', () => {
- const { wrapper } = setup();
- setTimeout(() => {
- expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1);
- const child = wrapper.find(MetricsControl);
- expect(child.props().savedMetrics).toEqual([VALID_METRIC]);
- fetchMock.reset();
- }, 0);
- });
-
- it('Makes no calls if endpoint is not set', () => {
- const { wrapper } = setup({
- getEndpoint: () => null,
- });
- setTimeout(() => {
- expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(0);
- expect(wrapper.state('validOptions')).toEqual(new Set());
- fetchMock.reset();
- }, 0);
- });
-
- it('Calls endpoint if control values change', () => {
- const { props, wrapper } = setup({
- controlValues: { metrics: 'sum__value' },
- });
- setTimeout(() => {
- expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1);
- fetchMock.reset();
- }, 0);
- wrapper.setProps({ ...props, controlValues: { metrics: 'avg__value' } });
- setTimeout(() => {
- expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1);
- fetchMock.reset();
- }, 0);
- });
-
- it('Returns no verified options if none are valid', () => {
- const { wrapper } = setup({}, []);
- setTimeout(() => {
- expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1);
- const child = wrapper.find(MetricsControl);
- expect(child.props().savedMetrics).toEqual([]);
- fetchMock.reset();
- }, 0);
- });
-});
diff --git a/superset-frontend/src/explore/actions/exploreActions.ts b/superset-frontend/src/explore/actions/exploreActions.ts
index af71199..f312f68 100644
--- a/superset-frontend/src/explore/actions/exploreActions.ts
+++ b/superset-frontend/src/explore/actions/exploreActions.ts
@@ -25,7 +25,7 @@ import {
QueryFormData,
} from '@superset-ui/core';
import { Dispatch } from 'redux';
-import { addDangerToast } from 'src/messageToasts/actions';
+import { addDangerToast, toastActions } from 'src/messageToasts/actions';
import { Slice } from 'src/types/Chart';
const FAVESTAR_BASE_URL = '/superset/favstar/slice';
@@ -148,6 +148,7 @@ export function sliceUpdated(slice: Slice) {
}
export const exploreActions = {
+ ...toastActions,
setDatasourceType,
setDatasource,
setDatasources,
diff --git a/superset-frontend/src/explore/components/AdhocFilterOption.jsx b/superset-frontend/src/explore/components/AdhocFilterOption.jsx
index 42ba81d..c7cf9ee 100644
--- a/superset-frontend/src/explore/components/AdhocFilterOption.jsx
+++ b/superset-frontend/src/explore/components/AdhocFilterOption.jsx
@@ -53,15 +53,11 @@ class AdhocFilterOption extends React.PureComponent {
};
}
- componentDidMount() {
- const { adhocFilter } = this.props;
- // isNew is used to auto-open the popup. Once popup is opened, it's not
- // considered new anymore.
- // put behind setTimeout so in case consequetive re-renderings are triggered
- // for some reason, the popup can still show up.
- setTimeout(() => {
- adhocFilter.isNew = false;
- });
+ componentWillUnmount() {
+ // isNew is used to auto-open the popup. Once popup is viewed, it's not
+ // considered new anymore. We mutate the prop directly because we don't
+ // want excessive rerenderings.
+ this.props.adhocFilter.isNew = false;
}
onPopoverResize() {
@@ -69,11 +65,12 @@ class AdhocFilterOption extends React.PureComponent {
}
closePopover() {
- this.setState({ popoverVisible: false });
+ this.togglePopover(false);
}
togglePopover(visible) {
this.setState(({ popoverVisible }) => {
+ this.props.adhocFilter.isNew = false;
return {
popoverVisible: visible === undefined ? !popoverVisible : visible,
};
@@ -116,7 +113,7 @@ class AdhocFilterOption extends React.PureComponent {
placement="right"
trigger="click"
content={overlayContent}
- defaultVisible={adhocFilter.isNew}
+ defaultVisible={this.state.popoverVisible || adhocFilter.isNew}
visible={this.state.popoverVisible}
onVisibleChange={this.togglePopover}
>
diff --git a/superset-frontend/src/explore/components/AdhocMetricOption.jsx b/superset-frontend/src/explore/components/AdhocMetricOption.jsx
index d030e67..c62cf7c 100644
--- a/superset-frontend/src/explore/components/AdhocMetricOption.jsx
+++ b/superset-frontend/src/explore/components/AdhocMetricOption.jsx
@@ -50,15 +50,11 @@ class AdhocMetricOption extends React.PureComponent {
};
}
- componentDidMount() {
- const { adhocMetric } = this.props;
- // isNew is used to auto-open the popup. Once popup is opened, it's not
- // considered new anymore.
- // put behind setTimeout so in case consequetive re-renderings are triggered
- // for some reason, the popup can still show up.
- setTimeout(() => {
- adhocMetric.isNew = false;
- });
+ componentWillUnmount() {
+ // isNew is used to auto-open the popup. Once popup is viewed, it's not
+ // considered new anymore. We mutate the prop directly because we don't
+ // want excessive rerenderings.
+ this.props.adhocMetric.isNew = false;
}
onLabelChange(e) {
@@ -76,11 +72,12 @@ class AdhocMetricOption extends React.PureComponent {
}
closePopover() {
- this.setState({ popoverVisible: false });
+ this.togglePopover(false);
}
togglePopover(visible) {
this.setState(({ popoverVisible }) => {
+ this.props.adhocMetric.isNew = false;
return {
popoverVisible: visible === undefined ? !popoverVisible : visible,
};
@@ -124,7 +121,7 @@ class AdhocMetricOption extends React.PureComponent {
trigger="click"
disabled
content={overlayContent}
- defaultVisible={adhocMetric.isNew}
+ defaultVisible={this.state.popoverVisible || adhocMetric.isNew}
visible={this.state.popoverVisible}
onVisibleChange={this.togglePopover}
title={popoverTitle}
diff --git a/superset-frontend/src/explore/components/Control.tsx b/superset-frontend/src/explore/components/Control.tsx
index 8fc6583..e728ad5 100644
--- a/superset-frontend/src/explore/components/Control.tsx
+++ b/superset-frontend/src/explore/components/Control.tsx
@@ -27,7 +27,7 @@ import './Control.less';
export type ControlProps = {
// the actual action dispatcher (via bindActionCreators) has identical
// signature to the original action factory.
- actions: ExploreActions;
+ actions: Partial<ExploreActions> & Pick<ExploreActions, 'setControlValue'>;
type: ControlType;
label: string;
name: string;
diff --git a/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx b/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx
index 28e6b42..2448982 100644
--- a/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx
+++ b/superset-frontend/src/explore/components/controls/AdhocFilterControl.jsx
@@ -47,6 +47,7 @@ const propTypes = {
PropTypes.oneOfType([PropTypes.string, adhocMetricType]),
),
}),
+ isLoading: PropTypes.bool,
};
const defaultProps = {
@@ -268,6 +269,7 @@ export default class AdhocFilterControl extends React.Component {
<ControlHeader {...this.props} />
<OnPasteSelect
isMulti
+ isLoading={this.props.isLoading}
name={`select-${this.props.name}`}
placeholder={t('choose a column or metric')}
options={this.state.options}
diff --git a/superset-frontend/src/explore/components/controls/MetricsControl.jsx b/superset-frontend/src/explore/components/controls/MetricsControl.jsx
index ad1a320..c7db3d0 100644
--- a/superset-frontend/src/explore/components/controls/MetricsControl.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricsControl.jsx
@@ -46,6 +46,7 @@ const propTypes = {
]),
columns: PropTypes.arrayOf(columnType),
savedMetrics: PropTypes.arrayOf(savedMetricType),
+ isLoading: PropTypes.bool,
multi: PropTypes.bool,
clearable: PropTypes.bool,
datasourceType: PropTypes.string,
@@ -335,6 +336,7 @@ export default class MetricsControl extends React.PureComponent {
<div className="metrics-select">
<ControlHeader {...this.props} />
<OnPasteSelect
+ isLoading={this.props.isLoading}
isMulti={this.props.multi}
name={`select-${this.props.name}`}
placeholder={t('choose a column or aggregate function')}
diff --git a/superset-frontend/src/explore/components/controls/index.js b/superset-frontend/src/explore/components/controls/index.js
index aa08450..5faf520 100644
--- a/superset-frontend/src/explore/components/controls/index.js
+++ b/superset-frontend/src/explore/components/controls/index.js
@@ -39,7 +39,6 @@ import VizTypeControl from './VizTypeControl';
import MetricsControl from './MetricsControl';
import AdhocFilterControl from './AdhocFilterControl';
import FilterBoxItemControl from './FilterBoxItemControl';
-import withVerification from './withVerification';
const controlMap = {
AnnotationLayerControl,
@@ -65,20 +64,5 @@ const controlMap = {
MetricsControl,
AdhocFilterControl,
FilterBoxItemControl,
- MetricsControlVerifiedOptions: withVerification(
- MetricsControl,
- 'metric_name',
- 'savedMetrics',
- ),
- SelectControlVerifiedOptions: withVerification(
- SelectControl,
- 'column_name',
- 'options',
- ),
- AdhocFilterControlVerifiedOptions: withVerification(
- AdhocFilterControl,
- 'column_name',
- 'columns',
- ),
};
export default controlMap;
diff --git a/superset-frontend/src/explore/components/controls/withAsyncVerification.tsx b/superset-frontend/src/explore/components/controls/withAsyncVerification.tsx
new file mode 100644
index 0000000..7e75862
--- /dev/null
+++ b/superset-frontend/src/explore/components/controls/withAsyncVerification.tsx
@@ -0,0 +1,224 @@
+/**
+ * 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, {
+ ComponentType,
+ useCallback,
+ useEffect,
+ useRef,
+ useState,
+} from 'react';
+import sharedControlComponents from '@superset-ui/chart-controls/lib/shared-controls/components';
+import { ExtraControlProps } from '@superset-ui/chart-controls';
+import { JsonArray, JsonValue, t } from '@superset-ui/core';
+import { ControlProps } from 'src/explore/components/Control';
+import builtInControlComponents from 'src/explore/components/controls';
+
+/**
+ * Full control component map.
+ */
+const controlComponentMap = {
+ ...builtInControlComponents,
+ ...sharedControlComponents,
+};
+
+export type SharedControlComponent = keyof typeof controlComponentMap;
+
+/**
+ * The actual props passed to the control component itself
+ * (not src/explore/components/Control.tsx).
+ */
+export type ControlPropsWithExtras = Omit<ControlProps, 'type'> &
+ ExtraControlProps;
+
+/**
+ * The full props passed to control component. Including withAsyncVerification
+ * related props and `onChange` event + `hovered` state from Control.tsx.
+ */
+export type FullControlProps = ControlPropsWithExtras & {
+ onChange?: (value: JsonValue) => void;
+ hovered?: boolean;
+ /**
+ * An extra flag for triggering async verification. Set it in mapStateToProps.
+ */
+ needAsyncVerification?: boolean;
+ /**
+ * Whether to show loading state when verification is still loading.
+ */
+ showLoadingState?: boolean;
+ verify?: AsyncVerify;
+};
+
+/**
+ * The async verification function that accepts control props and returns a
+ * promise resolving to extra props if overrides are needed.
+ */
+export type AsyncVerify = (
+ props: ControlPropsWithExtras,
+) => Promise<ExtraControlProps | undefined | null>;
+
+/**
+ * Whether the extra props will update the original props.
+ */
+function hasUpdates(
+ props: ControlPropsWithExtras,
+ newProps: ExtraControlProps,
+) {
+ return (
+ props !== newProps &&
+ Object.entries(newProps).some(([key, value]) => {
+ if (Array.isArray(props[key]) && Array.isArray(value)) {
+ const sourceValue: JsonArray = props[key];
+ return (
+ sourceValue.length !== value.length ||
+ sourceValue.some((x, i) => x !== value[i])
+ );
+ }
+ if (key === 'formData') {
+ return JSON.stringify(props[key]) !== JSON.stringify(value);
+ }
+ return props[key] !== value;
+ })
+ );
+}
+
+export type WithAsyncVerificationOptions = {
+ baseControl:
+ | SharedControlComponent
+ // allows custom `baseControl` to not handle some of the <Control />
+ // component props.
+ | ComponentType<Partial<FullControlProps>>;
+ showLoadingState?: boolean;
+ quiet?: boolean;
+ verify?: AsyncVerify;
+ onChange?: (value: JsonValue, props: ControlPropsWithExtras) => void;
+};
+
+/**
+ * Wrap Control with additional async verification. The <Control /> component
+ * will render twice, once with the original props, then later with the updated
+ * props after the async verification is finished.
+ *
+ * @param baseControl - The base control component.
+ * @param verify - An async function that returns a Promise which resolves with
+ * the updated and verified props. You should handle error within
+ * the promise itself. If the Promise returns nothing or null, then
+ * the control will not rerender.
+ * @param onChange - Additional event handler when values are changed by users.
+ * @param quiet - Whether to show a warning toast when verification failed.
+ */
+export default function withAsyncVerification({
+ baseControl,
+ onChange,
+ verify: defaultVerify,
+ quiet = false,
+ showLoadingState: defaultShowLoadingState = true,
+}: WithAsyncVerificationOptions) {
+ const ControlComponent: ComponentType<FullControlProps> =
+ typeof baseControl === 'string'
+ ? controlComponentMap[baseControl]
+ : baseControl;
+
+ return function ControlWithVerification(props: FullControlProps) {
+ const {
+ hovered,
+ onChange: basicOnChange,
+ needAsyncVerification = false,
+ isLoading: initialIsLoading = false,
+ showLoadingState = defaultShowLoadingState,
+ verify = defaultVerify,
+ ...restProps
+ } = props;
+ const otherPropsRef = useRef(restProps);
+ const [verifiedProps, setVerifiedProps] = useState({});
+ const [isLoading, setIsLoading] = useState<boolean>(initialIsLoading);
+ const { addWarningToast } = restProps.actions;
+
+ // memoize `restProps`, so that verification only triggers when material
+ // props are actually updated.
+ let otherProps = otherPropsRef.current;
+ if (hasUpdates(otherProps, restProps)) {
+ otherProps = otherPropsRef.current = restProps;
+ }
+
+ const handleChange = useCallback(
+ (value: JsonValue) => {
+ // the default onChange handler, triggers the `setControlValue` action
+ if (basicOnChange) {
+ basicOnChange(value);
+ }
+ if (onChange) {
+ onChange(value, { ...otherProps, ...verifiedProps });
+ }
+ },
+ [basicOnChange, otherProps, verifiedProps],
+ );
+
+ useEffect(() => {
+ if (needAsyncVerification && verify) {
+ if (showLoadingState) {
+ setIsLoading(true);
+ }
+ verify(otherProps)
+ .then(updatedProps => {
+ if (showLoadingState) {
+ setIsLoading(false);
+ }
+ if (updatedProps && hasUpdates(otherProps, updatedProps)) {
+ setVerifiedProps({
+ // save isLoading in combination with other props to avoid
+ // rendering twice.
+ ...updatedProps,
+ });
+ }
+ })
+ .catch((err: Error | string) => {
+ if (showLoadingState) {
+ setIsLoading(false);
+ }
+ if (!quiet && addWarningToast) {
+ addWarningToast(
+ t(
+ 'Failed to verify select options: %s',
+ (typeof err === 'string' ? err : err.message) ||
+ t('[unknown error]'),
+ ),
+ { noDuplicate: true },
+ );
+ }
+ });
+ }
+ }, [
+ needAsyncVerification,
+ showLoadingState,
+ verify,
+ otherProps,
+ addWarningToast,
+ ]);
+
+ return (
+ <ControlComponent
+ isLoading={isLoading}
+ hovered={hovered}
+ onChange={handleChange}
+ {...otherProps}
+ {...verifiedProps}
+ />
+ );
+ };
+}
diff --git a/superset-frontend/src/explore/components/controls/withVerification.jsx b/superset-frontend/src/explore/components/controls/withVerification.jsx
deleted file mode 100644
index b0a8e63..0000000
--- a/superset-frontend/src/explore/components/controls/withVerification.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 { SupersetClient, logging } from '@superset-ui/core';
-
-import { isEqual } from 'lodash';
-
-export default function withVerification(
- WrappedComponent,
- optionLabel,
- optionsName,
-) {
- /*
- * This function will verify control options before passing them to the control by calling an
- * endpoint on mount and when the controlValues change. controlValues should be set in
- * mapStateToProps that can be added as a control override along with getEndpoint.
- */
- class withVerificationComponent extends React.Component {
- constructor(props) {
- super(props);
- this.state = {
- validOptions: null,
- hasRunVerification: false,
- };
-
- this.getValidOptions = this.getValidOptions.bind(this);
- }
-
- componentDidMount() {
- this.getValidOptions();
- }
-
- componentDidUpdate(prevProps) {
- const { hasRunVerification } = this.state;
- if (
- !isEqual(this.props.controlValues, prevProps.controlValues) ||
- !hasRunVerification
- ) {
- this.getValidOptions();
- }
- }
-
- getValidOptions() {
- const endpoint = this.props.getEndpoint(this.props.controlValues);
- if (endpoint) {
- SupersetClient.get({
- endpoint,
- })
- .then(({ json }) => {
- if (Array.isArray(json)) {
- this.setState({ validOptions: new Set(json) || new Set() });
- }
- })
- .catch(error => logging.log(error));
-
- if (!this.state.hasRunVerification) {
- this.setState({ hasRunVerification: true });
- }
- }
- }
-
- render() {
- const { validOptions } = this.state;
- const options = this.props[optionsName];
- const verifiedOptions = validOptions
- ? options.filter(o => validOptions.has(o[optionLabel]))
- : options;
-
- const newProps = { ...this.props, [optionsName]: verifiedOptions };
-
- return <WrappedComponent {...newProps} />;
- }
- }
- withVerificationComponent.propTypes = WrappedComponent.propTypes;
- return withVerificationComponent;
-}
diff --git a/superset-frontend/src/messageToasts/actions/index.ts b/superset-frontend/src/messageToasts/actions/index.ts
index 4eeb2e1..3eaf51c 100644
--- a/superset-frontend/src/messageToasts/actions/index.ts
+++ b/superset-frontend/src/messageToasts/actions/index.ts
@@ -19,6 +19,8 @@
import shortid from 'shortid';
import { ToastType, ToastMeta } from '../types';
+type ToastOptions = Partial<Omit<ToastMeta, 'id' | 'toastType' | 'text'>>;
+
export function getToastUuid(type: ToastType) {
return `${type}-${shortid.generate()}`;
}
@@ -28,6 +30,7 @@ export function addToast({
toastType,
text,
duration = 8000,
+ noDuplicate = false,
}: Omit<ToastMeta, 'id'>) {
return {
type: ADD_TOAST,
@@ -36,6 +39,7 @@ export function addToast({
toastType,
text,
duration,
+ noDuplicate,
},
};
}
@@ -52,21 +56,48 @@ export function removeToast(id: string) {
// Different types of toasts
export const ADD_INFO_TOAST = 'ADD_INFO_TOAST';
-export function addInfoToast(text: string) {
- return addToast({ text, toastType: ToastType.INFO, duration: 4000 });
+export function addInfoToast(text: string, options?: ToastOptions) {
+ return addToast({
+ text,
+ toastType: ToastType.INFO,
+ duration: 4000,
+ ...options,
+ });
}
export const ADD_SUCCESS_TOAST = 'ADD_SUCCESS_TOAST';
-export function addSuccessToast(text: string) {
- return addToast({ text, toastType: ToastType.SUCCESS, duration: 4000 });
+export function addSuccessToast(text: string, options?: ToastOptions) {
+ return addToast({
+ text,
+ toastType: ToastType.SUCCESS,
+ duration: 4000,
+ ...options,
+ });
}
export const ADD_WARNING_TOAST = 'ADD_WARNING_TOAST';
-export function addWarningToast(text: string) {
- return addToast({ text, toastType: ToastType.WARNING, duration: 6000 });
+export function addWarningToast(text: string, options?: ToastOptions) {
+ return addToast({
+ text,
+ toastType: ToastType.WARNING,
+ duration: 6000,
+ ...options,
+ });
}
export const ADD_DANGER_TOAST = 'ADD_DANGER_TOAST';
-export function addDangerToast(text: string) {
- return addToast({ text, toastType: ToastType.DANGER, duration: 8000 });
+export function addDangerToast(text: string, options?: ToastOptions) {
+ return addToast({
+ text,
+ toastType: ToastType.DANGER,
+ duration: 8000,
+ ...options,
+ });
}
+
+export const toastActions = {
+ addInfoToast,
+ addSuccessToast,
+ addWarningToast,
+ addDangerToast,
+};
diff --git a/superset-frontend/src/messageToasts/reducers/index.js b/superset-frontend/src/messageToasts/reducers/index.js
index e82e297..2203bf8 100644
--- a/superset-frontend/src/messageToasts/reducers/index.js
+++ b/superset-frontend/src/messageToasts/reducers/index.js
@@ -22,7 +22,11 @@ export default function messageToastsReducer(toasts = [], action) {
switch (action.type) {
case ADD_TOAST: {
const { payload: toast } = action;
- return [toast, ...toasts];
+ const result = toasts.slice();
+ if (!toast.noDuplicate || !result.find(x => x.text === toast.text)) {
+ return [toast, ...toasts];
+ }
+ return toasts;
}
case REMOVE_TOAST: {
diff --git a/superset-frontend/src/messageToasts/types.ts b/superset-frontend/src/messageToasts/types.ts
index 0a72ec3..cd41927 100644
--- a/superset-frontend/src/messageToasts/types.ts
+++ b/superset-frontend/src/messageToasts/types.ts
@@ -28,4 +28,7 @@ export interface ToastMeta {
toastType: ToastType;
text: string;
duration: number;
+ /** Whether to skip displaying this message if there are another toast
+ * with the same message. */
+ noDuplicate?: boolean;
}