codeant-ai-for-open-source[bot] commented on code in PR #36927:
URL: https://github.com/apache/superset/pull/36927#discussion_r2665657572
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx:
##########
@@ -204,24 +204,38 @@ const FilterBar: FC<FiltersBarProps> = ({
dataMask: Partial<DataMask>,
) => {
setDataMaskSelected(draft => {
+ const existingDataMask = dataMaskSelectedRef.current[filter.id];
+ const appliedDataMask = dataMaskApplied[filter.id];
const isFirstTimeInitialization =
!initializedFilters.has(filter.id) &&
- dataMaskSelectedRef.current[filter.id]?.filterState?.value ===
- undefined;
+ existingDataMask?.filterState?.value === undefined;
Review Comment:
**Suggestion:** The `isFirstTimeInitialization` flag currently also depends
on `existingDataMask?.filterState?.value === undefined`, which means that if a
filter ever gets a value in `dataMaskSelected` before it has been marked as
initialized (for example via a default/permalink value without
`extraFormData`), it will permanently stop being treated as "first time" and
required-first auto-apply logic may never fire even though `initializedFilters`
was never updated; using only the explicit `initializedFilters` tracking avoids
this inconsistent state. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
const isFirstTimeInitialization = !initializedFilters.has(filter.id);
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current check couples "first-time" to the presence of an existing value
in the selected mask. That can create a surprising state where a filter
receives a value (e.g. from permalink/default) before initializedFilters is
ever updated, which makes isFirstTimeInitialization false even though we never
actually marked this filter as initialized. For requiredFirst handling we only
care whether the filter was recorded in initializedFilters; using the explicit
flag (!initializedFilters.has(filter.id)) is a clearer and more robust signal
and will restore expected required-first auto-apply behavior. This is a
functional fix, not just a cosmetic change.
</details>
<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/index.tsx
**Line:** 209:211
**Comment:**
*Logic Error: The `isFirstTimeInitialization` flag currently also
depends on `existingDataMask?.filterState?.value === undefined`, which means
that if a filter ever gets a value in `dataMaskSelected` before it has been
marked as initialized (for example via a default/permalink value without
`extraFormData`), it will permanently stop being treated as "first time" and
required-first auto-apply logic may never fire even though `initializedFilters`
was never updated; using only the explicit `initializedFilters` tracking avoids
this inconsistent state.
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.
```
</details>
--
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]
