Skip to content

Commit

Permalink
rollback transaction if destroy aborted (rubysherpas#485)
Browse files Browse the repository at this point in the history
Co-authored-by: Boris <penkovskiy@taxi21.ru>
Co-authored-by: Mathieu Jobin <mathieujobin@users.noreply.github.com>
  • Loading branch information
3 people authored and arun-zc committed Feb 6, 2024
1 parent e890f95 commit 31887da
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
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

0 comments on commit 31887da

Please sign in to comment.