You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Vitor-Avila (via GitHub)" <gi...@apache.org> on 2025/07/08 03:18:00 UTC
[PR] chore: Improve performance to load chart's save modal [superset]
Vitor-Avila opened a new pull request, #34097:
URL: https://github.com/apache/superset/pull/34097
### SUMMARY
**_Note: This is currently on draft pending https://github.com/dpgaspar/Flask-AppBuilder/pull/2372 to be included in a new FAB release._**
Follow up to: https://github.com/apache/superset/pull/34079 this uses `select_columns` when loading the SaveModal in Explore.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI changes.
### TESTING INSTRUCTIONS
Test updated.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Improve performance to load chart's save modal [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
korbit-ai[bot] commented on code in PR #34097:
URL: https://github.com/apache/superset/pull/34097#discussion_r2195183297
##########
superset-frontend/src/explore/actions/saveModalActions.ts:
##########
@@ -312,7 +312,7 @@ export const getSliceDashboards =
try {
const response = await SupersetClient.get({
endpoint: `/api/v1/chart/${slice.slice_id}?q=${rison.encode({
- columns: ['dashboards.id'],
+ select_columns: ['dashboards.id'],
Review Comment:
### Unverified API parameter change <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code change assumes the API endpoint supports 'select_columns' parameter, but there's no validation that this parameter actually exists and works in the API.
###### Why this matters
If the API endpoint doesn't support 'select_columns', the request will fail and potentially crash the chart saving functionality, leaving users unable to save their charts.
###### Suggested change ∙ *Feature Preview*
Verify the API endpoint supports the new parameter and include error handling for backward compatibility:
```typescript
try {
const response = await SupersetClient.get({
endpoint: `/api/v1/chart/${slice.slice_id}?q=${rison.encode({
select_columns: ['dashboards.id'],
// Fallback if needed
columns: ['dashboards.id'],
})}`,
});
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:b1039685-656a-4f93-bcc2-5bd10f0db526 -->
[](b1039685-656a-4f93-bcc2-5bd10f0db526)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] chore: Improve performance to load chart's save modal [superset]
Posted by "Vitor-Avila (via GitHub)" <gi...@apache.org>.
Vitor-Avila merged PR #34097:
URL: https://github.com/apache/superset/pull/34097
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org