From 1cbecb67cc3278463d2a37710466d69a341437f3 Mon Sep 17 00:00:00 2001 From: Mitsuhiro Shibuya Date: Sun, 25 Aug 2024 18:49:13 +0900 Subject: [PATCH] Follow-up for #3656 - Do not abbreviate 'representation' - Add specs --- .../config/fields/types/active_storage.rb | 10 +++------- .../fields/types/multiple_active_storage.rb | 10 +++------- .../config/fields/types/active_storage_spec.rb | 16 ++++++++++++++++ .../fields/types/multiple_active_storage_spec.rb | 16 ++++++++++++++++ 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/rails_admin/config/fields/types/active_storage.rb b/lib/rails_admin/config/fields/types/active_storage.rb index 57524b7b7..20fe7b472 100644 --- a/lib/rails_admin/config/fields/types/active_storage.rb +++ b/lib/rails_admin/config/fields/types/active_storage.rb @@ -18,7 +18,7 @@ class ActiveStorage < RailsAdmin::Config::Fields::Types::FileUpload end register_instance_option :image? do - value && (value.representable? || mime_type(value.filename).to_s.match?(/^image/)) + value && (value.representable? || value.content_type.match?(/^image/)) end register_instance_option :eager_load do @@ -50,9 +50,9 @@ def resource_url(thumb = false) if thumb && value.representable? thumb = thumb_method if thumb == true - repr = value.representation(thumb) + representation = value.representation(thumb) Rails.application.routes.url_helpers.rails_blob_representation_path( - repr.blob.signed_id, repr.variation.key, repr.blob.filename, only_path: true + representation.blob.signed_id, representation.variation.key, representation.blob.filename, only_path: true ) else Rails.application.routes.url_helpers.rails_blob_path(value, only_path: true) @@ -63,10 +63,6 @@ def value attachment = super attachment if attachment&.attached? end - - def mime_type(filename_obj) - Mime::Type.lookup_by_extension(filename_obj.extension_without_delimiter) - end end end end diff --git a/lib/rails_admin/config/fields/types/multiple_active_storage.rb b/lib/rails_admin/config/fields/types/multiple_active_storage.rb index 70f05610a..43d163e02 100644 --- a/lib/rails_admin/config/fields/types/multiple_active_storage.rb +++ b/lib/rails_admin/config/fields/types/multiple_active_storage.rb @@ -23,25 +23,21 @@ class ActiveStorageAttachment < RailsAdmin::Config::Fields::Types::MultipleFileU end register_instance_option :image? do - value && (value.representable? || mime_type(value.filename).to_s.match?(/^image/)) + value && (value.representable? || value.content_type.match?(/^image/)) end def resource_url(thumb = false) return nil unless value if thumb && value.representable? - repr = value.representation(thumb_method) + representation = value.representation(thumb_method) Rails.application.routes.url_helpers.rails_blob_representation_path( - repr.blob.signed_id, repr.variation.key, repr.blob.filename, only_path: true + representation.blob.signed_id, representation.variation.key, representation.blob.filename, only_path: true ) else Rails.application.routes.url_helpers.rails_blob_path(value, only_path: true) end end - - def mime_type(filename_obj) - Mime::Type.lookup_by_extension(filename_obj.extension_without_delimiter) - end end register_instance_option :attachment_class do diff --git a/spec/rails_admin/config/fields/types/active_storage_spec.rb b/spec/rails_admin/config/fields/types/active_storage_spec.rb index de9cd8349..835a6cb9b 100644 --- a/spec/rails_admin/config/fields/types/active_storage_spec.rb +++ b/spec/rails_admin/config/fields/types/active_storage_spec.rb @@ -42,6 +42,14 @@ expect(field.image?).to be_falsy end end + + context 'when attachment is a PDF file' do + let(:record) { FactoryBot.create :field_test, active_storage_asset: {io: StringIO.new('dummy'), filename: 'test.pdf', content_type: 'application/pdf'} } + + it 'returns true' do + expect(field.image?).to be_truthy + end + end end describe '#resource_url' do @@ -68,6 +76,14 @@ expect(field.resource_url(true)).not_to match(/representations/) end end + + context 'when attachment is a PDF file' do + let(:record) { FactoryBot.create :field_test, active_storage_asset: {io: StringIO.new('dummy'), filename: 'test.pdf', content_type: 'application/pdf'} } + + it 'returns variant\'s url' do + expect(field.resource_url(true)).to match(/representations/) + end + end end describe '#value' do diff --git a/spec/rails_admin/config/fields/types/multiple_active_storage_spec.rb b/spec/rails_admin/config/fields/types/multiple_active_storage_spec.rb index 42ff2f841..0ad966b93 100644 --- a/spec/rails_admin/config/fields/types/multiple_active_storage_spec.rb +++ b/spec/rails_admin/config/fields/types/multiple_active_storage_spec.rb @@ -63,6 +63,14 @@ expect(field.attachments[0].image?).to be_falsy end end + + context 'when attachment is a PDF file' do + let(:record) { FactoryBot.create :field_test, active_storage_assets: [{io: StringIO.new('dummy'), filename: 'test.pdf', content_type: 'application/pdf'}] } + + it 'returns true' do + expect(field.attachments[0].image?).to be_truthy + end + end end describe '#resource_url' do @@ -89,6 +97,14 @@ expect(field.attachments[0].resource_url(true)).not_to match(/representations/) end end + + context 'when attachment is a PDF file' do + let(:record) { FactoryBot.create :field_test, active_storage_assets: [{io: StringIO.new('dummy'), filename: 'test.pdf', content_type: 'application/pdf'}] } + + it 'returns variant\'s url' do + expect(field.attachments[0].resource_url(true)).to match(/representations/) + end + end end end