Allow overriding filename in Blob#service_url
#31650
Conversation
This is useful when we have several representations for the same underlying file, each one with a different name, and we need to provide a custom download URL based on that name and not that of the underlying file.
|
Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @georgeclaghorn (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. This repository is being automatically checked for code quality issues using Code Climate. You can see results for this analysis in the PR status below. Newly introduced issues should be fixed before a Pull Request is considered ready to review. Please see the contribution instructions for more information. |
| @@ -201,7 +201,7 @@ def representable? | |||
| # with users. Instead, the +service_url+ should only be exposed as a redirect from a stable, possibly authenticated URL. | |||
| # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And | |||
| # it allows permanent URLs that redirect to the +service_url+ to be cached in the view. | |||
| def service_url(expires_in: service.url_expires_in, disposition: :inline) | |||
| def service_url(expires_in: service.url_expires_in, disposition: :inline, filename: self.filename) | |||
There was a problem hiding this comment.
This is the simplest version I came up with, that assumes a ActiveStorage::Filename instance here. I was thinking that a string could also be allowed, and then do:
service.url ... filename: ActiveStorage::Filename.new(filename.to_s)
Or implement something like ActiveStorage::Filename#wrap and save doing the conversion twice in the default case:
service.url ... filename: ActiveStorage::Filename.wrap(filename)
Summary
This is a simple change that allows a
filenameto be passed toBlob#service_url, in the same way asexpires_inanddisposition.Other Information
This is useful when we have several representations that share the same underlying file, each one with a different name, and we need to provide a different download URL based on that name instead of using the file's, for each of them.
r? @georgeclaghorn