refactor(explore): improve typing for Dnd controls (#16362)

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.test.tsx

@@ -18,13 +18,17 @@
  */
 import React from 'react';
 import { render, screen } from 'spec/helpers/testing-library';
-import { LabelProps } from 'src/explore/components/controls/DndColumnSelectControl/types';
-import { DndColumnSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndColumnSelect';
+import {
+  DndColumnSelect,
+  DndColumnSelectProps,
+} from 'src/explore/components/controls/DndColumnSelectControl/DndColumnSelect';
 
-const defaultProps: LabelProps = {
+const defaultProps: DndColumnSelectProps = {
+  type: 'DndColumnSelect',
   name: 'Filter',
   onChange: jest.fn(),
   options: { string: { column_name: 'Column A' } },
+  actions: { setControlValue: jest.fn() },
 };
 
 test('renders with default props', () => {

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelect.tsx

@@ -20,7 +20,6 @@ import React, { useCallback, useMemo, useState } from 'react';
 import { FeatureFlag, isFeatureEnabled, tn } from '@superset-ui/core';
 import { ColumnMeta } from '@superset-ui/chart-controls';
 import { isEmpty } from 'lodash';
-import { LabelProps } from 'src/explore/components/controls/DndColumnSelectControl/types';
 import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
 import OptionWrapper from 'src/explore/components/controls/DndColumnSelectControl/OptionWrapper';
 import { OptionSelector } from 'src/explore/components/controls/DndColumnSelectControl/utils';
@@ -28,8 +27,13 @@ import { DatasourcePanelDndItem } from 'src/explore/components/DatasourcePanel/t
 import { DndItemType } from 'src/explore/components/DndItemType';
 import { useComponentDidUpdate } from 'src/common/hooks/useComponentDidUpdate';
 import ColumnSelectPopoverTrigger from './ColumnSelectPopoverTrigger';
+import { DndControlProps } from './types';
 
-export const DndColumnSelect = (props: LabelProps) => {
+export type DndColumnSelectProps = DndControlProps<string> & {
+  options: Record<string, ColumnMeta>;
+};
+
+export function DndColumnSelect(props: DndColumnSelectProps) {
   const {
     value,
     options,
@@ -68,6 +72,7 @@ export const DndColumnSelect = (props: LabelProps) => {
     ) {
       onChange(optionSelectorValues);
     }
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [JSON.stringify(value), JSON.stringify(optionSelector.getValues())]);
 
   // useComponentDidUpdate to avoid running this for the first render, to avoid
@@ -203,7 +208,7 @@ export const DndColumnSelect = (props: LabelProps) => {
 
   return (
     <div>
-      <DndSelectLabel<string | string[], ColumnMeta[]>
+      <DndSelectLabel
         onDrop={onDrop}
         canDrop={canDrop}
         valuesRenderer={valuesRenderer}
@@ -229,4 +234,4 @@ export const DndColumnSelect = (props: LabelProps) => {
       </ColumnSelectPopoverTrigger>
     </div>
   );
-};
+}

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx

@@ -23,17 +23,24 @@ import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetr
 import AdhocFilter, {
   EXPRESSION_TYPES,
 } from 'src/explore/components/controls/FilterControl/AdhocFilter';
-import { DndFilterSelect } from 'src/explore/components/controls/DndColumnSelectControl/DndFilterSelect';
+import {
+  DndFilterSelect,
+  DndFilterSelectProps,
+} from 'src/explore/components/controls/DndColumnSelectControl/DndFilterSelect';
+import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
+import { DEFAULT_FORM_DATA } from '@superset-ui/plugin-chart-echarts/lib/Timeseries/types';
 
-const defaultProps = {
+const defaultProps: DndFilterSelectProps = {
+  type: 'DndFilterSelect',
   name: 'Filter',
   value: [],
   columns: [],
-  datasource: {},
-  formData: {},
+  datasource: PLACEHOLDER_DATASOURCE,
+  formData: null,
   savedMetrics: [],
+  selectedMetrics: [],
   onChange: jest.fn(),
-  options: { string: { column_name: 'Column' } },
+  actions: { setControlValue: jest.fn() },
 };
 
 test('renders with default props', () => {
@@ -53,9 +60,15 @@ test('renders with value', () => {
 });
 
 test('renders options with saved metric', () => {
-  render(<DndFilterSelect {...defaultProps} formData={['saved_metric']} />, {
-    useDnd: true,
-  });
+  render(
+    <DndFilterSelect
+      {...defaultProps}
+      formData={{ ...DEFAULT_FORM_DATA, metrics: ['saved_metric'] }}
+    />,
+    {
+      useDnd: true,
+    },
+  );
   expect(screen.getByText('Drop columns or metrics here')).toBeInTheDocument();
 });
 
@@ -84,8 +97,14 @@ test('renders options with adhoc metric', () => {
     expression: 'AVG(birth_names.num)',
     metric_name: 'avg__num',
   });
-  render(<DndFilterSelect {...defaultProps} formData={[adhocMetric]} />, {
-    useDnd: true,
-  });
+  render(
+    <DndFilterSelect
+      {...defaultProps}
+      formData={{ ...DEFAULT_FORM_DATA, metrics: [adhocMetric] }}
+    />,
+    {
+      useDnd: true,
+    },
+  );
   expect(screen.getByText('Drop columns or metrics here')).toBeInTheDocument();
 });

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx

@@ -22,6 +22,8 @@ import {
   isFeatureEnabled,
   logging,
   Metric,
+  QueryFormData,
+  QueryFormMetric,
   SupersetClient,
   t,
 } from '@superset-ui/core';
@@ -30,11 +32,8 @@ import {
   OPERATOR_ENUM_TO_OPERATOR_TYPE,
   Operators,
 } from 'src/explore/constants';
-import { OptionSortType } from 'src/explore/types';
-import {
-  DndFilterSelectProps,
-  OptionValueType,
-} from 'src/explore/components/controls/DndColumnSelectControl/types';
+import { Datasource, OptionSortType } from 'src/explore/types';
+import { OptionValueType } from 'src/explore/components/controls/DndColumnSelectControl/types';
 import AdhocFilterPopoverTrigger from 'src/explore/components/controls/FilterControl/AdhocFilterPopoverTrigger';
 import OptionWrapper from 'src/explore/components/controls/DndColumnSelectControl/OptionWrapper';
 import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
@@ -48,6 +47,7 @@ import {
   DndItemValue,
 } from 'src/explore/components/DatasourcePanel/types';
 import { DndItemType } from 'src/explore/components/DndItemType';
+import { ControlComponentProps } from 'src/explore/components/Control';
 
 const DND_ACCEPTED_TYPES = [
   DndItemType.Column,
@@ -59,8 +59,16 @@ const DND_ACCEPTED_TYPES = [
 const isDictionaryForAdhocFilter = (value: OptionValueType) =>
   !(value instanceof AdhocFilter) && value?.expressionType;
 
+export interface DndFilterSelectProps
+  extends ControlComponentProps<OptionValueType[]> {
+  columns: ColumnMeta[];
+  savedMetrics: Metric[];
+  selectedMetrics: QueryFormMetric[];
+  datasource: Datasource;
+}
+
 export const DndFilterSelect = (props: DndFilterSelectProps) => {
-  const { datasource, onChange } = props;
+  const { datasource, onChange = () => {}, name: controlName } = props;
 
   const propsValues = Array.from(props.value ?? []);
   const [values, setValues] = useState(
@@ -74,7 +82,7 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
 
   const optionsForSelect = (
     columns: ColumnMeta[],
-    formData: Record<string, any>,
+    formData: QueryFormData | null | undefined,
   ) => {
     const options: OptionSortType[] = [
       ...columns,
@@ -369,7 +377,7 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
       setDroppedItem(item.value);
       togglePopover(true);
     },
-    [togglePopover],
+    [controlName, togglePopover],
   );
 
   const ghostButtonText = isFeatureEnabled(FeatureFlag.ENABLE_DND_WITH_CLICK_UX)
@@ -378,7 +386,7 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
 
   return (
     <>
-      <DndSelectLabel<OptionValueType, OptionValueType[]>
+      <DndSelectLabel
         onDrop={handleDrop}
         canDrop={canDrop}
         valuesRenderer={valuesRenderer}

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndMetricSelect.tsx

@@ -19,11 +19,13 @@
 
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import {
+  DatasourceType,
   ensureIsArray,
   FeatureFlag,
   GenericDataType,
   isFeatureEnabled,
   Metric,
+  QueryFormMetric,
   tn,
 } from '@superset-ui/core';
 import { ColumnMeta } from '@superset-ui/chart-controls';
@@ -32,12 +34,12 @@ import { usePrevious } from 'src/common/hooks/usePrevious';
 import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
 import AdhocMetricPopoverTrigger from 'src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger';
 import MetricDefinitionValue from 'src/explore/components/controls/MetricControl/MetricDefinitionValue';
-import { OptionValueType } from 'src/explore/components/controls/DndColumnSelectControl/types';
 import { DatasourcePanelDndItem } from 'src/explore/components/DatasourcePanel/types';
 import { DndItemType } from 'src/explore/components/DndItemType';
 import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
 import { savedMetricType } from 'src/explore/components/controls/MetricControl/types';
 import { AGGREGATES } from 'src/explore/constants';
+import { DndControlProps } from './types';
 
 const EMPTY_OBJECT = {};
 const DND_ACCEPTED_TYPES = [DndItemType.Column, DndItemType.Metric];
@@ -75,10 +77,12 @@ const getOptionsForSavedMetrics = (
       : savedMetric,
   ) ?? [];
 
+type ValueType = Metric | AdhocMetric | QueryFormMetric;
+
 const columnsContainAllMetrics = (
-  value: (string | AdhocMetric | ColumnMeta)[],
+  value: ValueType | ValueType[] | null | undefined,
   columns: ColumnMeta[],
-  savedMetrics: savedMetricType[],
+  savedMetrics: (savedMetricType | Metric)[],
 ) => {
   const columnNames = new Set(
     [...(columns || []), ...(savedMetrics || [])]
@@ -104,6 +108,12 @@ const columnsContainAllMetrics = (
   );
 };
 
+export type DndMetricSelectProps = DndControlProps<ValueType> & {
+  savedMetrics: savedMetricType[];
+  columns: ColumnMeta[];
+  datasourceType?: DatasourceType;
+};
+
 export const DndMetricSelect = (props: any) => {
   const { onChange, multi, columns, savedMetrics } = props;
 
@@ -130,7 +140,7 @@ export const DndMetricSelect = (props: any) => {
     [multi, onChange],
   );
 
-  const [value, setValue] = useState<(AdhocMetric | Metric | string)[]>(
+  const [value, setValue] = useState<ValueType[]>(
     coerceAdhocMetrics(props.value),
   );
   const [droppedItem, setDroppedItem] = useState<DatasourcePanelDndItem | null>(
@@ -176,7 +186,9 @@ export const DndMetricSelect = (props: any) => {
 
   const onNewMetric = useCallback(
     (newMetric: Metric) => {
-      const newValue = props.multi ? [...value, newMetric] : [newMetric];
+      const newValue = props.multi
+        ? [...value, newMetric.metric_name]
+        : [newMetric.metric_name];
       setValue(newValue);
       handleChange(newValue);
     },
@@ -191,7 +203,7 @@ export const DndMetricSelect = (props: any) => {
       const newValue = value.map(value => {
         if (
           // compare saved metrics
-          value === (oldMetric as Metric).metric_name ||
+          ('metric_name' in oldMetric && value === oldMetric.metric_name) ||
           // compare adhoc metrics
           typeof (value as AdhocMetric).optionName !== 'undefined'
             ? (value as AdhocMetric).optionName ===
@@ -254,7 +266,7 @@ export const DndMetricSelect = (props: any) => {
   );
 
   const valueRenderer = useCallback(
-    (option: Metric | AdhocMetric | string, index: number) => (
+    (option: ValueType, index: number) => (
       <MetricDefinitionValue
         key={index}
         index={index}
@@ -353,7 +365,7 @@ export const DndMetricSelect = (props: any) => {
 
   return (
     <div className="metrics-select">
-      <DndSelectLabel<OptionValueType, OptionValueType[]>
+      <DndSelectLabel
         onDrop={handleDrop}
         canDrop={canDrop}
         valuesRenderer={valuesRenderer}

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.test.tsx

@@ -19,16 +19,16 @@
 import React from 'react';
 import { render, screen } from 'spec/helpers/testing-library';
 import { DndItemType } from 'src/explore/components/DndItemType';
-import DndSelectLabel from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
+import DndSelectLabel, {
+  DndSelectLabelProps,
+} from 'src/explore/components/controls/DndColumnSelectControl/DndSelectLabel';
 
-const defaultProps = {
+const defaultProps: DndSelectLabelProps = {
   name: 'Column',
   accept: 'Column' as DndItemType,
   onDrop: jest.fn(),
   canDrop: () => false,
   valuesRenderer: () => <span />,
-  onChange: jest.fn(),
-  options: { string: { column_name: 'Column' } },
 };
 
 test('renders with default props', async () => {

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndSelectLabel.tsx

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import React from 'react';
+import React, { ReactNode } from 'react';
 import { useDrop } from 'react-dnd';
 import { t, useTheme } from '@superset-ui/core';
 import ControlHeader from 'src/explore/components/ControlHeader';
@@ -25,18 +25,35 @@ import {
   DndLabelsContainer,
   HeaderContainer,
 } from 'src/explore/components/controls/OptionControls';
-import { DatasourcePanelDndItem } from 'src/explore/components/DatasourcePanel/types';
+import {
+  DatasourcePanelDndItem,
+  DndItemValue,
+} from 'src/explore/components/DatasourcePanel/types';
 import Icons from 'src/components/Icons';
-import { DndColumnSelectProps } from './types';
+import { DndItemType } from '../../DndItemType';
 
-export default function DndSelectLabel<T, O>({
+export type DndSelectLabelProps = {
+  name: string;
+  accept: DndItemType | DndItemType[];
+  ghostButtonText?: string;
+  onDrop: (item: DatasourcePanelDndItem) => void;
+  canDrop: (item: DatasourcePanelDndItem) => boolean;
+  canDropValue?: (value: DndItemValue) => boolean;
+  onDropValue?: (value: DndItemValue) => void;
+  valuesRenderer: () => ReactNode;
+  displayGhostButton?: boolean;
+  onClickGhostButton?: () => void;
+};
+
+export default function DndSelectLabel({
   displayGhostButton = true,
+  accept,
   ...props
-}: DndColumnSelectProps<T, O>) {
+}: DndSelectLabelProps) {
   const theme = useTheme();
 
   const [{ isOver, canDrop }, datasourcePanelDrop] = useDrop({
-    accept: props.accept,
+    accept,
 
     drop: (item: DatasourcePanelDndItem) => {
       props.onDrop(item);

superset-frontend/src/explore/components/controls/DndColumnSelectControl/types.ts

@@ -17,13 +17,9 @@
  * under the License.
  */
 import { ReactNode } from 'react';
-import { Metric } from '@superset-ui/core';
+import { JsonValue } from '@superset-ui/core';
+import { ControlComponentProps } from 'src/explore/components/Control';
 import { ColumnMeta } from '@superset-ui/chart-controls';
-import {
-  DatasourcePanelDndItem,
-  DndItemValue,
-} from '../../DatasourcePanel/types';
-import { DndItemType } from '../../DndItemType';
 
 export interface OptionProps {
   children?: ReactNode;
@@ -42,40 +38,16 @@ export interface OptionItemInterface {
   dragIndex: number;
 }
 
-export interface LabelProps<T = string[] | string> {
-  name: string;
-  value?: T;
-  onChange: (value?: T) => void;
-  options: { string: ColumnMeta };
+/**
+ * Shared control props for all DnD control.
+ */
+export type DndControlProps<
+  ValueType extends JsonValue
+> = ControlComponentProps<ValueType | ValueType[] | null> & {
   multi?: boolean;
   canDelete?: boolean;
   ghostButtonText?: string;
-  label?: string;
-}
-
-export interface DndColumnSelectProps<
-  T = string[] | string,
-  O = string[] | string
-> extends LabelProps<T> {
-  onDrop: (item: DatasourcePanelDndItem) => void;
-  canDrop: (item: DatasourcePanelDndItem) => boolean;
-  canDropValue?: (value: DndItemValue) => boolean;
-  onDropValue?: (value: DndItemValue) => void;
-  valuesRenderer: () => ReactNode;
-  accept: DndItemType | DndItemType[];
-  ghostButtonText?: string;
-  displayGhostButton?: boolean;
-  onClickGhostButton?: () => void;
-}
+  onChange: (value: ValueType | ValueType[] | null | undefined) => void;
+};
 
 export type OptionValueType = Record<string, any>;
-export interface DndFilterSelectProps {
-  name: string;
-  value: OptionValueType[];
-  columns: ColumnMeta[];
-  datasource: Record<string, any>;
-  formData: Record<string, any>;
-  savedMetrics: Metric[];
-  onChange: (filters: OptionValueType[]) => void;
-  options: { string: ColumnMeta };
-}

superset-frontend/src/explore/components/controls/DndColumnSelectControl/utils/optionSelector.ts

@@ -22,25 +22,25 @@ import { ensureIsArray } from '@superset-ui/core';
 export class OptionSelector {
   values: ColumnMeta[];
 
-  options: { string: ColumnMeta };
+  options: Record<string, ColumnMeta>;
 
   multi: boolean;
 
   constructor(
-    options: { string: ColumnMeta },
+    options: Record<string, ColumnMeta>,
     multi: boolean,
-    initialValues?: string[] | string,
+    initialValues?: string[] | string | null,
   ) {
     this.options = options;
     this.multi = multi;
     this.values = ensureIsArray(initialValues)
       .map(value => {
-        if (value in options) {
+        if (value && value in options) {
           return options[value];
         }
         return null;
       })
-      .filter(Boolean);
+      .filter(Boolean) as ColumnMeta[];
   }
 
   add(value: string) {