Follow-up for #3656

- Do not abbreviate 'representation'
- Add specs

Author: mshibuya

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

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

spec/rails_admin/config/fields/types/active_storage_spec.rb

@@ -42,6 +42,15 @@
           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'} }
+        before { allow(ActiveStorage::Previewer::PopplerPDFPreviewer).to receive(:accept?).and_return(true) }
+
+        it 'returns true' do
+          expect(field.image?).to be_truthy
+        end
+      end
     end
 
     describe '#resource_url' do
@@ -68,6 +77,15 @@
           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'} }
+        before { allow(ActiveStorage::Previewer::PopplerPDFPreviewer).to receive(:accept?).and_return(true) }
+
+        it 'returns variant\'s url' do
+          expect(field.resource_url(true)).to match(/representations/)
+        end
+      end
     end
 
     describe '#value' do

spec/rails_admin/config/fields/types/multiple_active_storage_spec.rb

@@ -63,6 +63,15 @@
             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'}] }
+          before { allow(ActiveStorage::Previewer::PopplerPDFPreviewer).to receive(:accept?).and_return(true) }
+
+          it 'returns true' do
+            expect(field.attachments[0].image?).to be_truthy
+          end
+        end
       end
 
       describe '#resource_url' do
@@ -89,6 +98,15 @@
             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'}] }
+          before { allow(ActiveStorage::Previewer::PopplerPDFPreviewer).to receive(:accept?).and_return(true) }
+
+          it 'returns variant\'s url' do
+            expect(field.attachments[0].resource_url(true)).to match(/representations/)
+          end
+        end
       end
     end