Merge pull request rails#53623 from Edouard-chin/ec-autosave-blob

Prevent Active Storage Blob from autosaving Attachments:

Author: byroot

activestorage/CHANGELOG.md

@@ -1,2 +1,13 @@
+*   A Blob will no longer autosave associated Attachment.
+
+    This fixes an issue where a record with an attachment would have
+    its dirty attributes reset, preventing your `after commit` callbacks
+    on that record to behave as expected.
+
+    Note that this change doesn't require any changes on your application
+    and is supposed to be internal. Active Storage Attachment will continue
+    to be autosaved (through a different relation).
+
+    *Edouard-chin*
 
 Please check [8-0-stable](https://github.com/rails/rails/blob/8-0-stable/activestorage/CHANGELOG.md) for previous changes.

activestorage/app/models/active_storage/blob.rb

@@ -29,7 +29,7 @@ class ActiveStorage::Blob < ActiveStorage::Record
   # :method:
   #
   # Returns the associated ActiveStorage::Attachment instances.
-  has_many :attachments
+  has_many :attachments, autosave: false
 
   ##
   # :singleton-method:

activestorage/test/models/attachment_test.rb

@@ -38,6 +38,27 @@ class ActiveStorage::AttachmentTest < ActiveSupport::TestCase
     assert_predicate blob.reload, :analyzed?
   end
 
+  test "attaching a blob doesn't touch the record" do
+    data = "Something else entirely!"
+    io = StringIO.new(data)
+    blob = create_blob_before_direct_upload byte_size: data.size, checksum: OpenSSL::Digest::MD5.base64digest(data)
+    blob.upload(io)
+
+    user = User.create!(
+      name: "Roger",
+      avatar: blob.signed_id,
+      record_callbacks: true,
+    )
+
+    assert_equal(1, user.callback_counter)
+  end
+
+  test "attaching a record doesn't reset the previously_new_record flag" do
+    @user.highlights.attach(io: ::StringIO.new("dummy"), filename: "dummy.txt")
+
+    assert(@user.notification_sent)
+  end
+
   test "mirroring a directly-uploaded blob after attaching it" do
     with_service("mirror") do
       blob = directly_upload_file_blob

activestorage/test/test_helper.rb

@@ -121,6 +121,9 @@ def with_raise_on_open_redirects(service)
 ActiveRecord::Base.include GlobalID::Identification
 
 class User < ActiveRecord::Base
+  attr_accessor :record_callbacks, :callback_counter
+  attr_reader :notification_sent
+
   validates :name, presence: true
 
   has_one_attached :avatar
@@ -161,11 +164,25 @@ class User < ActiveRecord::Base
     attachable.variant :preview, resize_to_fill: [400, 400], preprocessed: true
   end
 
+  after_commit :increment_callback_counter
+  after_update_commit :notify
+
   accepts_nested_attributes_for :highlights_attachments, allow_destroy: true
 
   def should_preprocessed?
     name == "transform via method"
   end
+
+  def increment_callback_counter
+    if record_callbacks
+      @callback_counter ||= 0
+      @callback_counter += 1
+    end
+  end
+
+  def notify
+    @notification_sent = true if highlights_attachments.any?(&:previously_new_record?)
+  end
 end
 
 class Group < ActiveRecord::Base