sadpandajoe commented on code in PR #44069:
URL: https://github.com/apache/superset/pull/44069#discussion_r4155816851
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx:
##########
@@ -315,6 +333,86 @@ const FilterBar: FC<FiltersBarProps> = ({
const hasRequiredValue = isRequired && isEmptyValue;
+ // Cascade clearing: when a parent filter's value changes, every
+ // transitive descendant (child) dependent filter must have its
+ // selection reset. Otherwise the child keeps a stale value that no
+ // longer belongs to the parent's option set (e.g. Country=UK with a
+ // City value only valid under USA), producing impossible filter
+ // combinations that blank charts.
+ const prevMask = draft[filter.id];
+ const prevValue = prevMask?.filterState?.value;
+ const prevExtra = prevMask?.extraFormData;
+ const nextExtra = baseDataMask.extraFormData;
+ // Filters configured with defaultToFirstItem auto-select their first
+ // option on load. That seed is initialization, not a dependency
+ // change, and must not clear descendants. Persisted values reach the
+ // applied state through the sync effect rather than this callback, so
+ // any other first emission is a genuine user selection.
+ const isAutoSeedInit =
+ prevValue === undefined &&
!!filter.controlValues?.defaultToFirstItem;
+ // A filter being (re)initialized from persisted state re-emits its own
+ // saved mask on mount: first the reducer's empty extraFormData, then
+ // its saved clauses. Those synchronization emissions are not user
+ // changes and must not cascade-clear descendants, or opening a
+ // dashboard with a saved parent/child combination would wipe the child
+ // with no user action. The parent only counts as "live" once it has
+ // been initialized (received a value with non-empty extraFormData) or
+ // when it transitions from an empty/cleared state into a real
+ // selection.
+ const isInitializationEmission =
+ !initializedFilters.has(filter.id) &&
+ prevValue !== undefined &&
+ prevValue !== null;
+ // The effective dependency state is the parent's extraFormData (the
+ // clauses and time_range merged into descendants), not the raw
+ // selected value: inverse-selection toggles change the clause while
+ // the selected value stays identical.
+ const parentValueChanged =
+ !!prevMask &&
+ !isAutoSeedInit &&
+ !isInitializationEmission &&
+ !isEqual(prevExtra, nextExtra);
+ if (parentValueChanged) {
+ const childIds = resolveTransitiveChildIds(filter.id, filters);
+ childIds.forEach(childId => {
+ const childMask = draft[childId];
+ if (!childMask) return;
+ const childFilter = filters[childId];
+ const childInScope = inScopeFilterIds.has(childId);
+ childMask.extraFormData = {};
+ const { filterState } = childMask;
+ if (filterState) {
+ const childIsRequired =
+ !!childFilter?.controlValues?.enableEmptyFilter;
+ // A defaultToFirstItem child stages undefined (not null) so the
+ // Select plugin's init effect re-seeds the first option of the
+ // newly-scoped set: clearing it to null would leave it empty
even
+ // though its whole purpose is to resolve to the first value.
+ filterState.value = getClearedValue(
Review Comment:
If Apply is clicked before a default-to-first child's new options arrive,
this commits its undefined value and empty mask; when the first option later
appears, `needsAutoApply` rejects it because the applied value is undefined, so
the control shows a selection that charts do not use until a second Apply.
Could Apply wait for the child to resolve, or commit its resolved mask with the
parent change?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
