codeant-ai-for-open-source[bot] commented on code in PR #40475:
URL: https://github.com/apache/superset/pull/40475#discussion_r3530953080
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.tsx:
##########
@@ -113,19 +163,9 @@ export function ColumnSelect({
resetColumnField();
}
setColumns(result.columns);
- },
- async badResponse => {
- const { error, message } = await getClientErrorObject(badResponse);
- let errorText = message || error || t('An error has occurred');
- if (message === 'Forbidden') {
- errorText = t(
- 'You do not have permission to edit this dashboard',
- );
- }
- addDangerToast(errorText);
- },
- )
- .finally(() => setLoading(false));
+ }, handleError)
+ .finally(() => setLoading(false));
+ }
}
});
Review Comment:
**Suggestion:** The new dual-endpoint fetch path can race when users switch
datasource type or dataset quickly: earlier requests are not cancelled or
ignored, so a slower stale response can overwrite `columns` from a newer
selection and leave the UI showing the wrong column list. Track a request token
(or compare current `datasourceKey` before `setColumns`) and ignore stale
responses. [race condition]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ ColumnSelect may show columns from previous dataset.
- ⚠️ Filters can be configured against unintended columns.
- ⚠️ UI inconsistent when toggling semantic view and dataset.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dashboard's native filters configuration modal, which renders
`FiltersConfigForm` (`FiltersConfigForm.tsx` lines 31-51) and, via
`getControlItemsMap`, a
`ColumnSelect` for the groupby/column field (`getControlItemsMap.tsx` lines
69-88 and
140-160).
2. In `FiltersConfigForm`, observe that `datasetId` and `datasourceType` are
derived from
current form state and existing targets (`FiltersConfigForm.tsx` lines
134-151 and
153-166) and passed into `getControlItemsMap`, which forwards them as props
to
`ColumnSelect` (`getControlItemsMap.tsx` lines 140-146).
3. In `ColumnSelect`, note that `datasourceKey` is computed from `datasetId`
and
`datasourceType` and passed to `useChangeEffect` (`ColumnSelect.tsx` lines
95-100); when
this key changes, the effect issues an async `cachedSupersetGet` call to
either
`/api/v1/semantic_view/{id}/structure` or `/api/v1/dataset/{id}?q=...` and
then calls
`setColumns` in the `.then` handler (`ColumnSelect.tsx` lines 118-168).
4. Rapidly switch the dataset or datasource type in the FiltersConfig UI
(e.g., select
dataset A, then quickly dataset B or toggle between table and semantic view)
so that
multiple `cachedSupersetGet` calls are in flight; because
`cachedSupersetGet` is a simple
cache wrapper around `SupersetClient.get` with no cancellation
(`cachedSupersetGet.ts`
lines 20-29) and the `ColumnSelect` effect does not compare the current
`datasourceKey` or
track a request token before calling `setColumns` (`ColumnSelect.tsx` lines
136-166), a
slower stale response for the previous selection can resolve last and
overwrite `columns`
for the newer selection, leaving the ColumnSelect options out of sync with
the currently
selected dataset/datasourceType.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=76a3417170bd417d9ac605259acf427d&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=76a3417170bd417d9ac605259acf427d&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/FiltersConfigModal/FiltersConfigForm/ColumnSelect.tsx
**Line:** 99:170
**Comment:**
*Race Condition: The new dual-endpoint fetch path can race when users
switch datasource type or dataset quickly: earlier requests are not cancelled
or ignored, so a slower stale response can overwrite `columns` from a newer
selection and leave the UI showing the wrong column list. Track a request token
(or compare current `datasourceKey` before `setColumns`) and ignore stale
responses.
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%2F40475&comment_hash=2b5744266229da296610a668ecfe5c6d13b37ca744c053c1cc2d6aa80b759034&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40475&comment_hash=2b5744266229da296610a668ecfe5c6d13b37ca744c053c1cc2d6aa80b759034&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]
