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 4ac43f73007..d70a3701397 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 3cb79cb1142..4dc368881bf 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 = ({ 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) && ( - filterIds + (filterId: string) => { + // Build current dependency map + const dependencyMap = new Map(); + 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'), + ], ); /**