EnxDev commented on code in PR #38069:
URL: https://github.com/apache/superset/pull/38069#discussion_r4180546478
##########
superset-frontend/src/explore/exploreUtils/index.ts:
##########
@@ -355,9 +355,13 @@ export const exportChart = async ({
endpointType,
allowDomainSharding: false,
});
+ if (!url) {
+ console.warn('Failed to get explore url.');
+ return;
+ }
payload = formData;
} else {
- url = ensureAppRoot('/api/v1/chart/data');
+ url = SupersetClient.getUrl({ endpoint: '/api/v1/chart/data' });
Review Comment:
On master `exportChart` doesn't go through `postForm` anymore. The legacy
branch went away with the explore_json removal and the non-streaming path uses
`postBlob`.
So this hunk and the `exportChart.test.ts` changes can likely take master's
side on rebase. `exploreChart` below is the part that still matters.
##########
superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts:
##########
@@ -115,19 +116,18 @@ export default class SupersetClientClass {
return this.fetchCSRFToken();
}
- async postForm(
- endpoint: string,
- payload: Record<string, any>,
- target = '_blank',
- ) {
- if (endpoint) {
+ async postForm(postFormConfig: PostFormConfig) {
+ if (postFormConfig.endpoint || postFormConfig.url) {
await this.ensureAuth();
const hiddenForm = document.createElement('form');
- hiddenForm.action = this.getUrl({ endpoint });
+ hiddenForm.action = this.getUrl({
Review Comment:
Master wraps this in `sanitizeUrl(...)` now, which is the CodeQL fix and
probably why CodeQL is red here.
Worth keeping when you resolve the conflict, more so now that `url` passes
straight through `getUrl` untouched.
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -131,7 +132,13 @@ export default function Login() {
sessionStorage.setItem('login_attempted', 'true');
// Use standard form submission for Flask-AppBuilder compatibility
- SupersetClient.postForm(loginEndpoint, values, '');
+ SupersetClient.postForm({
+ endpoint: loginEndpoint,
+ payload: values,
+ target: '',
+ }).finally(() => {
+ setLoading(false);
+ });
Review Comment:
`postForm` resolves as soon as `submit()` fires, so this clears the spinner
while the login POST is still navigating and the button is clickable again.
If the point is to recover when `ensureAuth()` rejects, `.catch` covers that
without the flicker.
```suggestion
}).catch(() => {
setLoading(false);
});
```
--
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]
