Skip to content

Commit

Permalink
fix(Scope): Correct issue where filters appear out of scope when sort…
Browse files Browse the repository at this point in the history
… is unchecked. (apache#32115)
  • Loading branch information
LevisNgigi authored Feb 12, 2025
1 parent 937d40c commit af3589f
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 7 deletions.
126 changes: 126 additions & 0 deletions superset-frontend/src/dashboard/components/nativeFilters/state.test.ts
Original file line number Diff line number Diff line change
@@ -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([]);
});
});
25 changes: 18 additions & 7 deletions superset-frontend/src/dashboard/components/nativeFilters/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
);
}
Expand Down

0 comments on commit af3589f

Please # to comment.