mirror of
https://github.com/apache/superset.git
synced 2026-06-08 17:19:20 +00:00
fix(native-filters): prevent circular dependencies and improve dependency handling (#35317)
This commit is contained in:
@@ -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 },
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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={[
|
||||
|
||||
@@ -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'),
|
||||
],
|
||||
);
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user