Merge pull request #48339 from natematykiewicz/activestorage_remove_attachment_empty_string

Allow an ActiveStorage attachment to be removed via a form post

activestorage/CHANGELOG.md

@@ -1,3 +1,22 @@
+*   Allow an ActiveStorage attachment to be removed via a form post
+
+    Attachments can already be removed by updating the attachment to be nil such as:
+    ```ruby
+    User.find(params[:id]).update!(avatar: nil)
+    ```
+
+    However, a form cannot post a nil param, it can only post an empty string. But, posting an
+    empty string would result in an `ActiveSupport::MessageVerifier::InvalidSignature: mismatched digest`
+    error being raised, because it's being treated as a signed blob id.
+
+    Now, nil and an empty string are treated as a delete, which allows attachments to be removed via:
+    ```ruby
+    User.find(params[:id]).update!(params.require(:user).permit(:avatar))
+
+    ```
+
+    *Nate Matykiewicz*
+
 *   Remove mini_mime usage in favour of marcel.
 
     We have two libraries that are have similar usage. This change removes

activestorage/lib/active_storage/attached/model.rb

@@ -61,7 +61,7 @@ def #{name}
 
           def #{name}=(attachable)
             attachment_changes["#{name}"] =
-              if attachable.nil?
+              if attachable.nil? || attachable == ""
                 ActiveStorage::Attached::Changes::DeleteOne.new("#{name}", self)
               else
                 ActiveStorage::Attached::Changes::CreateOne.new("#{name}", self, attachable)

activestorage/test/models/attached/one_test.rb

@@ -289,7 +289,7 @@ def upload.open
     end
   end
 
-  test "updating an existing record to remove a dependent attachment" do
+  test "updating an existing record to nil to remove a dependent attachment" do
     create_blob(filename: "funky.jpg").tap do |blob|
       @user.avatar.attach blob
 
@@ -301,7 +301,19 @@ def upload.open
     end
   end
 
-  test "updating an existing record to remove an independent attachment" do
+  test "updating an existing record to an empty string to remove a dependent attachment" do
+    create_blob(filename: "funky.jpg").tap do |blob|
+      @user.avatar.attach blob
+
+      assert_enqueued_with job: ActiveStorage::PurgeJob, args: [ blob ] do
+        @user.update! avatar: ""
+      end
+
+      assert_not @user.avatar.attached?
+    end
+  end
+
+  test "updating an existing record to nil to remove an independent attachment" do
     create_blob(filename: "funky.jpg").tap do |blob|
       @user.cover_photo.attach blob
 
@@ -313,6 +325,18 @@ def upload.open
     end
   end
 
+  test "updating an existing record to an empty string to remove an independent attachment" do
+    create_blob(filename: "funky.jpg").tap do |blob|
+      @user.cover_photo.attach blob
+
+      assert_no_enqueued_jobs only: ActiveStorage::PurgeJob do
+        @user.update! cover_photo: ""
+      end
+
+      assert_not @user.cover_photo.attached?
+    end
+  end
+
   test "analyzing a new blob from an uploaded file after attaching it to an existing record" do
     perform_enqueued_jobs do
       @user.avatar.attach fixture_file_upload("racecar.jpg")