From 3fba9678565cbbffd048172eb3e01fac50ba31bc Mon Sep 17 00:00:00 2001 From: Ramiro Aquino Romero Date: Tue, 20 Jan 2026 14:18:47 -0400 Subject: [PATCH] fix(delete-filter): deleted native filters are still shown until [sc-96553] (#37012) --- .../src/dashboard/actions/nativeFilters.ts | 7 +- .../dashboard/reducers/nativeFilters.test.ts | 180 +++++++++++++++++- .../src/dashboard/reducers/nativeFilters.ts | 15 +- 3 files changed, 195 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/dashboard/actions/nativeFilters.ts b/superset-frontend/src/dashboard/actions/nativeFilters.ts index 4a628af3f4e..af3d711495a 100644 --- a/superset-frontend/src/dashboard/actions/nativeFilters.ts +++ b/superset-frontend/src/dashboard/actions/nativeFilters.ts @@ -21,6 +21,9 @@ import { FilterConfiguration, Filters, makeApi, + Divider, + ChartCustomization, + ChartCustomizationDivider, } from '@superset-ui/core'; import { Dispatch } from 'redux'; import { RootState } from 'src/dashboard/types'; @@ -44,7 +47,9 @@ export const SET_NATIVE_FILTERS_CONFIG_COMPLETE = 'SET_NATIVE_FILTERS_CONFIG_COMPLETE'; export interface SetNativeFiltersConfigComplete { type: typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE; - filterChanges: Filter[]; + filterChanges: Array< + Filter | Divider | ChartCustomization | ChartCustomizationDivider + >; } export const SET_NATIVE_FILTERS_CONFIG_FAIL = 'SET_NATIVE_FILTERS_CONFIG_FAIL'; diff --git a/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts b/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts index 65c4e546357..ceac844317b 100644 --- a/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts +++ b/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts @@ -16,7 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import { Filter, NativeFilterType } from '@superset-ui/core'; +import { + Filter, + NativeFilterType, + ChartCustomization, + ChartCustomizationType, +} from '@superset-ui/core'; import nativeFilterReducer from './nativeFilters'; import { SET_NATIVE_FILTERS_CONFIG_COMPLETE } from '../actions/nativeFilters'; import { HYDRATE_DASHBOARD } from '../actions/hydrate'; @@ -58,6 +63,40 @@ const createMockFilter = ( tabsInScope, }); +const createMockChartCustomization = ( + id: string, + chartsInScope?: number[], + tabsInScope?: string[], +): ChartCustomization => ({ + id, + type: ChartCustomizationType.ChartCustomization, + name: `Chart Customization ${id}`, + filterType: 'filter_select', + targets: [ + { + datasetId: 0, + column: { + name: 'test column', + displayName: 'test column', + }, + }, + ], + scope: { + rootPath: [], + excluded: [], + }, + defaultDataMask: { + filterState: { + value: null, + }, + }, + controlValues: { + sortAscending: true, + }, + chartsInScope, + tabsInScope, +}); + test('SET_NATIVE_FILTERS_CONFIG_COMPLETE updates filters with complete scope properties', () => { const initialState = { filters: { @@ -237,3 +276,142 @@ test('HYDRATE_DASHBOARD handles new filters without existing state', () => { expect(result.filters.filter1.chartsInScope).toEqual([1, 2]); expect(result.filters.filter1.tabsInScope).toEqual(['tab1']); }); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE removes deleted filters from state', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2], ['tab1']), + filter2: createMockFilter('filter2', [3, 4], ['tab2']), + filter3: createMockFilter('filter3', [5, 6], ['tab3']), + }, + }; + + // Backend response only includes filter1 and filter3 (filter2 was deleted) + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [ + createMockFilter('filter1', [1, 2], ['tab1']), + createMockFilter('filter3', [5, 6], ['tab3']), + ], + }; + + const result = nativeFilterReducer(initialState, action); + + // filter2 should be removed from state + expect(result.filters.filter1).toBeDefined(); + expect(result.filters.filter2).toBeUndefined(); + expect(result.filters.filter3).toBeDefined(); + expect(Object.keys(result.filters)).toHaveLength(2); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE removes all filters when backend returns empty array', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2], ['tab1']), + filter2: createMockFilter('filter2', [3, 4], ['tab2']), + }, + }; + + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(Object.keys(result.filters)).toHaveLength(0); + expect(result.filters).toEqual({}); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE handles mixed Filter and ChartCustomization types', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2], ['tab1']), + customization1: createMockChartCustomization( + 'customization1', + [3, 4], + ['tab2'], + ), + }, + }; + + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [ + createMockFilter('filter1', [5, 6], ['tab3']), + createMockChartCustomization('customization1', [7, 8], ['tab4']), + ], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(result.filters.filter1.chartsInScope).toEqual([5, 6]); + expect(result.filters.filter1.tabsInScope).toEqual(['tab3']); + expect(result.filters.customization1.chartsInScope).toEqual([7, 8]); + expect(result.filters.customization1.tabsInScope).toEqual(['tab4']); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE adds new filters while removing deleted ones', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2], ['tab1']), + filter2: createMockFilter('filter2', [3, 4], ['tab2']), + }, + }; + + // Backend response: keep filter1, delete filter2, add filter3 + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [ + createMockFilter('filter1', [1, 2], ['tab1']), + createMockFilter('filter3', [5, 6], ['tab3']), + ], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(result.filters.filter1).toBeDefined(); + expect(result.filters.filter2).toBeUndefined(); + expect(result.filters.filter3).toBeDefined(); + expect(result.filters.filter3.chartsInScope).toEqual([5, 6]); + expect(Object.keys(result.filters)).toHaveLength(2); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE treats backend response as source of truth', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2], ['tab1']), + filter2: createMockFilter('filter2', [3, 4], ['tab2']), + filter3: createMockFilter('filter3', [5, 6], ['tab3']), + customization1: createMockChartCustomization( + 'customization1', + [7, 8], + ['tab4'], + ), + }, + }; + + // Backend only returns filter2 and customization1 + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [ + createMockFilter('filter2', [10, 11], ['tab5']), + createMockChartCustomization('customization1', [12, 13], ['tab6']), + ], + }; + + const result = nativeFilterReducer(initialState, action); + + // Only filter2 and customization1 should remain + expect(result.filters.filter1).toBeUndefined(); + expect(result.filters.filter2).toBeDefined(); + expect(result.filters.filter3).toBeUndefined(); + expect(result.filters.customization1).toBeDefined(); + expect(Object.keys(result.filters)).toHaveLength(2); + + // Values should be from backend response + expect(result.filters.filter2.chartsInScope).toEqual([10, 11]); + expect(result.filters.filter2.tabsInScope).toEqual(['tab5']); + expect(result.filters.customization1.chartsInScope).toEqual([12, 13]); + expect(result.filters.customization1.tabsInScope).toEqual(['tab6']); +}); diff --git a/superset-frontend/src/dashboard/reducers/nativeFilters.ts b/superset-frontend/src/dashboard/reducers/nativeFilters.ts index c1b4c924fa4..37f406dbaf6 100644 --- a/superset-frontend/src/dashboard/reducers/nativeFilters.ts +++ b/superset-frontend/src/dashboard/reducers/nativeFilters.ts @@ -77,13 +77,18 @@ function handleFilterChangesComplete( Filter | Divider | ChartCustomization | ChartCustomizationDivider >, ) { - const modifiedFilters = { ...state.filters }; + // Create new filters object from backend response (deleted filters won't be included) + const newFilters: Record< + string, + Filter | Divider | ChartCustomization | ChartCustomizationDivider + > = {}; + filters.forEach(filter => { + const existingFilter = state.filters[filter.id]; if (filter.chartsInScope != null && filter.tabsInScope != null) { - modifiedFilters[filter.id] = filter; + newFilters[filter.id] = filter; } else { - const existingFilter = modifiedFilters[filter.id]; - modifiedFilters[filter.id] = { + newFilters[filter.id] = { ...filter, chartsInScope: filter.chartsInScope ?? existingFilter?.chartsInScope, tabsInScope: filter.tabsInScope ?? existingFilter?.tabsInScope, @@ -93,7 +98,7 @@ function handleFilterChangesComplete( return { ...state, - filters: modifiedFilters, + filters: newFilters, } as ExtendedNativeFiltersState; }