bito-code-review[bot] commented on code in PR #37131:
URL: https://github.com/apache/superset/pull/37131#discussion_r3395426050
##########
superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts:
##########
@@ -150,6 +150,25 @@ export default class SupersetClientClass {
}
}
+ /**
+ * POST request that returns a blob for file downloads.
+ * Unlike postForm, this uses AJAX so errors can be caught and handled.
+ * @param endpoint - API endpoint
+ * @param payload - Request payload
+ * @returns Promise resolving to Response with blob
+ */
+ async postBlob(
+ endpoint: string,
+ payload: Record<string, any>,
+ ): Promise<Response> {
+ await this.ensureAuth();
+ return this.post({
+ endpoint,
+ postPayload: payload,
+ parseMethod: 'raw',
+ });
+ }
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing Unit Tests for postBlob</b></div>
<div id="fix">
The new `postBlob` method lacks unit tests. Rule [11730] requires
comprehensive unit tests for new tools covering success paths, error scenarios,
and edge cases. Compare with `postForm` which has tests at lines 668-775 in the
same test file.
</div>
</div>
<small><i>Code Review Run #d807d9</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx:
##########
@@ -378,26 +378,51 @@ export const useExploreAdditionalActionsMenu = (
filename = `${safeChartName}${timestamp}.csv`;
}
- return exportChart({
- formData: latestQueryFormData as QueryFormData,
- ownState,
- resultType: 'full',
- resultFormat: 'csv',
- onStartStreamingExport: shouldUseStreaming
- ? exportParams => {
- if (exportParams.url) {
- setIsStreamingModalVisible(true);
- startExport({
- ...exportParams,
- url: exportParams.url,
- filename,
- expectedRows: actualRowCount,
- exportType: exportParams.exportType as 'csv' | 'xlsx',
- });
+ try {
+ await exportChart({
+ formData: latestQueryFormData as QueryFormData,
+ ownState,
+ resultType: 'full',
+ resultFormat: 'csv',
+ onStartStreamingExport: shouldUseStreaming
+ ? exportParams => {
+ if (exportParams.url) {
+ setIsStreamingModalVisible(true);
+ startExport({
+ ...exportParams,
+ url: exportParams.url,
+ filename,
+ expectedRows: actualRowCount,
+ exportType: exportParams.exportType as 'csv' | 'xlsx',
+ });
+ }
}
- }
- : null,
- });
+ : null,
+ });
+ } catch (error) {
+ const exportError = error as Error & {
+ status?: number;
+ statusText?: string;
+ response?: { status?: number };
+ };
+ const status = exportError.status || exportError.response?.status;
+ if (status === 413) {
+ addDangerToast(
+ t(
+ 'Export failed: The chart data is too large to download (413).
Please try reducing the date range, limiting rows, or using fewer columns.',
+ ),
+ );
+ } else {
+ const errorMessage =
+ exportError.message ||
+ exportError.statusText ||
+ t(
+ 'Failed to export chart data. Please try again or contact your
administrator.',
+ );
+ addDangerToast(errorMessage);
+ }
+ }
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Semantic duplication in error handling</b></div>
<div id="fix">
The `exportCSV` function duplicates the same error handling logic that
already exists in the `handleExportError` callback (lines 437-462). This
creates semantic duplication and inconsistent error messages (note the 413
error message differs: 'Export failed: The chart data is too large to download
(413).' vs 'The chart data is too large to download.'). Use
`handleExportError(error)` instead.
</div>
</div>
<small><i>Code Review Run #d807d9</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
