diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts index 693c7d374b2..782e0fff6bf 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts @@ -296,3 +296,214 @@ test('filter with chartsInScope takes precedence over rootPath', () => { const { result } = renderHook(() => useIsFilterInScope()); expect(result.current(filter)).toBe(true); }); + +test('filter should be hidden on excluded nested tab even when parent tab is active', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB-Parent1', 'TAB-P1_Child2'] }, + dashboardLayout: { + present: { + 'CHART-1': { + type: 'CHART', + meta: { chartId: 1 }, + parents: ['ROOT_ID', 'TAB-Parent1', 'TAB-P1_Child1'], + }, + 'CHART-2': { + type: 'CHART', + meta: { chartId: 2 }, + parents: ['ROOT_ID', 'TAB-Parent1', 'TAB-P1_Child1'], + }, + 'CHART-5': { + type: 'CHART', + meta: { chartId: 5 }, + parents: ['ROOT_ID', 'TAB-Parent2'], + }, + 'CHART-6': { + type: 'CHART', + meta: { chartId: 6 }, + parents: ['ROOT_ID', 'TAB-Parent2'], + }, + 'TAB-Parent1': { type: 'TAB', id: 'TAB-Parent1' }, + 'TAB-P1_Child1': { type: 'TAB', id: 'TAB-P1_Child1' }, + 'TAB-P1_Child2': { type: 'TAB', id: 'TAB-P1_Child2' }, + 'TAB-Parent2': { type: 'TAB', id: 'TAB-Parent2' }, + }, + }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_nested', + name: 'Nested Tab Filter', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [1, 2, 5, 6], + scope: { rootPath: ['TAB-P1_Child1', 'TAB-Parent2'], excluded: [3, 4] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter excluding P1_Child2', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(false); +}); + +test('filter should be visible on included nested tab', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB-Parent1', 'TAB-P1_Child1'] }, + dashboardLayout: { + present: { + 'CHART-1': { + type: 'CHART', + meta: { chartId: 1 }, + parents: ['ROOT_ID', 'TAB-Parent1', 'TAB-P1_Child1'], + }, + 'CHART-2': { + type: 'CHART', + meta: { chartId: 2 }, + parents: ['ROOT_ID', 'TAB-Parent1', 'TAB-P1_Child1'], + }, + 'TAB-Parent1': { type: 'TAB', id: 'TAB-Parent1' }, + 'TAB-P1_Child1': { type: 'TAB', id: 'TAB-P1_Child1' }, + 'TAB-P1_Child2': { type: 'TAB', id: 'TAB-P1_Child2' }, + }, + }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_nested_visible', + name: 'Nested Tab Filter Visible', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [1, 2, 5, 6], + scope: { rootPath: ['TAB-P1_Child1', 'TAB-Parent2'], excluded: [3, 4] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter including P1_Child1', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); + +test('filter should be visible on top-level tab when charts have no nested parents', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB-Parent2'] }, + dashboardLayout: { + present: { + 'CHART-5': { + type: 'CHART', + meta: { chartId: 5 }, + parents: ['ROOT_ID', 'TAB-Parent2'], + }, + 'CHART-6': { + type: 'CHART', + meta: { chartId: 6 }, + parents: ['ROOT_ID', 'TAB-Parent2'], + }, + 'TAB-Parent2': { type: 'TAB', id: 'TAB-Parent2' }, + }, + }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_top_level', + name: 'Top Level Tab Filter', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [1, 2, 5, 6], + scope: { rootPath: ['TAB-P1_Child1', 'TAB-Parent2'], excluded: [3, 4] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter including Parent2', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); + +test('filter with chartsInScope referencing non-existent chart should still work', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB-Parent1'] }, + dashboardLayout: { + present: { + 'CHART-1': { + type: 'CHART', + meta: { chartId: 1 }, + parents: ['ROOT_ID', 'TAB-Parent1'], + }, + 'TAB-Parent1': { type: 'TAB', id: 'TAB-Parent1' }, + }, + }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_missing_chart', + name: 'Filter With Missing Chart', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [999], + scope: { rootPath: ['TAB-Parent1'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter referencing non-existent chart', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); + +test('filter with mix of existing and non-existent charts in chartsInScope', () => { + (useSelector as jest.Mock).mockImplementation((selector: Function) => { + const mockState = { + dashboardState: { activeTabs: ['TAB-Parent2'] }, + dashboardLayout: { + present: { + 'CHART-1': { + type: 'CHART', + meta: { chartId: 1 }, + parents: ['ROOT_ID', 'TAB-Parent1'], + }, + 'TAB-Parent1': { type: 'TAB', id: 'TAB-Parent1' }, + 'TAB-Parent2': { type: 'TAB', id: 'TAB-Parent2' }, + }, + }, + }; + return selector(mockState); + }); + + const filter: Filter = { + id: 'filter_mixed_charts', + name: 'Filter With Mixed Charts', + filterType: 'filter_select', + type: NativeFilterType.NativeFilter, + chartsInScope: [1, 999], + scope: { rootPath: ['TAB-Parent1'], excluded: [] }, + controlValues: {}, + defaultDataMask: {}, + cascadeParentIds: [], + targets: [{ column: { name: 'column_name' }, datasetId: 1 }], + description: 'Filter with mix of existing and non-existent charts', + }; + + const { result } = renderHook(() => useIsFilterInScope()); + expect(result.current(filter)).toBe(true); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts index 1989d879e2a..431994da6f8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.ts @@ -203,10 +203,12 @@ export function useIsFilterInScope() { (filter: FilterElement | Divider) => { if (filter.type === NativeFilterType.Divider) return true; - const isChartInScope = - Array.isArray(filter.chartsInScope) && - filter.chartsInScope.length > 0 && - filter.chartsInScope.some((chartId: number) => { + const hasChartsInScope = + Array.isArray(filter.chartsInScope) && filter.chartsInScope.length > 0; + + let isChartInScope = false; + if (hasChartsInScope) { + isChartInScope = filter.chartsInScope!.some((chartId: number) => { const tabParents = selectChartTabParents(chartId); return ( !tabParents || @@ -214,6 +216,7 @@ export function useIsFilterInScope() { tabParents.every(tab => activeTabs.includes(tab)) ); }); + } if (isChartCustomization(filter)) { const isCustomizationInActiveTab = filter.tabsInScope?.some(tab => @@ -222,11 +225,13 @@ export function useIsFilterInScope() { return isChartInScope || isCustomizationInActiveTab; } - const isFilterInActiveTab = filter.scope?.rootPath?.some(tab => - activeTabs.includes(tab), - ); + if (hasChartsInScope) { + return isChartInScope; + } - return isChartInScope || isFilterInActiveTab; + return ( + filter.scope?.rootPath?.some(tab => activeTabs.includes(tab)) ?? false + ); }, [selectChartTabParents, activeTabs], ); diff --git a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.test.ts b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.test.ts new file mode 100644 index 00000000000..07aeb62067b --- /dev/null +++ b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.test.ts @@ -0,0 +1,370 @@ +/** + * 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 { getChartIdsInFilterScope } from './getChartIdsInFilterScope'; +import { CHART_TYPE } from './componentTypes'; +import { LayoutItem } from '../types'; + +/** + * Creates a minimal valid LayoutItem for testing. + * Only includes fields required by the type and used by getChartIdsInFilterScope. + */ +const createChartLayoutItem = ( + id: string, + chartId: number, + parents: string[], +): LayoutItem => ({ + id, + type: CHART_TYPE, + children: [], + parents, + meta: { + chartId, + height: 100, + width: 100, + uuid: `test-uuid-${id}`, + }, +}); + +const createNestedTabsLayout = (): LayoutItem[] => { + return [ + createChartLayoutItem('CHART-1', 1, [ + 'ROOT_ID', + 'TABS-1', + 'TAB-Parent1', + 'TABS-nested', + 'TAB-P1_Child1', + ]), + createChartLayoutItem('CHART-2', 2, [ + 'ROOT_ID', + 'TABS-1', + 'TAB-Parent1', + 'TABS-nested', + 'TAB-P1_Child1', + ]), + createChartLayoutItem('CHART-3', 3, [ + 'ROOT_ID', + 'TABS-1', + 'TAB-Parent1', + 'TABS-nested', + 'TAB-P1_Child2', + ]), + createChartLayoutItem('CHART-4', 4, [ + 'ROOT_ID', + 'TABS-1', + 'TAB-Parent1', + 'TABS-nested', + 'TAB-P1_Child2', + ]), + createChartLayoutItem('CHART-5', 5, ['ROOT_ID', 'TABS-1', 'TAB-Parent2']), + createChartLayoutItem('CHART-6', 6, ['ROOT_ID', 'TABS-1', 'TAB-Parent2']), + ]; +}; + +const allChartIds = [1, 2, 3, 4, 5, 6]; + +test('filter scoped to all panels should include all charts', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['ROOT_ID'], + excluded: [], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]); +}); + +test('filter scoped to Parent1 tab should include only charts in Parent1 (including nested tabs)', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-Parent1'], + excluded: [], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4]); +}); + +test('filter scoped to Parent2 tab should include only charts in Parent2', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-Parent2'], + excluded: [], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([5, 6]); +}); + +test('filter scoped to P1_Child1 nested tab should include only charts in that tab', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-P1_Child1'], + excluded: [], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2]); +}); + +test('filter excluding P1_Child2 tab should not include charts 3 and 4', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-P1_Child1', 'TAB-Parent2'], + excluded: [3, 4], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 5, 6]); +}); + +test('filter with ROOT_ID rootPath excluding charts 3 and 4 should work correctly', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['ROOT_ID'], + excluded: [3, 4], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 5, 6]); +}); + +test('filter scoped to multiple top-level tabs should include charts from all specified tabs', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-Parent1', 'TAB-Parent2'], + excluded: [], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]); +}); + +test('filter scoped to nested tab with exclusion should work', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-P1_Child1'], + excluded: [2], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope).toEqual([1]); +}); + +test('filter with selectedLayers should include charts from layer selections', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['ROOT_ID'], + excluded: [], + selectedLayers: ['chart-1-layer-0', 'chart-2-layer-1'], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]); +}); + +test('filter with selectedLayers should include both layer-selected charts and regular charts', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-Parent2'], + excluded: [], + selectedLayers: ['chart-1-layer-0'], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 5, 6]); +}); + +test('filter with selectedLayers should exclude charts that have layer selections from regular filtering', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-P1_Child1'], + excluded: [], + selectedLayers: ['chart-1-layer-0'], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2]); +}); + +test('filter with selectedLayers should ignore invalid layer key formats', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['ROOT_ID'], + excluded: [], + selectedLayers: [ + 'chart-1-layer-0', + 'invalid-format', + 'chart-2-layer-1', + 'chart-invalid-layer-0', + ], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]); +}); + +test('filter with selectedLayers should handle charts not in chartIds array', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['ROOT_ID'], + excluded: [], + selectedLayers: ['chart-1-layer-0', 'chart-999-layer-0'], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]); +}); + +test('filter with selectedLayers should deduplicate chart IDs', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['ROOT_ID'], + excluded: [], + selectedLayers: [ + 'chart-1-layer-0', + 'chart-1-layer-1', + 'chart-2-layer-0', + 'chart-2-layer-2', + ], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]); +}); + +test('filter with selectedLayers and rootPath should combine both correctly', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-Parent2'], + excluded: [], + selectedLayers: ['chart-1-layer-0'], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 5, 6]); +}); + +test('filter with selectedLayers should include charts even if they are in excluded array', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['ROOT_ID'], + excluded: [1, 2], + selectedLayers: ['chart-1-layer-0', 'chart-2-layer-0'], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]); +}); + +test('filter with selectedLayers and excluded should exclude regular charts but include layer-selected charts', () => { + const chartLayoutItems = createNestedTabsLayout(); + const filterScope = { + rootPath: ['TAB-Parent2'], + excluded: [5], + selectedLayers: ['chart-1-layer-0'], + }; + + const chartsInScope = getChartIdsInFilterScope( + filterScope, + allChartIds, + chartLayoutItems, + ); + + expect(chartsInScope.sort()).toEqual([1, 6]); +});