From bbb1f2d75705957a6239bd6084feadbdacc9d410 Mon Sep 17 00:00:00 2001 From: simcha90 <56388545+simcha90@users.noreply.github.com> Date: Mon, 3 May 2021 14:56:41 +0300 Subject: [PATCH] perf(native-filters): avoid unnecessary reloading of charts (#14408) * fix:fix get permission function * refactor: filter default value * refactor: update default value loading * refactor: apply defaultValues * lint: fix lint * lint: fix lint * test: fix test * refactor: use extraFormData for reload charts * test: fix tests * test: fix tests * test: fix tests --- .../spec/fixtures/mockNativeFilters.ts | 20 +++-- .../dashboard/components/Dashboard_spec.jsx | 2 +- .../dashboard/fixtures/mockNativeFilters.ts | 7 +- .../util/getFormDataWithExtraFilters_spec.ts | 1 - .../spec/javascripts/filters/utils_spec.ts | 12 +-- superset-frontend/src/chart/Chart.jsx | 7 -- .../src/chart/ChartContainer.jsx | 7 +- .../src/dashboard/actions/nativeFilters.ts | 17 +--- .../src/dashboard/components/Dashboard.jsx | 32 ++++++-- .../CascadeFilters/CascadePopover/index.tsx | 3 +- .../FilterBar/CascadeFilters/types.ts | 5 +- .../FilterBar/FilterBar.test.tsx | 25 +++--- .../FilterBar/FilterControls/state.ts | 4 +- .../FilterBar/FilterControls/types.ts | 4 +- .../FilterBar/FilterSets/EditSection.tsx | 4 +- .../FilterBar/FilterSets/index.tsx | 26 +++--- .../nativeFilters/FilterBar/index.tsx | 61 +++++++------- .../nativeFilters/FilterBar/state.ts | 70 +++++----------- .../nativeFilters/FilterBar/utils.ts | 7 ++ .../FiltersConfigForm/ControlItems.tsx | 2 +- .../FiltersConfigForm/FiltersConfigForm.tsx | 20 ++--- .../nativeFilters/FiltersConfigModal/types.ts | 3 +- .../nativeFilters/FiltersConfigModal/utils.ts | 3 +- .../components/nativeFilters/types.ts | 3 +- .../components/nativeFilters/utils.ts | 4 +- .../src/dashboard/containers/Dashboard.ts | 7 +- .../src/dashboard/reducers/nativeFilters.ts | 12 +-- .../src/dashboard/reducers/types.ts | 1 - superset-frontend/src/dashboard/types.ts | 4 +- .../util/activeAllDashboardFilters.ts | 12 ++- superset-frontend/src/dataMask/actions.ts | 9 ++- superset-frontend/src/dataMask/reducer.ts | 80 ++++++++++++++----- superset-frontend/src/dataMask/types.ts | 2 +- superset-frontend/src/filters/utils.ts | 27 +++---- 34 files changed, 260 insertions(+), 243 deletions(-) diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index 12abae7877..afc4959d39 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -21,7 +21,6 @@ import { NativeFiltersState } from 'src/dashboard/reducers/types'; import { DataMaskStateWithId } from '../../src/dataMask/types'; export const nativeFilters: NativeFiltersState = { - isInitialized: true, filterSets: {}, filters: { 'NATIVE_FILTER-e7Q8zKixx': { @@ -36,7 +35,11 @@ export const nativeFilters: NativeFiltersState = { }, }, ], - defaultValue: null, + defaultDataMask: { + filterState: { + value: null, + }, + }, cascadeParentIds: [], scope: { rootPath: ['ROOT_ID'], @@ -61,7 +64,11 @@ export const nativeFilters: NativeFiltersState = { }, }, ], - defaultValue: null, + defaultDataMask: { + filterState: { + value: null, + }, + }, cascadeParentIds: [], scope: { rootPath: ['ROOT_ID'], @@ -115,14 +122,17 @@ export const extraFormData: ExtraFormData = { export const NATIVE_FILTER_ID = 'NATIVE_FILTER-p4LImrSgA'; export const singleNativeFiltersState = { - isInitialized: true, filters: { [NATIVE_FILTER_ID]: { id: [NATIVE_FILTER_ID], name: 'eth', type: 'text', targets: [{ datasetId: 13, column: { name: 'ethnic_minority' } }], - defaultValue: null, + defaultDataMask: { + filterState: { + value: null, + }, + }, cascadeParentIds: [], scope: { rootPath: ['ROOT_ID'], excluded: [227, 229] }, inverseSelection: false, diff --git a/superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx index 61730c5e9e..a881d0cdad 100644 --- a/superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx +++ b/superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx @@ -167,7 +167,7 @@ describe('Dashboard', () => { ...OVERRIDE_FILTERS, [NATIVE_FILTER_ID]: { scope: [230], - values: [extraFormData], + values: extraFormData, }, }); }); diff --git a/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts b/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts index da7377d217..8a106a2b5e 100644 --- a/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts @@ -30,7 +30,6 @@ export const mockDataMaskInfo: DataMaskStateWithId = { }; export const nativeFiltersInfo: NativeFiltersState = { - isInitialized: true, filterSets: { 'set-id': { id: 'DefaultsID', @@ -54,7 +53,11 @@ export const nativeFiltersInfo: NativeFiltersState = { }, }, ], - defaultValue: null, + defaultDataMask: { + filterState: { + value: null, + }, + }, scope: { rootPath: [], excluded: [], diff --git a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts index c183a2e3d9..38c29e0f5d 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts +++ b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts @@ -52,7 +52,6 @@ describe('getFormDataWithExtraFilters', () => { }, sliceId: chartId, nativeFilters: { - isInitialized: true, filterSets: {}, filters: { [filterId]: ({ diff --git a/superset-frontend/spec/javascripts/filters/utils_spec.ts b/superset-frontend/spec/javascripts/filters/utils_spec.ts index d997e6495c..9d225f4c2f 100644 --- a/superset-frontend/spec/javascripts/filters/utils_spec.ts +++ b/superset-frontend/spec/javascripts/filters/utils_spec.ts @@ -129,21 +129,15 @@ describe('Filter utils', () => { ); }); it('getSelectExtraFormData - col: "testCol", value: [], emptyFilter: false, inverseSelection: false', () => { - expect(getSelectExtraFormData('testCol', [], false, false)).toEqual({ - filters: [], - }); + expect(getSelectExtraFormData('testCol', [], false, false)).toEqual({}); }); it('getSelectExtraFormData - col: "testCol", value: undefined, emptyFilter: false, inverseSelection: false', () => { expect( getSelectExtraFormData('testCol', undefined, false, false), - ).toEqual({ - filters: [], - }); + ).toEqual({}); }); it('getSelectExtraFormData - col: "testCol", value: null, emptyFilter: false, inverseSelection: false', () => { - expect(getSelectExtraFormData('testCol', null, false, false)).toEqual({ - filters: [], - }); + expect(getSelectExtraFormData('testCol', null, false, false)).toEqual({}); }); }); diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx index 8e32731518..0ca590a130 100644 --- a/superset-frontend/src/chart/Chart.jsx +++ b/superset-frontend/src/chart/Chart.jsx @@ -121,13 +121,6 @@ class Chart extends React.PureComponent { } runQuery() { - if ( - this.props.dashboardId && // we on dashboard screen - isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) && - !this.props.isFiltersInitialized - ) { - return; - } if (this.props.chartId > 0 && isFeatureEnabled(FeatureFlag.CLIENT_CACHE)) { // Load saved chart with a GET request this.props.actions.getSavedChart( diff --git a/superset-frontend/src/chart/ChartContainer.jsx b/superset-frontend/src/chart/ChartContainer.jsx index 7c88667c01..9925986ad2 100644 --- a/superset-frontend/src/chart/ChartContainer.jsx +++ b/superset-frontend/src/chart/ChartContainer.jsx @@ -37,9 +37,4 @@ function mapDispatchToProps(dispatch) { }; } -export default connect( - ({ nativeFilters }) => ({ - isFiltersInitialized: nativeFilters?.isInitialized, - }), - mapDispatchToProps, -)(Chart); +export default connect(null, mapDispatchToProps)(Chart); diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts index e7c8266c3e..660f422699 100644 --- a/superset-frontend/src/dashboard/actions/nativeFilters.ts +++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts @@ -22,8 +22,8 @@ import { Dispatch } from 'redux'; import { FilterConfiguration } from 'src/dashboard/components/nativeFilters/types'; import { DataMaskType, DataMaskStateWithId } from 'src/dataMask/types'; import { - SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE, SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL, + setDataMaskForFilterConfigComplete, } from 'src/dataMask/actions'; import { HYDRATE_DASHBOARD } from './hydrate'; import { dashboardInfoChanged } from './dashboardInfo'; @@ -65,14 +65,6 @@ export interface SetFilterSetsConfigFail { type: typeof SET_FILTER_SETS_CONFIG_FAIL; filterSetsConfig: FilterSet[]; } -export const SET_FILTERS_INITIALIZED = 'SET_FILTERS_INITIALIZED'; -export interface SetFiltersInitialized { - type: typeof SET_FILTERS_INITIALIZED; -} - -export const setFiltersInitialized = (): SetFiltersInitialized => ({ - type: SET_FILTERS_INITIALIZED, -}); export const setFilterConfiguration = ( filterConfig: FilterConfiguration, @@ -108,11 +100,7 @@ export const setFilterConfiguration = ( type: SET_FILTER_CONFIG_COMPLETE, filterConfig, }); - dispatch({ - type: SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE, - unitName: DataMaskType.NativeFilters, - filterConfig, - }); + dispatch(setDataMaskForFilterConfigComplete(filterConfig)); } catch (err) { dispatch({ type: SET_FILTER_CONFIG_FAIL, filterConfig }); dispatch({ type: SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL, filterConfig }); @@ -200,6 +188,5 @@ export type AnyFilterAction = | SetFilterSetsConfigBegin | SetFilterSetsConfigComplete | SetFilterSetsConfigFail - | SetFiltersInitialized | SaveFilterSets | SetBooststapData; diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx index b504a8d20f..4a5c8831dc 100644 --- a/superset-frontend/src/dashboard/components/Dashboard.jsx +++ b/superset-frontend/src/dashboard/components/Dashboard.jsx @@ -18,7 +18,7 @@ */ import React from 'react'; import PropTypes from 'prop-types'; -import { t } from '@superset-ui/core'; +import { isFeatureEnabled, t, FeatureFlag } from '@superset-ui/core'; import { PluginContext } from 'src/components/DynamicPlugins'; import Loading from 'src/components/Loading'; @@ -56,6 +56,7 @@ const propTypes = { charts: PropTypes.objectOf(chartPropShape).isRequired, slices: PropTypes.objectOf(slicePropShape).isRequired, activeFilters: PropTypes.object.isRequired, + chartConfiguration: PropTypes.object.isRequired, datasources: PropTypes.object.isRequired, ownDataCharts: PropTypes.object.isRequired, layout: PropTypes.object.isRequired, @@ -120,6 +121,11 @@ class Dashboard extends React.PureComponent { }; } window.addEventListener('visibilitychange', this.onVisibilityChange); + this.applyCharts(); + } + + componentDidUpdate() { + this.applyCharts(); } UNSAFE_componentWillReceiveProps(nextProps) { @@ -147,15 +153,28 @@ class Dashboard extends React.PureComponent { } } - componentDidUpdate() { + applyCharts() { const { hasUnsavedChanges, editMode } = this.props.dashboardState; const { appliedFilters, appliedOwnDataCharts } = this; - const { activeFilters, ownDataCharts } = this.props; + const { activeFilters, ownDataCharts, chartConfiguration } = this.props; + if ( + isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS) && + !chartConfiguration + ) { + // For a first loading we need to wait for cross filters charts data loaded to get all active filters + // for correct comparing of filters to avoid unnecessary requests + return; + } + if ( !editMode && - (!areObjectsEqual(appliedOwnDataCharts, ownDataCharts) || - !areObjectsEqual(appliedFilters, activeFilters)) + (!areObjectsEqual(appliedOwnDataCharts, ownDataCharts, { + ignoreUndefined: true, + }) || + !areObjectsEqual(appliedFilters, activeFilters, { + ignoreUndefined: true, + })) ) { this.applyFilters(); } @@ -223,6 +242,9 @@ class Dashboard extends React.PureComponent { !areObjectsEqual( appliedFilters[filterKey].values, activeFilters[filterKey].values, + { + ignoreUndefined: true, + }, ) ) { affectedChartIds.push(...activeFilters[filterKey].scope); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx index 60396caea1..f25610816b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadePopover/index.tsx @@ -28,6 +28,7 @@ import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/Filt import CascadeFilterControl from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl'; import { CascadeFilter } from 'src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types'; import { Filter } from 'src/dashboard/components/nativeFilters/types'; +import { RootState } from 'src/dashboard/types'; interface CascadePopoverProps { filter: CascadeFilter; @@ -82,7 +83,7 @@ const CascadePopover: React.FC = ({ directPathToChild, }) => { const [currentPathToChild, setCurrentPathToChild] = useState(); - const dataMask = useSelector( + const dataMask = useSelector( state => state.dataMask[filter.id] ?? getInitialDataMask(filter.id), ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts index 435404669e..c5a9d1388c 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CascadeFilters/types.ts @@ -17,8 +17,9 @@ * under the License. */ +import { DataMask } from '@superset-ui/core'; import { Filter } from '../../types'; -export interface CascadeFilter extends Filter { +export type CascadeFilter = Filter & { dataMask?: DataMask } & { cascadeChildren: CascadeFilter[]; -} +}; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx index d6420df458..5f2342cbe7 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx @@ -84,7 +84,6 @@ const addFilterFlow = () => { const addFilterSetFlow = async () => { // add filter set userEvent.click(screen.getByText('Filter Sets (0)')); - expect(screen.getByTestId(getTestId('new-filter-set-button'))).toBeDisabled(); // check description expect(screen.getByText('Filters (1)')).toBeInTheDocument(); @@ -92,7 +91,6 @@ const addFilterSetFlow = async () => { expect(screen.getAllByText('Last week').length).toBe(2); // apply filters - userEvent.click(screen.getByTestId(getTestId('apply-button'))); expect(screen.getByTestId(getTestId('new-filter-set-button'))).toBeEnabled(); // create filter set @@ -139,7 +137,7 @@ describe('FilterBar', () => { "name":"${FILTER_NAME}", "filterType":"filter_time", "targets":[{"datasetId":11,"column":{"name":"color"}}], - "defaultValue":null, + "defaultDataMask":{"filterState":{"value":null}}, "controlValues":{}, "cascadeParentIds":[], "scope":{"rootPath":["ROOT_ID"],"excluded":[]}, @@ -154,7 +152,7 @@ describe('FilterBar', () => { "name":"${FILTER_NAME}", "filterType":"filter_time", "targets":[{}], - "defaultValue":"Last week", + "defaultDataMask":{"filterState":{"value":"Last week"},"extraFormData":{"time_range":"Last week"}}, "controlValues":{}, "cascadeParentIds":[], "scope":{"rootPath":["ROOT_ID"],"excluded":[]}, @@ -163,7 +161,7 @@ describe('FilterBar', () => { }, "dataMask":{ "${filterId}":{ - "extraFormData":{"override_form_data":{"time_range":"Last week"}}, + "extraFormData":{"time_range":"Last week"}, "filterState":{"value":"Last week"}, "ownState":{}, "id":"${filterId}" @@ -308,10 +306,6 @@ describe('FilterBar', () => { addFilterFlow(); await screen.findByText('All Filters (1)'); - - // apply filter - expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(); - userEvent.click(screen.getByTestId(getTestId('apply-button'))); expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); }); @@ -319,6 +313,7 @@ describe('FilterBar', () => { it.skip('add and apply filter set', async () => { // @ts-ignore global.featureFlags = { + [FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true, [FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET]: true, }; renderWrapper(openedBarProps, stateWithoutNativeFilters); @@ -326,21 +321,23 @@ describe('FilterBar', () => { addFilterFlow(); await screen.findByText('All Filters (1)'); - expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(); await addFilterSetFlow(); // change filter - userEvent.click(screen.getByText('All Filters (1)')); expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); await changeFilterValue(); await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(2)); // apply new filter value - expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(); + await waitFor(() => + expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(), + ); userEvent.click(screen.getByTestId(getTestId('apply-button'))); - expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); + await waitFor(() => + expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(), + ); // applying filter set userEvent.click(screen.getByText('Filter Sets (1)')); @@ -357,7 +354,6 @@ describe('FilterBar', () => { expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); }); - // TODO: fix flakiness and re-enable it.skip('add and edit filter set', async () => { // @ts-ignore global.featureFlags = { @@ -369,7 +365,6 @@ describe('FilterBar', () => { addFilterFlow(); await screen.findByText('All Filters (1)'); - expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(); await addFilterSetFlow(); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts index 802a12d195..7be5835780 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts @@ -19,7 +19,7 @@ import { useSelector } from 'react-redux'; import { NativeFiltersState } from 'src/dashboard/reducers/types'; import { mergeExtraFormData } from '../../utils'; -import { useDataMask } from '../state'; +import { useNativeFiltersDataMask } from '../state'; // eslint-disable-next-line import/prefer-default-export export function useCascadingFilters(id: string) { @@ -29,7 +29,7 @@ export function useCascadingFilters(id: string) { const filter = filters[id]; const cascadeParentIds: string[] = filter?.cascadeParentIds ?? []; let cascadedFilters = {}; - const nativeFiltersDataMask = useDataMask(); + const nativeFiltersDataMask = useNativeFiltersDataMask(); cascadeParentIds.forEach(parentId => { const parentState = nativeFiltersDataMask[parentId] || {}; const { extraFormData: parentExtra = {} } = parentState; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts index 67e50d562f..0b39dd210b 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/types.ts @@ -21,7 +21,9 @@ import { DataMask } from '@superset-ui/core'; import { Filter } from '../../types'; export interface FilterProps { - filter: Filter; + filter: Filter & { + dataMask?: DataMask; + }; icon?: React.ReactElement; directPathToChild?: string[]; onFilterSelectionChange: (filter: Filter, dataMask: DataMask) => void; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/EditSection.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/EditSection.tsx index 6ab44b452e..bd9f724af4 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/EditSection.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/EditSection.tsx @@ -25,7 +25,7 @@ import { setFilterSetsConfiguration } from 'src/dashboard/actions/nativeFilters' import { DataMaskState } from 'src/dataMask/types'; import { WarningOutlined } from '@ant-design/icons'; import { ActionButtons } from './Footer'; -import { useDataMask, useFilters, useFilterSets } from '../state'; +import { useNativeFiltersDataMask, useFilters, useFilterSets } from '../state'; import { APPLY_FILTERS_HINT, findExistingFilterSet } from './utils'; import { useFilterSetNameDuplicated } from './state'; import { getFilterBarTestId } from '../index'; @@ -72,7 +72,7 @@ const EditSection: FC = ({ dataMaskSelected, disabled, }) => { - const dataMaskApplied = useDataMask(); + const dataMaskApplied = useNativeFiltersDataMask(); const dispatch = useDispatch(); const filterSets = useFilterSets(); const filters = useFilters(); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/index.tsx index 8947cebb77..95676daa4a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/index.tsx @@ -26,7 +26,7 @@ import { Filters, FilterSet, FilterSets } from 'src/dashboard/reducers/types'; import { areObjectsEqual } from 'src/reduxUtils'; import { findExistingFilterSet, generateFiltersSetId } from './utils'; import { Filter } from '../../types'; -import { useFilters, useDataMask, useFilterSets } from '../state'; +import { useFilters, useNativeFiltersDataMask, useFilterSets } from '../state'; import Footer from './Footer'; import FilterSetUnit from './FilterSetUnit'; import { getFilterBarTestId } from '..'; @@ -85,7 +85,7 @@ const FilterSets: React.FC = ({ const dispatch = useDispatch(); const [filterSetName, setFilterSetName] = useState(DEFAULT_FILTER_SET_NAME); const [editMode, setEditMode] = useState(false); - const dataMaskApplied = useDataMask(); + const dataMaskApplied = useNativeFiltersDataMask(); const filterSets = useFilterSets(); const filterSetFilterValues = Object.values(filterSets); const filters = useFilters(); @@ -111,7 +111,9 @@ const FilterSets: React.FC = ({ filterSet?: FilterSet, ) => !filterValues.find(filter => filter?.id === id) || - !areObjectsEqual(filters[id], filterSet?.nativeFilters?.[id]); + !areObjectsEqual(filters[id], filterSet?.nativeFilters?.[id], { + ignoreUndefined: true, + }); const takeFilterSet = (id: string, target?: HTMLElement) => { const ignoreSelectorHeader = 'ant-collapse-header'; @@ -137,13 +139,15 @@ const FilterSets: React.FC = ({ const filterSet = filterSets[id]; - Object.values(filterSet?.dataMask ?? []).forEach(dataMask => { - const { extraFormData, filterState, id } = dataMask as DataMaskWithId; - if (isFilterMissingOrContainsInvalidMetadata(id, filterSet)) { - return; - } - onFilterSelectionChange({ id }, { extraFormData, filterState }); - }); + (Object.values(filterSet?.dataMask) ?? []).forEach( + (dataMask: DataMaskWithId) => { + const { extraFormData, filterState, id } = dataMask; + if (isFilterMissingOrContainsInvalidMetadata(id, filterSet)) { + return; + } + onFilterSelectionChange({ id }, { extraFormData, filterState }); + }, + ); }; const handleRebuild = (id: string) => { @@ -168,7 +172,7 @@ const FilterSets: React.FC = ({ dataMask: Object.keys(newFilters).reduce( (prev, nextFilterId) => ({ ...prev, - [nextFilterId]: filterSet.dataMask?.nativeFilters?.[nextFilterId], + [nextFilterId]: filterSet.dataMask?.[nextFilterId], }), {}, ), diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 3829cf448f..77f47d171d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -19,31 +19,38 @@ /* eslint-disable no-param-reassign */ import { HandlerFunction, styled, t } from '@superset-ui/core'; -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useMemo, useState } from 'react'; import { useDispatch } from 'react-redux'; import cx from 'classnames'; import Icon from 'src/components/Icon'; import { Tabs } from 'src/common/components'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import { updateDataMask } from 'src/dataMask/actions'; -import { DataMaskState } from 'src/dataMask/types'; +import { + DataMaskState, + DataMaskStateWithId, + DataMaskWithId, +} from 'src/dataMask/types'; import { useImmer } from 'use-immer'; import { areObjectsEqual } from 'src/reduxUtils'; import { testWithId } from 'src/utils/testUtils'; import { Filter } from 'src/dashboard/components/nativeFilters/types'; -import { setFiltersInitialized } from 'src/dashboard/actions/nativeFilters'; -import { mapParentFiltersToChildren, TabIds } from './utils'; +import { + getOnlyExtraFormData, + mapParentFiltersToChildren, + TabIds, +} from './utils'; import FilterSets from './FilterSets'; import { - useDataMask, + useNativeFiltersDataMask, useFilters, useFilterSets, - useFiltersInitialisation, useFilterUpdates, } from './state'; import EditSection from './FilterSets/EditSection'; import Header from './Header'; import FilterControls from './FilterControls/FilterControls'; +import { getInitialDataMask } from '../../../../dataMask/reducer'; const BAR_WIDTH = `250px`; @@ -155,18 +162,16 @@ const FilterBar: React.FC = ({ directPathToChild, }) => { const [editFilterSetId, setEditFilterSetId] = useState(null); - const [dataMaskSelected, setDataMaskSelected] = useImmer({}); - const [ - lastAppliedFilterData, - setLastAppliedFilterData, - ] = useImmer({}); + const [dataMaskSelected, setDataMaskSelected] = useImmer( + {}, + ); const dispatch = useDispatch(); const filterSets = useFilterSets(); const filterSetFilterValues = Object.values(filterSets); const [tab, setTab] = useState(TabIds.AllFilters); const filters = useFilters(); const filterValues = Object.values(filters); - const dataMaskApplied = useDataMask(); + const dataMaskApplied: DataMaskStateWithId = useNativeFiltersDataMask(); const [isFilterSetChanged, setIsFilterSetChanged] = useState(false); const cascadeChildren = useMemo( () => mapParentFiltersToChildren(filterValues), @@ -185,7 +190,10 @@ const FilterBar: React.FC = ({ dispatch(updateDataMask(filter.id, dataMask)); } - draft[filter.id] = dataMask; + draft[filter.id] = { + ...(getInitialDataMask(filter.id) as DataMaskWithId), + ...dataMask, + }; }); }; @@ -196,29 +204,18 @@ const FilterBar: React.FC = ({ dispatch(updateDataMask(filterId, dataMaskSelected[filterId])); } }); - setLastAppliedFilterData(() => dataMaskSelected); }; - const { isInitialized } = useFiltersInitialisation( - dataMaskSelected, - handleApply, - ); - - useEffect(() => { - if (isInitialized) { - dispatch(setFiltersInitialized()); - } - }, [dispatch, isInitialized]); - - useFilterUpdates( - dataMaskSelected, - setDataMaskSelected, - setLastAppliedFilterData, - ); + useFilterUpdates(dataMaskSelected, setDataMaskSelected); + const dataSelectedValues = Object.values(dataMaskSelected); + const dataAppliedValues = Object.values(dataMaskApplied); const isApplyDisabled = - !isInitialized || areObjectsEqual(dataMaskSelected, lastAppliedFilterData); - + areObjectsEqual( + getOnlyExtraFormData(dataMaskSelected), + getOnlyExtraFormData(dataMaskApplied), + { ignoreUndefined: true }, + ) || dataSelectedValues.length !== dataAppliedValues.length; return ( useSelector( @@ -35,44 +39,27 @@ export const useFilterSets = () => export const useFilters = () => useSelector(state => state.nativeFilters.filters); -export const useDataMask = () => - useSelector(state => state.dataMask); +export const useNativeFiltersDataMask = () => { + const dataMask = useSelector( + state => state.dataMask, + ); -export const useFiltersInitialisation = ( - dataMaskSelected: DataMaskState, - handleApply: () => void, -) => { - const [isInitialized, setIsInitialized] = useState(false); - const filters = useFilters(); - const filterValues = Object.values(filters); - useEffect(() => { - if (isInitialized) { - return; - } - const areFiltersInitialized = filterValues.every(filterValue => - areObjectsEqual( - filterValue?.defaultValue, - dataMaskSelected[filterValue?.id]?.filterState?.value, - ), - ); - if (areFiltersInitialized) { - handleApply(); - setIsInitialized(true); - } - }, [filterValues, dataMaskSelected, isInitialized]); - - return { - isInitialized, - }; + return Object.values(dataMask) + .filter((item: DataMaskWithId) => + String(item.id).startsWith(NATIVE_FILTER_PREFIX), + ) + .reduce( + (prev, next: DataMaskWithId) => ({ ...prev, [next.id]: next }), + {}, + ) as DataMaskStateWithId; }; export const useFilterUpdates = ( dataMaskSelected: DataMaskState, setDataMaskSelected: (arg0: (arg0: DataMaskState) => void) => void, - setLastAppliedFilterData: (arg0: (arg0: DataMaskState) => void) => void, ) => { const filters = useFilters(); - const dataMaskApplied = useDataMask(); + const dataMaskApplied = useNativeFiltersDataMask(); useEffect(() => { // Remove deleted filters from local state @@ -83,18 +70,5 @@ export const useFilterUpdates = ( }); } }); - Object.keys(dataMaskApplied).forEach(appliedId => { - if (!filters[appliedId]) { - setLastAppliedFilterData(draft => { - delete draft[appliedId]; - }); - } - }); - }, [ - dataMaskApplied, - dataMaskSelected, - filters, - setDataMaskSelected, - setLastAppliedFilterData, - ]); + }, [dataMaskApplied, dataMaskSelected, filters, setDataMaskSelected]); }; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts index dc7097a508..75cda7382e 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts @@ -17,6 +17,7 @@ * under the License. */ +import { DataMaskStateWithId } from 'src/dataMask/types'; import { Filter } from '../types'; export enum TabIds { @@ -39,3 +40,9 @@ export function mapParentFiltersToChildren( }); return cascadeChildren; } + +export const getOnlyExtraFormData = (data: DataMaskStateWithId) => + Object.values(data).reduce( + (prev, next) => ({ ...prev, [next.id]: next.extraFormData }), + {}, + ); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx index 4310a5b45f..82a46bf781 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx @@ -76,7 +76,7 @@ const ControlItems: FC = ({ return; } setNativeFilterFieldValues(form, filterId, { - defaultValue: null, + defaultDataMask: null, }); forceUpdate(); }} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index 2eca4e8b96..182e1e00f0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -126,9 +126,7 @@ export const FiltersConfigForm: React.FC = ({ const [metrics, setMetrics] = useState([]); const forceUpdate = useForceUpdate(); const [datasetDetails, setDatasetDetails] = useState>(); - const formFilter = form.getFieldValue('filters')?.[filterId] || {}; - const nativeFilterItems = getChartMetadataRegistry().items; const nativeFilterVizTypes = Object.entries(nativeFilterItems) // @ts-ignore @@ -191,7 +189,6 @@ export const FiltersConfigForm: React.FC = ({ const formData = getFormData({ datasetId: formFilter?.dataset?.value, groupby: formFilter?.column, - defaultValue: formFilter?.defaultValue, ...formFilter, }); setNativeFilterFieldValues(form, filterId, { @@ -242,7 +239,6 @@ export const FiltersConfigForm: React.FC = ({ const newFormData = getFormData({ datasetId, groupby: hasColumn ? formFilter?.column : undefined, - defaultValue: formFilter?.defaultValue, ...formFilter, }); @@ -294,7 +290,7 @@ export const FiltersConfigForm: React.FC = ({ onChange={({ value }: { value: string }) => { setNativeFilterFieldValues(form, filterId, { filterType: value, - defaultValue: null, + defaultDataMask: null, }); forceUpdate(); }} @@ -324,7 +320,7 @@ export const FiltersConfigForm: React.FC = ({ // We need reset column when dataset changed if (datasetId && e?.value !== datasetId) { setNativeFilterFieldValues(form, filterId, { - defaultValue: null, + defaultDataMask: null, column: null, }); } @@ -346,10 +342,10 @@ export const FiltersConfigForm: React.FC = ({ form={form} filterId={filterId} datasetId={datasetId} - onChange={e => { + onChange={() => { // We need reset default value when when column changed setNativeFilterFieldValues(form, filterId, { - defaultValue: null, + defaultDataMask: null, }); forceUpdate(); }} @@ -435,16 +431,16 @@ export const FiltersConfigForm: React.FC = ({ )} {t('Default Value')}} > {(!hasDataset || (!isDataDirty && hasFilledDataset)) && ( { + setDataMask={dataMask => { setNativeFilterFieldValues(form, filterId, { - defaultValue: filterState?.value, + defaultDataMask: dataMask, }); forceUpdate(); }} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts index 14c67e4ea4..60051e73f0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { AdhocFilter } from '@superset-ui/core'; +import { AdhocFilter, DataMask } from '@superset-ui/core'; import { Scope } from '../types'; export interface NativeFiltersFormItem { @@ -32,6 +32,7 @@ export interface NativeFiltersFormItem { [key: string]: any; }; defaultValue: any; + defaultDataMask: DataMask; parentFilter: { value: string; label: string; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts index 3b0cf2b855..d9ae39d3e5 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts @@ -18,6 +18,7 @@ */ import { FormInstance } from 'antd/lib/form'; import shortid from 'shortid'; +import { getInitialDataMask } from 'src/dataMask/reducer'; import { FilterRemoval, NativeFiltersForm } from './types'; import { Filter, FilterConfiguration, Target } from '../types'; @@ -148,7 +149,7 @@ export const createHandleSave = ( filterType: formInputs.filterType, // for now there will only ever be one target targets: [target], - defaultValue: formInputs.defaultValue || null, + defaultDataMask: formInputs.defaultDataMask ?? getInitialDataMask(), cascadeParentIds: formInputs.parentFilter ? [formInputs.parentFilter.value] : [], diff --git a/superset-frontend/src/dashboard/components/nativeFilters/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/types.ts index a7589359e9..07e347c437 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/types.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/types.ts @@ -41,8 +41,7 @@ export interface Target { export interface Filter { cascadeParentIds: string[]; - defaultValue: any; - dataMask?: DataMask; + defaultDataMask: DataMask; isInstant: boolean; id: string; // randomly generated at filter creation name: string; diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts index 5264e70735..a42540ac21 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts @@ -35,7 +35,7 @@ export const getFormData = ({ cascadingFilters = {}, groupby, inputRef, - defaultValue, + defaultDataMask, controlValues, filterType, sortMetric, @@ -73,7 +73,7 @@ export const getFormData = ({ metrics: ['count'], row_limit: 10000, showSearch: true, - defaultValue, + defaultValue: defaultDataMask?.filterState?.value, time_range, time_range_endpoints: ['inclusive', 'exclusive'], url_params: {}, diff --git a/superset-frontend/src/dashboard/containers/Dashboard.ts b/superset-frontend/src/dashboard/containers/Dashboard.ts index 526223d8ed..398dff7f29 100644 --- a/superset-frontend/src/dashboard/containers/Dashboard.ts +++ b/superset-frontend/src/dashboard/containers/Dashboard.ts @@ -18,9 +18,7 @@ */ import { bindActionCreators, Dispatch } from 'redux'; import { connect } from 'react-redux'; - import Dashboard from '../components/Dashboard'; - import { addSliceToDashboard, removeSliceFromDashboard, @@ -66,11 +64,12 @@ function mapStateToProps(state: RootState) { // eslint-disable-next-line camelcase chartConfiguration: dashboardInfo.metadata?.chart_configuration, nativeFilters: nativeFilters.filters, - dataMask: getRelevantDataMask(dataMask, 'isApplied'), + dataMask, layout: dashboardLayout.present, }), }, - ownDataCharts: getRelevantDataMask(dataMask, 'ownState', 'ownState'), + chartConfiguration: dashboardInfo.metadata?.chart_configuration, + ownDataCharts: getRelevantDataMask(dataMask, 'ownState'), slices: sliceEntities.slices, layout: dashboardLayout.present, impressionId, diff --git a/superset-frontend/src/dashboard/reducers/nativeFilters.ts b/superset-frontend/src/dashboard/reducers/nativeFilters.ts index e22af28d76..f434d69112 100644 --- a/superset-frontend/src/dashboard/reducers/nativeFilters.ts +++ b/superset-frontend/src/dashboard/reducers/nativeFilters.ts @@ -21,7 +21,6 @@ import { SAVE_FILTER_SETS, SET_FILTER_CONFIG_COMPLETE, SET_FILTER_SETS_CONFIG_COMPLETE, - SET_FILTERS_INITIALIZED, } from 'src/dashboard/actions/nativeFilters'; import { FilterSet, NativeFiltersState } from './types'; import { FilterConfiguration } from '../components/nativeFilters/types'; @@ -36,9 +35,7 @@ export function getInitialState({ filterConfig?: FilterConfiguration; state?: NativeFiltersState; }): NativeFiltersState { - const state: Partial = { - isInitialized: prevState?.isInitialized, - }; + const state: Partial = {}; const filters = {}; if (filterConfig) { @@ -66,7 +63,6 @@ export function getInitialState({ export default function nativeFilterReducer( state: NativeFiltersState = { - isInitialized: false, filters: {}, filterSets: {}, }, @@ -95,12 +91,6 @@ export default function nativeFilterReducer( case SET_FILTER_CONFIG_COMPLETE: return getInitialState({ filterConfig: action.filterConfig, state }); - case SET_FILTERS_INITIALIZED: - return { - ...state, - isInitialized: true, - }; - case SET_FILTER_SETS_CONFIG_COMPLETE: return getInitialState({ filterSetsConfig: action.filterSetsConfig, diff --git a/superset-frontend/src/dashboard/reducers/types.ts b/superset-frontend/src/dashboard/reducers/types.ts index 9ded03540e..770d561a32 100644 --- a/superset-frontend/src/dashboard/reducers/types.ts +++ b/superset-frontend/src/dashboard/reducers/types.ts @@ -97,7 +97,6 @@ export type Filters = { }; export type NativeFiltersState = { - isInitialized: boolean; filters: Filters; filterSets: FilterSets; }; diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index 3ed1c1841d..f01c2ac66f 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { ChartProps, JsonObject } from '@superset-ui/core'; +import { ChartProps, ExtraFormData, JsonObject } from '@superset-ui/core'; import { chart } from 'src/chart/chartReducer'; import componentTypes from 'src/dashboard/util/componentTypes'; import { DataMaskStateWithId } from '../dataMask/types'; @@ -95,7 +95,7 @@ export type LayoutItem = { type ActiveFilter = { scope: number[]; - values: any[]; + values: ExtraFormData; }; export type ActiveFilters = { diff --git a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts index c3210aa77f..2243c25d56 100644 --- a/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts +++ b/superset-frontend/src/dashboard/util/activeAllDashboardFilters.ts @@ -20,7 +20,7 @@ import { DataMaskStateWithId } from 'src/dataMask/types'; import { JsonObject } from '@superset-ui/core'; import { CHART_TYPE } from './componentTypes'; import { Scope } from '../components/nativeFilters/types'; -import { ActiveFilters, LayoutItem } from '../types'; +import { ActiveFilters, Layout, LayoutItem } from '../types'; import { ChartConfiguration, Filters } from '../reducers/types'; import { DASHBOARD_ROOT_ID } from './constants'; @@ -51,12 +51,11 @@ export const findAffectedCharts = ({ // eslint-disable-next-line no-param-reassign activeFilters[filterId] = { scope: [], - values: [], + values: extraFormData, }; } // Add not excluded chart scopes(to know what charts refresh) and values(refresh only if its value changed) activeFilters[filterId].scope.push(chartId); - activeFilters[filterId].values.push(extraFormData); return; } // If child is not chart, recursive iterate over its children @@ -74,11 +73,10 @@ export const findAffectedCharts = ({ export const getRelevantDataMask = ( dataMask: DataMaskStateWithId, - filterBy: string, - prop?: string, + prop: string, ): JsonObject | DataMaskStateWithId => Object.values(dataMask) - .filter(item => item[filterBy]) + .filter(item => item[prop]) .reduce( (prev, next) => ({ ...prev, [next.id]: prop ? next[prop] : next }), {}, @@ -93,7 +91,7 @@ export const getAllActiveFilters = ({ chartConfiguration: ChartConfiguration; dataMask: DataMaskStateWithId; nativeFilters: Filters; - layout: { [key: string]: LayoutItem }; + layout: Layout; }): ActiveFilters => { const activeFilters = {}; diff --git a/superset-frontend/src/dataMask/actions.ts b/superset-frontend/src/dataMask/actions.ts index 9d557da6b5..7a9bb3f387 100644 --- a/superset-frontend/src/dataMask/actions.ts +++ b/superset-frontend/src/dataMask/actions.ts @@ -42,7 +42,14 @@ export interface SetDataMaskForFilterConfigFail { type: typeof SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL; filterConfig: FilterConfiguration; } - +export function setDataMaskForFilterConfigComplete( + filterConfig: FilterConfiguration, +): SetDataMaskForFilterConfigComplete { + return { + type: SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE, + filterConfig, + }; +} export function updateDataMask( filterId: string, dataMask: DataMask, diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index 22c3e66c84..c4ee71fba2 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -20,23 +20,57 @@ /* eslint-disable no-param-reassign */ // <- When we work with Immer, we need reassign, so disabling lint import produce from 'immer'; +import { DataMask, FeatureFlag } from '@superset-ui/core'; +import { NATIVE_FILTER_PREFIX } from 'src/dashboard/components/nativeFilters/FiltersConfigModal/utils'; +import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate'; +import { isFeatureEnabled } from 'src/featureFlags'; import { DataMaskStateWithId, DataMaskWithId } from './types'; import { AnyDataMaskAction, SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE, UPDATE_DATA_MASK, } from './actions'; -import { NATIVE_FILTER_PREFIX } from '../dashboard/components/nativeFilters/FiltersConfigModal/utils'; -import { Filter } from '../dashboard/components/nativeFilters/types'; +import { + Filter, + FilterConfiguration, +} from '../dashboard/components/nativeFilters/types'; +export function getInitialDataMask(id?: string): DataMask; export function getInitialDataMask(id: string): DataMaskWithId { + let otherProps = {}; + if (id) { + otherProps = { + id, + }; + } return { - id, + ...otherProps, extraFormData: {}, - filterState: {}, + filterState: { + value: null, + }, ownState: {}, - isApplied: false, - }; + } as DataMaskWithId; +} + +function fillNativeFilters( + data: FilterConfiguration, + cleanState: DataMaskStateWithId, + draft: DataMaskStateWithId, +) { + data.forEach((filter: Filter) => { + cleanState[filter.id] = { + ...getInitialDataMask(filter.id), // take initial data + ...filter.defaultDataMask, // if something new came from BE - take it + ...draft[filter.id], // keep local filter data + }; + }); + // Get back all other non-native filters + Object.values(draft).forEach(filter => { + if (!String(filter?.id).startsWith(NATIVE_FILTER_PREFIX)) { + cleanState[filter?.id] = filter; + } + }); } const dataMaskReducer = produce( @@ -48,21 +82,31 @@ const dataMaskReducer = produce( ...getInitialDataMask(action.filterId), ...draft[action.filterId], ...action.dataMask, - isApplied: true, }; return draft; - + // TODO: update hydrate to .ts + // @ts-ignore + case HYDRATE_DASHBOARD: + if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { + Object.keys( + // @ts-ignore + action.data.dashboardInfo?.metadata?.chart_configuration, + ).forEach(id => { + cleanState[id] = { + ...getInitialDataMask(id), // take initial data + }; + }); + } + fillNativeFilters( + // @ts-ignore + action.data.dashboardInfo?.metadata?.native_filter_configuration ?? + [], + cleanState, + draft, + ); + return cleanState; case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE: - (action.filterConfig ?? []).forEach((filter: Filter) => { - cleanState[filter.id] = - draft[filter.id] ?? getInitialDataMask(filter.id); - }); - // Get back all other non-native filters - Object.values(draft).forEach(filter => { - if (!String(filter?.id).startsWith(NATIVE_FILTER_PREFIX)) { - cleanState[filter?.id] = filter; - } - }); + fillNativeFilters(action.filterConfig ?? [], cleanState, draft); return cleanState; default: diff --git a/superset-frontend/src/dataMask/types.ts b/superset-frontend/src/dataMask/types.ts index 2e335b2ece..95d2073a07 100644 --- a/superset-frontend/src/dataMask/types.ts +++ b/superset-frontend/src/dataMask/types.ts @@ -25,5 +25,5 @@ export enum DataMaskType { export type DataMaskState = { [id: string]: DataMask }; -export type DataMaskWithId = { id: string; isApplied?: boolean } & DataMask; +export type DataMaskWithId = { id: string } & DataMask; export type DataMaskStateWithId = { [filterId: string]: DataMaskWithId }; diff --git a/superset-frontend/src/filters/utils.ts b/superset-frontend/src/filters/utils.ts index 6ca88dd058..ecd7268adf 100644 --- a/superset-frontend/src/filters/utils.ts +++ b/superset-frontend/src/filters/utils.ts @@ -41,17 +41,14 @@ export const getSelectExtraFormData = ( sqlExpression: '1 = 0', }, ]; - } else { - extra.filters = - value === undefined || value === null || value.length === 0 - ? [] - : [ - { - col, - op: inverseSelection ? ('NOT IN' as const) : ('IN' as const), - val: value, - }, - ]; + } else if (value !== undefined && value !== null && value.length !== 0) { + extra.filters = [ + { + col, + op: inverseSelection ? ('NOT IN' as const) : ('IN' as const), + val: value, + }, + ]; } return extra; }; @@ -69,9 +66,11 @@ export const getRangeExtraFormData = ( filters.push({ col, op: '<=', val: upper }); } - return { - filters, - }; + return filters.length + ? { + filters, + } + : {}; }; export interface DataRecordValueFormatter {