From af3589fe91f369656a19972f08adde28ed731e9e Mon Sep 17 00:00:00 2001 From: Levis Mbote <111055098+LevisNgigi@users.noreply.github.com> Date: Wed, 12 Feb 2025 16:32:20 +0300 Subject: [PATCH] fix(Scope): Correct issue where filters appear out of scope when sort is unchecked. (#32115) --- .../components/nativeFilters/state.test.ts | 126 ++++++++++++++++++ .../components/nativeFilters/state.ts | 25 +++- 2 files changed, 144 insertions(+), 7 deletions(-) create mode 100644 superset-frontend/src/dashboard/components/nativeFilters/state.test.ts diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts new file mode 100644 index 000000000..7cd694b4c --- /dev/null +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.test.ts @@ -0,0 +1,126 @@ +/** + * 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 { renderHook } from '@testing-library/react-hooks'; +import { useSelector } from 'react-redux'; +import { NativeFilterType, Filter, Divider } from '@superset-ui/core'; +import { useIsFilterInScope, useSelectFiltersInScope } from './state'; + +jest.mock('react-redux', () => { + const actual = jest.requireActual('react-redux'); + return { + ...actual, + useSelector: jest.fn(), + }; +}); + +beforeEach(() => { + (useSelector as jest.Mock).mockImplementation(selector => { + if (selector.name === 'useActiveDashboardTabs') { + return ['TAB_1']; + } + return []; + }); +}); + +describe('useIsFilterInScope', () => { + it('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); + }); + + it('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); + }); + + it('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); + }); +}); + +describe('useSelectFiltersInScope', () => { + it('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', + }, + ]; + + const { result } = renderHook(() => useSelectFiltersInScope(filters)); + expect(result.current[0]).toEqual(filters); + expect(result.current[1]).toEqual([]); + }); +}); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/state.ts b/superset-frontend/src/dashboard/components/nativeFilters/state.ts index 498fcf9a7..340410749 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/state.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/state.ts @@ -106,16 +106,27 @@ export function useIsFilterInScope() { // Chart is in an active tab tree if all of its ancestors of type TAB are active // Dividers are always in scope return useCallback( - (filter: Filter | Divider) => - isFilterDivider(filter) || - ('chartsInScope' in filter && - filter.chartsInScope?.some((chartId: number) => { + (filter: Filter | Divider) => { + if (isFilterDivider(filter)) return true; + + const isChartInScope = + Array.isArray(filter.chartsInScope) && + filter.chartsInScope.length > 0 && + filter.chartsInScope.some((chartId: number) => { const tabParents = selectChartTabParents(chartId); return ( - tabParents?.length === 0 || - tabParents?.every(tab => activeTabs.includes(tab)) + !tabParents || + tabParents.length === 0 || + tabParents.every(tab => activeTabs.includes(tab)) ); - })), + }); + + const isFilterInActiveTab = + filter.scope?.rootPath && + filter.scope.rootPath.some(tab => activeTabs.includes(tab)); + + return isChartInScope || isFilterInActiveTab; + }, [selectChartTabParents, activeTabs], ); }