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

rollback transaction if destroy aborted (fix #484) #485

Merged
merged 3 commits into from
Mar 23, 2022
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
6 changes: 4 additions & 2 deletions lib/paranoia.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def restore(id_or_ids, opts = {})

def paranoia_destroy
with_transaction_returning_status do
run_callbacks(:destroy) do
result = run_callbacks(:destroy) do
@_disable_counter_cache = deleted?
result = paranoia_delete
next result unless result && ActiveRecord::VERSION::STRING >= '4.2'
Expand All @@ -73,7 +73,9 @@ def paranoia_destroy
@_disable_counter_cache = false
result
end
end
raise ActiveRecord::Rollback, "Not destroyed" unless self.deleted?
result
end || false
end
alias_method :destroy, :paranoia_destroy

Expand Down
30 changes: 26 additions & 4 deletions test/paranoia_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def setup!
'active_column_models' => 'paranoid_model_id INTEGER, deleted_at DATETIME, active BOOLEAN',
'active_column_model_with_uniqueness_validations' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN',
'paranoid_model_with_belongs_to_active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN, active_column_model_with_has_many_relationship_id INTEGER',
'active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN',
'active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN',
'without_default_scope_models' => 'deleted_at DATETIME'
}.each do |table_name, columns_as_sql_string|
ActiveRecord::Base.connection.execute "CREATE TABLE #{table_name} (id INTEGER NOT NULL PRIMARY KEY, #{columns_as_sql_string})"
Expand Down Expand Up @@ -260,11 +260,11 @@ def test_scoping_behavior_for_paranoid_models
p2 = ParanoidModel.create(:parent_model => parent2)
p1.destroy
p2.destroy

assert_equal 0, parent1.paranoid_models.count
assert_equal 1, parent1.paranoid_models.only_deleted.count

assert_equal 2, ParanoidModel.only_deleted.joins(:parent_model).count
assert_equal 2, ParanoidModel.only_deleted.joins(:parent_model).count
assert_equal 1, parent1.paranoid_models.deleted.count
assert_equal 0, parent1.paranoid_models.without_deleted.count
p3 = ParanoidModel.create(:parent_model => parent1)
Expand Down Expand Up @@ -302,7 +302,7 @@ def test_only_deleted_with_joins
c1 = ActiveColumnModelWithHasManyRelationship.create(name: 'Jacky')
c2 = ActiveColumnModelWithHasManyRelationship.create(name: 'Thomas')
p1 = ParanoidModelWithBelongsToActiveColumnModelWithHasManyRelationship.create(name: 'Hello', active_column_model_with_has_many_relationship: c1)

c1.destroy
assert_equal 1, ActiveColumnModelWithHasManyRelationship.count
assert_equal 1, ActiveColumnModelWithHasManyRelationship.only_deleted.count
Expand Down Expand Up @@ -635,6 +635,16 @@ def test_real_destroy_dependent_destroy
refute NonParanoidModel.unscoped.exists?(child3.id)
end

def test_not_destroy_child_if_abort_destroy
parent = ParentModel.create
child = parent.very_related_models.create
parent.destroy_unavailable = true
parent.destroy

assert_nil parent.reload.deleted_at, "Parent must be not deleted"
assert_nil child.reload.deleted_at, "Child must be not deleted"
end

def test_real_destroy_dependent_destroy_after_normal_destroy
parent = ParentModel.create
child = parent.very_related_models.create
Expand Down Expand Up @@ -1281,6 +1291,7 @@ def remove_called_variables
end

class ParentModel < ActiveRecord::Base
attr_accessor :destroy_unavailable
acts_as_paranoid
has_many :paranoid_models, dependent: :destroy
has_many :related_models
Expand All @@ -1289,6 +1300,17 @@ class ParentModel < ActiveRecord::Base
has_one :non_paranoid_model, dependent: :destroy
has_many :asplode_models, dependent: :destroy
has_one :polymorphic_model, as: :parent, dependent: :destroy
before_destroy :validate_destroy

def validate_destroy
return unless self.destroy_unavailable

if ActiveRecord::VERSION::MAJOR < 5
false
else
throw :abort
end
end
end

class ParentModelWithCounterCacheColumn < ActiveRecord::Base
Expand Down