codeant-ai-for-open-source[bot] commented on code in PR #37131:
URL: https://github.com/apache/superset/pull/37131#discussion_r3454668418
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -406,46 +438,59 @@ export const useExploreAdditionalActionsMenu = (
streamingThreshold,
slice,
startExport,
+ handleExportError,
]);
- const exportCSVPivoted = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'post_processed',
- resultFormat: 'csv',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportCSVPivoted = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'post_processed',
+ resultFormat: 'csv',
+ });
+ } catch (error) {
+ handleExportError(error);
+ }
+ return null;
+ }, [canDownloadCSV, latestQueryFormData, ownState, handleExportError]);
- const exportJson = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'results',
- resultFormat: 'json',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportJson = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'results',
+ resultFormat: 'json',
+ });
Review Comment:
**Suggestion:** The "Export All Data → JSON" path now calls the API with
`resultType: 'results'`, which returns only the current-view/result subset
instead of the full dataset. This breaks the "all data" contract and can
silently truncate exports; use the full-export result type for this menu
action. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Explore "Export All Data → JSON" returns partial dataset.
- ❌ JSON export semantics differ from CSV full-data export.
- ⚠️ Users may trust truncated JSON as complete data.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Explore view for any chart; the Explore header uses
`useExploreAdditionalActionsMenu` from
`superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx`
to
build the "Data Export Options" menu (see menu construction around lines
208–230 in the
tool output).
2. In `useExploreAdditionalActionsMenu`, the "Export All Data" submenu is
defined with
children that include an item labeled `t('Export to .JSON')` whose `onClick`
handler calls
`exportJson()` (lines 32–47 in the menu snippet at the bottom of `index.tsx`
as returned
by the BulkRead tool).
3. The `exportJson` callback is defined earlier in the same file at lines
162–177 (tool
output): it calls `exportChart({ formData: latestQueryFormData as
QueryFormData, ownState,
resultType: 'results', resultFormat: 'json' })`, which is precisely the
snippet at lines
467–471 in the PR diff.
4. In `superset-frontend/src/explore/exploreUtils/index.ts` lines 19–49,
`exportChart`
forwards `resultType` into the v1 `/api/v1/chart/data` payload; elsewhere in
`useExploreAdditionalActionsMenu` the "Export Current View → CSV" path
explicitly
documents that `resultType: 'results'` is used "to export the *current view*
(as opposed
to 'full')". Therefore, clicking "Data Export Options → Export All Data →
Export to .JSON"
invokes the current-view export mode rather than the full export mode,
causing the
exported JSON to include only the current results instead of the full
dataset whenever
those differ.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=64c420bb6a0741549a395e7279b4047d&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=64c420bb6a0741549a395e7279b4047d&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/useExploreAdditionalActionsMenu/index.tsx
**Line:** 467:471
**Comment:**
*Logic Error: The "Export All Data → JSON" path now calls the API with
`resultType: 'results'`, which returns only the current-view/result subset
instead of the full dataset. This breaks the "all data" contract and can
silently truncate exports; use the full-export result type for this menu action.
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%2F37131&comment_hash=6ab09c998a43711f2a1608736a3198fe92188b5485c1accc7c0d859a96c26a4a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37131&comment_hash=6ab09c998a43711f2a1608736a3198fe92188b5485c1accc7c0d859a96c26a4a&reaction=dislike'>👎</a>
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -406,46 +438,59 @@ export const useExploreAdditionalActionsMenu = (
streamingThreshold,
slice,
startExport,
+ handleExportError,
]);
- const exportCSVPivoted = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'post_processed',
- resultFormat: 'csv',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportCSVPivoted = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'post_processed',
+ resultFormat: 'csv',
+ });
+ } catch (error) {
+ handleExportError(error);
+ }
+ return null;
+ }, [canDownloadCSV, latestQueryFormData, ownState, handleExportError]);
- const exportJson = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'results',
- resultFormat: 'json',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportJson = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'results',
+ resultFormat: 'json',
+ });
+ } catch (error) {
+ handleExportError(error);
+ }
+ return null;
+ }, [canDownloadCSV, latestQueryFormData, ownState, handleExportError]);
- const exportExcel = useCallback(
- () =>
- canDownloadCSV
- ? exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'results',
- resultFormat: 'xlsx',
- })
- : null,
- [canDownloadCSV, latestQueryFormData, ownState],
- );
+ const exportExcel = useCallback(async () => {
+ if (!canDownloadCSV) {
+ return null;
+ }
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'results',
+ resultFormat: 'xlsx',
+ });
Review Comment:
**Suggestion:** The "Export All Data → Excel" flow also uses `resultType:
'results'`, so it exports the current result slice rather than the full dataset
expected by the menu label. This creates inconsistent behavior versus CSV
all-data export and causes partial Excel downloads. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ "Export All Data → Excel" omits expected rows.
- ❌ Excel export behavior inconsistent with CSV full export.
- ⚠️ Analysts may misinterpret incomplete Excel downloads.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the Explore view for any chart; the Explore header wires in
`useExploreAdditionalActionsMenu` from
`superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx`
to
populate the "Data Export Options → Export All Data" submenu (see menu items
pushed into
`allDataChildren` at lines 32–85 in the tool output).
2. For the "Export All Data → Export to Excel" item, the `onClick` handler
calls
`exportExcel()` (lines 71–85 of the menu snippet in `index.tsx`), which is
specifically
used under the "Export All Data" group, not the "Export Current View" group.
3. The `exportExcel` callback is defined earlier in the same file at lines
179–194 (tool
output): it calls `exportChart({ formData: latestQueryFormData as
QueryFormData, ownState,
resultType: 'results', resultFormat: 'xlsx' })`, matching the PR diff
snippet at lines
484–488.
4. In contrast, the "Export All Data → Export to .CSV" path in `index.tsx`
uses
`exportChart` with `resultType: 'full'` (lines 70–143), while the "Export
Current View →
CSV" current-view path explicitly uses `resultType: 'results'` and comments
that
`'results'` corresponds to exporting the *current view* rather than the full
dataset.
Because the Excel all-data item reuses `resultType: 'results'`, clicking
"Export All Data
→ Export to Excel" exports only the current-view slice instead of the full
dataset,
leading to partial Excel files whenever the current view is a subset of the
full data.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ef640b0e42df4b60b8f9fbfb5a4e562c&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=ef640b0e42df4b60b8f9fbfb5a4e562c&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/useExploreAdditionalActionsMenu/index.tsx
**Line:** 484:488
**Comment:**
*Logic Error: The "Export All Data → Excel" flow also uses `resultType:
'results'`, so it exports the current result slice rather than the full dataset
expected by the menu label. This creates inconsistent behavior versus CSV
all-data export and causes partial Excel downloads.
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%2F37131&comment_hash=b9e8e636aae3bfd6a9c58e830b7bbae6b4966898c252f342ef47b2d0fbacd343&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37131&comment_hash=b9e8e636aae3bfd6a9c58e830b7bbae6b4966898c252f342ef47b2d0fbacd343&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]
