diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts index 660f422699..2afb9fef7e 100644 --- a/superset-frontend/src/dashboard/actions/nativeFilters.ts +++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts @@ -74,6 +74,7 @@ export const setFilterConfiguration = ( filterConfig, }); const { id, metadata } = getState().dashboardInfo; + const oldFilters = getState().nativeFilters?.filters; // TODO extract this out when makeApi supports url parameters const updateDashboard = makeApi< @@ -100,7 +101,7 @@ export const setFilterConfiguration = ( type: SET_FILTER_CONFIG_COMPLETE, filterConfig, }); - dispatch(setDataMaskForFilterConfigComplete(filterConfig)); + dispatch(setDataMaskForFilterConfigComplete(filterConfig, oldFilters)); } catch (err) { dispatch({ type: SET_FILTER_CONFIG_FAIL, filterConfig }); dispatch({ type: SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL, filterConfig }); 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 afe0c0a8d1..4507e381d6 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.test.tsx @@ -18,7 +18,7 @@ */ import React from 'react'; -import { render, screen, cleanup } from 'spec/helpers/testing-library'; +import { render, screen } from 'spec/helpers/testing-library'; import { Provider } from 'react-redux'; import userEvent from '@testing-library/user-event'; import { @@ -71,7 +71,7 @@ const getDateControlTestId = testWithId( const FILTER_NAME = 'Time filter 1'; const FILTER_SET_NAME = 'New filter set'; -const addFilterFlow = () => { +const addFilterFlow = async () => { // open filter config modal userEvent.click(screen.getByTestId(getTestId('collapsable'))); userEvent.click(screen.getByTestId(getTestId('create-filter'))); @@ -80,6 +80,7 @@ const addFilterFlow = () => { userEvent.click(screen.getByText('Time filter')); userEvent.type(screen.getByTestId(getModalTestId('name-input')), FILTER_NAME); userEvent.click(screen.getByText('Save')); + await screen.findByText('All Filters (1)'); }; const addFilterSetFlow = async () => { @@ -175,7 +176,7 @@ describe('FilterBar', () => { }); beforeEach(() => { - toggleFiltersBar.mockClear(); + jest.clearAllMocks(); fetchMock.get( 'http://localhost/api/v1/time_range/?q=%27Last%20day%27', { @@ -203,11 +204,6 @@ describe('FilterBar', () => { mockCore.makeApi = jest.fn(() => mockApi); }); - afterEach(() => { - cleanup(); - jest.clearAllMocks(); - }); - const renderWrapper = (props = closedBarProps, state?: object) => render( @@ -306,14 +302,12 @@ describe('FilterBar', () => { renderWrapper(openedBarProps, stateWithoutNativeFilters); expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); - addFilterFlow(); + await addFilterFlow(); - await screen.findByText('All Filters (1)'); expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); }); - // TODO: fix flakiness and re-enable - it.skip('add and apply filter set', async () => { + it('add and apply filter set', async () => { // @ts-ignore global.featureFlags = { [FeatureFlag.DASHBOARD_NATIVE_FILTERS]: true, @@ -321,22 +315,17 @@ describe('FilterBar', () => { }; renderWrapper(openedBarProps, stateWithoutNativeFilters); - addFilterFlow(); - - await screen.findByText('All Filters (1)'); + await addFilterFlow(); await addFilterSetFlow(); // change filter expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); - + userEvent.click(await screen.findByText('All Filters (1)')); await changeFilterValue(); await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(2)); // apply new filter value - await waitFor(() => - expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(), - ); userEvent.click(screen.getByTestId(getTestId('apply-button'))); await waitFor(() => expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(), @@ -352,12 +341,11 @@ describe('FilterBar', () => { ).not.toHaveAttribute('data-selected', 'true'); userEvent.click(screen.getByTestId(getTestId('filter-set-wrapper'))); expect(await screen.findByText('Last week')).toBeInTheDocument(); - expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(); userEvent.click(screen.getByTestId(getTestId('apply-button'))); expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); }); - it.skip('add and edit filter set', async () => { + it('add and edit filter set', async () => { // @ts-ignore global.featureFlags = { [FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET]: true, @@ -365,9 +353,7 @@ describe('FilterBar', () => { }; renderWrapper(openedBarProps, stateWithoutNativeFilters); - addFilterFlow(); - - await screen.findByText('All Filters (1)'); + await addFilterFlow(); await addFilterSetFlow(); @@ -375,12 +361,13 @@ describe('FilterBar', () => { userEvent.click(screen.getByText('Edit')); await changeFilterValue(); - await waitFor(() => expect(screen.getAllByText('Last day').length).toBe(1)); // apply new changes and save them - expect( - screen.getByTestId(getTestId('filter-set-edit-save')), - ).toBeDisabled(); + await waitFor(() => + expect( + screen.getByTestId(getTestId('filter-set-edit-save')), + ).toBeDisabled(), + ); expect(screen.getByTestId(getTestId('apply-button'))).toBeEnabled(); userEvent.click(screen.getByTestId(getTestId('apply-button'))); expect(screen.getByTestId(getTestId('apply-button'))).toBeDisabled(); 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 82a46bf781..ae20a88860 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ControlItems.tsx @@ -23,13 +23,15 @@ import { import React, { FC } from 'react'; import { Checkbox } from 'src/common/components'; import { FormInstance } from 'antd/lib/form'; -import { getChartControlPanelRegistry } from '@superset-ui/core'; +import { getChartControlPanelRegistry, t } from '@superset-ui/core'; +import { Tooltip } from 'src/components/Tooltip'; import { getControlItems, setNativeFilterFieldValues } from './utils'; import { NativeFiltersForm, NativeFiltersFormItem } from '../types'; import { StyledCheckboxFormItem } from './FiltersConfigForm'; import { Filter } from '../../types'; type ControlItemsProps = { + disabled: boolean; filterId: string; forceUpdate: Function; filterToEdit?: Filter; @@ -38,6 +40,7 @@ type ControlItemsProps = { }; const ControlItems: FC = ({ + disabled, forceUpdate, form, filterId, @@ -59,38 +62,48 @@ const ControlItems: FC = ({ controlItem?.config?.renderTrigger, ) .map(controlItem => ( - - { - if (!controlItem.config.resetConfig) { - forceUpdate(); - return; - } - setNativeFilterFieldValues(form, filterId, { - defaultDataMask: null, - }); - forceUpdate(); - }} + - {controlItem.config.label}{' '} - {controlItem.config.description && ( - - )} - - + { + if (!controlItem.config.resetConfig) { + forceUpdate(); + return; + } + setNativeFilterFieldValues(form, filterId, { + defaultDataMask: null, + }); + forceUpdate(); + }} + > + {controlItem.config.label}{' '} + {controlItem.config.description && ( + + )} + + + ))} ); 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 d3c3298b69..fe5d048474 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -260,6 +260,8 @@ export const FiltersConfigForm: React.FC = ({ label: filter.title, })); + const showDefaultValue = !hasDataset || (!isDataDirty && hasFilledDataset); + return ( <> {t('Settings')} @@ -434,7 +436,7 @@ export const FiltersConfigForm: React.FC = ({ data-test="default-input" label={{t('Default Value')}} > - {(!hasDataset || (!isDataDirty && hasFilledDataset)) && ( + {showDefaultValue ? ( { setNativeFilterFieldValues(form, filterId, { @@ -447,6 +449,10 @@ export const FiltersConfigForm: React.FC = ({ form={form} formData={newFormData} /> + ) : hasFilledDataset ? ( + t('Click "Populate" to get "Default Value" ->') + ) : ( + t('Fill all required fields to enable "Default Value"') )} @@ -461,6 +467,7 @@ export const FiltersConfigForm: React.FC = ({ { const isInstant = formValues.filters[filterId] ? formValues.filters[filterId].isInstant diff --git a/superset-frontend/src/dataMask/actions.ts b/superset-frontend/src/dataMask/actions.ts index 7a9bb3f387..76a286c833 100644 --- a/superset-frontend/src/dataMask/actions.ts +++ b/superset-frontend/src/dataMask/actions.ts @@ -19,6 +19,7 @@ import { DataMask } from '@superset-ui/core'; import { FilterConfiguration } from '../dashboard/components/nativeFilters/types'; import { FeatureFlag, isFeatureEnabled } from '../featureFlags'; +import { Filters } from '../dashboard/reducers/types'; export const UPDATE_DATA_MASK = 'UPDATE_DATA_MASK'; export interface UpdateDataMask { @@ -33,6 +34,7 @@ export const SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE = export interface SetDataMaskForFilterConfigComplete { type: typeof SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE; filterConfig: FilterConfiguration; + filters?: Filters; } export const SET_DATA_MASK_FOR_FILTER_CONFIG_FAIL = @@ -44,10 +46,12 @@ export interface SetDataMaskForFilterConfigFail { } export function setDataMaskForFilterConfigComplete( filterConfig: FilterConfiguration, + filters?: Filters, ): SetDataMaskForFilterConfigComplete { return { type: SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE, filterConfig, + filters, }; } export function updateDataMask( diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index c4ee71fba2..a0b0e38609 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -34,6 +34,8 @@ import { Filter, FilterConfiguration, } from '../dashboard/components/nativeFilters/types'; +import { areObjectsEqual } from '../reduxUtils'; +import { Filters } from '../dashboard/reducers/types'; export function getInitialDataMask(id?: string): DataMask; export function getInitialDataMask(id: string): DataMaskWithId { @@ -54,21 +56,37 @@ export function getInitialDataMask(id: string): DataMaskWithId { } function fillNativeFilters( - data: FilterConfiguration, - cleanState: DataMaskStateWithId, - draft: DataMaskStateWithId, + filterConfig: FilterConfiguration, + mergedDataMask: DataMaskStateWithId, + draftDataMask: DataMaskStateWithId, + currentFilters?: Filters, ) { - data.forEach((filter: Filter) => { - cleanState[filter.id] = { + filterConfig.forEach((filter: Filter) => { + mergedDataMask[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 + ...draftDataMask[filter.id], // keep local filter data }; + // if we came from filters config modal and particular filters changed take it's dataMask + if ( + currentFilters && + !areObjectsEqual( + filter.defaultDataMask, + currentFilters[filter.id]?.defaultDataMask, + { ignoreUndefined: true }, + ) + ) { + mergedDataMask[filter.id] = { + ...mergedDataMask[filter.id], + ...filter.defaultDataMask, + }; + } }); + // Get back all other non-native filters - Object.values(draft).forEach(filter => { + Object.values(draftDataMask).forEach(filter => { if (!String(filter?.id).startsWith(NATIVE_FILTER_PREFIX)) { - cleanState[filter?.id] = filter; + mergedDataMask[filter?.id] = filter; } }); } @@ -106,7 +124,12 @@ const dataMaskReducer = produce( ); return cleanState; case SET_DATA_MASK_FOR_FILTER_CONFIG_COMPLETE: - fillNativeFilters(action.filterConfig ?? [], cleanState, draft); + fillNativeFilters( + action.filterConfig ?? [], + cleanState, + draft, + action.filters, + ); return cleanState; default: diff --git a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts index 33922d0ec6..5b9b898e64 100644 --- a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts +++ b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts @@ -37,6 +37,7 @@ const config: ControlPanelConfig = { type: 'CheckboxControl', label: t('Multiple select'), default: multiSelect, + affectsDataMask: true, resetConfig: true, renderTrigger: true, description: t('Allow selecting multiple values'), diff --git a/superset-frontend/src/filters/components/Select/controlPanel.ts b/superset-frontend/src/filters/components/Select/controlPanel.ts index a866c987fe..1fad9305b6 100644 --- a/superset-frontend/src/filters/components/Select/controlPanel.ts +++ b/superset-frontend/src/filters/components/Select/controlPanel.ts @@ -61,6 +61,7 @@ const config: ControlPanelConfig = { label: t('Multiple select'), default: multiSelect, resetConfig: true, + affectsDataMask: true, renderTrigger: true, description: t('Allow selecting multiple values'), }, @@ -89,6 +90,7 @@ const config: ControlPanelConfig = { label: t('Default to first item'), default: defaultToFirstItem, resetConfig: true, + affectsDataMask: true, renderTrigger: true, description: t('Select first item by default'), }, @@ -100,6 +102,7 @@ const config: ControlPanelConfig = { config: { type: 'CheckboxControl', renderTrigger: true, + affectsDataMask: true, label: t('Inverse selection'), default: inverseSelection, description: t('Exclude selected values'),