You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2025/10/27 13:44:20 UTC
(superset) branch master updated: fix(native-filters): prevent circular dependencies and improve dependency handling (#35317)
This is an automated email from the ASF dual-hosted git repository.
diegopucci 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 0bf34d4d6f fix(native-filters): prevent circular dependencies and improve dependency handling (#35317)
0bf34d4d6f is described below
commit 0bf34d4d6f244989eec5e1cd2e652f52a3627b5c
Author: ngokturkkarli <95...@users.noreply.github.com>
AuthorDate: Mon Oct 27 16:44:14 2025 +0300
fix(native-filters): prevent circular dependencies and improve dependency handling (#35317)
---
.../cypress/e2e/dashboard/nativeFilters.test.ts | 51 ++++++++++++++++++++++
.../FiltersConfigForm/DependencyList.tsx | 24 ++++++++--
.../FiltersConfigForm/FiltersConfigForm.tsx | 8 +++-
.../FiltersConfigModal/FiltersConfigModal.tsx | 41 +++++++++++++++--
4 files changed, 115 insertions(+), 9 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts
index 4ac43f7300..d70a370139 100644
--- a/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts
@@ -160,6 +160,57 @@ describe('Native filters', () => {
);
});
+ it('user cannot create bi-directional dependencies between filters', () => {
+ prepareDashboardFilters([
+ { name: 'region', column: 'region', datasetId: 2 },
+ { name: 'country_name', column: 'country_name', datasetId: 2 },
+ { name: 'country_code', column: 'country_code', datasetId: 2 },
+ { name: 'year', column: 'year', datasetId: 2 },
+ ]);
+ enterNativeFilterEditModal();
+
+ // First, make country_name dependent on region
+ selectFilter(1);
+ cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(
+ () => {
+ cy.contains('Values are dependent on other filters')
+ .should('be.visible')
+ .click();
+ },
+ );
+ addParentFilterWithValue(0, testItems.topTenChart.filterColumnRegion);
+
+ // Second, make country_code dependent on country_name
+ selectFilter(2);
+ cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(
+ () => {
+ cy.contains('Values are dependent on other filters')
+ .should('be.visible')
+ .click();
+ },
+ );
+ addParentFilterWithValue(0, testItems.topTenChart.filterColumn);
+
+ // Now select region filter and try to add dependency
+ selectFilter(0);
+ cy.get(nativeFilters.filterConfigurationSections.displayedSection).within(
+ () => {
+ cy.contains('Values are dependent on other filters')
+ .should('be.visible')
+ .click();
+
+ // Verify that only 'year' is available as dependency for region
+ // 'country_name' and 'country_code' should not be available (would create circular dependency)
+ cy.get('input[aria-label^="Limit type"]').click({ force: true });
+ cy.get('[role="listbox"]').should('be.visible');
+ cy.get('[role="listbox"]').should('contain', 'year');
+ cy.get('[role="listbox"]').should('not.contain', 'country_name');
+ cy.get('[role="listbox"]').should('not.contain', 'country_code');
+ cy.get('[role="listbox"]').contains('year').click();
+ },
+ );
+ });
+
it('Dependent filter selects first item based on parent filter selection', () => {
prepareDashboardFilters([
{ name: 'region', column: 'region', datasetId: 2 },
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx
index 3cb79cb114..4dc368881b 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DependencyList.tsx
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { useState } from 'react';
+import { useState, useEffect } from 'react';
import { styled, t } from '@superset-ui/core';
import { Icons } from '@superset-ui/core/components/Icons';
import { Select } from '@superset-ui/core/components';
@@ -189,10 +189,28 @@ const DependencyList = ({
const hasAvailableFilters = availableFilters.length > 0;
const hasDependencies = dependencies.length > 0;
+ // Clean up invalid dependencies when available filters change
+ useEffect(() => {
+ if (dependencies.length > 0) {
+ const availableFilterIds = new Set(availableFilters.map(f => f.value));
+ const validDependencies = dependencies.filter(dep =>
+ availableFilterIds.has(dep),
+ );
+
+ // If some dependencies are no longer valid, update the list
+ if (validDependencies.length !== dependencies.length) {
+ onDependenciesChange(validDependencies);
+ }
+ }
+ }, [availableFilters, dependencies, onDependenciesChange]);
+
const onCheckChanged = (value: boolean) => {
const newDependencies: string[] = [];
if (value && !hasDependencies && hasAvailableFilters) {
- newDependencies.push(getDependencySuggestion());
+ const suggestion = getDependencySuggestion();
+ if (suggestion) {
+ newDependencies.push(suggestion);
+ }
}
onDependenciesChange(newDependencies);
};
@@ -201,7 +219,7 @@ const DependencyList = ({
<MainPanel>
<CollapsibleControl
title={t('Values are dependent on other filters')}
- initialValue={hasDependencies}
+ checked={hasDependencies}
onChange={onCheckChanged}
tooltip={t(
'Values selected in other filters will affect the filter options to only show relevant values',
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
index cb21434462..20f0e83138 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -584,7 +584,10 @@ const FiltersConfigForm = (
return Promise.reject(new Error(t('Pre-filter is required')));
};
- const availableFilters = getAvailableFilters(filterId);
+ const availableFilters = useMemo(
+ () => getAvailableFilters(filterId),
+ [getAvailableFilters, filterId, filters],
+ );
const hasAvailableFilters = availableFilters.length > 0;
const hasTimeDependency = availableFilters
.filter(filter => filter.type === 'filter_time')
@@ -918,7 +921,8 @@ const FiltersConfigForm = (
children: (
<>
{canDependOnOtherFilters &&
- hasAvailableFilters && (
+ (hasAvailableFilters ||
+ dependencies.length > 0) && (
<StyledRowFormItem
expanded={expanded}
name={[
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
index b2d458d80b..68a151eca8 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx
@@ -319,16 +319,49 @@ function FiltersConfigModal({
);
const getAvailableFilters = useCallback(
- (filterId: string) =>
- filterIds
+ (filterId: string) => {
+ // Build current dependency map
+ const dependencyMap = new Map<string, string[]>();
+ const filters = form.getFieldValue('filters');
+ if (filters) {
+ Object.keys(filters).forEach(key => {
+ const formItem = filters[key];
+ const configItem = filterConfigMap[key];
+ let array: string[] = [];
+ if (formItem && 'dependencies' in formItem) {
+ array = [...formItem.dependencies];
+ } else if (configItem?.cascadeParentIds) {
+ array = [...configItem.cascadeParentIds];
+ }
+ dependencyMap.set(key, array);
+ });
+ }
+
+ return filterIds
.filter(id => id !== filterId)
.filter(id => canBeUsedAsDependency(id))
+ .filter(id => {
+ // Check if adding this dependency would create a circular dependency
+ const currentDependencies = dependencyMap.get(filterId) || [];
+ const testDependencies = [...currentDependencies, id];
+ const testMap = new Map(dependencyMap);
+ testMap.set(filterId, testDependencies);
+ return !hasCircularDependency(testMap, filterId);
+ })
.map(id => ({
label: getFilterTitle(id),
value: id,
type: filterConfigMap[id]?.filterType,
- })),
- [canBeUsedAsDependency, filterConfigMap, filterIds, getFilterTitle],
+ }));
+ },
+ [
+ canBeUsedAsDependency,
+ filterConfigMap,
+ filterIds,
+ getFilterTitle,
+ form,
+ form.getFieldValue('filters'),
+ ],
);
/**