Re: [PR] chore: Improve performance to load chart's save modal [superset]

[via GitHub]

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>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</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
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea7cd3bc-0a53-43ba-989b-290bda6dc56f?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](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-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org