This is an automated email from the ASF dual-hosted git repository. geido pushed a commit to branch geido/fix-required-filters in repository https://gitbox.apache.org/repos/asf/superset.git
commit f13262e8356a70f124382a808edf3369d9790c95 Author: Diego Pucci <[email protected]> AuthorDate: Tue May 26 21:08:18 2026 +0300 fix(dashboard): required filters reliably apply default + Apply enables on change Three small fixes for native filter state desync: 1. utils.ts — remove the selectedCount !== appliedCount early-return in checkIsApplyDisabled (regression from #37681). It disabled Apply whenever Selected had an entry Applied lacked — i.e. precisely when a user typed a value into a filter whose default never made it to Applied. dataEqual with ignoreUndefined already handles the "no real change" case correctly. 2. reducer.ts updateDataMaskForFilterChanges — only preserve existing filterState/extraFormData when there's actually a non-empty value. Previously, modifying a required filter would preserve empty existing state and wipe the newly defined default. 3. reducer.ts fillNativeFilters — on HYDRATE_DASHBOARD the shallow spread of the permalink-loaded dataMask wiped filter.defaultDataMask.filterState when the loaded mask was captured mid-initialization. Now defaults survive when the loaded mask lacks a real value. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --- .../nativeFilters/FilterBar/utils.test.ts | 7 +++-- .../components/nativeFilters/FilterBar/utils.ts | 10 +++----- superset-frontend/src/dataMask/reducer.ts | 30 ++++++++++++++++++++-- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.test.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.test.ts index accb766a7b2..5e725a563f5 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.test.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.test.ts @@ -299,7 +299,10 @@ test('checkIsApplyDisabled returns true when required filter is missing value in ); }); -test('checkIsApplyDisabled handles filter count mismatch', () => { +test('checkIsApplyDisabled enables Apply when Selected has a filter value not yet in Applied', () => { + // Regression: when a required filter's default isn't applied (Applied missing + // the entry) and the user types a value, Selected gains an entry Applied + // doesn't have. Apply must be enabled so the user can commit the value. const dataMaskSelected: DataMaskStateWithId = { 'filter-1': { id: 'filter-1', @@ -322,7 +325,7 @@ test('checkIsApplyDisabled handles filter count mismatch', () => { const filters = [createFilter('filter-1'), createFilter('filter-2')]; expect(checkIsApplyDisabled(dataMaskSelected, dataMaskApplied, filters)).toBe( - true, + false, ); }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts index b28a24bcf1d..98f941c03e0 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts @@ -74,13 +74,9 @@ export const checkIsApplyDisabled = ( const selectedExtraFormData = getOnlyExtraFormData(dataMaskSelected); const appliedExtraFormData = getOnlyExtraFormData(dataMaskApplied); - // Check counts first - const selectedCount = Object.keys(selectedExtraFormData).length; - const appliedCount = Object.keys(appliedExtraFormData).length; - - if (selectedCount !== appliedCount) return true; - - // Check for changes + // Check for changes. ignoreUndefined drops empty keys on both sides so that + // a filter present in Selected with a real value but absent (or undefined) + // in Applied is correctly detected as a change. const dataEqual = areObjectsEqual( selectedExtraFormData, appliedExtraFormData, diff --git a/superset-frontend/src/dataMask/reducer.ts b/superset-frontend/src/dataMask/reducer.ts index bba15c0aea9..2fa5c2f9aa7 100644 --- a/superset-frontend/src/dataMask/reducer.ts +++ b/superset-frontend/src/dataMask/reducer.ts @@ -109,10 +109,28 @@ function fillNativeFilters( ) { filterConfig.forEach((filter: Filter) => { const dataMask = initialDataMask || {}; + const loaded = dataMask[filter.id]; + + // The shallow spread of `loaded` would override `filter.defaultDataMask` + // even when `loaded.filterState` is empty (e.g. a permalink captured + // mid-initialization), wiping out a valid default. Only let `loaded` + // override `filterState`/`extraFormData` when it actually carries a value. + const loadedValue = loaded?.filterState?.value; + const loadedHasValue = + loadedValue !== undefined && + loadedValue !== null && + !(Array.isArray(loadedValue) && loadedValue.length === 0); + mergedDataMask[filter.id] = { ...getInitialDataMask(filter.id), // take initial data ...filter.defaultDataMask, // if something new came from BE - take it - ...dataMask[filter.id], + ...loaded, + ...(loadedHasValue + ? {} + : { + filterState: filter.defaultDataMask?.filterState, + extraFormData: filter.defaultDataMask?.extraFormData, + }), }; if ( currentFilters && @@ -155,10 +173,18 @@ function updateDataMaskForFilterChanges( // Check if targets are equal const areTargetsEqual = isEqual(prevFilterDef?.targets, filter?.targets); - // Preserve state only if filter exists, has enableEmptyFilter=true and targets match + // Preserve state only when there's actually a value to preserve. Otherwise + // the empty existing state would wipe the (possibly newly-defined) default. + const existingValue = existingFilter?.filterState?.value; + const hasExistingValue = + existingValue !== undefined && + existingValue !== null && + !(Array.isArray(existingValue) && existingValue.length === 0); + const shouldPreserveState = existingFilter && areTargetsEqual && + hasExistingValue && (filter.controlValues?.enableEmptyFilter || filter.controlValues?.defaultToFirstItem);
