From edf7d0ec8c7af797cb1b98c0bf74934b07e2c1b9 Mon Sep 17 00:00:00 2001
From: DJ <6138914+bkDJ@users.noreply.github.com>
Date: Wed, 23 Mar 2022 04:32:54 +0100
Subject: [PATCH] Fix create callback called on destroy (#513)

* update Rails

* add specs for broken case

* apply fix

Co-authored-by: Djilani Kebaili <djilani.kebaili@epfl.ch>
---
 Gemfile               |  2 +-
 lib/paranoia.rb       |  4 ++--
 test/paranoia_test.rb | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Gemfile b/Gemfile
index 9518a9cb..fe5f736b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -20,7 +20,7 @@ if RUBY_ENGINE == 'rbx'
   end
 end
 
-rails = ENV['RAILS'] || '~> 5.2.0'
+rails = ENV['RAILS'] || '~> 6.0.4'
 
 if rails == 'edge'
   gem 'rails', github: 'rails/rails'
diff --git a/lib/paranoia.rb b/lib/paranoia.rb
index 8ce62933..6ed19261 100644
--- a/lib/paranoia.rb
+++ b/lib/paranoia.rb
@@ -58,7 +58,7 @@ def restore(id_or_ids, opts = {})
   end
 
   def paranoia_destroy
-    transaction do
+    with_transaction_returning_status do
       run_callbacks(:destroy) do
         @_disable_counter_cache = deleted?
         result = paranoia_delete
@@ -143,7 +143,7 @@ def paranoia_destroyed?
   alias :deleted? :paranoia_destroyed?
 
   def really_destroy!
-    transaction do
+    with_transaction_returning_status do
       run_callbacks(:real_destroy) do
         @_disable_counter_cache = paranoia_destroyed?
         dependent_reflections = self.class.reflections.select do |name, reflection|
diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb
index 1a1413fc..1ba75be9 100644
--- a/test/paranoia_test.rb
+++ b/test/paranoia_test.rb
@@ -30,6 +30,7 @@ def setup!
     'featureful_models' => 'deleted_at DATETIME, name VARCHAR(32)',
     'plain_models' => 'deleted_at DATETIME',
     'callback_models' => 'deleted_at DATETIME',
+    'after_commit_callback_models' => 'deleted_at DATETIME',
     'fail_callback_models' => 'deleted_at DATETIME',
     'related_models' => 'parent_model_id INTEGER, parent_model_with_counter_cache_column_id INTEGER, deleted_at DATETIME',
     'asplode_models' => 'parent_model_id INTEGER, deleted_at DATETIME',
@@ -158,6 +159,20 @@ def test_destroy_behavior_for_freshly_loaded_plain_models_callbacks
     assert model.instance_variable_get(:@after_commit_callback_called)
   end
 
+  def test_destroy_behavior_for_freshly_saved_models_after_commit_callbacks
+    model = AfterCommitCallbackModel.create!
+
+    assert_equal 1, model.after_create_commit_called_times
+    assert_equal 0, model.after_destroy_commit_called_times
+
+    # clear the counters, but do not reload from DB
+    model.remove_called_variables
+
+    model.destroy
+    assert_equal 0, model.after_create_commit_called_times
+    assert_equal 1, model.after_destroy_commit_called_times
+  end
+
   def test_delete_behavior_for_plain_models_callbacks
     model = CallbackModel.new
     model.save
@@ -1196,6 +1211,33 @@ def remove_called_variables
   end
 end
 
+class AfterCommitCallbackModel < ActiveRecord::Base
+  acts_as_paranoid
+
+  after_commit :increment_after_create_commit_called_times, on: :create
+  after_commit :increment_after_destroy_commit_called_times, on: :destroy
+
+  def increment_after_create_commit_called_times
+    @after_create_commit_called_times = after_create_commit_called_times + 1
+  end
+
+  def increment_after_destroy_commit_called_times
+    @after_destroy_commit_called_times = after_destroy_commit_called_times + 1
+  end
+
+  def after_create_commit_called_times
+    @after_create_commit_called_times || 0
+  end
+
+  def after_destroy_commit_called_times
+    @after_destroy_commit_called_times || 0
+  end
+
+  def remove_called_variables
+    instance_variables.each {|name| (name.to_s.end_with?('_called_times')) ? remove_instance_variable(name) : nil}
+  end
+end
+
 class ParentModel < ActiveRecord::Base
   acts_as_paranoid
   has_many :paranoid_models, dependent: :destroy