diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts index 32aeaa9290a1..3cb4925e9089 100644 --- a/superset-frontend/spec/fixtures/mockNativeFilters.ts +++ b/superset-frontend/spec/fixtures/mockNativeFilters.ts @@ -55,6 +55,7 @@ export const nativeFilters: NativeFiltersState = { }, type: NativeFilterType.NATIVE_FILTER, description: '', + chartsInScope: [18], }, 'NATIVE_FILTER-x9QPw0so1': { id: 'NATIVE_FILTER-x9QPw0so1', @@ -85,6 +86,7 @@ export const nativeFilters: NativeFiltersState = { }, type: NativeFilterType.NATIVE_FILTER, description: '2 letter code', + chartsInScope: [18], }, }, }; diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx index 50e53b9c7fc7..b08a7cd6339f 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx @@ -18,7 +18,7 @@ */ // ParentSize uses resize observer so the dashboard will update size // when its container size changes, due to e.g., builder side panel opening -import React, { FC, useEffect, useState } from 'react'; +import React, { FC, useEffect, useMemo, useState } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { FeatureFlag, @@ -27,35 +27,55 @@ import { isFeatureEnabled, } from '@superset-ui/core'; import { ParentSize } from '@vx/responsive'; +import pick from 'lodash/pick'; import Tabs from 'src/components/Tabs'; import DashboardGrid from 'src/dashboard/containers/DashboardGrid'; +import { + ChartsState, + DashboardLayout, + LayoutItem, + RootState, +} from 'src/dashboard/types'; import getLeafComponentIdFromPath from 'src/dashboard/util/getLeafComponentIdFromPath'; -import { DashboardLayout, LayoutItem, RootState } from 'src/dashboard/types'; import { DASHBOARD_GRID_ID, DASHBOARD_ROOT_DEPTH, } from 'src/dashboard/util/constants'; +import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope'; +import findTabIndexByComponentId from 'src/dashboard/util/findTabIndexByComponentId'; +import { setInScopeStatusOfFilters } from 'src/dashboard/actions/nativeFilters'; import { getRootLevelTabIndex, getRootLevelTabsComponent } from './utils'; -import { getChartIdsInFilterScope } from '../../util/activeDashboardFilters'; -import findTabIndexByComponentId from '../../util/findTabIndexByComponentId'; import { findTabsWithChartsInScope } from '../nativeFilters/utils'; -import { setInScopeStatusOfFilters } from '../../actions/nativeFilters'; import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils'; type DashboardContainerProps = { topLevelTabs?: LayoutItem; }; +const useNativeFilterScopes = () => { + const nativeFilters = useSelector( + state => state.nativeFilters?.filters, + ); + return useMemo( + () => + nativeFilters + ? Object.values(nativeFilters).map((filter: Filter) => + pick(filter, ['id', 'scope', 'type']), + ) + : [], + [JSON.stringify(nativeFilters)], + ); +}; + const DashboardContainer: FC = ({ topLevelTabs }) => { const dashboardLayout = useSelector( state => state.dashboardLayout.present, ); - const nativeFilters = useSelector( - state => state.nativeFilters?.filters, - ); + const nativeFilterScopes = useNativeFilterScopes(); const directPathToChild = useSelector( state => state.dashboardState.directPathToChild, ); + const charts = useSelector(state => state.charts); const [tabIndex, setTabIndex] = useState( getRootLevelTabIndex(dashboardLayout, directPathToChild), ); @@ -72,22 +92,14 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { } }, [getLeafComponentIdFromPath(directPathToChild)]); - // recalculate charts and tabs in scopes of native filters only when a scope or dashboard layout changes - const filterScopes = Object.values(nativeFilters ?? {}).map( - (filter: Filter) => ({ - id: filter.id, - scope: filter.scope, - type: filter.type, - }), - ); useEffect(() => { if ( !isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS) || - filterScopes.length === 0 + nativeFilterScopes.length === 0 ) { return; } - const scopes = filterScopes.map(filterScope => { + const scopes = nativeFilterScopes.map(filterScope => { if (filterScope.id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX)) { return { filterId: filterScope.id, @@ -95,13 +107,11 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { chartsInScope: [], }; } - const { scope } = filterScope; - const chartsInScope: number[] = getChartIdsInFilterScope({ - filterScope: { - scope: scope.rootPath, - immune: scope.excluded, - }, - }); + const chartsInScope: number[] = getChartIdsInFilterScope( + filterScope.scope, + charts, + dashboardLayout, + ); const tabsInScope = findTabsWithChartsInScope( dashboardLayout, chartsInScope, @@ -113,7 +123,7 @@ const DashboardContainer: FC = ({ topLevelTabs }) => { }; }); dispatch(setInScopeStatusOfFilters(scopes)); - }, [JSON.stringify(filterScopes), dashboardLayout, dispatch]); + }, [nativeFilterScopes, dashboardLayout, dispatch]); const childIds: string[] = topLevelTabs ? topLevelTabs.children diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts index 2c6022a8b235..0b84c73411db 100644 --- a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts +++ b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts @@ -27,11 +27,11 @@ import { NativeFilterType, } from '@superset-ui/core'; import { NO_TIME_RANGE, TIME_FILTER_MAP } from 'src/explore/constants'; -import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; +import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters'; +import { CHART_TYPE } from 'src/dashboard/util/componentTypes'; import { ChartConfiguration } from 'src/dashboard/reducers/types'; +import { Layout } from 'src/dashboard/types'; import { areObjectsEqual } from 'src/reduxUtils'; -import { Layout } from '../../types'; -import { getTreeCheckedItems } from '../nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/utils'; export enum IndicatorStatus { Unset = 'UNSET', @@ -123,7 +123,7 @@ const selectIndicatorsForChartFromFilter = ( return Object.keys(filter.columns) .filter(column => - getChartIdsInFilterScope({ + getChartIdsInFilterBoxScope({ filterScope: filter.scopes[column], }).includes(chartId), ) @@ -274,10 +274,7 @@ export const selectNativeIndicatorsForChart = ( .filter( nativeFilter => nativeFilter.type === NativeFilterType.NATIVE_FILTER && - getTreeCheckedItems(nativeFilter.scope, dashboardLayout).some( - layoutItem => - dashboardLayout[layoutItem]?.meta?.chartId === chartId, - ), + nativeFilter.chartsInScope?.includes(chartId), ) .map(nativeFilter => { const column = nativeFilter.targets?.[0]?.column?.name; @@ -295,14 +292,19 @@ export const selectNativeIndicatorsForChart = ( let crossFilterIndicators: any = []; if (isFeatureEnabled(FeatureFlag.DASHBOARD_CROSS_FILTERS)) { + const dashboardLayoutValues = Object.values(dashboardLayout); + const chartLayoutItem = dashboardLayoutValues.find( + layoutItem => layoutItem?.meta?.chartId === chartId, + ); crossFilterIndicators = Object.values(chartConfiguration) - .filter(chartConfig => - getTreeCheckedItems( - chartConfig?.crossFilters?.scope, - dashboardLayout, - ).some( - layoutItem => dashboardLayout[layoutItem]?.meta?.chartId === chartId, - ), + .filter( + chartConfig => + !chartConfig.crossFilters.scope.excluded.includes(chartId) && + chartConfig.crossFilters.scope.rootPath.some( + elementId => + chartLayoutItem?.type === CHART_TYPE && + chartLayoutItem?.parents?.includes(elementId), + ), ) .map(chartConfig => { const filterState = dataMask[chartConfig.id]?.filterState; @@ -310,7 +312,7 @@ export const selectNativeIndicatorsForChart = ( const filtersState = filterState?.filters; const column = filtersState && Object.keys(filtersState)[0]; - const dashboardLayoutItem = Object.values(dashboardLayout).find( + const dashboardLayoutItem = dashboardLayoutValues.find( layoutItem => layoutItem?.meta?.chartId === chartConfig.id, ); return { diff --git a/superset-frontend/src/dashboard/components/filterscope/FilterScopeSelector.jsx b/superset-frontend/src/dashboard/components/filterscope/FilterScopeSelector.jsx index afb37075a8f9..74a9a9080cd4 100644 --- a/superset-frontend/src/dashboard/components/filterscope/FilterScopeSelector.jsx +++ b/superset-frontend/src/dashboard/components/filterscope/FilterScopeSelector.jsx @@ -30,7 +30,7 @@ import getKeyForFilterScopeTree from 'src/dashboard/util/getKeyForFilterScopeTre import getSelectedChartIdForFilterScopeTree from 'src/dashboard/util/getSelectedChartIdForFilterScopeTree'; import getFilterScopeFromNodesTree from 'src/dashboard/util/getFilterScopeFromNodesTree'; import getRevertedFilterScope from 'src/dashboard/util/getRevertedFilterScope'; -import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; +import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters'; import { getChartIdAndColumnFromFilterKey, getDashboardFilterKey, @@ -106,7 +106,7 @@ export default class FilterScopeSelector extends React.PureComponent { const expanded = getFilterScopeParentNodes(nodes, 1); // force display filter_box chart as unchecked, but show checkbox as disabled const chartIdsInFilterScope = ( - getChartIdsInFilterScope({ + getChartIdsInFilterBoxScope({ filterScope: dashboardFilters[filterId].scopes[columnName], }) || [] ).filter(id => id !== filterId); diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx index 7a205fa17aae..465d646e7bd1 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx @@ -22,7 +22,7 @@ import cx from 'classnames'; import { useTheme } from '@superset-ui/core'; import { useSelector, connect } from 'react-redux'; -import { getChartIdsInFilterScope } from 'src/dashboard/util/activeDashboardFilters'; +import { getChartIdsInFilterBoxScope } from 'src/dashboard/util/activeDashboardFilters'; import Chart from '../../containers/Chart'; import AnchorLink from '../../../components/AnchorLink'; import DeleteComponentButton from '../DeleteComponentButton'; @@ -142,7 +142,7 @@ const FilterFocusHighlight = React.forwardRef( } } else if ( chartId === focusedFilterScope.chartId || - getChartIdsInFilterScope({ + getChartIdsInFilterBoxScope({ filterScope: focusedFilterScope.scope, }).includes(chartId) ) { diff --git a/superset-frontend/src/dashboard/util/activeDashboardFilters.js b/superset-frontend/src/dashboard/util/activeDashboardFilters.js index 20e420d8de2a..41871f7c0b08 100644 --- a/superset-frontend/src/dashboard/util/activeDashboardFilters.js +++ b/superset-frontend/src/dashboard/util/activeDashboardFilters.js @@ -18,7 +18,6 @@ */ import { isEmpty } from 'lodash'; import { mapValues, flow, keyBy } from 'lodash/fp'; - import { getChartIdAndColumnFromFilterKey, getDashboardFilterKey, @@ -62,7 +61,10 @@ export function getAppliedFilterValues(chartId) { return appliedFilterValuesByChart[chartId]; } -export function getChartIdsInFilterScope({ filterScope }) { +// Legacy - getChartIdsInFilterBoxScope is used only by +// components and functions related to filter box +// Please use src/dashboard/util/getChartIdsInFilterScope instead +export function getChartIdsInFilterBoxScope({ filterScope }) { function traverse(chartIds = [], component = {}, immuneChartIds = []) { if (!component) { return; @@ -117,7 +119,7 @@ export function buildActiveFilters({ dashboardFilters = {}, components = {} }) { : columns[column] !== undefined ) { // remove filter itself - const scope = getChartIdsInFilterScope({ + const scope = getChartIdsInFilterBoxScope({ filterScope: scopes[column], }).filter(id => chartId !== id); diff --git a/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts b/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts index 523577520ee7..f6083f4e9e40 100644 --- a/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts +++ b/superset-frontend/src/dashboard/util/filterboxMigrationHelper.ts @@ -26,7 +26,7 @@ import { } from 'src/explore/constants'; import { DASHBOARD_FILTER_SCOPE_GLOBAL } from 'src/dashboard/reducers/dashboardFilters'; import { Filter, NativeFilterType, TimeGranularity } from '@superset-ui/core'; -import { getChartIdsInFilterScope } from './activeDashboardFilters'; +import { getChartIdsInFilterBoxScope } from './activeDashboardFilters'; import getFilterConfigsFromFormdata from './getFilterConfigsFromFormdata'; interface FilterConfig { @@ -147,7 +147,7 @@ const getFilterboxDependencies = (filterScopes: FilterScopesMetadata) => { Object.entries(filterScopes).forEach(([key, filterFields]) => { filterFieldsDependencies[key] = {}; Object.entries(filterFields).forEach(([filterField, filterScope]) => { - filterFieldsDependencies[key][filterField] = getChartIdsInFilterScope({ + filterFieldsDependencies[key][filterField] = getChartIdsInFilterBoxScope({ filterScope, }).filter( chartId => filterChartIds.includes(chartId) && String(chartId) !== key, diff --git a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts new file mode 100644 index 000000000000..516bbf5045c6 --- /dev/null +++ b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts @@ -0,0 +1,44 @@ +/** + * 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 { NativeFilterScope } from '@superset-ui/core'; +import { CHART_TYPE } from './componentTypes'; +import { ChartsState, Layout } from '../types'; + +export function getChartIdsInFilterScope( + filterScope: NativeFilterScope, + charts: ChartsState, + layout: Layout, +) { + const layoutItems = Object.values(layout); + return Object.values(charts) + .filter( + chart => + !filterScope.excluded.includes(chart.id) && + layoutItems + .find( + layoutItem => + layoutItem?.type === CHART_TYPE && + layoutItem.meta?.chartId === chart.id, + ) + ?.parents?.some(elementId => + filterScope.rootPath.includes(elementId), + ), + ) + .map(chart => chart.id); +}