refactor(native-filters): decouple params from filter config modal (first phase) (#13021)

* refactor: decouple params from filter config modal (first phase) * refactor: remove unnecessary fields * refactor: fix CR notes * fix: fix controlValues * refactor: fix Cr notes Co-authored-by: amitmiran137 <amit.miran@nielsen.com>
2026-04-19 08:04:53 +00:00 · 2021-02-14 10:38:15 +02:00
parent bc4c837da3
commit 2010e64c94
13 changed files with 105 additions and 144 deletions
--- a/superset-frontend/spec/fixtures/mockNativeFilters.ts
+++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts
@@ -39,10 +39,12 @@ export const nativeFilters: NativeFiltersState = {
        rootPath: ['ROOT_ID'],
        excluded: [],
      },
-      inverseSelection: false,
      isInstant: true,
-      allowsMultipleValues: false,
-      isRequired: false,
+      controlValues: {
+        multiSelect: false,
+        enableEmptyFilter: false,
+        inverseSelection: false,
+      },
    },
    'NATIVE_FILTER-x9QPw0so1': {
      id: 'NATIVE_FILTER-x9QPw0so1',
@@ -62,10 +64,12 @@ export const nativeFilters: NativeFiltersState = {
        rootPath: ['ROOT_ID'],
        excluded: [],
      },
-      inverseSelection: false,
+      controlValues: {
+        multiSelect: false,
+        enableEmptyFilter: false,
+        inverseSelection: false,
+      },
      isInstant: true,
-      allowsMultipleValues: false,
-      isRequired: false,
    },
  },
  filtersState: {
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx
@@ -48,14 +48,7 @@ const FilterValue: React.FC<FilterProps> = ({
  directPathToChild,
  onFilterSelectionChange,
 }) => {
-  const {
-    id,
-    allowsMultipleValues,
-    inverseSelection,
-    targets,
-    defaultValue,
-    filterType,
-  } = filter;
+  const { id, targets, filterType } = filter;
  const cascadingFilters = useCascadingFilters(id);
  const filterState = useFilterState(id);
  const [loading, setLoading] = useState<boolean>(true);
@@ -72,11 +65,9 @@ const FilterValue: React.FC<FilterProps> = ({
      datasetId,
      cascadingFilters,
      groupby,
-      allowsMultipleValues,
-      defaultValue,
      currentValue,
-      inverseSelection,
      inputRef,
+      ...filter,
    });
    if (!areObjectsEqual(formData || {}, newFormData)) {
      setFormData(newFormData);
@@ -95,7 +86,7 @@ const FilterValue: React.FC<FilterProps> = ({
          setLoading(false);
        });
    }
-  }, [cascadingFilters, datasetId, groupby, defaultValue, currentValue]);
+  }, [cascadingFilters, datasetId, groupby, filter.defaultValue, currentValue]);

  useEffect(() => {
    if (directPathToChild?.[0] === filter.id) {
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx
@@ -16,7 +16,12 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import { styled, SuperChart, t } from '@superset-ui/core';
+import {
+  styled,
+  SuperChart,
+  t,
+  getChartControlPanelRegistry,
+} from '@superset-ui/core';
 import { FormInstance } from 'antd/lib/form';
 import React, { useCallback } from 'react';
 import {
@@ -30,10 +35,16 @@ import { Select } from 'src/components/Select/SupersetStyledSelect';
 import SupersetResourceSelect from 'src/components/SupersetResourceSelect';
 import { addDangerToast } from 'src/messageToasts/actions';
 import { ClientErrorObject } from 'src/utils/getClientErrorObject';
+import { CustomControlItem } from '@superset-ui/chart-controls';
 import { ColumnSelect } from './ColumnSelect';
 import { NativeFiltersForm } from './types';
 import FilterScope from './FilterScope';
-import { FilterTypeNames, setFilterFieldValues, useForceUpdate } from './utils';
+import {
+  FilterTypeNames,
+  getControlItems,
+  setFilterFieldValues,
+  useForceUpdate,
+} from './utils';
 import { useBackendFormUpdate } from './state';
 import { getFormData } from '../utils';
 import { Filter, FilterType } from '../types';
@@ -104,8 +115,12 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
  form,
  parentFilters,
 }) => {
+  const controlPanelRegistry = getChartControlPanelRegistry();
  const forceUpdate = useForceUpdate();
  const formFilter = (form.getFieldValue('filters') || {})[filterId];
+  const controlItems = getControlItems(
+    controlPanelRegistry.get(formFilter?.filterType),
+  );
  useBackendFormUpdate(form, filterId, filterToEdit);

  const initDatasetId = filterToEdit?.targets[0].datasetId;
@@ -113,9 +128,8 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
  const newFormData = getFormData({
    datasetId: formFilter?.dataset?.value,
    groupby: formFilter?.column,
-    allowsMultipleValues: formFilter?.allowsMultipleValues,
    defaultValue: formFilter?.defaultValue,
-    inverseSelection: formFilter?.inverseSelection,
+    ...formFilter,
  });

  const onDatasetSelectError = useCallback(
@@ -289,39 +303,33 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
          {t('Apply changes instantly')}
        </Checkbox>
      </StyledCheckboxFormItem>
-      <StyledCheckboxFormItem
-        name={['filters', filterId, 'allowsMultipleValues']}
-        initialValue={filterToEdit?.allowsMultipleValues}
-        valuePropName="checked"
-        colon={false}
-      >
-        <Checkbox
-          onChange={() => {
-            setFilterFieldValues(form, filterId, {
-              defaultValue: null,
-            });
-            forceUpdate();
-          }}
-        >
-          {t('Allow multiple selections')}
-        </Checkbox>
-      </StyledCheckboxFormItem>
-      <StyledCheckboxFormItem
-        name={['filters', filterId, 'inverseSelection']}
-        initialValue={filterToEdit?.inverseSelection}
-        valuePropName="checked"
-        colon={false}
-      >
-        <Checkbox>{t('Inverse selection')}</Checkbox>
-      </StyledCheckboxFormItem>
-      <StyledCheckboxFormItem
-        name={['filters', filterId, 'isRequired']}
-        initialValue={filterToEdit?.isRequired}
-        valuePropName="checked"
-        colon={false}
-      >
-        <Checkbox>{t('Required')}</Checkbox>
-      </StyledCheckboxFormItem>
+      {controlItems
+        .filter(
+          (controlItem: CustomControlItem) =>
+            controlItem?.config?.renderTrigger,
+        )
+        .map(controlItem => (
+          <StyledCheckboxFormItem
+            name={['filters', filterId, 'controlValues', controlItem.name]}
+            initialValue={filterToEdit?.controlValues?.[controlItem.name]}
+            valuePropName="checked"
+            colon={false}
+          >
+            <Checkbox
+              onChange={() => {
+                if (!controlItem.config.resetConfig) {
+                  return;
+                }
+                setFilterFieldValues(form, filterId, {
+                  defaultValue: null,
+                });
+                forceUpdate();
+              }}
+            >
+              {controlItem.config.label}
+            </Checkbox>
+          </StyledCheckboxFormItem>
+        ))}
      <FilterScope
        filterId={filterId}
        filterToEdit={filterToEdit}
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigModal.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigModal.tsx
@@ -54,7 +54,7 @@ const StyledForm = styled(Form)`
 const StyledSpan = styled.span`
  cursor: pointer;
  color: ${({ theme }) => theme.colors.primary.dark1};
-  &: hover {
+  &:hover {
    color: ${({ theme }) => theme.colors.primary.dark2};
  }
 `;
@@ -402,6 +402,7 @@ export function FilterConfigModal({
        if (!formInputs) return filterConfigMap[id];
        return {
          id,
+          controlValues: formInputs.controlValues,
          name: formInputs.name,
          filterType: formInputs.filterType,
          // for now there will only ever be one target
@@ -418,10 +419,7 @@ export function FilterConfigModal({
            ? [formInputs.parentFilter.value]
            : [],
          scope: formInputs.scope,
-          inverseSelection: !!formInputs.inverseSelection,
-          isInstant: !!formInputs.isInstant,
-          allowsMultipleValues: !!formInputs.allowsMultipleValues,
-          isRequired: !!formInputs.isRequired,
+          isInstant: formInputs.isInstant,
        };
      });

--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/state.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/state.ts
@@ -96,9 +96,8 @@ export const useBackendFormUpdate = (
    const formData = getFormData({
      datasetId: formFilter?.dataset?.value,
      groupby: formFilter?.column,
-      allowsMultipleValues: formFilter?.allowsMultipleValues,
      defaultValue: formFilter?.defaultValue,
-      inverseSelection: formFilter?.inverseSelection,
+      ...formFilter,
    });
    getChartDataRequest({
      formData,
@@ -108,8 +107,7 @@ export const useBackendFormUpdate = (
      if (
        filterToEdit?.filterType === formFilter?.filterType &&
        filterToEdit?.targets[0].datasetId === formFilter?.dataset?.value &&
-        formFilter?.column === filterToEdit?.targets[0]?.column?.name &&
-        filterToEdit?.allowsMultipleValues === formFilter?.allowsMultipleValues
+        formFilter?.column === filterToEdit?.targets[0]?.column?.name
      ) {
        resolvedDefaultValue = filterToEdit?.defaultValue;
      }
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/types.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/types.ts
@@ -27,7 +27,7 @@ export enum Scoping {
 // Using to pass setState React callbacks directly to And components
 export type AntCallback = (value1?: any, value2?: any) => void;

-interface NativeFiltersFormItem {
+export interface NativeFiltersFormItem {
  scope: Scope;
  name: string;
  filterType: FilterType;
@@ -36,15 +36,15 @@ interface NativeFiltersFormItem {
    label: string;
  };
  column: string;
+  controlValues: {
+    [key: string]: any;
+  };
  defaultValue: any;
  parentFilter: {
    value: string;
    label: string;
  };
-  inverseSelection: boolean;
  isInstant: boolean;
-  allowsMultipleValues: boolean;
-  isRequired: boolean;
 }

 export interface NativeFiltersForm {
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/utils.ts
@@ -17,6 +17,7 @@
 * under the License.
 */
 import { t } from '@superset-ui/core';
+import { flatMapDeep } from 'lodash';
 import { Charts, Layout, LayoutItem } from 'src/dashboard/types';
 import {
  CHART_TYPE,
@@ -26,6 +27,7 @@ import {
 import { FormInstance } from 'antd/lib/form';
 import React from 'react';
 import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
+import { CustomControlItem } from '@superset-ui/chart-controls';
 import { TreeItem } from './types';
 import { FilterType, Scope } from '../types';

@@ -176,5 +178,16 @@ export const setFilterFieldValues = (
  });
 };

+export const getControlItems = (
+  controlConfig: { [key: string]: any } = {},
+): CustomControlItem[] =>
+  (flatMapDeep(controlConfig.controlPanelSections)?.reduce(
+    (acc: any, { controlSetRows = [] }: any) => [
+      ...acc,
+      ...flatMapDeep(controlSetRows),
+    ],
+    [],
+  ) as CustomControlItem[]) ?? [];
+
 export const isScopingAll = (scope: Scope) =>
  !scope || (scope.rootPath[0] === DASHBOARD_ROOT_ID && !scope.excluded.length);
--- a/superset-frontend/src/dashboard/components/nativeFilters/types.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/types.ts
@@ -44,13 +44,10 @@ export interface Target {
 }

 export interface Filter {
-  allowsMultipleValues: boolean;
  cascadeParentIds: string[];
  defaultValue: any;
  currentValue?: any;
-  inverseSelection: boolean;
  isInstant: boolean;
-  isRequired: boolean;
  id: string; // randomly generated at filter creation
  name: string;
  scope: Scope;
@@ -58,6 +55,9 @@ export interface Filter {
  // for now there will only ever be one target
  // when multiple targets are supported, change this to Target[]
  targets: [Target];
+  controlValues: {
+    [key: string]: any;
+  };
 }

 export type FilterConfiguration = Filter[];
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
@@ -33,11 +33,10 @@ export const getFormData = ({
  datasetId = 18,
  cascadingFilters = {},
  groupby,
-  allowsMultipleValues = false,
-  defaultValue,
  currentValue,
-  inverseSelection,
  inputRef,
+  defaultValue,
+  controlValues,
 }: Partial<Filter> & {
  datasetId?: number;
  inputRef?: RefObject<HTMLInputElement>;
@@ -50,19 +49,17 @@ export const getFormData = ({
  extra_form_data: cascadingFilters,
  granularity_sqla: 'ds',
  groupby: [groupby],
-  inverseSelection,
  metrics: ['count'],
-  multiSelect: allowsMultipleValues,
  row_limit: 10000,
  showSearch: true,
  currentValue,
+  defaultValue,
  time_range: 'No filter',
  time_range_endpoints: ['inclusive', 'exclusive'],
  url_params: {},
  viz_type: 'filter_select',
-  // TODO: need process per filter type after will be decided approach
-  defaultValue,
  inputRef,
+  ...controlValues,
 });

 export function mergeExtraFormData(
@@ -86,9 +83,9 @@ export function mergeExtraFormData(
  appendKeys.forEach(key => {
    appendFormData[key] = [
      // @ts-ignore
-      ...(originalAppend[key] || []),
+      ...(originalAppend?.[key] || []),
      // @ts-ignore
-      ...(newAppend[key] || []),
+      ...(newAppend?.[key] || []),
    ];
  });

--- a/superset-frontend/src/filters/components/Range/controlPanel.ts
+++ b/superset-frontend/src/filters/components/Range/controlPanel.ts
@@ -16,29 +16,12 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import { t, validateNonEmpty } from '@superset-ui/core';
 import { ControlPanelConfig, sections } from '@superset-ui/chart-controls';

 const config: ControlPanelConfig = {
-  // For control input types, see: superset-frontend/src/explore/components/controls/index.js
-  controlPanelSections: [
-    // @ts-ignore
-    sections.legacyRegularTime,
-    {
-      label: t('Query'),
-      expanded: true,
-      controlSetRows: [['groupby'], ['adhoc_filters']],
-    },
-  ],
-  controlOverrides: {
-    groupby: {
-      validators: [validateNonEmpty],
-      clearable: false,
-    },
-    row_limit: {
-      default: 100,
-    },
-  },
+  // @ts-ignore
+  controlPanelSections: [sections.legacyRegularTime],
+  // TODO: here to add the relevant controls
 };

 export default config;
--- a/superset-frontend/src/filters/components/Select/AntdSelectFilter.tsx
+++ b/superset-frontend/src/filters/components/Select/AntdSelectFilter.tsx
@@ -19,7 +19,7 @@
 import { styled } from '@superset-ui/core';
 import React, { useEffect, useState } from 'react';
 import { Select } from 'src/common/components';
-import { DEFAULT_FORM_DATA, AntdPluginFilterSelectProps } from './types';
+import { AntdPluginFilterSelectProps } from './types';
 import { AntdPluginFilterStylesProps } from '../types';
 import { getSelectExtraFormData } from '../../utils';

@@ -42,10 +42,7 @@ export default function AntdPluginFilterSelect(
    currentValue,
    inverseSelection,
    inputRef,
-  } = {
-    ...DEFAULT_FORM_DATA,
-    ...formData,
-  };
+  } = formData;

  const [values, setValues] = useState<(string | number)[]>(defaultValue ?? []);

--- a/superset-frontend/src/filters/components/Select/controlPanel.ts
+++ b/superset-frontend/src/filters/components/Select/controlPanel.ts
@@ -20,13 +20,7 @@ import { t, validateNonEmpty } from '@superset-ui/core';
 import { ControlPanelConfig, sections } from '@superset-ui/chart-controls';
 import { DEFAULT_FORM_DATA } from './types';

-const {
-  enableEmptyFilter,
-  fetchPredicate,
-  inverseSelection,
-  multiSelect,
-  showSearch,
-} = DEFAULT_FORM_DATA;
+const { enableEmptyFilter, inverseSelection, multiSelect } = DEFAULT_FORM_DATA;

 const config: ControlPanelConfig = {
  controlPanelSections: [
@@ -35,10 +29,12 @@ const config: ControlPanelConfig = {
    {
      label: t('Query'),
      expanded: true,
+      controlSetRows: [['groupby']],
+    },
+    {
+      label: t('UI Configuration'),
+      expanded: true,
      controlSetRows: [
-        ['groupby'],
-        ['metrics'],
-        ['adhoc_filters'],
        [
          {
            name: 'multiSelect',
@@ -46,6 +42,8 @@ const config: ControlPanelConfig = {
              type: 'CheckboxControl',
              label: t('Multiple select'),
              default: multiSelect,
+              resetConfig: true,
+              renderTrigger: true,
              description: t('Allow selecting multiple values'),
            },
          },
@@ -57,6 +55,7 @@ const config: ControlPanelConfig = {
              type: 'CheckboxControl',
              label: t('Enable empty filter'),
              default: enableEmptyFilter,
+              renderTrigger: true,
              description: t(
                'When selection is empty, should an always false filter event be emitted',
              ),
@@ -68,38 +67,13 @@ const config: ControlPanelConfig = {
            name: 'inverseSelection',
            config: {
              type: 'CheckboxControl',
+              renderTrigger: true,
              label: t('Inverse selection'),
              default: inverseSelection,
              description: t('Exclude selected values'),
            },
          },
        ],
-        [
-          {
-            name: 'showSearch',
-            config: {
-              type: 'CheckboxControl',
-              label: t('Search field'),
-              default: showSearch,
-              description: t('Allow typing search terms'),
-            },
-          },
-        ],
-        [
-          {
-            name: 'fetchPredicate',
-            config: {
-              type: 'TextControl',
-              label: t('Fetch predicate'),
-              default: fetchPredicate,
-              description: t(
-                'Predicate applied when fetching distinct value to populate the filter control component.',
-              ),
-            },
-          },
-          null,
-        ],
-        ['row_limit', null],
      ],
    },
  ],
--- a/superset-frontend/src/filters/components/Select/types.ts
+++ b/superset-frontend/src/filters/components/Select/types.ts
@@ -31,7 +31,6 @@ interface AntdPluginFilterSelectCustomizeProps {
  fetchPredicate?: string;
  inverseSelection: boolean;
  multiSelect: boolean;
-  showSearch: boolean;
  inputRef?: RefObject<HTMLInputElement>;
 }

@@ -51,6 +50,5 @@ export const DEFAULT_FORM_DATA: AntdPluginFilterSelectCustomizeProps = {
  enableEmptyFilter: false,
  fetchPredicate: '',
  inverseSelection: false,
-  multiSelect: true,
-  showSearch: true,
+  multiSelect: false,
 };