You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2021/02/14 08:39:08 UTC
[superset] branch master updated: refactor(native-filters):
decouple params from filter config modal (first phase) (#13021)
This is an automated email from the ASF dual-hosted git repository.
villebro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 2010e64 refactor(native-filters): decouple params from filter config modal (first phase) (#13021)
2010e64 is described below
commit 2010e64c9456f7c2c9f20d227213f7ceffd0d914
Author: simcha90 <56...@users.noreply.github.com>
AuthorDate: Sun Feb 14 10:38:15 2021 +0200
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 <am...@nielsen.com>
---
.../spec/fixtures/mockNativeFilters.ts | 16 +++--
.../nativeFilters/FilterBar/FilterValue.tsx | 15 +---
.../FilterConfigModal/FilterConfigForm.tsx | 82 ++++++++++++----------
.../FilterConfigModal/FilterConfigModal.tsx | 8 +--
.../nativeFilters/FilterConfigModal/state.ts | 6 +-
.../nativeFilters/FilterConfigModal/types.ts | 8 +--
.../nativeFilters/FilterConfigModal/utils.ts | 13 ++++
.../dashboard/components/nativeFilters/types.ts | 6 +-
.../dashboard/components/nativeFilters/utils.ts | 15 ++--
.../src/filters/components/Range/controlPanel.ts | 23 +-----
.../filters/components/Select/AntdSelectFilter.tsx | 7 +-
.../src/filters/components/Select/controlPanel.ts | 46 +++---------
.../src/filters/components/Select/types.ts | 4 +-
13 files changed, 105 insertions(+), 144 deletions(-)
diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts
index 6765019..42404fc 100644
--- 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: {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterValue.tsx
index 5104ec1..d0e2024 100644
--- 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) {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigForm.tsx
index 7848bba..44e0683 100644
--- 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}
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/FilterConfigModal.tsx
index ea08d62..18a6026 100644
--- 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,
};
});
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/state.ts
index d17a587..ef6b291 100644
--- 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;
}
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/types.ts
index a18dcfa..aa9c531 100644
--- 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 {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal/utils.ts
index 06cec67..000779d 100644
--- 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);
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/types.ts
index 12e8820..c8ec615 100644
--- 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[];
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
index d4417b2..ea8cb1a 100644
--- 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] || []),
];
});
diff --git a/superset-frontend/src/filters/components/Range/controlPanel.ts b/superset-frontend/src/filters/components/Range/controlPanel.ts
index bfc1c09..e7f31e3 100644
--- 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;
diff --git a/superset-frontend/src/filters/components/Select/AntdSelectFilter.tsx b/superset-frontend/src/filters/components/Select/AntdSelectFilter.tsx
index 01d6f2d..ccac766 100644
--- 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 ?? []);
diff --git a/superset-frontend/src/filters/components/Select/controlPanel.ts b/superset-frontend/src/filters/components/Select/controlPanel.ts
index 22ef1b2..d5de783 100644
--- 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],
],
},
],
diff --git a/superset-frontend/src/filters/components/Select/types.ts b/superset-frontend/src/filters/components/Select/types.ts
index 5720978..476c436 100644
--- 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,
};