Re: [PR] feat: Use dashboard name for screenshot download [superset]

Tue, 02 Sep 2025 08:48:41 -0700 


korbit-ai[bot] commented on code in PR #34988:
URL: https://github.com/apache/superset/pull/34988#discussion_r2316496436


##########
superset/dashboards/api.py:
##########
@@ -1204,6 +1205,10 @@ def screenshot(self, pk: int, digest: str) -> 
WerkzeugResponse:
                 image = cache_payload.get_image()
             except ScreenshotImageNotAvailableException:
                 return self.response_404()
+
+            filename = get_filename(
+                dashboard.dashboard_title, dashboard.id, skip_id=True
+            )

Review Comment:
   ### Potential filename collision for dashboards with identical titles 
<sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The get_filename function is called with skip_id=True, which always excludes 
the ID, potentially leading to filename collisions if multiple dashboards have 
the same title.
   
   
   ###### Why this matters
   When dashboards share the same title, their downloaded files could overwrite 
each other in the user's system since the filenames would be identical.
   
   ###### Suggested change ∙ *Feature Preview*
   Make skip_id parameter optional and default to False to include the 
dashboard ID in the filename, ensuring uniqueness:
   ```python
   filename = get_filename(
       dashboard.dashboard_title, dashboard.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/4dd31562-c90c-46cf-a194-f642bc84b30b/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4dd31562-c90c-46cf-a194-f642bc84b30b?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/4dd31562-c90c-46cf-a194-f642bc84b30b?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/4dd31562-c90c-46cf-a194-f642bc84b30b?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/4dd31562-c90c-46cf-a194-f642bc84b30b)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:95709984-3117-415d-9c74-dfa43bfdce8f -->
   
   
   [](95709984-3117-415d-9c74-dfa43bfdce8f)



-- 
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]

Reply via email to