sadpandajoe commented on code in PR #44069:
URL: https://github.com/apache/superset/pull/44069#discussion_r3980400420
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx:
##########
@@ -311,6 +315,46 @@ 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 prevValue = draft[filter.id]?.filterState?.value;
+ const nextValue = baseDataMask.filterState?.value;
+ const parentValueChanged =
Review Comment:
This value-only test misses dependency changes that do not look like a
defined value changing to a different value: an optional parent's first user
selection is suppressed by `prevValue !== undefined`, while an `is`/`is not`
toggle keeps the same value but changes the parent's `IN`/`NOT IN` mask. In
both cases the child re-queries against different constraints without clearing
its existing selection; should cascade detection compare the effective
dependency mask while separately distinguishing initialization?
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -513,6 +515,27 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
}
}, [clearAllTrigger, onClearAllComplete, updateDataMask]);
+ useEffect(() => {
+ // When a parent filter's value changes, a cascading clear signals this
+ // dependent filter to reset its visual selection. Same behavior as a
+ // global clear-all but scoped to one descendant.
+ if (cascadeClearTrigger) {
Review Comment:
A pending LIKE-input debounce can run after this effect clears the child
because the cancellation effect only watches `clearAllTrigger`. If someone
types in a dependent Contains/Starts with filter and changes its parent before
`SLOW_DEBOUNCE` expires, the delayed callback restores the stale child value;
should cascade clear cancel `debouncedLikeChange` too?
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx:
##########
@@ -311,6 +315,46 @@ 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 prevValue = draft[filter.id]?.filterState?.value;
+ const nextValue = baseDataMask.filterState?.value;
+ const parentValueChanged =
+ prevValue !== undefined && !isEqual(prevValue, nextValue);
+ if (parentValueChanged) {
+ const childIds = resolveTransitiveChildIds(filter.id, filters);
+ childIds.forEach(childId => {
+ const childMask = draft[childId];
+ if (!childMask) return;
+ childMask.extraFormData = {};
+ const { filterState } = childMask;
+ if (filterState) {
+ const childIsRequired =
+ !!filters[childId]?.controlValues?.enableEmptyFilter;
+ // Mirror handleClearAll: range filters use [null, null] as the
+ // canonical cleared value. Bare null would be ignored by
+ // RangeFilterPlugin's sync effect, leaving stale UI.
+ filterState.value =
Review Comment:
`[null, null]` is the range clear sentinel, but both required-value checks
only treat `null`/`undefined` (or an empty array) as empty. When the Range
plugin syncs this value back, it removes the staged validation error, so a
required child can be applied with empty `extraFormData`; should all-null
ranges remain invalid?
--
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]
