diff --git a/superset-frontend/spec/helpers/testing-library.tsx b/superset-frontend/spec/helpers/testing-library.tsx index 7366b8d4c..56489cce8 100644 --- a/superset-frontend/spec/helpers/testing-library.tsx +++ b/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 & { 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 = {result}; } + if (useRouter) { + result = {result}; + } + return result; }; } diff --git a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js b/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js index c986c6fb4..8ceac1ab2 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getDashboardUrl_spec.js +++ b/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', ); }); }); 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 4507064ea..d76f09f85 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx +++ b/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( - - - , - ); + render(, { + 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' }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx index 159178023..64a2dd1d4 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/Header.test.tsx +++ b/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: { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx index e1d273ed1..16714ac8a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Header/index.tsx +++ b/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 = ({ onApply, + onClearAll, isApplyDisabled, dataMaskSelected, dataMaskApplied, @@ -82,21 +83,11 @@ const Header: FC = ({ }) => { const theme = useTheme(); const filters = useFilters(); - const dispatch = useDispatch(); const filterValues = Object.values(filters); const canEdit = useSelector( ({ 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 = ({ disabled={isClearAllDisabled} buttonStyle="tertiary" buttonSize="small" - onClick={handleClearAll} + onClick={onClearAll} {...getFilterBarTestId('clear-button')} > {t('Clear all')} diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index e175fc012..9a96331dd 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/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 = ({ height, offset, }) => { + const history = useHistory(); const dataMaskApplied: DataMaskStateWithId = useNativeFiltersDataMask(); const [editFilterSetId, setEditFilterSetId] = useState(null); const [dataMaskSelected, setDataMaskSelected] = useImmer( @@ -158,8 +161,6 @@ const FilterBar: React.FC = ({ const previousFilters = usePrevious(filters); const filterValues = Object.values(filters); const [isFilterSetChanged, setIsFilterSetChanged] = useState(false); - const preselectNativeFilters = usePreselectNativeFilters(); - const [initializedFilters, setInitializedFilters] = useState([]); useEffect(() => { setDataMaskSelected(() => dataMaskApplied); @@ -189,33 +190,8 @@ const FilterBar: React.FC = ({ ) => { 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 = ({ }); }; + 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 = ({ }); }; + 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 = ({
{ - const filterState = value?.filterState?.value; - return { - ...agg, - [key]: filterState || null, - }; - }, - {}, - ); newSearchParams.set( URL_PARAMS.nativeFilters.name, - rison.encode(filterStates), + rison.encode(replaceUndefinedByNull(dataMask)), ); } diff --git a/superset-frontend/src/dashboard/util/replaceUndefinedByNull.ts b/superset-frontend/src/dashboard/util/replaceUndefinedByNull.ts new file mode 100644 index 000000000..bcf5004b3 --- /dev/null +++ b/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); +} diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index 4a4e240d4..f5fd88339 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/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( diff --git a/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx b/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx index 6a1ab956b..d49e756a5 100644 --- a/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx +++ b/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, ); diff --git a/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx b/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx index cb615c898..5db92c3c3 100644 --- a/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx +++ b/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')