refactor: rewrite and enhance chart control withVerification (#11435)

* refactor: rewrite and enhance chart control withVerification

* Add toasts for failed messages; fixes popover render

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}

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')}

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;

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}
+      />
+    );
+  };
+}

superset-frontend/src/explore/components/controls/withVerification.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 { 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;
-}