From 8f52d93576ec23c8e87c96f048585445f871efe5 Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Mon, 8 Jan 2018 15:44:14 +0100 Subject: Allow overriding filename in `Blob#service_url` 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. --- activestorage/app/models/active_storage/blob.rb | 2 +- activestorage/test/models/blob_test.rb | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) (limited to 'activestorage') 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 -- cgit v1.2.3