chore: Preserve native filters selection after refresh (#15583)

superset-frontend/spec/helpers/testing-library.tsx

@@ -20,6 +20,7 @@ import '@testing-library/jest-dom/extend-expect';
 import React, { ReactNode, ReactElement } from 'react';
 import { render, RenderOptions } from '@testing-library/react';
 import { ThemeProvider, supersetTheme } from '@superset-ui/core';
+import { BrowserRouter } from 'react-router-dom';
 import { Provider } from 'react-redux';
 import { combineReducers, createStore, applyMiddleware, compose } from 'redux';
 import thunk from 'redux-thunk';
@@ -32,13 +33,20 @@ type Options = Omit<RenderOptions, 'queries'> & {
   useRedux?: boolean;
   useDnd?: boolean;
   useQueryParams?: boolean;
+  useRouter?: boolean;
   initialState?: {};
   reducers?: {};
 };
 
 function createWrapper(options?: Options) {
-  const { useDnd, useRedux, useQueryParams, initialState, reducers } =
-    options || {};
+  const {
+    useDnd,
+    useRedux,
+    useQueryParams,
+    useRouter,
+    initialState,
+    reducers,
+  } = options || {};
 
   return ({ children }: { children?: ReactNode }) => {
     let result = (
@@ -63,6 +71,10 @@ function createWrapper(options?: Options) {
       result = <QueryParamProvider>{result}</QueryParamProvider>;
     }
 
+    if (useRouter) {
+      result = <BrowserRouter>{result}</BrowserRouter>;
+    }
+
     return result;
   };
 }

superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js

@@ -91,7 +91,7 @@ describe('getChartIdsFromLayout', () => {
       },
     });
     expect(urlWithNativeFilters).toBe(
-      'path?preselect_filters=%7B%7D&native_filters=%28NATIVE_FILTER-bar456%3A%21n%2CNATIVE_FILTER-foo123%3A%21%28a%2Cb%29%29',
+      'path?preselect_filters=%7B%7D&native_filters=%28NATIVE_FILTER-bar456%3A%28filterState%3A%28value%3A%21n%29%29%2CNATIVE_FILTER-foo123%3A%28filterState%3A%28label%3A%27custom+label%27%2Cvalue%3A%21%28a%2Cb%29%29%29%29',
     );
   });
 });

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx

@@ -19,13 +19,8 @@
 
 import React from 'react';
 import { render, screen } from 'spec/helpers/testing-library';
-import { Provider } from 'react-redux';
 import userEvent from '@testing-library/user-event';
-import {
-  getMockStore,
-  mockStore,
-  stateWithoutNativeFilters,
-} from 'spec/fixtures/mockStore';
+import { stateWithoutNativeFilters } from 'spec/fixtures/mockStore';
 import * as mockCore from '@superset-ui/core';
 import { testWithId } from 'src/utils/testUtils';
 import { FeatureFlag } from 'src/featureFlags';
@@ -224,11 +219,11 @@ describe('FilterBar', () => {
   });
 
   const renderWrapper = (props = closedBarProps, state?: object) =>
-    render(
-      <Provider store={state ? getMockStore(state) : mockStore}>
-        <FilterBar {...props} width={280} height={400} offset={0} />
-      </Provider>,
-    );
+    render(<FilterBar {...props} width={280} height={400} offset={0} />, {
+      useRedux: true,
+      initialState: state,
+      useRouter: true,
+    });
 
   it('should render', () => {
     const { container } = renderWrapper();
@@ -260,13 +255,6 @@ describe('FilterBar', () => {
     expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
   });
 
-  it('should render the filter control name', async () => {
-    renderWrapper();
-    expect(
-      await screen.findByText('test', {}, { timeout: 2000 }),
-    ).toBeInTheDocument();
-  });
-
   it('should toggle', () => {
     renderWrapper();
     const collapse = screen.getByRole('img', { name: 'collapse' });

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx

@@ -24,6 +24,7 @@ import Header from './index';
 const createProps = () => ({
   toggleFiltersBar: jest.fn(),
   onApply: jest.fn(),
+  onClearAll: jest.fn(),
   dataMaskSelected: {
     DefaultsID: {
       filterState: {

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx

@@ -21,8 +21,7 @@ import { styled, t, useTheme } from '@superset-ui/core';
 import React, { FC } from 'react';
 import Icons from 'src/components/Icons';
 import Button from 'src/components/Button';
-import { clearDataMask } from 'src/dataMask/actions';
-import { useDispatch, useSelector } from 'react-redux';
+import { useSelector } from 'react-redux';
 import { DataMaskState, DataMaskStateWithId } from 'src/dataMask/types';
 import FilterConfigurationLink from 'src/dashboard/components/nativeFilters/FilterBar/FilterConfigurationLink';
 import { useFilters } from 'src/dashboard/components/nativeFilters/FilterBar/state';
@@ -68,6 +67,7 @@ const Wrapper = styled.div`
 type HeaderProps = {
   toggleFiltersBar: (arg0: boolean) => void;
   onApply: () => void;
+  onClearAll: () => void;
   dataMaskSelected: DataMaskState;
   dataMaskApplied: DataMaskStateWithId;
   isApplyDisabled: boolean;
@@ -75,6 +75,7 @@ type HeaderProps = {
 
 const Header: FC<HeaderProps> = ({
   onApply,
+  onClearAll,
   isApplyDisabled,
   dataMaskSelected,
   dataMaskApplied,
@@ -82,21 +83,11 @@ const Header: FC<HeaderProps> = ({
 }) => {
   const theme = useTheme();
   const filters = useFilters();
-  const dispatch = useDispatch();
   const filterValues = Object.values<Filter>(filters);
   const canEdit = useSelector<RootState, boolean>(
     ({ dashboardInfo }) => dashboardInfo.dash_edit_perm,
   );
 
-  const handleClearAll = () => {
-    const filterIds = Object.keys(dataMaskSelected);
-    filterIds.forEach(filterId => {
-      if (dataMaskSelected[filterId]) {
-        dispatch(clearDataMask(filterId));
-      }
-    });
-  };
-
   const isClearAllDisabled = Object.values(dataMaskApplied).every(
     filter =>
       dataMaskSelected[filter.id]?.filterState?.value === null ||
@@ -129,7 +120,7 @@ const Header: FC<HeaderProps> = ({
           disabled={isClearAllDisabled}
           buttonStyle="tertiary"
           buttonSize="small"
-          onClick={handleClearAll}
+          onClick={onClearAll}
           {...getFilterBarTestId('clear-button')}
         >
           {t('Clear all')}

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx

@@ -19,21 +19,24 @@
 
 /* eslint-disable no-param-reassign */
 import { DataMask, HandlerFunction, styled, t } from '@superset-ui/core';
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useState, useCallback } from 'react';
 import { useDispatch } from 'react-redux';
 import cx from 'classnames';
 import Icons from 'src/components/Icons';
 import { Tabs } from 'src/common/components';
+import { useHistory } from 'react-router-dom';
 import { usePrevious } from 'src/common/hooks/usePrevious';
+import rison from 'rison';
 import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
-import { updateDataMask } from 'src/dataMask/actions';
+import { updateDataMask, clearDataMask } from 'src/dataMask/actions';
 import { DataMaskStateWithId, DataMaskWithId } from 'src/dataMask/types';
 import { useImmer } from 'use-immer';
 import { testWithId } from 'src/utils/testUtils';
 import { Filter } from 'src/dashboard/components/nativeFilters/types';
 import Loading from 'src/components/Loading';
 import { getInitialDataMask } from 'src/dataMask/reducer';
-import { areObjectsEqual } from 'src/reduxUtils';
+import { URL_PARAMS } from 'src/constants';
+import replaceUndefinedByNull from 'src/dashboard/util/replaceUndefinedByNull';
 import { checkIsApplyDisabled, TabIds } from './utils';
 import FilterSets from './FilterSets';
 import {
@@ -46,7 +49,6 @@ import {
 import EditSection from './FilterSets/EditSection';
 import Header from './Header';
 import FilterControls from './FilterControls/FilterControls';
-import { usePreselectNativeFilters } from '../state';
 
 export const FILTER_BAR_TEST_ID = 'filter-bar';
 export const getFilterBarTestId = testWithId(FILTER_BAR_TEST_ID);
@@ -145,6 +147,7 @@ const FilterBar: React.FC<FiltersBarProps> = ({
   height,
   offset,
 }) => {
+  const history = useHistory();
   const dataMaskApplied: DataMaskStateWithId = useNativeFiltersDataMask();
   const [editFilterSetId, setEditFilterSetId] = useState<string | null>(null);
   const [dataMaskSelected, setDataMaskSelected] = useImmer<DataMaskStateWithId>(
@@ -158,8 +161,6 @@ const FilterBar: React.FC<FiltersBarProps> = ({
   const previousFilters = usePrevious(filters);
   const filterValues = Object.values<Filter>(filters);
   const [isFilterSetChanged, setIsFilterSetChanged] = useState(false);
-  const preselectNativeFilters = usePreselectNativeFilters();
-  const [initializedFilters, setInitializedFilters] = useState<any[]>([]);
 
   useEffect(() => {
     setDataMaskSelected(() => dataMaskApplied);
@@ -189,33 +190,8 @@ const FilterBar: React.FC<FiltersBarProps> = ({
   ) => {
     setIsFilterSetChanged(tab !== TabIds.AllFilters);
     setDataMaskSelected(draft => {
-      // check if a filter has preselect filters
-      if (
-        preselectNativeFilters?.[filter.id] !== undefined &&
-        !initializedFilters.includes(filter.id)
-      ) {
-        /**
-         * since preselect filters don't have extraFormData, they need to iterate
-         * a few times to populate the full state necessary for proper filtering.
-         * Once both filterState and extraFormData are identical, we can coclude
-         * that the filter has been fully initialized.
-         */
-        if (
-          areObjectsEqual(
-            dataMask.filterState,
-            dataMaskSelected[filter.id]?.filterState,
-          ) &&
-          areObjectsEqual(
-            dataMask.extraFormData,
-            dataMaskSelected[filter.id]?.extraFormData,
-          )
-        ) {
-          setInitializedFilters(prevState => [...prevState, filter.id]);
-        }
-        dispatch(updateDataMask(filter.id, dataMask));
-      }
       // force instant updating on initialization for filters with `requiredFirst` is true or instant filters
-      else if (
+      if (
         // filterState.value === undefined - means that value not initialized
         dataMask.filterState?.value !== undefined &&
         dataMaskSelected[filter.id]?.filterState?.value === undefined &&
@@ -231,6 +207,37 @@ const FilterBar: React.FC<FiltersBarProps> = ({
     });
   };
 
+  const publishDataMask = useCallback(
+    (dataMaskSelected: DataMaskStateWithId) => {
+      const { location } = history;
+      const { search } = location;
+      const previousParams = new URLSearchParams(search);
+      const newParams = new URLSearchParams();
+
+      previousParams.forEach((value, key) => {
+        if (key !== URL_PARAMS.nativeFilters.name) {
+          newParams.append(key, value);
+        }
+      });
+
+      newParams.set(
+        URL_PARAMS.nativeFilters.name,
+        rison.encode(replaceUndefinedByNull(dataMaskSelected)),
+      );
+
+      history.replace({
+        search: newParams.toString(),
+      });
+    },
+    [history],
+  );
+
+  const dataMaskAppliedText = JSON.stringify(dataMaskApplied);
+  useEffect(() => {
+    publishDataMask(dataMaskApplied);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [dataMaskAppliedText, publishDataMask]);
+
   const handleApply = () => {
     const filterIds = Object.keys(dataMaskSelected);
     filterIds.forEach(filterId => {
@@ -240,6 +247,15 @@ const FilterBar: React.FC<FiltersBarProps> = ({
     });
   };
 
+  const handleClearAll = () => {
+    const filterIds = Object.keys(dataMaskSelected);
+    filterIds.forEach(filterId => {
+      if (dataMaskSelected[filterId]) {
+        dispatch(clearDataMask(filterId));
+      }
+    });
+  };
+
   useFilterUpdates(dataMaskSelected, setDataMaskSelected);
   const isApplyDisabled = checkIsApplyDisabled(
     dataMaskSelected,
@@ -270,6 +286,7 @@ const FilterBar: React.FC<FiltersBarProps> = ({
         <Header
           toggleFiltersBar={toggleFiltersBar}
           onApply={handleApply}
+          onClearAll={handleClearAll}
           isApplyDisabled={isApplyDisabled}
           dataMaskSelected={dataMaskSelected}
           dataMaskApplied={dataMaskApplied}

superset-frontend/src/dashboard/util/getDashboardUrl.ts

@@ -19,6 +19,7 @@
 import rison from 'rison';
 import { JsonObject } from '@superset-ui/core';
 import { URL_PARAMS } from 'src/constants';
+import replaceUndefinedByNull from './replaceUndefinedByNull';
 import serializeActiveFilterValues from './serializeActiveFilterValues';
 import { DataMaskState } from '../../dataMask/types';
 
@@ -49,19 +50,9 @@ export default function getDashboardUrl({
   }
 
   if (dataMask) {
-    const filterStates = Object.entries(dataMask).reduce(
-      (agg, [key, value]) => {
-        const filterState = value?.filterState?.value;
-        return {
-          ...agg,
-          [key]: filterState || null,
-        };
-      },
-      {},
-    );
     newSearchParams.set(
       URL_PARAMS.nativeFilters.name,
-      rison.encode(filterStates),
+      rison.encode(replaceUndefinedByNull(dataMask)),
     );
   }
 

superset-frontend/src/dashboard/util/replaceUndefinedByNull.ts

@@ -0,0 +1,36 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { cloneDeep } from 'lodash';
+
+function processObject(object: Object) {
+  const result = object;
+  Object.keys(result).forEach(key => {
+    if (result[key] === undefined) {
+      result[key] = null;
+    } else if (result[key] !== null && typeof result[key] === 'object') {
+      result[key] = processObject(result[key]);
+    }
+  });
+  return result;
+}
+
+export default function replaceUndefinedByNull(object: Object) {
+  const copy = cloneDeep(object);
+  return processObject(copy);
+}

superset-frontend/src/dataMask/reducer.ts

@@ -24,6 +24,8 @@ 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 { getUrlParam } from 'src/utils/urlUtils';
+import { URL_PARAMS } from 'src/constants';
 import { DataMaskStateWithId, DataMaskWithId } from './types';
 import {
   AnyDataMaskAction,
@@ -68,13 +70,13 @@ function fillNativeFilters(
   draftDataMask: DataMaskStateWithId,
   currentFilters?: Filters,
 ) {
+  const dataMaskFromUrl = getUrlParam(URL_PARAMS.nativeFilters) || {};
   filterConfig.forEach((filter: Filter) => {
     mergedDataMask[filter.id] = {
       ...getInitialDataMask(filter.id), // take initial data
       ...filter.defaultDataMask, // if something new came from BE - take it
-      ...draftDataMask[filter.id], // keep local filter data
+      ...dataMaskFromUrl[filter.id],
     };
-    // if we came from filters config modal and particular filters changed take it's dataMask
     if (
       currentFilters &&
       !areObjectsEqual(

superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx

@@ -64,16 +64,16 @@ export default function PluginFilterTimeColumn(
     });
   };
 
-  useEffect(() => {
-    handleChange(filterState.value ?? null);
-  }, [JSON.stringify(filterState.value)]);
-
   useEffect(() => {
     handleChange(defaultValue ?? null);
     // 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)]);
 
+  useEffect(() => {
+    handleChange(filterState.value ?? null);
+  }, [JSON.stringify(filterState.value)]);
+
   const timeColumns = (data || []).filter(
     row => row.dtype === GenericDataType.TEMPORAL,
   );

superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx

@@ -78,16 +78,16 @@ export default function PluginFilterTimegrain(
     });
   };
 
-  useEffect(() => {
-    handleChange(filterState.value ?? []);
-  }, [JSON.stringify(filterState.value)]);
-
   useEffect(() => {
     handleChange(defaultValue ?? []);
     // 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)]);
 
+  useEffect(() => {
+    handleChange(filterState.value ?? []);
+  }, [JSON.stringify(filterState.value)]);
+
   const placeholderText =
     (data || []).length === 0
       ? t('No data')