codeant-ai-for-open-source[bot] commented on code in PR #40978:
URL: https://github.com/apache/superset/pull/40978#discussion_r3465609124
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx:
##########
@@ -228,6 +241,21 @@ const FilterValue: FC<FilterValueProps> = ({
// direct parents) so the counts line up with `dependencies`, which is
// itself built from the transitive chain by `useFilterDependencies`.
+ // Block if any parent with defaultToFirstItem hasn't auto-selected yet.
+ // Without this, the child fetches unfiltered options before the parent
+ // auto-selects, leading to a stale first-value dispatch that never
+ // gets corrected because subsequent re-selections are not
first-initialization.
+ const hasDefaultFirstParentPending = transitiveParentIds.some(pId => {
+ const parentMask = dataMaskSelected?.[pId];
+ return (
+ parentDefaultToFirstItem[pId] &&
+ parentMask?.filterState?.value === undefined
+ );
Review Comment:
**Suggestion:** The readiness guard only treats `undefined` as "parent not
ready", but native filter state is also represented as `null` for an unselected
value in this codebase. If a `defaultToFirstItem` parent is in the `null`
state, this condition incorrectly lets the child fetch early and can
reintroduce the same unfiltered first-fetch race. Treat both `null` and
`undefined` (and optionally other empty-state forms you use) as pending.
[incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Cascading native filters can fetch with stale parent state.
- ❌ Charts may initialize with inconsistent ancestor/child filters.
- ⚠️ Dashboard default-to-first-item behavior becomes race-condition prone.
- ⚠️ Filter readiness logic inconsistent with null-missing convention.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In a dashboard with native filters enabled, configure a parent filter with
`defaultToFirstItem = true` and at least one child cascade filter, so that
the child's
`transitiveParentIds` array includes the parent ID (as used in `FilterValue`
at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx`,
lines 16–18 of the tool output where `useTransitiveParentIds(id)` is called).
2. Ensure the Redux state for the parent filter has
`controlValues.defaultToFirstItem =
true` so that `parentDefaultToFirstItem[pId]` is truthy (derived via
`useSelector` in
`FilterValue.tsx`, lines 20–31 of the tool output), and that the parent's
`dataMaskSelected[pId].filterState.value` is `null` to represent "no
selection yet" (this
`null`-as-missing convention is established by `checkIsMissingRequiredValue`
in
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts`,
lines
16–19, which explicitly treats both `value === null` and `value ===
undefined` as
missing).
3. Mount the child filter so it comes into view and `inViewFirstTime` is
`true`, causing
the `useEffect` in `FilterValue.tsx` (lines 24–83 of the tool output) to
execute. The
readiness guard computes `hasDefaultFirstParentPending =
transitiveParentIds.some(pId => {
const parentMask = dataMaskSelected?.[pId]; return (
parentDefaultToFirstItem[pId] &&
parentMask?.filterState?.value === undefined ); });` (lines 29–35 of the
tool output,
corresponding to diff lines 248–253). Because `filterState.value` is `null`,
not
`undefined`, this condition is false, `hasDefaultFirstParentPending` is
`false`, and the
guard does not early-return.
4. The child filter proceeds to issue a backend `getChartDataRequest` (lines
86–91 of the
tool output in `FilterValue.tsx`) using `dependencies` and `extraFormData`
built from
ancestor filters, even though the parent's `filterState.value` is still
effectively
unselected (`null`). This reproduces the buggy behavior: the child fetches
and potentially
auto-selects options based on an unfiltered parent state, while other parts
of the code
(e.g., `checkIsMissingRequiredValue` in `FilterBar/utils.ts`) still consider
`null` a
"missing" value, proving that the `=== undefined` check in the readiness
guard is too
narrow and allows premature child fetching when the parent is not actually
ready.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4f656aefdbb246339d81dc1330164069&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4f656aefdbb246339d81dc1330164069&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
**Line:** 250:253
**Comment:**
*Incorrect Condition Logic: The readiness guard only treats `undefined`
as "parent not ready", but native filter state is also represented as `null`
for an unselected value in this codebase. If a `defaultToFirstItem` parent is
in the `null` state, this condition incorrectly lets the child fetch early and
can reintroduce the same unfiltered first-fetch race. Treat both `null` and
`undefined` (and optionally other empty-state forms you use) as pending.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40978&comment_hash=9dbaed64717a8ec364cf0b3bafc316cbbf4a7359aa62b3138479c0192e410df6&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40978&comment_hash=9dbaed64717a8ec364cf0b3bafc316cbbf4a7359aa62b3138479c0192e410df6&reaction=dislike'>👎</a>
--
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]
