Merge pull request rails#46197 from bensheldon/double_destroy_after_commit

Don't trigger `after_commit :destroy` callback again on destroy if record previously was destroyed

Author: jhawthorn

activerecord/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Only trigger `after_commit :destroy` callbacks when a database row is deleted.
+
+    This prevents `after_commit :destroy` callbacks from being triggered again
+    when `destroy` is called multiple times on the same record.
+
+    *Ben Sheldon*
+
 *   Fix `ciphertext_for` for yet-to-be-encrypted values.
 
     Previously, `ciphertext_for` returned the cleartext of values that had not

activerecord/lib/active_record/persistence.rb

@@ -676,11 +676,7 @@ def delete
     def destroy
       _raise_readonly_record_error if readonly?
       destroy_associations
-      @_trigger_destroy_callback = if persisted?
-        destroy_row > 0
-      else
-        true
-      end
+      @_trigger_destroy_callback = persisted? && destroy_row > 0
       @destroyed = true
       freeze
     end

activerecord/test/cases/transaction_callbacks_test.rb

@@ -204,12 +204,12 @@ def test_only_call_after_commit_on_create_after_transaction_commits_for_new_reco
     assert_equal [], reply.history
   end
 
-  def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_new_record
+  def test_no_after_commit_on_destroy_after_transaction_commits_for_destroyed_new_record
     new_record = TopicWithCallbacks.new(title: "New topic", written_on: Date.today)
     add_transaction_execution_blocks new_record
 
     new_record.destroy
-    assert_equal [:commit_on_destroy], new_record.history
+    assert_equal [], new_record.history
   end
 
   def test_save_in_after_create_commit_wont_invoke_extra_after_create_commit
@@ -674,7 +674,7 @@ class TopicWithCallbacksOnUpdate < TopicWithHistory
       def before_save_for_transaction; end
   end
 
-  def test_trigger_once_on_multiple_deletions
+  def test_trigger_once_on_multiple_deletion_within_transaction
     TopicWithCallbacksOnDestroy.clear_history
     topic = TopicWithCallbacksOnDestroy.new
     topic.save
@@ -689,6 +689,19 @@ def test_trigger_once_on_multiple_deletions
     assert_equal [:commit_on_destroy], TopicWithCallbacksOnDestroy.history
   end
 
+  def test_trigger_once_on_multiple_deletions
+    TopicWithCallbacksOnDestroy.clear_history
+    topic = TopicWithCallbacksOnDestroy.new
+    topic.save
+    topic_clone = TopicWithCallbacksOnDestroy.find(topic.id)
+
+    topic.destroy
+    topic.destroy
+    topic_clone.destroy
+
+    assert_equal [:commit_on_destroy], TopicWithCallbacksOnDestroy.history
+  end
+
   def test_rollback_on_multiple_deletions
     TopicWithCallbacksOnDestroy.clear_history
     topic = TopicWithCallbacksOnDestroy.new