codeant-ai-for-open-source[bot] commented on code in PR #34785:
URL: https://github.com/apache/superset/pull/34785#discussion_r3664361032
##########
superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx:
##########
@@ -113,7 +117,7 @@ const SelectAsyncControl = ({
if (!loaded) {
loadOptions();
}
- }, [addDangerToast, dataEndpoint, mutator, value, loaded]);
+ }, [addDangerToast, dataEndpoint, mutator, value, loaded, searchParams]);
Review Comment:
**Suggestion:** Adding `searchParams` to the effect dependencies does not
reload options when it changes because `loaded` remains true after the first
request. If a `ChartSelect` instance is reused with a different `datasetId`,
the endpoint query changes but no request is made, leaving charts from the
previous dataset displayed. Reset `loaded` and options when the endpoint
parameters change, or track the loaded parameters instead of using a one-shot
flag. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Chart selector can show charts from the wrong dataset.
- ⚠️ Dataset drill-to-details configuration may save an invalid chart.
- ⚠️ Dataset editor options become stale after dataset changes.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render `ChartSelectUsingAsync()` from
`superset-frontend/src/components/Select/ChartSelect.tsx:48` with a dataset
ID; it derives
an encoded `q` filter from `datasetId` at lines 56-80 and passes it as
`searchParams` at
line 96.
2. On the initial render, `SelectAsyncControl` requests `/api/v1/chart/`
with those
parameters at
`superset-frontend/src/explore/components/controls/SelectAsyncControl/index.tsx:101-105`,
stores the response, and sets `loaded` to true at lines 113-115.
3. Reuse the same component instance with a different `datasetId`, causing
`ChartSelect`
to produce new `searchParams` and the effect at line 120 to rerun.
4. The effect still sees `loaded === true`, so the `if (!loaded)` guard at
lines 117-119
skips the request; the options remain from the previous dataset even though
the API query
has changed. The production caller is the dataset editor at
`superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx:1065-1069`,
where the selector is rendered with `datasource.id`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=10df5863cb7e4ce392763bde8f17cebd&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=10df5863cb7e4ce392763bde8f17cebd&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/explore/components/controls/SelectAsyncControl/index.tsx
**Line:** 120:120
**Comment:**
*Stale Reference: Adding `searchParams` to the effect dependencies does
not reload options when it changes because `loaded` remains true after the
first request. If a `ChartSelect` instance is reused with a different
`datasetId`, the endpoint query changes but no request is made, leaving charts
from the previous dataset displayed. Reset `loaded` and options when the
endpoint parameters change, or track the loaded parameters instead of using a
one-shot flag.
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%2F34785&comment_hash=4efbc87351db1349b5a3a4ba8b849df32ef99bf80a1b5c552d60ce2c8015af3b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=4efbc87351db1349b5a3a4ba8b849df32ef99bf80a1b5c552d60ce2c8015af3b&reaction=dislike'>👎</a>
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -277,16 +275,13 @@ const ResultSet = ({
const openInNewWindow = clickEvent.metaKey;
logAction(LOG_ACTIONS_SQLLAB_CREATE_CHART, {});
if (results?.query_id) {
- const key = await postFormData(results.query_id, 'query', {
+ const url = await generateExploreUrl(results.query_id, 'query', {
Review Comment:
**Suggestion:** The new asynchronous URL generation can reject when the
form-data POST fails, but this click handler has no catch or user-facing error
path. Clicking Create chart during a network or server failure therefore
produces an unhandled promise rejection and no feedback to the user. Catch the
error and show the same danger toast used for other explore URL generation
failures. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ SQL Lab chart creation fails silently on network or server errors.
- ⚠️ Users receive no actionable error feedback.
- ⚠️ Failed URL generation creates unhandled promise rejections.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open SQL Lab and execute a query whose results include a query ID;
`ResultSet` renders
the Create chart action and binds it to `createExploreResultsOnClick()` at
`superset-frontend/src/SqlLab/components/ResultSet/index.tsx:341`.
2. Click Create chart, invoking `createExploreResultsOnClick()` at
`superset-frontend/src/SqlLab/components/ResultSet/index.tsx:272`, which
calls
`generateExploreUrl()` at line 278.
3. The URL helper posts form data through `postFormData()` in
`superset-frontend/src/explore/exploreUtils/formData.ts:37`; a failed
request rejects the
returned promise.
4. Because `createExploreResultsOnClick()` has no `try/catch` or rejection
handler around
line 278, the click produces an unhandled promise rejection and neither
`history.push()`
nor a danger toast executes. The analogous drill-through handler already
catches this
failure and calls `addDangerToast()` at
`superset-frontend/src/components/Chart/DrillDetail/DrillDetailModal.tsx:41-44`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2cbcebf6b1d54559a77a3d6405a58688&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=2cbcebf6b1d54559a77a3d6405a58688&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/SqlLab/components/ResultSet/index.tsx
**Line:** 278:278
**Comment:**
*Possible Bug: The new asynchronous URL generation can reject when the
form-data POST fails, but this click handler has no catch or user-facing error
path. Clicking Create chart during a network or server failure therefore
produces an unhandled promise rejection and no feedback to the user. Catch the
error and show the same danger toast used for other explore URL generation
failures.
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%2F34785&comment_hash=7bc1110b3a364c9b78ff33c72e6272b8c1063c5531d80f2fa5b3020d9437385a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F34785&comment_hash=7bc1110b3a364c9b78ff33c72e6272b8c1063c5531d80f2fa5b3020d9437385a&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]
