Merge pull request #3597 from AlchemyCMS/fix-filename-conversion

tvdeyen · web-flow · commit d9112f4f787e · 2026-01-28T10:14:40.000+01:00
fix: Use Ruby/Rails internals to create human readable file names
diff --git a/app/controllers/alchemy/admin/pictures_controller.rb b/app/controllers/alchemy/admin/pictures_controller.rb
@@ -64,7 +64,6 @@ def url
 
       def create
         @picture = Picture.new(picture_params)
-        @picture.name = @picture.humanized_name
         if @picture.save
           render successful_uploader_response(file: @picture)
         else
diff --git a/app/models/alchemy/attachment.rb b/app/models/alchemy/attachment.rb
@@ -25,6 +25,8 @@ class Attachment < BaseRecord
     include Alchemy::TouchElements
     include Alchemy::RelatableResource
 
+    before_save :set_name, if: -> { Alchemy.storage_adapter.set_attachment_name?(self) }
+
     include Alchemy.storage_adapter.attachment_class_methods
 
     stampable stamper_class_name: Alchemy.config.user_class_name
@@ -97,8 +99,6 @@ def ransackable_scopes(_auth_object = nil)
     validate :file_type_allowed,
       unless: -> { self.class.allowed_filetypes.include?("*") }
 
-    before_save :set_name, if: -> { Alchemy.storage_adapter.set_attachment_name?(self) }
-
     scope :with_file_type, ->(file_type) { where(file_mime_type: file_type) }
 
     # Instance methods
@@ -179,7 +179,7 @@ def file_type_allowed
     end
 
     def set_name
-      self.name ||= convert_to_humanized_name(file_name, extension)
+      self.name ||= Alchemy.storage_adapter.file_basename(self).humanize
     end
   end
 end
diff --git a/app/models/alchemy/picture.rb b/app/models/alchemy/picture.rb
@@ -62,6 +62,8 @@ def self.preprocessor_class=(klass)
       @_preprocessor_class = klass
     end
 
+    before_create :set_name, if: :image_file_name
+
     include Alchemy.storage_adapter.picture_class_methods
 
     # We need to define this method here to have it available in the validations below.
@@ -195,14 +197,6 @@ def urlname
       end
     end
 
-    # Returns a humanized, readable name from image filename.
-    #
-    def humanized_name
-      return "" if image_file_name.blank?
-
-      convert_to_humanized_name(image_file_name, image_file_extension)
-    end
-
     # Returns the format the image should be rendered with
     #
     # Only returns a format differing from original if an +image_output_format+
@@ -287,6 +281,12 @@ def image_file_dimensions
 
     private
 
+    # Returns a humanized, readable name from image filename.
+    #
+    def set_name
+      self.name ||= Alchemy.storage_adapter.image_file_basename(self).humanize
+    end
+
     def image_file_type_allowed
       unless image_file_extension&.in?(self.class.allowed_filetypes)
         errors.add(:image_file, Alchemy.t("not a valid image"))
diff --git a/app/models/alchemy/storage_adapter.rb b/app/models/alchemy/storage_adapter.rb
@@ -9,12 +9,14 @@ class UnknownAdapterError < StandardError; end
       :by_file_format_scope,
       :by_file_type_scope,
       :not_file_type_scope,
+      :file_basename,
       :file_extension,
       :file_formats,
       :file_mime_type,
       :file_name,
       :file_size,
       :has_convertible_format?,
+      :image_file_basename,
       :image_file_extension,
       :image_file_format,
       :image_file_height,
diff --git a/app/models/alchemy/storage_adapter/active_storage.rb b/app/models/alchemy/storage_adapter/active_storage.rb
@@ -96,6 +96,13 @@ def not_file_type_scope(file_type)
         Attachment.with_attached_file.joins(:file_blob).where.not(active_storage_blobs: {content_type: file_type})
       end
 
+      # @param [Alchemy::Attachment]
+      # @return [String]
+      def file_basename(attachment)
+        filename = attachment.file&.filename.to_s
+        File.basename(filename, File.extname(filename))
+      end
+
       # @param [Alchemy::Attachment]
       # @return [String]
       def file_name(attachment)
@@ -126,6 +133,13 @@ def has_convertible_format?(picture)
         picture.image_file&.variable?
       end
 
+      # @param [Alchemy::Picture]
+      # @return [String]
+      def image_file_basename(picture)
+        filename = picture.image_file&.filename.to_s
+        File.basename(filename, File.extname(filename))
+      end
+
       # @param [Alchemy::Picture]
       # @return [String]
       def image_file_name(picture)
diff --git a/app/models/alchemy/storage_adapter/dragonfly.rb b/app/models/alchemy/storage_adapter/dragonfly.rb
@@ -127,6 +127,12 @@ def file_name(attachment)
         attachment.read_attribute(:file_name)
       end
 
+      # @param [Alchemy::Attachment]
+      # @return [String]
+      def file_basename(attachment)
+        attachment.file&.basename&.to_s
+      end
+
       # @param [Alchemy::Attachment]
       # @return [Integer]
       def file_size(attachment)
@@ -152,6 +158,12 @@ def has_convertible_format?(picture)
         image_file_extension(picture).in?(CONVERTIBLE_FILE_FORMATS)
       end
 
+      # @param [Alchemy::Picture]
+      # @return [String]
+      def image_file_basename(picture)
+        picture.image_file&.basename&.to_s
+      end
+
       # @param [Alchemy::Picture]
       # @return [String]
       def image_file_name(picture)
diff --git a/lib/alchemy/name_conversions.rb b/lib/alchemy/name_conversions.rb
@@ -17,12 +17,6 @@ def convert_to_urlname(name)
         .parameterize
     end
 
-    # Converts a filename and suffix into a human readable name.
-    #
-    def convert_to_humanized_name(name, suffix)
-      name.gsub(/\.#{::Regexp.quote(suffix)}$/i, "").tr("_", " ").strip
-    end
-
     # Sanitizes a given filename by removing directory traversal attempts and HTML entities.
     def sanitized_filename(file_name)
       file_name = File.basename(file_name)
diff --git a/spec/controllers/alchemy/admin/pictures_controller_spec.rb b/spec/controllers/alchemy/admin/pictures_controller_spec.rb
@@ -141,15 +141,14 @@ module Alchemy
     describe "#create" do
       subject { post :create, params: params }
 
-      let(:params) { {picture: {name: ""}} }
-      let(:picture) { mock_model("Picture", humanized_name: "Cute kittens") }
-
       context "with passing validations" do
-        before do
-          expect(Picture).to receive(:new).and_return(picture)
-          expect(picture).to receive(:name=).and_return("Cute kittens")
-          expect(picture).to receive(:name).and_return("Cute kittens")
-          expect(picture).to receive(:save).and_return(true)
+        let(:params) do
+          {picture: {image_file: fixture_file_upload("my FileNämü.png")}}
+        end
+
+        it "creates a new picture with human friendly name" do
+          expect { subject }.to change { Picture.count }.by(1)
+          expect(Picture.last.name).to eq("My filenämü")
         end
 
         it "renders json response with success message" do
@@ -162,6 +161,8 @@ module Alchemy
       end
 
       context "with failing validations" do
+        let(:params) { {picture: {name: ""}} }
+
         it_behaves_like "having a json uploader error message"
       end
     end
diff --git a/spec/fixtures/files/cute_kitten.png b/spec/fixtures/files/cute_kitten.png
diff --git a/spec/fixtures/files/image with spaces.jpgi.png b/spec/fixtures/files/image with spaces.jpgi.png
diff --git a/spec/models/alchemy/attachment_spec.rb b/spec/models/alchemy/attachment_spec.rb
@@ -57,7 +57,7 @@ module Alchemy
 
       it "should have a humanized name" do
         save
-        expect(attachment.name).to eq("image with spaces")
+        expect(attachment.name).to eq("Image with spaces")
       end
 
       context "when file_name has not changed" do
diff --git a/spec/models/alchemy/picture_spec.rb b/spec/models/alchemy/picture_spec.rb
@@ -55,24 +55,27 @@ module Alchemy
       end
     end
 
-    describe "#humanized_name" do
-      it "should return a humanized version of original filename" do
-        allow(picture).to receive(:image_file_name).and_return("cute_kitten.JPG")
-        allow(picture).to receive(:image_file_extension).and_return("jpg")
-        expect(picture.humanized_name).to eq("cute kitten")
+    describe "after create" do
+      let(:image_file) { fixture_file_upload("image with spaces.png") }
+      let(:picture) { create(:alchemy_picture, name: nil, image_file: image_file) }
+
+      it "should have a humanized version of original filename" do
+        expect(picture.name).to eq("Image with spaces")
       end
 
-      it "should not remove incidents of suffix from filename" do
-        allow(picture).to receive(:image_file_name).and_return("cute_kitten_mo.jpgi.JPG")
-        allow(picture).to receive(:image_file_extension).and_return("jpg")
-        expect(picture.humanized_name).to eq("cute kitten mo.jpgi")
+      context "image has suffix-like in filename" do
+        let(:image_file) { fixture_file_upload("image with spaces.jpgi.png") }
+
+        it "should not have incidents of suffix from filename" do
+          expect(picture.name).to eq("Image with spaces.jpgi")
+        end
       end
 
-      context "image has no suffix" do
-        it "should return humanized name" do
-          allow(picture).to receive(:image_file_name).and_return("cute_kitten")
-          allow(picture).to receive(:image_file_extension).and_return("")
-          expect(picture.humanized_name).to eq("cute kitten")
+      context "image has underscores" do
+        let(:image_file) { fixture_file_upload("cute_kitten.png") }
+
+        it "should have humanized name" do
+          expect(picture.name).to eq("Cute kitten")
         end
       end
     end
