codeant-ai-for-open-source[bot] commented on code in PR #37131:
URL: https://github.com/apache/superset/pull/37131#discussion_r2694287505
##########
superset-frontend/packages/superset-ui-core/src/connection/SupersetClient.ts:
##########
@@ -51,6 +51,7 @@ const SupersetClient: SupersetClientInterface = {
reAuthenticate: () => getInstance().reAuthenticate(),
request: request => getInstance().request(request),
getCSRFToken: () => getInstance().getCSRFToken(),
+ postBlob: (endpoint, payload) => getInstance().postBlob(endpoint, payload),
Review Comment:
**Suggestion:** The wrapper uses two separate parameters `(endpoint,
payload)` but callers likely pass a single request object; this will cause the
instance `postBlob` to receive an unexpected shape (the whole request as
`endpoint` and `payload` undefined), leading to runtime errors—forward the
single request argument to the instance method. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Explore export (exportChart) fails producing no download.
- ❌ Dashboard chart/table export fails to download.
- ⚠️ Browser console shows uncaught export-related errors.
- ⚠️ Automated tests for exports may start failing.
```
</details>
```suggestion
postBlob: request => getInstance().postBlob(request),
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Ensure SupersetClient is configured and the singleton exists by calling
SupersetClient.configure(...). (See
superset-frontend/packages/superset-ui-core/src/connection/SupersetClient.ts
around
getInstance()/configure; the wrapper live at line 54.)
2. Trigger a UI chart export that the PR description changed to use postBlob
— e.g., click
"Download CSV" in Explore which calls exportChart in exploreUtils (PR
changes reference
exploreUtils/index.js:exportChart). That code calls
SupersetClient.postBlob(request) with
a single request object (request contains endpoint, payload, filename
metadata).
3. At runtime the wrapper in SupersetClient.ts line 54 executes, treating
the single
object as the `endpoint` parameter and leaving `payload` undefined, then
calls
getInstance().postBlob(endpoint, payload) with unexpected shapes.
4. If SupersetClientClass.postBlob expects a single request object, the
instance method
receives wrong arguments and may throw, return an error, or produce
malformed network
requests — observed as export failures or uncaught exceptions in the browser
console and
failed downloads.
Note: the reproduction maps to the PR description which replaced form
submissions with
SupersetClient.postBlob usage in exploreUtils and Dashboard chart export
flows; the
wrapper at SupersetClient.ts:54 must forward the same single-argument shape
used by those
callers.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/connection/SupersetClient.ts
**Line:** 54:54
**Comment:**
*Logic Error: The wrapper uses two separate parameters `(endpoint,
payload)` but callers likely pass a single request object; this will cause the
instance `postBlob` to receive an unexpected shape (the whole request as
`endpoint` and `payload` undefined), leading to runtime errors—forward the
single request argument to the instance method.
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.
```
</details>
##########
superset-frontend/packages/superset-ui-core/src/connection/types.ts:
##########
@@ -152,6 +152,7 @@ export interface SupersetClientInterface extends Pick<
| 'get'
| 'post'
| 'postForm'
+ | 'postBlob'
| 'put'
| 'request'
| 'init'
Review Comment:
**Suggestion:** Adding `'postBlob'` to the Pick list assumes
`SupersetClientClass` already declares that method; if it doesn't, the Pick
will not include it and consumers expecting `postBlob` will break. Make
`postBlob` an optional member on the exported interface (typed to the correct
request/response types) or ensure the class exposes the method. This preserves
backward compatibility while providing proper types for the new API. [type
error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ TypeScript build failure blocking CI pipelines.
- ❌ Explore exportChart runtime TypeError on export.
- ❌ Dashboard chart export runtime errors.
- ⚠️ Consumers relying on SupersetClient typings mismatched.
```
</details>
```suggestion
| 'put'
| 'request'
| 'init'
| 'isAuthenticated'
| 'reAuthenticate'
| 'getGuestToken'
> {
// `postBlob` may be a new API implemented on SupersetClientClass.
// Declare it optional here to avoid breaking consumers if the class
// implementation isn't present at compile time, and provide a precise
type.
postBlob?: (request: RequestConfig) => Promise<SupersetClientResponse>;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run TypeScript build for the frontend package: from repo root run `cd
superset-frontend
&& npm run build` (this triggers tsc for packages). Type errors originate
from
`superset-frontend/packages/superset-ui-core/src/connection/types.ts:152`
where the Pick
includes `'postBlob'`. Expect TypeScript error like: "Type '"postBlob"' is
not assignable
to parameter of type 'keyof SupersetClientClass'".
2. Inspect the imported class at
`superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass`
(import
path from `types.ts` line 1: `import SupersetClientClass from
'./SupersetClientClass';`).
If that class file does not declare a `postBlob` member, the Pick in
`types.ts` will fail
compilation as shown in step 1.
3. If TypeScript checking is skipped (e.g., during a Babel-only transpile or
in a runtime
test environment), consumer code added in this PR calls
`SupersetClient.postBlob(...)`
from Explore export functions (PR description references
`superset-frontend/src/explore/exploreUtils/index.js` exportChart usage). At
runtime, if
`SupersetClientClass` instance lacks `postBlob`, this results in a runtime
TypeError:
"SupersetClient.postBlob is not a function" when `exportChart` calls it.
4. Confirm by opening the PR-modified consumers:
`superset-frontend/src/explore/exploreUtils/index.js` (exportChart),
`superset-frontend/src/dashboard/Chart.jsx` (table export), and
`superset-frontend/src/hooks/useExploreAdditionalActionsMenu`
(CSV/Excel/JSON exports). If
these files call `SupersetClient.postBlob` but `SupersetClientClass` does
not declare it,
either TypeScript build fails (step 1) or runtime failures occur (step 3).
If the addition
to the Pick was intentional, verify `SupersetClientClass` implements
`postBlob` in
`superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts`.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/packages/superset-ui-core/src/connection/types.ts
**Line:** 158:158
**Comment:**
*Type Error: Adding `'postBlob'` to the Pick list assumes
`SupersetClientClass` already declares that method; if it doesn't, the Pick
will not include it and consumers expecting `postBlob` will break. Make
`postBlob` an optional member on the exported interface (typed to the correct
request/response types) or ensure the class exposes the method. This preserves
backward compatibility while providing proper types for the new API.
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.
```
</details>
--
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]
