diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx new file mode 100644 index 00000000000..126b25d3458 --- /dev/null +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.test.tsx @@ -0,0 +1,389 @@ +/** + * 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 { render, waitFor } from 'spec/helpers/testing-library'; +import fetchMock from 'fetch-mock'; +import { storeWithState } from 'spec/fixtures/mockStore'; +import mockState from 'spec/fixtures/mockState'; +import { sliceId } from 'spec/fixtures/mockChartQueries'; +import { NativeFilterType } from '@superset-ui/core'; +import { CHART_TYPE } from '../../util/componentTypes'; +import DashboardContainer from './DashboardContainer'; +import * as nativeFiltersActions from '../../actions/nativeFilters'; + +fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {}); +fetchMock.put('glob:*/api/v1/dashboard/*/colors*', {}); +fetchMock.post('glob:*/superset/log/?*', {}); + +jest.mock('@visx/responsive', () => ({ + ParentSize: ({ + children, + }: { + children: (props: { width: number }) => JSX.Element; + }) => children({ width: 800 }), +})); + +jest.mock('src/dashboard/containers/DashboardGrid', () => ({ + __esModule: true, + default: () =>
, +})); + +function createTestState(overrides = {}) { + return { + ...mockState, + dashboardState: { + ...mockState.dashboardState, + sliceIds: [sliceId], + }, + dashboardLayout: { + ...mockState.dashboardLayout, + present: { + ...mockState.dashboardLayout.present, + CHART_ID: { + id: 'CHART_ID', + type: CHART_TYPE, + meta: { + chartId: sliceId, + width: 4, + height: 10, + }, + parents: ['ROOT_ID', 'GRID_ID', 'ROW_ID'], + }, + }, + }, + nativeFilters: { + filters: { + 'FILTER-1': { + id: 'FILTER-1', + name: 'Test Filter', + filterType: 'filter_select', + targets: [ + { + datasetId: 1, + column: { name: 'country' }, + }, + ], + defaultDataMask: { + filterState: { value: null }, + }, + cascadeParentIds: [], + scope: { + rootPath: ['ROOT_ID'], + excluded: [], + }, + controlValues: {}, + type: NativeFilterType.NativeFilter, + }, + }, + }, + ...overrides, + }; +} + +function setup(overrideState = {}) { + const initialState = createTestState(overrideState); + return render(, { + useRedux: true, + store: storeWithState(initialState), + }); +} + +function setupWithStore(overrideState = {}) { + const initialState = createTestState(overrideState); + const store = storeWithState(initialState); + const renderResult = render(, { + useRedux: true, + store, + }); + return { store, ...renderResult }; +} + +let setInScopeStatusMock: jest.SpyInstance; + +beforeEach(() => { + setInScopeStatusMock = jest.spyOn( + nativeFiltersActions, + 'setInScopeStatusOfFilters', + ); + setInScopeStatusMock.mockReturnValue(jest.fn()); +}); + +afterEach(() => { + setInScopeStatusMock.mockRestore(); +}); + +test('calculates chartsInScope correctly for filters', async () => { + setup(); + + await waitFor(() => { + expect(setInScopeStatusMock).toHaveBeenCalled(); + }); + + expect(setInScopeStatusMock).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + filterId: 'FILTER-1', + chartsInScope: [sliceId], + }), + ]), + ); +}); + +test('recalculates chartsInScope when filter non-scope properties change', async () => { + const { store } = setupWithStore(); + + await waitFor(() => { + expect(setInScopeStatusMock).toHaveBeenCalled(); + }); + + setInScopeStatusMock.mockClear(); + + // Bug scenario: Editing non-scope properties (e.g., "Sort filter values") + // triggers backend save, but response lacks chartsInScope. + // The fix ensures useEffect recalculates chartsInScope anyway. + const initialState = store.getState(); + store.dispatch({ + type: 'SET_NATIVE_FILTERS_CONFIG_COMPLETE', + filterChanges: [ + { + ...initialState.nativeFilters.filters['FILTER-1'], + controlValues: { + ...initialState.nativeFilters.filters['FILTER-1'].controlValues, + sortAscending: false, + }, + }, + ], + }); + + await waitFor(() => { + expect(setInScopeStatusMock).toHaveBeenCalled(); + }); +}); + +test('handles multiple filters with different scopes', async () => { + const baseDashboardLayout = mockState.dashboardLayout.present; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { CHART_ID: _removed, ...cleanLayout } = baseDashboardLayout; + + const stateWithMultipleFilters = { + ...mockState, + dashboardState: { + ...mockState.dashboardState, + sliceIds: [18, 19], + }, + dashboardLayout: { + ...mockState.dashboardLayout, + present: { + ...cleanLayout, + CHART_ID_1: { + id: 'CHART_ID_1', + type: CHART_TYPE, + meta: { chartId: 18, width: 4, height: 10 }, + parents: ['ROOT_ID', 'GRID_ID', 'ROW_ID'], + }, + CHART_ID_2: { + id: 'CHART_ID_2', + type: CHART_TYPE, + meta: { chartId: 19, width: 4, height: 10 }, + parents: ['ROOT_ID', 'GRID_ID', 'ROW_ID'], + }, + }, + }, + nativeFilters: { + filters: { + 'FILTER-1': { + id: 'FILTER-1', + name: 'Filter 1', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'country' } }], + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + controlValues: {}, + type: NativeFilterType.NativeFilter, + }, + 'FILTER-2': { + id: 'FILTER-2', + name: 'Filter 2', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'region' } }], + scope: { rootPath: ['ROOT_ID'], excluded: [19] }, + controlValues: {}, + type: NativeFilterType.NativeFilter, + }, + }, + }, + }; + + setup(stateWithMultipleFilters); + + await waitFor(() => { + expect(setInScopeStatusMock).toHaveBeenCalled(); + }); + + expect(setInScopeStatusMock).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + filterId: 'FILTER-1', + chartsInScope: expect.arrayContaining([18, 19]), + }), + expect.objectContaining({ + filterId: 'FILTER-2', + chartsInScope: [18], + }), + ]), + ); +}); + +test('handles filters with no charts in scope', async () => { + const stateWithExcludedFilter = createTestState({ + nativeFilters: { + filters: { + 'FILTER-1': { + id: 'FILTER-1', + name: 'Excluded Filter', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'country' } }], + scope: { + rootPath: ['ROOT_ID'], + excluded: [sliceId], + }, + controlValues: {}, + type: NativeFilterType.NativeFilter, + }, + }, + }, + }); + + setup(stateWithExcludedFilter); + + await waitFor(() => { + expect(setInScopeStatusMock).toHaveBeenCalled(); + }); + + expect(setInScopeStatusMock).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + filterId: 'FILTER-1', + chartsInScope: [], + }), + ]), + ); +}); + +test('does not dispatch when there are no filters', () => { + const stateWithoutFilters = createTestState({ + nativeFilters: { + filters: {}, + }, + }); + + setup(stateWithoutFilters); + + expect(setInScopeStatusMock).not.toHaveBeenCalled(); +}); + +test('calculates tabsInScope for filters with tab-scoped charts', async () => { + const baseDashboardLayout = mockState.dashboardLayout.present; + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { CHART_ID: _removed, ...cleanLayout } = baseDashboardLayout; + + const stateWithTabs = { + ...mockState, + dashboardState: { + ...mockState.dashboardState, + sliceIds: [20, 21, 22], + }, + dashboardLayout: { + ...mockState.dashboardLayout, + present: { + ...cleanLayout, + ROOT_ID: { + id: 'ROOT_ID', + type: 'ROOT', + children: ['TABS-1'], + }, + 'TABS-1': { + id: 'TABS-1', + type: 'TABS', + children: ['TAB-A', 'TAB-B'], + parents: ['ROOT_ID'], + }, + 'TAB-A': { + id: 'TAB-A', + type: 'TAB', + children: ['CHART-A1', 'CHART-A2'], + parents: ['ROOT_ID', 'TABS-1'], + meta: { text: 'Tab A' }, + }, + 'TAB-B': { + id: 'TAB-B', + type: 'TAB', + children: ['CHART-B1'], + parents: ['ROOT_ID', 'TABS-1'], + meta: { text: 'Tab B' }, + }, + 'CHART-A1': { + id: 'CHART-A1', + type: CHART_TYPE, + meta: { chartId: 20, width: 4, height: 10 }, + parents: ['ROOT_ID', 'TABS-1', 'TAB-A'], + }, + 'CHART-A2': { + id: 'CHART-A2', + type: CHART_TYPE, + meta: { chartId: 21, width: 4, height: 10 }, + parents: ['ROOT_ID', 'TABS-1', 'TAB-A'], + }, + 'CHART-B1': { + id: 'CHART-B1', + type: CHART_TYPE, + meta: { chartId: 22, width: 4, height: 10 }, + parents: ['ROOT_ID', 'TABS-1', 'TAB-B'], + }, + }, + }, + nativeFilters: { + filters: { + 'FILTER-TAB-SCOPED': { + id: 'FILTER-TAB-SCOPED', + name: 'Tab Scoped Filter', + filterType: 'filter_select', + targets: [{ datasetId: 1, column: { name: 'region' } }], + scope: { rootPath: ['ROOT_ID'], excluded: [22] }, + controlValues: {}, + type: NativeFilterType.NativeFilter, + }, + }, + }, + }; + + setup(stateWithTabs); + + await waitFor(() => { + expect(setInScopeStatusMock).toHaveBeenCalled(); + }); + + expect(setInScopeStatusMock).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + filterId: 'FILTER-TAB-SCOPED', + chartsInScope: expect.arrayContaining([20, 21]), + tabsInScope: expect.arrayContaining(['TAB-A']), + }), + ]), + ); +}); diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index 9ff41a99c29..10e3379f907 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -103,6 +103,9 @@ const TOP_OF_PAGE_RANGE = 220; const DashboardContainer: FC = ({ topLevelTabs }) => { const nativeFilterScopes = useNativeFilterScopes(); + const nativeFilters = useSelector( + state => state.nativeFilters?.filters, + ); const dispatch = useDispatch(); const dashboardLayout = useSelector( @@ -192,7 +195,13 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { }; }); dispatch(setInScopeStatusOfFilters(scopes)); - }, [chartIds, JSON.stringify(nativeFilterScopes), dashboardLayout, dispatch]); + }, [ + chartIds, + JSON.stringify(nativeFilterScopes), + dashboardLayout, + dispatch, + JSON.stringify(nativeFilters), + ]); const childIds: string[] = useMemo( () => (topLevelTabs ? topLevelTabs.children : [DASHBOARD_GRID_ID]), diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts index 24cd92b99cf..693c7d374b2 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts @@ -39,90 +39,260 @@ beforeEach(() => { }); }); -// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks -describe('useIsFilterInScope', () => { - test('should return true for dividers (always in scope)', () => { - const divider: Divider = { - id: 'divider_1', - type: NativeFilterType.Divider, - title: 'Sample Divider', - description: 'Divider description', - }; +test('useIsFilterInScope should return true for dividers (always in scope)', () => { + const divider: Divider = { + id: 'divider_1', + type: NativeFilterType.Divider, + title: 'Sample Divider', + description: 'Divider description', + }; - const { result } = renderHook(() => useIsFilterInScope()); - expect(result.current(divider)).toBe(true); - }); + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(divider)).toBe(true); +}); - test('should return true for filters with charts in active tabs', () => { - const filter: Filter = { +test('useIsFilterInScope should return true for filters with charts in active tabs', () => { + const filter: Filter = { + id: 'filter_1', + name: 'Test Filter', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [123], + scope: { rootPath: ['TAB_1'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Sample filter description', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); + +test('useIsFilterInScope should return false for filters with inactive rootPath', () => { + const filter: Filter = { + id: 'filter_3', + name: 'Test Filter 3', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + scope: { rootPath: ['TAB_99'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 3 }], + description: 'Sample filter description', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(false); +}); + +test('useSelectFiltersInScope should return all filters in scope when no tabs exist', () => { + const filters: Filter[] = [ + { id: 'filter_1', - name: 'Test Filter', + name: 'Filter 1', filterType: 'filter_select', type: NativeFilterType.NativeFilter, - chartsInScope: [123], - scope: { rootPath: ['TAB_1'], excluded: [] }, + scope: { rootPath: [], excluded: [] }, controlValues: {}, defaultDataMask: {}, cascadeParentIds: [], targets: [{ column: { name: 'column_name' }, datasetId: 1 }], description: 'Sample filter description', - }; + }, + { + id: 'filter_2', + name: 'Filter 2', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + scope: { rootPath: [], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 2 }], + description: 'Sample filter description', + }, + ]; - const { result } = renderHook(() => useIsFilterInScope()); - expect(result.current(filter)).toBe(true); + const { result } = renderHook(() => useSelectFiltersInScope(filters)); + expect(result.current[0]).toEqual(filters); + expect(result.current[1]).toEqual([]); +}); + +// Tests for filter scope persistence when chartsInScope is missing +// (Bug fix: filters incorrectly marked out of scope after editing non-scope properties) +test('filter without chartsInScope should fall back to rootPath check', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB_1'] }, + dashboardLayout: { present: {} }, + }; + return selector(mockState); }); - test('should return false for filters with inactive rootPath', () => { - const filter: Filter = { - id: 'filter_3', - name: 'Test Filter 3', + const filter: Filter = { + id: 'filter_fallback', + name: 'Filter Without Charts', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + scope: { rootPath: ['TAB_1'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter with missing chartsInScope', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); + +test('filter with empty chartsInScope array should check rootPath', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB_1'] }, + dashboardLayout: { present: {} }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_empty_charts', + name: 'Filter With Empty Charts', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [], + scope: { rootPath: ['TAB_1'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter with empty chartsInScope', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); + +test('filter without chartsInScope and inactive rootPath should be out of scope', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB_1'] }, + dashboardLayout: { present: {} }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_inactive', + name: 'Inactive Filter', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + scope: { rootPath: ['INACTIVE_TAB'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter in inactive tab', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(false); +}); + +test('filter with ROOT_ID in rootPath should be in scope when chartsInScope is missing', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['ROOT_ID'] }, + dashboardLayout: { present: {} }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_root', + name: 'Global Filter', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + scope: { rootPath: ['ROOT_ID'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Global filter with missing chartsInScope', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); + +test('useSelectFiltersInScope correctly categorizes filters with missing chartsInScope', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB_1'] }, + dashboardLayout: { + present: { + tab1: { type: 'TAB', id: 'TAB_1' }, + }, + }, + }; + return selector(mockState); + }); + + const filters: Filter[] = [ + { + id: 'filter_in_scope', + name: 'In Scope Filter', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + scope: { rootPath: ['TAB_1'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter that should be in scope', + }, + { + id: 'filter_out_scope', + name: 'Out of Scope Filter', filterType: 'filter_select', type: NativeFilterType.NativeFilter, scope: { rootPath: ['TAB_99'], excluded: [] }, controlValues: {}, defaultDataMask: {}, cascadeParentIds: [], - targets: [{ column: { name: 'column_name' }, datasetId: 3 }], - description: 'Sample filter description', - }; + targets: [{ column: { name: 'column_name' }, datasetId: 2 }], + description: 'Filter that should be out of scope', + }, + ]; - const { result } = renderHook(() => useIsFilterInScope()); - expect(result.current(filter)).toBe(false); - }); + const { result } = renderHook(() => useSelectFiltersInScope(filters)); + + expect(result.current[0]).toContainEqual( + expect.objectContaining({ id: 'filter_in_scope' }), + ); + expect(result.current[1]).toContainEqual( + expect.objectContaining({ id: 'filter_out_scope' }), + ); }); -// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks -describe('useSelectFiltersInScope', () => { - test('should return all filters in scope when no tabs exist', () => { - const filters: Filter[] = [ - { - id: 'filter_1', - name: 'Filter 1', - filterType: 'filter_select', - type: NativeFilterType.NativeFilter, - scope: { rootPath: [], excluded: [] }, - controlValues: {}, - defaultDataMask: {}, - cascadeParentIds: [], - targets: [{ column: { name: 'column_name' }, datasetId: 1 }], - description: 'Sample filter description', - }, - { - id: 'filter_2', - name: 'Filter 2', - filterType: 'filter_select', - type: NativeFilterType.NativeFilter, - scope: { rootPath: [], excluded: [] }, - controlValues: {}, - defaultDataMask: {}, - cascadeParentIds: [], - targets: [{ column: { name: 'column_name' }, datasetId: 2 }], - description: 'Sample filter description', - }, - ]; +test('filter with chartsInScope takes precedence over rootPath', () => { + const filter: Filter = { + id: 'filter_priority', + name: 'Priority Filter', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [123, 456], + scope: { rootPath: ['INACTIVE_TAB'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter with chartsInScope should ignore rootPath', + }; - const { result } = renderHook(() => useSelectFiltersInScope(filters)); - expect(result.current[0]).toEqual(filters); - expect(result.current[1]).toEqual([]); - }); + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); }); diff --git a/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts b/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts new file mode 100644 index 00000000000..dc8b2a77fc8 --- /dev/null +++ b/superset-frontend/src/dashboard/reducers/nativeFilters.test.ts @@ -0,0 +1,162 @@ +/** + * 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 { Filter, NativeFilterType } from '@superset-ui/core'; +import nativeFilterReducer from './nativeFilters'; +import { SET_NATIVE_FILTERS_CONFIG_COMPLETE } from '../actions/nativeFilters'; + +const createMockFilter = ( + id: string, + chartsInScope?: number[], + tabsInScope?: string[], +): Filter => ({ + cascadeParentIds: [], + id, + name: `Filter ${id}`, + filterType: 'filter_select', + targets: [ + { + datasetId: 0, + column: { + name: 'test column', + displayName: 'test column', + }, + }, + ], + defaultDataMask: { + filterState: { + value: null, + }, + }, + scope: { + rootPath: [], + excluded: [], + }, + controlValues: { + allowsMultipleValues: true, + isRequired: false, + }, + type: NativeFilterType.NativeFilter, + description: '', + chartsInScope, + tabsInScope, +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE updates filters with complete scope properties', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2], ['tab1']), + }, + }; + + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [createMockFilter('filter1', [3, 4], ['tab2'])], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(result.filters.filter1.chartsInScope).toEqual([3, 4]); + expect(result.filters.filter1.tabsInScope).toEqual(['tab2']); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE preserves existing chartsInScope when missing from update', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2, 3], ['tab1']), + }, + }; + + const filterWithoutChartsInScope: Filter = { + ...createMockFilter('filter1', undefined, ['tab2']), + chartsInScope: undefined, + }; + + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [filterWithoutChartsInScope], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(result.filters.filter1.chartsInScope).toEqual([1, 2, 3]); + expect(result.filters.filter1.tabsInScope).toEqual(['tab2']); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE preserves existing tabsInScope when missing from update', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2], ['tab1', 'tab2']), + }, + }; + + const filterWithoutTabsInScope: Filter = { + ...createMockFilter('filter1', [3, 4], undefined), + tabsInScope: undefined, + }; + + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [filterWithoutTabsInScope], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(result.filters.filter1.chartsInScope).toEqual([3, 4]); + expect(result.filters.filter1.tabsInScope).toEqual(['tab1', 'tab2']); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE handles undefined scope properties for new filters', () => { + const initialState = { + filters: {}, + }; + + const filterWithUndefinedScopes: Filter = { + ...createMockFilter('filter1', undefined, undefined), + chartsInScope: undefined, + tabsInScope: undefined, + }; + + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [filterWithUndefinedScopes], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(result.filters.filter1.chartsInScope).toBeUndefined(); + expect(result.filters.filter1.tabsInScope).toBeUndefined(); +}); + +test('SET_NATIVE_FILTERS_CONFIG_COMPLETE treats empty arrays as valid scope properties', () => { + const initialState = { + filters: { + filter1: createMockFilter('filter1', [1, 2, 3], ['tab1', 'tab2']), + }, + }; + + const action = { + type: SET_NATIVE_FILTERS_CONFIG_COMPLETE as typeof SET_NATIVE_FILTERS_CONFIG_COMPLETE, + filterChanges: [createMockFilter('filter1', [], [])], + }; + + const result = nativeFilterReducer(initialState, action); + + expect(result.filters.filter1.chartsInScope).toEqual([]); + expect(result.filters.filter1.tabsInScope).toEqual([]); +}); diff --git a/superset-frontend/src/dashboard/reducers/nativeFilters.ts b/superset-frontend/src/dashboard/reducers/nativeFilters.ts index 6f225cb51b5..5161bb31d9e 100644 --- a/superset-frontend/src/dashboard/reducers/nativeFilters.ts +++ b/superset-frontend/src/dashboard/reducers/nativeFilters.ts @@ -69,7 +69,16 @@ function handleFilterChangesComplete( ) { const modifiedFilters = { ...state.filters }; filters.forEach(filter => { - modifiedFilters[filter.id] = filter; + if (filter.chartsInScope != null && filter.tabsInScope != null) { + modifiedFilters[filter.id] = filter; + } else { + const existingFilter = modifiedFilters[filter.id]; + modifiedFilters[filter.id] = { + ...filter, + chartsInScope: filter.chartsInScope ?? existingFilter?.chartsInScope, + tabsInScope: filter.tabsInScope ?? existingFilter?.tabsInScope, + }; + } }); return {