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 818d6ebda6e0..8fe2da0b87c4 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts @@ -22,6 +22,7 @@ import { dataTestChartName, } from 'cypress/support/directories'; +import { waitForChartLoad } from 'cypress/utils'; import { addParentFilterWithValue, applyNativeFilterValueWithIndex, @@ -160,6 +161,74 @@ describe('Native filters', () => { ); }); + it('Dependent filter selects first item based on parent filter selection', () => { + prepareDashboardFilters([ + { name: 'region', column: 'region', datasetId: 2 }, + { name: 'country_name', column: 'country_name', datasetId: 2 }, + ]); + + enterNativeFilterEditModal(); + + selectFilter(0); + cy.get(nativeFilters.filterConfigurationSections.displayedSection).within( + () => { + cy.contains('Select first filter value by default') + .should('be.visible') + .click(); + }, + ); + cy.get(nativeFilters.filterConfigurationSections.displayedSection).within( + () => { + cy.contains('Can select multiple values ') + .should('be.visible') + .click(); + }, + ); + + selectFilter(1); + cy.get(nativeFilters.filterConfigurationSections.displayedSection).within( + () => { + cy.contains('Values are dependent on other filters') + .should('be.visible') + .click(); + }, + ); + cy.get(nativeFilters.filterConfigurationSections.displayedSection).within( + () => { + cy.contains('Can select multiple values ') + .should('be.visible') + .click(); + }, + ); + addParentFilterWithValue(0, testItems.topTenChart.filterColumnRegion); + cy.get(nativeFilters.filterConfigurationSections.displayedSection).within( + () => { + cy.contains('Select first filter value by default') + .should('be.visible') + .click(); + }, + ); + + // cannot use saveNativeFilterSettings because there is a bug which + // sometimes does not allow charts to load when enabling the 'Select first filter value by default' + // to be saved when using dependent filters so, + // you reload the window. + cy.get(nativeFilters.modal.footer) + .contains('Save') + .should('be.visible') + .click({ force: true }); + + cy.get(nativeFilters.modal.container).should('not.exist'); + cy.reload(); + + applyNativeFilterValueWithIndex(0, 'North America'); + + // Check that dependent filter auto-selects the first item + cy.get(nativeFilters.filterFromDashboardView.filterContent) + .eq(1) + .should('contain.text', 'Bermuda'); + }); + it('User can create filter depend on 2 other filters', () => { prepareDashboardFilters([ { name: 'region', column: 'region', datasetId: 2 }, diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 16263024d29b..97a6327f751f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -155,6 +155,34 @@ const FilterValue: FC = ({ dashboardId, }); const filterOwnState = filter.dataMask?.ownState || {}; + if (filter?.cascadeParentIds?.length) { + // Prevent unnecessary backend requests by validating parent filter selections first + + let selectedParentFilterValueCounts = 0; + + filter?.cascadeParentIds?.forEach(pId => { + const extraFormData = dataMaskSelected?.[pId]?.extraFormData; + if (extraFormData?.filters?.length) { + selectedParentFilterValueCounts += extraFormData.filters.length; + } else if (extraFormData?.time_range) { + selectedParentFilterValueCounts += 1; + } + }); + + // check if all parent filters with defaults have a value selected + + let depsCount = dependencies.filters?.length ?? 0; + + if (dependencies?.time_range) { + depsCount += 1; + } + if (selectedParentFilterValueCounts !== depsCount) { + // child filter should not request backend until it + // has all the required information from parent filters + return; + } + } + // TODO: We should try to improve our useEffect hooks to depend more on // granular information instead of big objects that require deep comparison. const customizer = ( @@ -226,6 +254,7 @@ const FilterValue: FC = ({ hasDataSource, isRefreshing, shouldRefresh, + dataMaskSelected, ]); useEffect(() => { diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx index baa52d58e485..6739a068c8dd 100644 --- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx @@ -151,6 +151,8 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { const [col] = groupby; const [initialColtypeMap] = useState(coltypeMap); const [search, setSearch] = useState(''); + const isChangedByUser = useRef(false); + const prevDataRef = useRef(data); const [dataMask, dispatchDataMask] = useImmerReducer(reducer, { extraFormData: {}, filterState, @@ -271,6 +273,8 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { } else { updateDataMask(values); } + + isChangedByUser.current = true; }, [updateDataMask, formData.nativeFilterId, clearAllTrigger], ); @@ -368,6 +372,58 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) { inverseSelection, ]); + useEffect(() => { + const prev = prevDataRef.current; + const curr = data; + + const hasDataChanged = prev?.length !== curr?.length || + prev?.some((row, i) => { + const prevVal = row[col]; + const currVal = curr[i][col]; + return typeof prevVal === 'bigint' || typeof currVal === 'bigint' + ? prevVal?.toString() !== currVal?.toString() + : prevVal !== currVal; + }); + + // If data actually changed (e.g., due to parent filter), reset flag + if (hasDataChanged) { + isChangedByUser.current = false; + prevDataRef.current = data; + } + }, [data, col]); + + useEffect(() => { + if ( + isChangedByUser.current && + filterState.value && + filterState.value.every((value?: any) => data.some(row => row[col] === value)) + ) + return; + + const firstItem: SelectValue = data[0] + ? (groupby.map(col => data[0][col]) as string[]) + : null; + + if ( + defaultToFirstItem && + Object.keys(formData?.extraFormData || {}).length && + filterState.value !== undefined && + firstItem !== null && + filterState.value !== firstItem + ) { + if (firstItem?.[0] !== undefined) { + updateDataMask(firstItem); + } + } + }, [ + defaultToFirstItem, + updateDataMask, + formData, + data, + JSON.stringify(filterState.value), + isChangedByUser.current, + ]); + useEffect(() => { setDataMask(dataMask); }, [JSON.stringify(dataMask)]);