Merge pull request #31650 from rosa/master

Allow overriding filename in `Blob#service_url`

2 files changed, 15 insertions, 4 deletions

diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb
index 8fb53fa787..df4f6f6075 100644
--- a/activestorage/app/models/active_storage/blob.rb
+++ b/activestorage/app/models/active_storage/blob.rb
@@ -201,7 +201,7 @@ class ActiveStorage::Blob < ActiveRecord::Base
   # 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)
     service.url key, expires_in: expires_in, disposition: forcibly_serve_as_binary? ? :attachment : disposition, filename: filename, content_type: content_type
   end
 
diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
index fa8b935da0..2db516c088 100644
--- a/activestorage/test/models/blob_test.rb
+++ b/activestorage/test/models/blob_test.rb
@@ -50,6 +50,16 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
     end
   end
 
+  test "urls allow for custom filename" do
+    blob = create_blob(filename: "original.txt")
+    new_filename = ActiveStorage::Filename.new("new.txt")
+
+    freeze_time do
+      assert_equal expected_url_for(blob), blob.service_url
+      assert_equal expected_url_for(blob, filename: new_filename), blob.service_url(filename: new_filename)
+    end
+  end
+
   test "purge deletes file from external service" do
     blob = create_blob
 
@@ -66,8 +76,9 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
   end
 
   private
-    def expected_url_for(blob, disposition: :inline)
-      query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{blob.filename.parameters}" }.to_param
-      "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{blob.filename}?#{query_string}"
+    def expected_url_for(blob, disposition: :inline, filename: nil)
+      filename ||= blob.filename
+      query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{filename.parameters}" }.to_param
+      "/rails/active_storage/disk/#{ActiveStorage.verifier.generate(blob.key, expires_in: 5.minutes, purpose: :blob_key)}/#{filename}?#{query_string}"
     end
 end