Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix create callback called on destroy #513

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ platforms :rbx do
gem 'rubysl-test-unit'
end

rails = ENV['RAILS'] || '~> 5.2.0'
rails = ENV['RAILS'] || '~> 6.0.4'

if rails == 'master'
gem 'rails', github: 'rails/rails'
Expand Down
4 changes: 2 additions & 2 deletions lib/paranoia.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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|
Expand Down
42 changes: 42 additions & 0 deletions test/paranoia_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -147,6 +148,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
Expand Down Expand Up @@ -1136,6 +1151,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
Expand Down