Skip to content

Commit

Permalink
Raise RecordNotDestroyed when children can't be replaced
Browse files Browse the repository at this point in the history
Fixes #12812
Raise `ActiveRecord::RecordNotDestroyed` when a child marked with
`dependent: destroy` can't be destroyed.

The following code:
```ruby
class Post < ActiveRecord::Base
  has_many :comments, dependent: :destroy
end

class Comment < ActiveRecord::Base
  before_destroy do
    return false
  end
end

post = Post.create!(comments: [Comment.create!])
post.comments = [Comment.create!]
````

would result in a `post` with two `comments`.
With this commit, the same code would raise a `RecordNotDestroyed`
exception, keeping the `post` with the same `comment`.
  • Loading branch information
brianstorti committed Nov 25, 2013
1 parent 19dd216 commit 5aab0c0
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 1 deletion.
6 changes: 6 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1042,4 +1042,10 @@

*Slava Markevich*

* Raise `ActiveRecord::RecordNotDestroyed` when a replaced child marked with `dependent: destroy` fails to be destroyed.

Fix #12812

*Brian Thomas Storti*

Please check [4-0-stable](https://github.com/rails/rails/blob/4-0-stable/activerecord/CHANGELOG.md) for previous changes.
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def inverse_updates_counter_cache?(reflection = reflection)
# Deletes the records according to the <tt>:dependent</tt> option.
def delete_records(records, method)
if method == :destroy
records.each { |r| r.destroy }
records.each(&:destroy!)
update_counter(-records.length) unless inverse_updates_counter_cache?
else
if records == :all
Expand Down
11 changes: 11 additions & 0 deletions activerecord/test/cases/associations/has_many_associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1770,4 +1770,15 @@ def test_collection_association_with_private_kernel_method
assert_equal [bulb1], car.bulbs
assert_equal [bulb1, bulb2], car.all_bulbs.sort_by(&:id)
end

test "raises RecordNotDestroyed when replaced child can't be destroyed" do
car = Car.create!
original_child = FailedBulb.create!(car: car)

assert_raise(ActiveRecord::RecordNotDestroyed) do
car.failed_bulbs = [FailedBulb.create!]
end

assert_equal [original_child], car.reload.failed_bulbs
end
end
6 changes: 6 additions & 0 deletions activerecord/test/models/bulb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,9 @@ class FunkyBulb < Bulb
raise "before_destroy was called"
end
end

class FailedBulb < Bulb
before_destroy do
false
end
end
1 change: 1 addition & 0 deletions activerecord/test/models/car.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class Car < ActiveRecord::Base
has_many :bulbs
has_many :all_bulbs, -> { unscope where: :name }, class_name: "Bulb"
has_many :funky_bulbs, class_name: 'FunkyBulb', dependent: :destroy
has_many :failed_bulbs, class_name: 'FailedBulb', dependent: :destroy
has_many :foo_bulbs, -> { where(:name => 'foo') }, :class_name => "Bulb"

has_one :bulb
Expand Down

0 comments on commit 5aab0c0

Please sign in to comment.