From fac29f9dff8ca504006b20e1b81f094d4d9761b4 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Mon, 2 Nov 2020 15:06:20 -0800 Subject: [PATCH] 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 +++++++++++ .../components/withVerification_spec.jsx | 127 ---------- .../src/explore/actions/exploreActions.ts | 3 +- .../explore/components/AdhocFilterOption.jsx | 19 +- .../explore/components/AdhocMetricOption.jsx | 19 +- .../src/explore/components/Control.tsx | 2 +- .../controls/AdhocFilterControl.jsx | 2 + .../components/controls/MetricsControl.jsx | 2 + .../src/explore/components/controls/index.js | 16 -- .../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(-) create mode 100644 superset-frontend/spec/javascripts/explore/components/withAsyncVerification_spec.tsx delete mode 100644 superset-frontend/spec/javascripts/explore/components/withVerification_spec.jsx create mode 100644 superset-frontend/src/explore/components/controls/withAsyncVerification.tsx delete mode 100644 superset-frontend/src/explore/components/controls/withVerification.jsx 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 0000000000..73805c74ad --- /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 & { + extraProps?: ExtraControlProps; +} = {}) { + const props = { + ...defaultProps, + ...extraProps, + }; + const verifier = verify('savedMetrics'); + const VerifiedControl = withAsyncVerification({ + baseControl, + verify: verifier, + onChange, + }); + type Wrapper = ReactWrapper; + let wrapper: Wrapper | undefined; + await act(async () => { + wrapper = mount(); + }); + 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 ed9457267a..0000000000 --- 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(); - 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 af71199ad7..f312f68e1c 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 42ba81dab5..c7cf9eeb6d 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 d030e670e2..c62cf7cc65 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 8fc6583f23..e728ad59d9 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 & Pick; 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 28e6b42275..24489825ce 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 { & + 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; + +/** + * 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 + // component props. + | ComponentType>; + showLoadingState?: boolean; + quiet?: boolean; + verify?: AsyncVerify; + onChange?: (value: JsonValue, props: ControlPropsWithExtras) => void; +}; + +/** + * Wrap Control with additional async verification. The 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 = + 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(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 ( + + ); + }; +} 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 b0a8e63c62..0000000000 --- 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 ; - } - } - 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 4eeb2e12d5..3eaf51c3ce 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>; + export function getToastUuid(type: ToastType) { return `${type}-${shortid.generate()}`; } @@ -28,6 +30,7 @@ export function addToast({ toastType, text, duration = 8000, + noDuplicate = false, }: Omit) { 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 e82e2972b9..2203bf8373 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 0a72ec39b7..cd41927847 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; }