codeant-ai-for-open-source[bot] commented on code in PR #41937:
URL: https://github.com/apache/superset/pull/41937#discussion_r3560856304
##########
superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:
##########
@@ -377,6 +371,63 @@ export default function DrillByModal({
formData,
]);
+ const handleDownload = useCallback(
+ (exportType: 'csv' | 'xlsx') => {
+ Promise.resolve(
+ exportChart({
+ formData: drilledFormData,
+ resultFormat: exportType,
+ resultType: 'full',
+ }),
+ ).catch(error => {
+ addDangerToast(
+ t('Failed to generate download: %s', error?.message || error),
+ );
+ });
+ },
+ [drilledFormData, addDangerToast],
+ );
+
+ const handleDownloadCSV = useCallback(
+ () => handleDownload('csv'),
+ [handleDownload],
+ );
+
+ const handleDownloadXLSX = useCallback(
+ () => handleDownload('xlsx'),
+ [handleDownload],
+ );
+
+ const handleReload = useCallback(() => {
+ setChartDataResult(undefined);
+ setIsChartDataLoading(true);
+ const [useLegacyApi] = getQuerySettings(drilledFormData);
+ getChartDataRequest({
+ formData: drilledFormData,
+ })
+ .then(({ response, json }) =>
+ handleChartDataResponse(response, json, useLegacyApi),
+ )
+ .then(queriesResponse => {
+ setChartDataResult(queriesResponse);
+ })
+ .catch(() => {
+ addDangerToast(t('Failed to load chart data.'));
+ })
+ .finally(() => {
+ setIsChartDataLoading(false);
+ });
+ }, [addDangerToast, drilledFormData]);
Review Comment:
**Suggestion:** The new reload flow issues uncancelled concurrent requests
that all mutate the same `chartDataResult`/`isChartDataLoading` state, so rapid
reload clicks (or reload during an in-flight auto-fetch) can show stale results
and prematurely hide the loading spinner when an earlier request finishes
first. Add request cancellation or a request-id guard so only the latest reload
response updates state. [race condition]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Drill-by table can show outdated query results.
- ⚠️ Reload icon clears spinner while request still running.
- ⚠️ Concurrent auto-fetch and reload yield non-deterministic results.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a dashboard and trigger the Drill By modal for a chart, rendering
`DrillByModal`
in `superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:169-201`
(modal title
and layout at lines 117-201 in the file read via tools).
2. Switch the modal to table mode so that `drillByDisplayMode ===
DrillByType.Table` and
`chartDataResult` is populated by the auto-fetch effect at
`DrillByModal.tsx:93-114`,
which calls `getChartDataRequest({ formData: drilledFormData })` and updates
`chartDataResult`/`isChartDataLoading`.
3. In the table view, use the reload icon wired in
`DataTableControls.tsx:65-73`, where
`<Icons.ReloadOutlined ... onClick={onReload} />` calls the `onReload` prop
provided by
`SingleQueryResultPane` (`SingleQueryResultPane.tsx:75-92`) and ultimately
`useResultsTableView` (`useResultsTableView.tsx:34-81`), which passes
`handleReload` from
`DrillByModal.tsx` (lines 2-21 in the 400–659 read) as `onReload`.
4. Click the reload icon multiple times rapidly (or click it once while the
auto-fetch
effect request is still in flight); each click invokes `handleReload` at
`DrillByModal.tsx:2-21`, which issues a new uncancelled
`getChartDataRequest` and
independently calls `setChartDataResult(...)` and
`setIsChartDataLoading(false)` in its
`.then`/`.finally` chain. Because neither the effect-based request (lines
93-114) nor the
reload path (lines 2-21) uses an `AbortController` or a request-id guard,
whichever
request (initial auto-fetch or any reload) finishes first will clear the
loading state and
may overwrite `chartDataResult`, so an earlier, stale response can hide the
spinner while
a newer request is still running and can replace the latest results with
older data when
it completes later.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=00dfdd2eaeb140c7a4832d0a269e5ffd&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=00dfdd2eaeb140c7a4832d0a269e5ffd&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/components/Chart/DrillBy/DrillByModal.tsx
**Line:** 401:420
**Comment:**
*Race Condition: The new reload flow issues uncancelled concurrent
requests that all mutate the same `chartDataResult`/`isChartDataLoading` state,
so rapid reload clicks (or reload during an in-flight auto-fetch) can show
stale results and prematurely hide the loading spinner when an earlier request
finishes first. Add request cancellation or a request-id guard so only the
latest reload response updates 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.
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%2F41937&comment_hash=a723846e7e4c71ddc552a77d097314385566a079e9061ec52138b2a88b426cbb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41937&comment_hash=a723846e7e4c71ddc552a77d097314385566a079e9061ec52138b2a88b426cbb&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]
