You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by dp...@apache.org on 2026/08/04 19:24:32 UTC
(superset) branch master updated: fix(dashboard): preserve duplicate chart holders in filter scopes (#42502)
This is an automated email from the ASF dual-hosted git repository.
DamianPendrak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9377659ffaa fix(dashboard): preserve duplicate chart holders in filter scopes (#42502)
9377659ffaa is described below
commit 9377659ffaa08493fea07b0a37e1961eb8d28d17
Author: Dang Dao <78...@users.noreply.github.com>
AuthorDate: Wed Aug 5 02:24:25 2026 +0700
fix(dashboard): preserve duplicate chart holders in filter scopes (#42502)
---
.../DashboardBuilder/DashboardContainer.tsx | 15 ++--
.../CrossFilters/ScopingModal/ScopingModal.tsx | 19 +++--
.../components/nativeFilters/utils.test.ts | 37 ++++++++-
.../dashboard/components/nativeFilters/utils.ts | 32 +++++---
.../src/dashboard/util/calculateScopes.ts | 15 ++--
.../src/dashboard/util/crossFilters.ts | 30 +++----
.../util/getChartIdsInFilterScope.test.ts | 42 +++++++++-
.../src/dashboard/util/getChartIdsInFilterScope.ts | 92 ++++++++++++++--------
8 files changed, 197 insertions(+), 85 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
index 4e133189337..97f3549c160 100644
--- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
+++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
@@ -64,11 +64,11 @@ import {
} from 'src/dashboard/actions/dashboardState';
import { getColorNamespace, resetColors } from 'src/utils/colorScheme';
import { calculateScopes } from 'src/dashboard/util/calculateScopes';
+import { createChartLayoutItemMap } from 'src/dashboard/util/getChartIdsInFilterScope';
import {
isLegacyChartCustomizationFormat,
migrateChartCustomization,
} from 'src/dashboard/util/migrateChartCustomization';
-import { CHART_TYPE } from 'src/dashboard/util/componentTypes';
import { NATIVE_FILTER_DIVIDER_PREFIX } from '../nativeFilters/FiltersConfigModal/utils';
import { selectFilterConfiguration } from '../nativeFilters/state';
import { getRootLevelTabsComponent } from './utils';
@@ -195,9 +195,8 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
prevRenderedChartIds.current = [];
}, [dashboardInfo?.metadata?.color_namespace, dispatch]);
- const chartLayoutItems = useMemo(
- () =>
- Object.values(dashboardLayout).filter(item => item?.type === CHART_TYPE),
+ const chartLayoutItemMap = useMemo(
+ () => createChartLayoutItemMap(Object.values(dashboardLayout)),
[dashboardLayout],
);
@@ -209,7 +208,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
const scopes = calculateScopes(
filterItems,
chartIds,
- chartLayoutItems,
+ chartLayoutItemMap,
item =>
item.id.startsWith(NATIVE_FILTER_DIVIDER_PREFIX) ||
item.type === NativeFilterType.Divider,
@@ -223,7 +222,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
prevFilterScopesRef.current = scopes;
dispatch(setInScopeStatusOfFilters(scopes));
}
- }, [chartIds, filterItems, chartLayoutItems, dispatch]);
+ }, [chartIds, filterItems, chartLayoutItemMap, dispatch]);
useEffect(() => {
if (chartCustomizations.length === 0) {
@@ -239,7 +238,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
const scopes = calculateScopes(
normalizedCustomizations,
chartIds,
- chartLayoutItems,
+ chartLayoutItemMap,
item => item.type === ChartCustomizationType.Divider,
).map(scope => ({
customizationId: scope.id,
@@ -251,7 +250,7 @@ const DashboardContainer: FC<DashboardContainerProps> = ({ topLevelTabs }) => {
prevCustomizationScopesRef.current = scopes;
dispatch(setInScopeStatusOfCustomizations(scopes));
}
- }, [chartIds, chartCustomizations, chartLayoutItems, dispatch]);
+ }, [chartIds, chartCustomizations, chartLayoutItemMap, dispatch]);
const childIds: string[] = useMemo(
() => (topLevelTabs ? topLevelTabs.children : [DASHBOARD_GRID_ID]),
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx
index 395c6a4e271..8a22df93a3c 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/CrossFilters/ScopingModal/ScopingModal.tsx
@@ -29,7 +29,10 @@ import {
GLOBAL_SCOPE_POINTER,
ChartCrossFiltersConfig,
} from 'src/dashboard/types';
-import { getChartIdsInFilterScope } from 'src/dashboard/util/getChartIdsInFilterScope';
+import {
+ createChartLayoutItemMap,
+ getChartIdsInFilterScope,
+} from 'src/dashboard/util/getChartIdsInFilterScope';
import { useChartIds } from 'src/dashboard/util/charts/useChartIds';
import { saveChartConfiguration } from 'src/dashboard/actions/dashboardInfo';
import { DEFAULT_CROSS_FILTER_SCOPING } from 'src/dashboard/constants';
@@ -82,6 +85,10 @@ export const ScopingModal = ({
}: ScopingModalProps) => {
const dispatch = useDispatch();
const chartLayoutItems = useChartLayoutItems();
+ const chartLayoutItemMap = useMemo(
+ () => createChartLayoutItemMap(chartLayoutItems),
+ [chartLayoutItems],
+ );
const chartIds = useChartIds();
const [currentChartId, setCurrentChartId] = useState(initialChartId);
const initialChartConfig = useSelector<RootState, ChartConfiguration>(
@@ -160,7 +167,7 @@ export const ScopingModal = ({
chartsInScope: getChartIdsInFilterScope(
scope,
chartIds,
- chartLayoutItems,
+ chartLayoutItemMap,
),
},
},
@@ -169,7 +176,7 @@ export const ScopingModal = ({
const globalChartsInScope = getChartIdsInFilterScope(
scope,
chartIds,
- chartLayoutItems,
+ chartLayoutItemMap,
);
setGlobalChartConfig({
scope,
@@ -183,7 +190,7 @@ export const ScopingModal = ({
);
}
},
- [currentChartId, chartIds, chartLayoutItems],
+ [currentChartId, chartIds, chartLayoutItemMap],
);
const removeCustomScope = useCallback(
@@ -251,7 +258,7 @@ export const ScopingModal = ({
chartsInScope: getChartIdsInFilterScope(
newScope,
chartIds,
- chartLayoutItems,
+ chartLayoutItemMap,
),
},
};
@@ -286,7 +293,7 @@ export const ScopingModal = ({
currentChartId,
globalChartConfig.chartsInScope,
globalChartConfig.scope,
- chartLayoutItems,
+ chartLayoutItemMap,
],
);
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts
index 2873e43300b..be5e59846da 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.test.ts
@@ -17,8 +17,9 @@
* under the License.
*/
import { Behavior } from '@superset-ui/core';
-import { DashboardLayout } from 'src/dashboard/types';
+import { DashboardLayout, LayoutItem } from 'src/dashboard/types';
import { CHART_TYPE } from 'src/dashboard/util/componentTypes';
+import { createChartLayoutItemMap } from 'src/dashboard/util/getChartIdsInFilterScope';
import {
nativeFilterGate,
findTabsWithChartsInScope,
@@ -86,6 +87,40 @@ test('findTabsWithChartsInScope should handle a recursive layout structure', ()
);
});
+test('findTabsWithChartsInScope includes tabs from duplicate chart holders', () => {
+ const chartLayoutItems: LayoutItem[] = [
+ {
+ id: 'CHART-7-first',
+ type: CHART_TYPE,
+ children: [],
+ parents: ['ROOT_ID', 'TAB-parent', 'TABS-nested', 'TAB-first'],
+ meta: {
+ chartId: 7,
+ height: 100,
+ width: 100,
+ uuid: 'test-uuid-CHART-7-first',
+ },
+ },
+ {
+ id: 'CHART-7-second',
+ type: CHART_TYPE,
+ children: [],
+ parents: ['ROOT_ID', 'TAB-parent', 'TABS-nested', 'TAB-second'],
+ meta: {
+ chartId: 7,
+ height: 100,
+ width: 100,
+ uuid: 'test-uuid-CHART-7-second',
+ },
+ },
+ ];
+ const chartLayoutItemMap = createChartLayoutItemMap(chartLayoutItems);
+
+ expect(
+ Array.from(findTabsWithChartsInScope(chartLayoutItemMap, [7])),
+ ).toEqual(['TAB-parent', 'TAB-first', 'TAB-second']);
+});
+
test('getFormData should include persisted time_grains for time grain filters', () => {
const formData = getFormData({
dashboardId: 10,
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
index 40398855ca0..5ebd0785094 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
@@ -32,8 +32,9 @@ import {
ExtraFormDataOverride,
ExtraFormDataAppend,
} from '@superset-ui/core';
-import { LayoutItem } from 'src/dashboard/types';
import extractUrlParams from 'src/dashboard/util/extractUrlParams';
+import { getChartLayoutItemMap } from 'src/dashboard/util/getChartIdsInFilterScope';
+import type { ChartLayoutItems } from 'src/dashboard/util/getChartIdsInFilterScope';
import { isIterable } from 'src/utils/types';
import { TAB_TYPE } from '../../util/componentTypes';
import getBootstrapData from '../../../utils/getBootstrapData';
@@ -173,19 +174,24 @@ export function nativeFilterGate(behaviors: Behavior[]): boolean {
}
export const findTabsWithChartsInScope = (
- chartLayoutItems: LayoutItem[],
+ chartLayoutItems: ChartLayoutItems,
chartsInScope: number[],
-) =>
- new Set<string>(
- chartsInScope
- .map(chartId =>
- chartLayoutItems
- .find(item => item?.meta?.chartId === chartId)
- ?.parents?.filter(parent => parent.startsWith(`${TAB_TYPE}-`)),
- )
- .filter(id => id !== undefined)
- .flat() as string[],
- );
+) => {
+ const chartLayoutItemMap = getChartLayoutItemMap(chartLayoutItems);
+ const tabsInScope = new Set<string>();
+
+ chartsInScope.forEach(chartId => {
+ chartLayoutItemMap.get(chartId)?.forEach(layoutItem => {
+ layoutItem.parents?.forEach(parent => {
+ if (parent.startsWith(`${TAB_TYPE}-`)) {
+ tabsInScope.add(parent);
+ }
+ });
+ });
+ });
+
+ return tabsInScope;
+};
export const getFilterValueForDisplay = (
value?: string[] | null | string | number | object,
diff --git a/superset-frontend/src/dashboard/util/calculateScopes.ts b/superset-frontend/src/dashboard/util/calculateScopes.ts
index 6cf7ae3efc4..9a1b85845a4 100644
--- a/superset-frontend/src/dashboard/util/calculateScopes.ts
+++ b/superset-frontend/src/dashboard/util/calculateScopes.ts
@@ -17,8 +17,11 @@
* under the License.
*/
import { NativeFilterScope } from '@superset-ui/core';
-import { LayoutItem } from '../types';
-import { getChartIdsInFilterScope } from './getChartIdsInFilterScope';
+import {
+ getChartLayoutItemMap,
+ getChartIdsInFilterScope,
+} from './getChartIdsInFilterScope';
+import type { ChartLayoutItems } from './getChartIdsInFilterScope';
import { findTabsWithChartsInScope } from '../components/nativeFilters/utils';
export interface ScopeItem {
@@ -35,9 +38,11 @@ export interface CalculatedScope {
export function calculateScopes<T extends ScopeItem>(
items: T[],
chartIds: number[],
- chartLayoutItems: LayoutItem[],
+ chartLayoutItems: ChartLayoutItems,
isDivider: (item: T) => boolean = () => false,
): CalculatedScope[] {
+ const chartLayoutItemMap = getChartLayoutItemMap(chartLayoutItems);
+
return items.map(item => {
if (isDivider(item)) {
return {
@@ -58,11 +63,11 @@ export function calculateScopes<T extends ScopeItem>(
const chartsInScope = getChartIdsInFilterScope(
item.scope,
chartIds,
- chartLayoutItems,
+ chartLayoutItemMap,
);
const tabsInScope = findTabsWithChartsInScope(
- chartLayoutItems,
+ chartLayoutItemMap,
chartsInScope,
);
diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts b/superset-frontend/src/dashboard/util/crossFilters.ts
index 3269c93d676..f9183f11c6e 100644
--- a/superset-frontend/src/dashboard/util/crossFilters.ts
+++ b/superset-frontend/src/dashboard/util/crossFilters.ts
@@ -20,10 +20,12 @@ import { cloneDeep } from 'lodash-es';
import {
Behavior,
getChartMetadataRegistry,
- isDefined,
NativeFilterScope,
} from '@superset-ui/core';
-import { getChartIdsInFilterScope } from './getChartIdsInFilterScope';
+import {
+ createChartLayoutItemMap,
+ getChartIdsInFilterScope,
+} from './getChartIdsInFilterScope';
import {
ChartConfiguration,
ChartsState,
@@ -33,7 +35,6 @@ import {
isCrossFilterScopeGlobal,
} from '../types';
import { DEFAULT_CROSS_FILTER_SCOPING } from '../constants';
-import { CHART_TYPE } from './componentTypes';
export const isCrossFiltersEnabled = (
metadataCrossFiltersEnabled: boolean | undefined,
@@ -48,34 +49,29 @@ export const getCrossFiltersConfiguration = (
>,
charts: ChartsState,
) => {
- const chartLayoutItems = Object.values(dashboardLayout).filter(
- item => item?.type === CHART_TYPE,
+ const chartLayoutItemMap = createChartLayoutItemMap(
+ Object.values(dashboardLayout),
);
+ const chartIds = Object.values(charts).map(chart => chart.id);
const globalChartConfiguration = metadata.global_chart_configuration?.scope
? {
scope: metadata.global_chart_configuration.scope,
chartsInScope: getChartIdsInFilterScope(
metadata.global_chart_configuration.scope,
- Object.values(charts).map(chart => chart.id),
- chartLayoutItems,
+ chartIds,
+ chartLayoutItemMap,
),
}
: {
scope: DEFAULT_CROSS_FILTER_SCOPING,
- chartsInScope: Object.values(charts).map(chart => chart.id),
+ chartsInScope: chartIds,
};
// If user just added cross filter to dashboard it's not saving its scope on server,
// so we tweak it until user will update scope and will save it in server
const chartConfiguration: ChartConfiguration = {};
- chartLayoutItems.forEach(layoutItem => {
- const chartId = layoutItem.meta?.chartId;
-
- if (!isDefined(chartId)) {
- return;
- }
-
+ chartLayoutItemMap.forEach((_, chartId) => {
const behaviors =
(
getChartMetadataRegistry().get(charts[chartId]?.form_data?.viz_type) ??
@@ -106,8 +102,8 @@ export const getCrossFiltersConfiguration = (
: getChartIdsInFilterScope(
chartConfiguration[chartId].crossFilters
.scope as NativeFilterScope,
- Object.values(charts).map(chart => chart.id),
- chartLayoutItems,
+ chartIds,
+ chartLayoutItemMap,
);
}
});
diff --git a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.test.ts b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.test.ts
index 52476810844..71d111fa06f 100644
--- a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.test.ts
+++ b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.test.ts
@@ -16,7 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { getChartIdsInFilterScope } from './getChartIdsInFilterScope';
+import {
+ createChartLayoutItemMap,
+ getChartIdsInFilterScope,
+} from './getChartIdsInFilterScope';
import { CHART_TYPE } from './componentTypes';
import { LayoutItem } from '../types';
@@ -92,6 +95,43 @@ test('filter scoped to all panels should include all charts', () => {
expect(chartsInScope.sort()).toEqual([1, 2, 3, 4, 5, 6]);
});
+test('filter scope accepts a precomputed chart layout item map', () => {
+ const chartLayoutItemMap = createChartLayoutItemMap(createNestedTabsLayout());
+ const filterScope = {
+ rootPath: ['TAB-P1_Child1'],
+ excluded: [],
+ };
+
+ const chartsInScope = getChartIdsInFilterScope(
+ filterScope,
+ allChartIds,
+ chartLayoutItemMap,
+ );
+
+ expect(chartsInScope).toEqual([1, 2]);
+});
+
+test('filter scope includes a chart when any duplicate layout holder is in scope', () => {
+ const chartLayoutItems = [
+ createChartLayoutItem('CHART-7-first', 7, ['ROOT_ID', 'TAB-first']),
+ createChartLayoutItem('CHART-7-second', 7, ['ROOT_ID', 'TAB-second']),
+ ];
+ const chartLayoutItemMap = createChartLayoutItemMap(chartLayoutItems);
+ const filterScope = {
+ rootPath: ['TAB-second'],
+ excluded: [],
+ };
+
+ const chartsInScope = getChartIdsInFilterScope(
+ filterScope,
+ [7],
+ chartLayoutItemMap,
+ );
+
+ expect(chartLayoutItemMap.get(7)).toEqual(chartLayoutItems);
+ expect(chartsInScope).toEqual([7]);
+});
+
test('filter scoped to Parent1 tab should include only charts in Parent1 (including nested tabs)', () => {
const chartLayoutItems = createNestedTabsLayout();
const filterScope = {
diff --git a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts
index f6b3c91ae41..81dadfeeee1 100644
--- a/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts
+++ b/superset-frontend/src/dashboard/util/getChartIdsInFilterScope.ts
@@ -24,65 +24,89 @@ interface ExtendedNativeFilterScope extends NativeFilterScope {
selectedLayers?: string[];
}
+export type ChartLayoutItemMap = ReadonlyMap<number, readonly LayoutItem[]>;
+export type ChartLayoutItems = readonly LayoutItem[] | ChartLayoutItemMap;
+
+const isChartLayoutItemArray = (
+ chartLayoutItems: ChartLayoutItems,
+): chartLayoutItems is readonly LayoutItem[] => Array.isArray(chartLayoutItems);
+
+export function createChartLayoutItemMap(
+ layoutItems: readonly (LayoutItem | null | undefined)[],
+): ChartLayoutItemMap {
+ const chartLayoutItemMap = new Map<number, LayoutItem[]>();
+
+ layoutItems.forEach(layoutItem => {
+ const chartId = layoutItem?.meta?.chartId;
+ if (layoutItem?.type === CHART_TYPE && typeof chartId === 'number') {
+ const chartLayoutItems = chartLayoutItemMap.get(chartId);
+ if (chartLayoutItems) {
+ chartLayoutItems.push(layoutItem);
+ } else {
+ chartLayoutItemMap.set(chartId, [layoutItem]);
+ }
+ }
+ });
+
+ return chartLayoutItemMap;
+}
+
+export function getChartLayoutItemMap(
+ chartLayoutItems: ChartLayoutItems,
+): ChartLayoutItemMap {
+ if (isChartLayoutItemArray(chartLayoutItems)) {
+ return createChartLayoutItemMap(chartLayoutItems);
+ }
+ return chartLayoutItems;
+}
+
export function getChartIdsInFilterScope(
filterScope: ExtendedNativeFilterScope,
chartIds: number[],
- layoutItems: LayoutItem[],
+ chartLayoutItems: ChartLayoutItems,
): number[] {
+ const chartLayoutItemMap = getChartLayoutItemMap(chartLayoutItems);
+ const excludedChartIds = new Set(filterScope.excluded);
+ const rootPath = new Set(filterScope.rootPath);
+ const isChartInScope = (chartId: number) =>
+ !excludedChartIds.has(chartId) &&
+ chartLayoutItemMap
+ .get(chartId)
+ ?.some(layoutItem =>
+ layoutItem.parents?.some(elementId => rootPath.has(elementId)),
+ );
+
if (filterScope.selectedLayers && filterScope.selectedLayers.length > 0) {
const targetChartIds: number[] = [];
+ const targetChartIdSet = new Set<number>();
+ const chartIdSet = new Set(chartIds);
+ const chartsWithLayerSelections = new Set<number>();
filterScope.selectedLayers.forEach(selectionKey => {
const layerMatch = selectionKey.match(/^chart-(\d+)-layer-(\d+)$/);
if (layerMatch) {
const chartId = parseInt(layerMatch[1], 10);
- if (chartIds.includes(chartId) && !targetChartIds.includes(chartId)) {
+ chartsWithLayerSelections.add(chartId);
+ if (chartIdSet.has(chartId) && !targetChartIdSet.has(chartId)) {
targetChartIds.push(chartId);
+ targetChartIdSet.add(chartId);
}
}
});
- const chartsWithLayerSelections = new Set<number>();
- filterScope.selectedLayers.forEach(selectionKey => {
- const layerMatch = selectionKey.match(/^chart-(\d+)-layer-(\d+)$/);
- if (layerMatch) {
- chartsWithLayerSelections.add(parseInt(layerMatch[1], 10));
- }
- });
const regularChartIds = chartIds.filter(
chartId =>
- !filterScope.excluded.includes(chartId) &&
- !chartsWithLayerSelections.has(chartId) &&
- layoutItems
- .find(
- layoutItem =>
- layoutItem?.type === CHART_TYPE &&
- layoutItem.meta?.chartId === chartId,
- )
- ?.parents?.some(elementId =>
- filterScope.rootPath.includes(elementId),
- ),
+ !chartsWithLayerSelections.has(chartId) && isChartInScope(chartId),
);
regularChartIds.forEach(chartId => {
- if (!targetChartIds.includes(chartId)) {
+ if (!targetChartIdSet.has(chartId)) {
targetChartIds.push(chartId);
+ targetChartIdSet.add(chartId);
}
});
return targetChartIds;
}
- const traditionalResult = chartIds.filter(
- chartId =>
- !filterScope.excluded.includes(chartId) &&
- layoutItems
- .find(
- layoutItem =>
- layoutItem?.type === CHART_TYPE &&
- layoutItem.meta?.chartId === chartId,
- )
- ?.parents?.some(elementId => filterScope.rootPath.includes(elementId)),
- );
-
- return traditionalResult;
+ return chartIds.filter(isChartInScope);
}