From 2ff874124840561277b1c480803fda55d11fb0c5 Mon Sep 17 00:00:00 2001 From: simcha90 <56388545+simcha90@users.noreply.github.com> Date: Thu, 18 Feb 2021 15:43:10 +0200 Subject: [PATCH] feat(filter-sets): Saving filter sets in metadata (#13205) * feat: POC adding filters set feature * lint: fix TS * fix: fix FF name * refactor: fix CR notes * fix: fix update values in filter bar * refactor: save filter sets in meta * feat(filter-sets): save filters sets in metadata --- .../spec/fixtures/mockNativeFilters.ts | 1 + .../util/getFormDataWithExtraFilters_spec.ts | 1 + .../src/dashboard/actions/nativeFilters.ts | 64 ++++++++++- .../nativeFilters/FilterBar/FilterBar.tsx | 108 ++++++++++++++---- .../nativeFilters/FilterBar/FilterValue.tsx | 13 ++- .../src/dashboard/reducers/getInitialState.js | 7 +- .../src/dashboard/reducers/nativeFilters.ts | 64 ++++++++--- .../src/dashboard/reducers/types.ts | 6 +- .../components/Range/RangeFilterPlugin.tsx | 4 +- superset/dashboards/schemas.py | 2 + 10 files changed, 215 insertions(+), 55 deletions(-) diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index f0c3a32efef..207a9394b76 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -93,6 +93,7 @@ export const nativeFilters: NativeFiltersState = { 'NATIVE_FILTER-x9QPw0so1': { id: 'NATIVE_FILTER-x9QPw0so1', extraFormData: {}, + currentState: {}, }, }, }; diff --git a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts index fd8ae3577f8..c5db75ff834 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts +++ b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts @@ -65,6 +65,7 @@ describe('getFormDataWithExtraFilters', () => { [filterId]: { id: filterId, extraFormData: {}, + currentState: {}, }, }, }, diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts index 29227b0cf66..6b9c4971cc3 100644 --- a/superset-frontend/src/dashboard/actions/nativeFilters.ts +++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts @@ -24,7 +24,11 @@ import { FilterConfiguration, } from 'src/dashboard/components/nativeFilters/types'; import { dashboardInfoChanged } from './dashboardInfo'; -import { CurrentFilterState, NativeFilterState } from '../reducers/types'; +import { + CurrentFilterState, + FiltersSet, + NativeFilterState, +} from '../reducers/types'; import { SelectedValues } from '../components/nativeFilters/FilterConfigModal/types'; export const SET_FILTER_CONFIG_BEGIN = 'SET_FILTER_CONFIG_BEGIN'; @@ -42,6 +46,22 @@ export interface SetFilterConfigFail { type: typeof SET_FILTER_CONFIG_FAIL; filterConfig: FilterConfiguration; } +export const SET_FILTER_SETS_CONFIG_BEGIN = 'SET_FILTER_SETS_CONFIG_BEGIN'; +export interface SetFilterSetsConfigBegin { + type: typeof SET_FILTER_SETS_CONFIG_BEGIN; + filterSetsConfig: FiltersSet[]; +} +export const SET_FILTER_SETS_CONFIG_COMPLETE = + 'SET_FILTER_SETS_CONFIG_COMPLETE'; +export interface SetFilterSetsConfigComplete { + type: typeof SET_FILTER_SETS_CONFIG_COMPLETE; + filterSetsConfig: FiltersSet[]; +} +export const SET_FILTER_SETS_CONFIG_FAIL = 'SET_FILTER_SETS_CONFIG_FAIL'; +export interface SetFilterSetsConfigFail { + type: typeof SET_FILTER_SETS_CONFIG_FAIL; + filterSetsConfig: FiltersSet[]; +} export const SET_FILTER_STATE = 'SET_FILTER_STATE'; export interface SetFilterState { @@ -95,6 +115,45 @@ export const setFilterConfiguration = ( } }; +export const setFilterSetsConfiguration = ( + filterSetsConfig: FiltersSet[], +) => async (dispatch: Dispatch, getState: () => any) => { + dispatch({ + type: SET_FILTER_SETS_CONFIG_BEGIN, + filterSetsConfig, + }); + const { id, metadata } = getState().dashboardInfo; + + // TODO extract this out when makeApi supports url parameters + const updateDashboard = makeApi< + Partial, + { result: DashboardInfo } + >({ + method: 'PUT', + endpoint: `/api/v1/dashboard/${id}`, + }); + + try { + const response = await updateDashboard({ + json_metadata: JSON.stringify({ + ...metadata, + filter_sets_configuration: filterSetsConfig, + }), + }); + dispatch( + dashboardInfoChanged({ + metadata: JSON.parse(response.result.json_metadata), + }), + ); + dispatch({ + type: SET_FILTER_SETS_CONFIG_COMPLETE, + filterSetsConfig, + }); + } catch (err) { + dispatch({ type: SET_FILTER_SETS_CONFIG_FAIL, filterSetsConfig }); + } +}; + export const SET_EXTRA_FORM_DATA = 'SET_EXTRA_FORM_DATA'; export interface SetExtraFormData { type: typeof SET_EXTRA_FORM_DATA; @@ -174,6 +233,9 @@ export type AnyFilterAction = | SetFilterConfigBegin | SetFilterConfigComplete | SetFilterConfigFail + | SetFilterSetsConfigBegin + | SetFilterSetsConfigComplete + | SetFilterSetsConfigFail | SetFiltersState | SetExtraFormData | SaveFilterSets diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx index 97837e9536b..f5db6c6c01b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx @@ -22,14 +22,14 @@ import { useDispatch, useSelector } from 'react-redux'; import cx from 'classnames'; import Button from 'src/components/Button'; import Icon from 'src/components/Icon'; -import { CurrentFilterState } from 'src/dashboard/reducers/types'; +import { + CurrentFilterState, + FiltersSet, + NativeFilterState, +} from 'src/dashboard/reducers/types'; import { Input, Select } from 'src/common/components'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; -import { - saveFilterSets, - setFiltersState, -} from 'src/dashboard/actions/nativeFilters'; -import { SelectValue } from 'antd/lib/select'; +import { setFilterSetsConfiguration } from 'src/dashboard/actions/nativeFilters'; import FilterConfigurationLink from './FilterConfigurationLink'; import { useFilters, @@ -193,12 +193,33 @@ const FilterBar: React.FC = ({ const filtersState = useFiltersState(); const filterSets = useFilterSets(); const filterConfigs = useFilterConfiguration(); + const filterSetsConfigs = useSelector( + state => state.dashboardInfo?.metadata?.filter_sets_configuration || [], + ); const filters = useFilters(); const [filtersSetName, setFiltersSetName] = useState(''); + const [selectedFiltersSetId, setSelectedFiltersSetId] = useState< + string | null + >(null); const canEdit = useSelector( ({ dashboardInfo }) => dashboardInfo.dash_edit_perm, ); const [visiblePopoverId, setVisiblePopoverId] = useState(null); + const [isInitialized, setIsInitialized] = useState(false); + + useEffect(() => { + if (isInitialized) { + return; + } + const areFiltersInitialized = filterConfigs.every( + filterConfig => + filterConfig.defaultValue === + filterData[filterConfig.id]?.currentState?.value, + ); + if (areFiltersInitialized) { + setIsInitialized(true); + } + }, [filterConfigs, filterData, isInitialized]); useEffect(() => { if (filterConfigs.length === 0 && filtersOpen) { @@ -217,18 +238,20 @@ const FilterBar: React.FC = ({ currentValue: filterData[filter.id]?.currentState?.value, })); return buildCascadeFiltersTree(filtersWithValue); - }, [filterConfigs]); + }, [filterConfigs, filterData]); const handleFilterSelectionChange = ( - filter: Filter, + filter: Pick & Partial, extraFormData: ExtraFormData, currentState: CurrentFilterState, ) => { - let isInitialized = false; setFilterData(prevFilterData => { - if (filter.id in prevFilterData) { - isInitialized = true; + const children = cascadeChildren[filter.id] || []; + // force instant updating on initialization or for parent filters + if (filter.isInstant || children.length > 0) { + setExtraFormData(filter.id, extraFormData, currentState); } + return { ...prevFilterData, [filter.id]: { @@ -237,12 +260,22 @@ const FilterBar: React.FC = ({ }, }; }); + }; - const children = cascadeChildren[filter.id] || []; - // force instant updating on initialization or for parent filters - if (!isInitialized || filter.isInstant || children.length > 0) { - setExtraFormData(filter.id, extraFormData, currentState); + const takeFiltersSet = (value: string) => { + setSelectedFiltersSetId(value); + if (!value) { + return; } + const filtersSet = filterSets[value]; + Object.values(filtersSet.filtersState).forEach(filterState => { + const { + extraFormData, + currentState, + id, + } = filterState as NativeFilterState; + handleFilterSelectionChange({ id }, extraFormData, currentState); + }); }; const handleApply = () => { @@ -258,17 +291,40 @@ const FilterBar: React.FC = ({ }); }; + useEffect(() => { + if (isInitialized) { + handleApply(); + } + }, [isInitialized]); + const handleSaveFilterSets = () => { dispatch( - saveFilterSets( - filtersSetName.trim(), - generateFiltersSetId(), - filtersState, + setFilterSetsConfiguration( + filterSetsConfigs.concat([ + { + name: filtersSetName.trim(), + id: generateFiltersSetId(), + // TODO: After merge https://github.com/apache/superset/pull/13137, compare if data changed (meantime save only clicking `apply`) + filtersState, + }, + ]), ), ); setFiltersSetName(''); }; + const handleDeleteFilterSets = () => { + dispatch( + setFilterSetsConfiguration( + filterSetsConfigs.filter( + filtersSet => filtersSet.id !== selectedFiltersSetId, + ), + ), + ); + setFiltersSetName(''); + setSelectedFiltersSetId(null); + }; + const handleResetAll = () => { filterConfigs.forEach(filter => { setExtraFormData(filter.id, filterData[filter.id]?.extraFormData, { @@ -278,10 +334,6 @@ const FilterBar: React.FC = ({ }); }; - const takeFiltersSet = (value: SelectValue) => { - dispatch(setFiltersState(filterSets[String(value)]?.filtersState)); - }; - return ( = ({ +
{t('Name')}
= ({ }) => { const { id, targets, filterType } = filter; const cascadingFilters = useCascadingFilters(id); - const filterState = useFilterState(id); const [state, setState] = useState([]); const [error, setError] = useState(false); const [formData, setFormData] = useState>({}); @@ -61,7 +60,6 @@ const FilterValue: React.FC = ({ column = {}, }: Partial<{ datasetId: number; column: { name?: string } }> = target; const { name: groupby } = column; - const currentValue = filterState.currentState?.value; const hasDataSource = !!(datasetId && groupby); const [loading, setLoading] = useState(hasDataSource); useEffect(() => { @@ -70,7 +68,6 @@ const FilterValue: React.FC = ({ datasetId, cascadingFilters, groupby, - currentValue, inputRef, }); if (!areObjectsEqual(formData || {}, newFormData)) { @@ -93,7 +90,13 @@ const FilterValue: React.FC = ({ setLoading(false); }); } - }, [cascadingFilters, datasetId, groupby, filter.defaultValue, currentValue]); + }, [ + cascadingFilters, + datasetId, + groupby, + JSON.stringify(filter), + hasDataSource, + ]); useEffect(() => { if (directPathToChild?.[0] === filter.id) { diff --git a/superset-frontend/src/dashboard/reducers/getInitialState.js b/superset-frontend/src/dashboard/reducers/getInitialState.js index e7864dcd983..c44fde25450 100644 --- a/superset-frontend/src/dashboard/reducers/getInitialState.js +++ b/superset-frontend/src/dashboard/reducers/getInitialState.js @@ -258,9 +258,10 @@ export default function getInitialState(bootstrapData) { directPathToChild.push(directLinkComponentId); } - const nativeFilters = getInitialNativeFilterState( - dashboard.metadata.filter_configuration || [], - ); + const nativeFilters = getInitialNativeFilterState({ + filterConfig: dashboard.metadata.filter_configuration || [], + filterSetsConfig: dashboard.metadata.filter_sets_configuration || [], + }); return { datasources, diff --git a/superset-frontend/src/dashboard/reducers/nativeFilters.ts b/superset-frontend/src/dashboard/reducers/nativeFilters.ts index 66b33f2f24c..2aaa8807b90 100644 --- a/superset-frontend/src/dashboard/reducers/nativeFilters.ts +++ b/superset-frontend/src/dashboard/reducers/nativeFilters.ts @@ -21,36 +21,58 @@ import { SAVE_FILTER_SETS, SET_EXTRA_FORM_DATA, SET_FILTER_CONFIG_COMPLETE, + SET_FILTER_SETS_CONFIG_COMPLETE, SET_FILTERS_STATE, } from 'src/dashboard/actions/nativeFilters'; -import { NativeFiltersState, NativeFilterState } from './types'; +import { FiltersSet, NativeFiltersState, NativeFilterState } from './types'; import { FilterConfiguration } from '../components/nativeFilters/types'; export function getInitialFilterState(id: string): NativeFilterState { return { id, extraFormData: {}, + currentState: {}, }; } -export function getInitialState( - filterConfig: FilterConfiguration, - prevState: NativeFiltersState, -): NativeFiltersState { +export function getInitialState({ + filterSetsConfig, + filterConfig, + state: prevState, +}: { + filterSetsConfig?: FiltersSet[]; + filterConfig?: FilterConfiguration; + state?: NativeFiltersState; +}): NativeFiltersState { + const state: Partial = {}; + const filters = {}; const filtersState = {}; - const state = { - filters, - filtersState, - filterSets: prevState?.filterSets ?? {}, - }; - filterConfig.forEach(filter => { - const { id } = filter; - filters[id] = filter; - filtersState[id] = - prevState?.filtersState?.[id] || getInitialFilterState(id); - }); - return state; + if (filterConfig) { + filterConfig.forEach(filter => { + const { id } = filter; + filters[id] = filter; + filtersState[id] = + prevState?.filtersState?.[id] || getInitialFilterState(id); + }); + state.filters = filters; + state.filtersState = filtersState; + } else { + state.filters = prevState?.filters ?? {}; + state.filtersState = prevState?.filtersState ?? {}; + } + + if (filterSetsConfig) { + const filterSets = {}; + filterSetsConfig.forEach(filtersSet => { + const { id } = filtersSet; + filterSets[id] = filtersSet; + }); + state.filterSets = filterSets; + } else { + state.filterSets = prevState?.filterSets ?? {}; + } + return state as NativeFiltersState; } export default function nativeFilterReducer( @@ -94,7 +116,13 @@ export default function nativeFilterReducer( }; case SET_FILTER_CONFIG_COMPLETE: - return getInitialState(action.filterConfig, state); + return getInitialState({ filterConfig: action.filterConfig, state }); + + case SET_FILTER_SETS_CONFIG_COMPLETE: + return getInitialState({ + filterSetsConfig: action.filterSetsConfig, + state, + }); // TODO handle SET_FILTER_CONFIG_FAIL action default: diff --git a/superset-frontend/src/dashboard/reducers/types.ts b/superset-frontend/src/dashboard/reducers/types.ts index 46749d4a769..493bbc12ee3 100644 --- a/superset-frontend/src/dashboard/reducers/types.ts +++ b/superset-frontend/src/dashboard/reducers/types.ts @@ -47,7 +47,7 @@ export type Layout = { [key: string]: LayoutItem }; /** State of nativeFilters currentState */ export type CurrentFilterState = JsonObject & { - value: any; + value?: any; }; /** State of charts in redux */ @@ -75,8 +75,8 @@ export type LayoutItem = { /** Current state of the filter, stored in `nativeFilters` in redux */ export type NativeFilterState = { id: string; // ties this filter state to the config object - extraFormData?: ExtraFormData; - currentState?: CurrentFilterState; + extraFormData: ExtraFormData; + currentState: CurrentFilterState; }; export type FiltersSet = { diff --git a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx index 9320edc225c..fe007d538d7 100644 --- a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx @@ -55,11 +55,11 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) { }; useEffect(() => { - handleChange(currentValue ?? [min, max]); + handleAfterChange(currentValue ?? [min, max]); }, [JSON.stringify(currentValue)]); useEffect(() => { - handleChange(defaultValue ?? [min, max]); + handleAfterChange(defaultValue ?? [min, max]); // I think after Config Modal update some filter it re-creates default value for all other filters // so we can process it like this `JSON.stringify` or start to use `Immer` }, [JSON.stringify(defaultValue)]); diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 17e46d0354f..360f94f74d9 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -106,6 +106,8 @@ def validate_json_metadata(value: Union[bytes, bytearray, str]) -> None: class DashboardJSONMetadataSchema(Schema): # filter_configuration is for dashboard-native filters filter_configuration = fields.List(fields.Dict(), allow_none=True) + # filter_sets_configuration is for dashboard-native filters + filter_sets_configuration = fields.List(fields.Dict(), allow_none=True) timed_refresh_immune_slices = fields.List(fields.Integer()) # deprecated wrt dashboard-native filters filter_scopes = fields.Dict()