From 465d986617233ab401c99f9eab0f55488e0dae09 Mon Sep 17 00:00:00 2001 From: simcha90 <56388545+simcha90@users.noreply.github.com> Date: Tue, 2 Feb 2021 11:28:12 +0200 Subject: [PATCH] feat(native-filters): Add defaultValue for Native filters modal (#12199) * refactor: sync Scoping tree with Forms data * refactor: update scoping tree * refactor: update scope tree logic to be more UX friendly * test: fix tests * lint: fix lin CR notes * chore: temp * fix: fix jsx * feat: Init value * refactor: move effect to utils * chore: add comments * feat: updates for default value in native filters * refactor: move multi values management to Modal * feat: added currentState to filterState fix: Reset all fixed * style: update filter styles * fix: process selection of same filter * fix: fix double choose select * fix: fix order of cascading filters * fix: fix CR comments * fix: fix CR comments --- .../spec/fixtures/mockNativeFilters.ts | 9 +- .../src/dashboard/actions/nativeFilters.ts | 5 + .../nativeFilters/CascadePopover.tsx | 51 ++--- .../components/nativeFilters/FilterBar.tsx | 184 +++++++++--------- .../nativeFilters/FilterConfigForm.tsx | 129 ++++++++++-- .../nativeFilters/FilterConfigModal.tsx | 4 +- .../components/nativeFilters/state.ts | 108 ++++++++-- .../components/nativeFilters/types.ts | 24 ++- .../components/nativeFilters/utils.ts | 66 ++++++- .../src/dashboard/reducers/nativeFilters.ts | 1 + .../components/Select/AntdSelectFilter.tsx | 54 +++-- .../src/filters/components/Select/types.ts | 6 +- superset-frontend/src/filters/utils.ts | 2 +- 13 files changed, 461 insertions(+), 182 deletions(-) diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index 9aa6e1010d067..24872d8a67a6c 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -16,14 +16,17 @@ * specific language governing permissions and limitations * under the License. */ -import { NativeFiltersState } from 'src/dashboard/components/nativeFilters/types'; +import { + FilterType, + NativeFiltersState, +} from 'src/dashboard/components/nativeFilters/types'; export const nativeFilters: NativeFiltersState = { filters: { 'NATIVE_FILTER-e7Q8zKixx': { id: 'NATIVE_FILTER-e7Q8zKixx', name: 'region', - type: 'text', + filterType: FilterType.filter_select, targets: [ { datasetId: 2, @@ -46,7 +49,7 @@ export const nativeFilters: NativeFiltersState = { 'NATIVE_FILTER-x9QPw0so1': { id: 'NATIVE_FILTER-x9QPw0so1', name: 'country_code', - type: 'text', + filterType: FilterType.filter_select, targets: [ { datasetId: 2, diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts index 2bafb73abe98a..a327ebaea6e57 100644 --- a/superset-frontend/src/dashboard/actions/nativeFilters.ts +++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts @@ -20,6 +20,7 @@ import { ExtraFormData, makeApi } from '@superset-ui/core'; import { Dispatch } from 'redux'; import { + CurrentFilterState, Filter, FilterConfiguration, SelectedValues, @@ -99,6 +100,7 @@ export interface SetExtraFormData { type: typeof SET_EXTRA_FORM_DATA; filterId: string; extraFormData: ExtraFormData; + currentState: CurrentFilterState; } export function setFilterState( @@ -117,15 +119,18 @@ export function setFilterState( * Sets the selected option(s) for a given filter * @param filterId the id of the native filter * @param extraFormData the selection translated into extra form data + * @param currentState */ export function setExtraFormData( filterId: string, extraFormData: ExtraFormData, + currentState: CurrentFilterState, ): SetExtraFormData { return { type: SET_EXTRA_FORM_DATA, filterId, extraFormData, + currentState, }; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/CascadePopover.tsx b/superset-frontend/src/dashboard/components/nativeFilters/CascadePopover.tsx index e12c93348dcd5..b131ba6ad7d2c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/CascadePopover.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/CascadePopover.tsx @@ -22,14 +22,19 @@ import Popover from 'src/common/components/Popover'; import Icon from 'src/components/Icon'; import { Pill } from 'src/dashboard/components/FiltersBadge/Styles'; import { CascadeFilterControl, FilterControl } from './FilterBar'; -import { Filter, CascadeFilter } from './types'; +import { Filter, CascadeFilter, CurrentFilterState } from './types'; +import { useFilterState } from './state'; interface CascadePopoverProps { filter: CascadeFilter; visible: boolean; directPathToChild?: string[]; onVisibleChange: (visible: boolean) => void; - onExtraFormDataChange: (filter: Filter, extraFormData: ExtraFormData) => void; + onFilterSelectionChange: ( + filter: Filter, + extraFormData: ExtraFormData, + currentState: CurrentFilterState, + ) => void; } const StyledTitleBox = styled.div` @@ -73,10 +78,11 @@ const CascadePopover: React.FC = ({ filter, visible, onVisibleChange, - onExtraFormDataChange, + onFilterSelectionChange, directPathToChild, }) => { const [currentPathToChild, setCurrentPathToChild] = useState(); + const filterState = useFilterState(filter.id); useEffect(() => { setCurrentPathToChild(directPathToChild); @@ -86,26 +92,27 @@ const CascadePopover: React.FC = ({ return () => clearTimeout(timeout); }, [directPathToChild, setCurrentPathToChild]); - const getActiveChildren = useCallback((filter: CascadeFilter): - | CascadeFilter[] - | null => { - const children = filter.cascadeChildren || []; - const currentValue = filter.currentValue || []; + const getActiveChildren = useCallback( + (filter: CascadeFilter): CascadeFilter[] | null => { + const children = filter.cascadeChildren || []; + const currentValue = filterState.currentState?.value; - const activeChildren = children.flatMap( - childFilter => getActiveChildren(childFilter) || [], - ); + const activeChildren = children.flatMap( + childFilter => getActiveChildren(childFilter) || [], + ); - if (activeChildren.length > 0) { - return activeChildren; - } + if (activeChildren.length > 0) { + return activeChildren; + } - if (currentValue.length > 0) { - return [filter]; - } + if (currentValue) { + return [filter]; + } - return null; - }, []); + return null; + }, + [filterState], + ); const getAllFilters = (filter: CascadeFilter): CascadeFilter[] => { const children = filter.cascadeChildren || []; @@ -139,7 +146,7 @@ const CascadePopover: React.FC = ({ ); } @@ -160,7 +167,7 @@ const CascadePopover: React.FC = ({ key={filter.id} filter={filter} directPathToChild={visible ? currentPathToChild : undefined} - onExtraFormDataChange={onExtraFormDataChange} + onFilterSelectionChange={onFilterSelectionChange} /> ); @@ -180,7 +187,7 @@ const CascadePopover: React.FC = ({ diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx index 617928ce8c00b..e7ad83f16b79d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar.tsx @@ -23,16 +23,9 @@ import { t, ExtraFormData, } from '@superset-ui/core'; -import React, { - useState, - useEffect, - useMemo, - useCallback, - useRef, -} from 'react'; +import React, { useState, useEffect, useMemo, useRef } from 'react'; import { useSelector } from 'react-redux'; import cx from 'classnames'; -import { Form } from 'src/common/components'; import Button from 'src/components/Button'; import Icon from 'src/components/Icon'; import { getChartDataRequest } from 'src/chart/chartAction'; @@ -40,15 +33,19 @@ import { areObjectsEqual } from 'src/reduxUtils'; import Loading from 'src/components/Loading'; import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert'; import FilterConfigurationLink from './FilterConfigurationLink'; -// import FilterScopeModal from 'src/dashboard/components/filterscope/FilterScopeModal'; - import { useCascadingFilters, useFilterConfiguration, + useFilters, + useFilterState, useSetExtraFormData, } from './state'; -import { Filter, CascadeFilter } from './types'; -import { buildCascadeFiltersTree, mapParentFiltersToChildren } from './utils'; +import { Filter, CascadeFilter, CurrentFilterState } from './types'; +import { + buildCascadeFiltersTree, + getFormData, + mapParentFiltersToChildren, +} from './utils'; import CascadePopover from './CascadePopover'; const barWidth = `250px`; @@ -60,6 +57,10 @@ const BarWrapper = styled.div` } `; +const FilterItem = styled.div` + padding-bottom: 10px; +`; + const Bar = styled.div` position: absolute; top: 0; @@ -201,7 +202,11 @@ interface FilterProps { filter: Filter; icon?: React.ReactElement; directPathToChild?: string[]; - onExtraFormDataChange: (filter: Filter, extraFormData: ExtraFormData) => void; + onFilterSelectionChange: ( + filter: Filter, + extraFormData: ExtraFormData, + currentState: CurrentFilterState, + ) => void; } interface FiltersBarProps { @@ -213,17 +218,18 @@ interface FiltersBarProps { const FilterValue: React.FC = ({ filter, directPathToChild, - onExtraFormDataChange, + onFilterSelectionChange, }) => { const { id, allowsMultipleValues, inverseSelection, targets, - currentValue, defaultValue, + filterType, } = filter; const cascadingFilters = useCascadingFilters(id); + const filterState = useFilterState(id); const [loading, setLoading] = useState(true); const [state, setState] = useState([]); const [error, setError] = useState(false); @@ -232,29 +238,17 @@ const FilterValue: React.FC = ({ const [target] = targets; const { datasetId = 18, column } = target; const { name: groupby } = column; - - const getFormData = (): Partial => ({ - adhoc_filters: [], - datasource: `${datasetId}__table`, - extra_filters: [], - extra_form_data: cascadingFilters, - granularity_sqla: 'ds', - groupby: [groupby], - inverseSelection, - metrics: ['count'], - multiSelect: allowsMultipleValues, - row_limit: 10000, - showSearch: true, - time_range: 'No filter', - time_range_endpoints: ['inclusive', 'exclusive'], - url_params: {}, - viz_type: 'filter_select', - defaultValues: currentValue || defaultValue || [], - inputRef, - }); - + const currentValue = filterState.currentState?.value; useEffect(() => { - const newFormData = getFormData(); + const newFormData = getFormData({ + datasetId, + cascadingFilters, + groupby, + allowsMultipleValues, + defaultValue, + currentValue, + inverseSelection, + }); if (!areObjectsEqual(formData || {}, newFormData)) { setFormData(newFormData); getChartDataRequest({ @@ -272,7 +266,7 @@ const FilterValue: React.FC = ({ setLoading(false); }); } - }, [cascadingFilters, datasetId, groupby]); + }, [cascadingFilters, datasetId, groupby, defaultValue, currentValue]); useEffect(() => { if (directPathToChild?.[0] === filter.id) { @@ -285,8 +279,13 @@ const FilterValue: React.FC = ({ return undefined; }, [inputRef, directPathToChild, filter.id]); - const setExtraFormData = (extraFormData: ExtraFormData) => - onExtraFormDataChange(filter, extraFormData); + const setExtraFormData = ({ + extraFormData, + currentState, + }: { + extraFormData: ExtraFormData; + currentState: CurrentFilterState; + }) => onFilterSelectionChange(filter, extraFormData, currentState); if (loading) { return ( @@ -307,29 +306,24 @@ const FilterValue: React.FC = ({ } return ( -
{ - setExtraFormData(values.value); - }} - > - - - -
+ + + ); }; export const FilterControl: React.FC = ({ filter, icon, - onExtraFormDataChange, + onFilterSelectionChange, directPathToChild, }) => { const { name = '' } = filter; @@ -342,7 +336,7 @@ export const FilterControl: React.FC = ({ ); @@ -351,13 +345,17 @@ export const FilterControl: React.FC = ({ interface CascadeFilterControlProps { filter: CascadeFilter; directPathToChild?: string[]; - onExtraFormDataChange: (filter: Filter, extraFormData: ExtraFormData) => void; + onFilterSelectionChange: ( + filter: Filter, + extraFormData: ExtraFormData, + currentState: CurrentFilterState, + ) => void; } export const CascadeFilterControl: React.FC = ({ filter, directPathToChild, - onExtraFormDataChange, + onFilterSelectionChange, }) => ( <> @@ -365,7 +363,7 @@ export const CascadeFilterControl: React.FC = ({ @@ -375,7 +373,7 @@ export const CascadeFilterControl: React.FC = ({ ))} @@ -388,11 +386,15 @@ const FilterBar: React.FC = ({ toggleFiltersBar, directPathToChild, }) => { - const [filterData, setFilterData] = useState<{ [id: string]: ExtraFormData }>( - {}, - ); + const [filterData, setFilterData] = useState<{ + [id: string]: { + extraFormData: ExtraFormData; + currentState: CurrentFilterState; + }; + }>({}); const setExtraFormData = useSetExtraFormData(); const filterConfigs = useFilterConfiguration(); + const filters = useFilters(); const canEdit = useSelector( ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, ); @@ -404,25 +406,6 @@ const FilterBar: React.FC = ({ } }, [filterConfigs]); - const getFilterValue = useCallback( - (filter: Filter): (string | number | boolean)[] | null => { - const filters = filterData[filter.id]?.append_form_data?.filters; - if (filters?.length) { - const filter = filters[0]; - if ('val' in filter) { - // need to nest these if statements to get a reference to val to appease TS - const { val } = filter; - if (Array.isArray(val)) { - return val; - } - return [val]; - } - } - return null; - }, - [filterData], - ); - const cascadeChildren = useMemo( () => mapParentFiltersToChildren(filterConfigs), [filterConfigs], @@ -431,24 +414,28 @@ const FilterBar: React.FC = ({ const cascadeFilters = useMemo(() => { const filtersWithValue = filterConfigs.map(filter => ({ ...filter, - currentValue: getFilterValue(filter), + currentValue: filterData[filter.id]?.currentState?.value, })); return buildCascadeFiltersTree(filtersWithValue); - }, [filterConfigs, getFilterValue]); + }, [filterConfigs]); - const handleExtraFormDataChange = ( + const handleFilterSelectionChange = ( filter: Filter, extraFormData: ExtraFormData, + currentState: CurrentFilterState, ) => { setFilterData(prevFilterData => ({ ...prevFilterData, - [filter.id]: extraFormData, + [filter.id]: { + extraFormData, + currentState, + }, })); const children = cascadeChildren[filter.id] || []; // force instant updating for parent filters if (filter.isInstant || children.length > 0) { - setExtraFormData(filter.id, extraFormData); + setExtraFormData(filter.id, extraFormData, currentState); } }; @@ -456,18 +443,21 @@ const FilterBar: React.FC = ({ const filterIds = Object.keys(filterData); filterIds.forEach(filterId => { if (filterData[filterId]) { - setExtraFormData(filterId, filterData[filterId]); + setExtraFormData( + filterId, + filterData[filterId]?.extraFormData, + filterData[filterId]?.currentState, + ); } }); }; const handleResetAll = () => { - setFilterData({}); - const filterIds = Object.keys(filterData); - filterIds.forEach(filterId => { - if (filterData[filterId]) { - setExtraFormData(filterId, {}); - } + filterConfigs.forEach(filter => { + setExtraFormData(filter.id, filterData[filter.id]?.extraFormData, { + ...filterData[filter.id]?.currentState, + value: filters[filter.id]?.defaultValue, + }); }); }; @@ -521,7 +511,7 @@ const FilterBar: React.FC = ({ setVisiblePopoverId(visible ? filter.id : null) } filter={filter} - onExtraFormDataChange={handleExtraFormDataChange} + onFilterSelectionChange={handleFilterSelectionChange} directPathToChild={directPathToChild} /> ))} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx index 94892090ef954..82269d03b6722 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx @@ -16,9 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { styled, t } from '@superset-ui/core'; +import { styled, SuperChart, t } from '@superset-ui/core'; import { FormInstance } from 'antd/lib/form'; -import React, { useCallback, useState } from 'react'; +import React, { useCallback } from 'react'; import { Button, Checkbox, @@ -27,14 +27,19 @@ import { Typography, } from 'src/common/components'; import { Select } from 'src/components/Select/SupersetStyledSelect'; -import SupersetResourceSelect, { - Value, -} from 'src/components/SupersetResourceSelect'; +import SupersetResourceSelect from 'src/components/SupersetResourceSelect'; import { addDangerToast } from 'src/messageToasts/actions'; import { ClientErrorObject } from 'src/utils/getClientErrorObject'; import { ColumnSelect } from './ColumnSelect'; -import { Filter, NativeFiltersForm } from './types'; +import { Filter, FilterType, NativeFiltersForm } from './types'; import FilterScope from './FilterScope'; +import { + FilterTypeNames, + getFormData, + setFilterFieldValues, + useForceUpdate, +} from './utils'; +import { useBackendFormUpdate } from './state'; type DatasetSelectValue = { value: number; @@ -77,6 +82,10 @@ const StyledLabel = styled.span` text-transform: uppercase; `; +const CleanFormItem = styled(Form.Item)` + margin-bottom: 0; +`; + export interface FilterConfigFormProps { filterId: string; filterToEdit?: Filter; @@ -98,11 +107,19 @@ export const FilterConfigForm: React.FC = ({ form, parentFilters, }) => { - const [dataset, setDataset] = useState | undefined>( - filterToEdit?.targets[0].datasetId - ? { label: '', value: filterToEdit?.targets[0].datasetId } - : undefined, - ); + const forceUpdate = useForceUpdate(); + const formFilter = (form.getFieldValue('filters') || {})[filterId]; + useBackendFormUpdate(form, filterId, filterToEdit); + + const initDatasetId = filterToEdit?.targets[0].datasetId; + const initColumn = filterToEdit?.targets[0]?.column?.name; + const newFormData = getFormData({ + datasetId: formFilter?.dataset?.value, + groupby: formFilter?.column, + allowsMultipleValues: formFilter?.allowsMultipleValues, + defaultValue: formFilter?.defaultValue, + inverseSelection: formFilter?.inverseSelection, + }); const onDatasetSelectError = useCallback( ({ error, message }: ClientErrorObject) => { @@ -146,21 +163,30 @@ export const FilterConfigForm: React.FC = ({ > - {t('Datasource')}} rules={[{ required: !removed, message: t('Datasource is required') }]} data-test="datasource-input" > { + // We need reset column when dataset changed + const datasetId = formFilter?.dataset?.value; + if (datasetId && e?.value !== datasetId) { + setFilterFieldValues(form, filterId, { + column: null, + }); + } + forceUpdate(); + }} /> @@ -168,7 +194,7 @@ export const FilterConfigForm: React.FC = ({ // don't show the column select unless we have a dataset // style={{ display: datasetId == null ? undefined : 'none' }} name={['filters', filterId, 'column']} - initialValue={filterToEdit?.targets[0]?.column?.name} + initialValue={initColumn} label={{t('Field')}} rules={[{ required: !removed, message: t('Field is required') }]} data-test="field-input" @@ -176,9 +202,68 @@ export const FilterConfigForm: React.FC = ({ + {t('Filter Type')}} + > +