Fix that successive ActiveStorage::Attached::Many#attach calls would overwrite previous attachments

2 files changed, 22 insertions, 27 deletions

diff --git a/activestorage/lib/active_storage/attached/changes/create_many.rb b/activestorage/lib/active_storage/attached/changes/create_many.rb
index af19328a61..a7a8553e0f 100644
--- a/activestorage/lib/active_storage/attached/changes/create_many.rb
+++ b/activestorage/lib/active_storage/attached/changes/create_many.rb
@@ -21,7 +21,8 @@ module ActiveStorage
     end
 
     def save
-      record.public_send("#{name}_attachments=", attachments)
+      assign_associated_attachments
+      reset_associated_blobs
     end
 
     private
@@ -32,5 +33,14 @@ module ActiveStorage
       def build_subchange_from(attachable)
         ActiveStorage::Attached::Changes::CreateOneOfMany.new(name, record, attachable)
       end
+
+
+      def assign_associated_attachments
+        record.public_send("#{name}_attachments=", attachments)
+      end
+
+      def reset_associated_blobs
+        record.public_send("#{name}_blobs").reset
+      end
   end
 end
diff --git a/activestorage/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb
index 2a45829da8..61a80b5dda 100644
--- a/activestorage/test/models/attached/many_test.rb
+++ b/activestorage/test/models/attached/many_test.rb
@@ -106,6 +106,17 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
     assert_equal "video.mp4", @user.highlights.second.filename.to_s
   end
 
+  test "attaching existing records to an existing blob one at a time" do
+    @user.highlights.attach create_blob(filename: "funky.jpg")
+    @user.highlights.attach create_blob(filename: "town.jpg")
+    assert_equal "funky.jpg", @user.highlights.first.filename.to_s
+    assert_equal "town.jpg", @user.highlights.second.filename.to_s
+
+    @user.reload
+    assert_equal "funky.jpg", @user.highlights.first.filename.to_s
+    assert_equal "town.jpg", @user.highlights.second.filename.to_s
+  end
+
   test "updating an existing record to attach existing blobs" do
     @user.update! highlights: [ create_file_blob(filename: "racecar.jpg"), create_file_blob(filename: "video.mp4") ]
     assert_equal "racecar.jpg", @user.highlights.first.filename.to_s
@@ -195,32 +206,6 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
     end
   end
 
-  test "removing dependent attachments from an existing record" do
-    [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs|
-      @user.highlights.attach blobs
-
-      assert_enqueued_with job: ActiveStorage::PurgeJob, args: [ blobs.first ] do
-        assert_enqueued_with job: ActiveStorage::PurgeJob, args: [ blobs.second ] do
-          @user.highlights.attach []
-        end
-      end
-
-      assert_not @user.highlights.attached?
-    end
-  end
-
-  test "removing independent attachments from an existing record" do
-    [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs|
-      @user.vlogs.attach blobs
-
-      assert_no_enqueued_jobs only: ActiveStorage::PurgeJob do
-        @user.vlogs.attach []
-      end
-
-      assert_not @user.vlogs.attached?
-    end
-  end
-
   test "updating an existing record to remove dependent attachments" do
     [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs|
       @user.highlights.attach blobs