Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Unset association when existing record is destroyed. #5248

Merged
merged 1 commit into from

4 participants

@jcoleman

To avoid foreign key errors (and invalid data) in the database, when a belongs_to association is destroyed, it should also be nil'd out on the parent object.

That is, the association id should not be kept on the record after save. This is a corruption of data integrity in the database.

@jcoleman jcoleman Unset association when existing record is destroyed.
To avoid foreign key errors (and invalid data) in the database, when a belongs_to association is destroyed, it should also be nil'd out on the parent object.
217a8b0
@isabanin

I probably missed something, but isn't this the same as using :dependent => :nullify on the other side of the association (has_many, has_one)?

@steveklabnik
Collaborator

This is the pull request that fixes #5235, right?

@jcoleman

@steveklabnik It is.

@isabanin Sorry I didn't respond, I didn't have a chance to test :dependent => :nullify. But I don't think that workaround invalidates the bug/pull request. That is, if you destroy a belongs_to through the association, then the association should be unset. Otherwise you have invalid data being persisted. That should just be default behavior--there shouldn't be a need to have a special option (I don't think the two are equivalent anyway.)

@steveklabnik
Collaborator

@jonleighton @tenderlove any thoughts on this?

@jonleighton jonleighton merged commit 834d6da into rails:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 2, 2012
  1. @jcoleman

    Unset association when existing record is destroyed.

    jcoleman authored
    To avoid foreign key errors (and invalid data) in the database, when a belongs_to association is destroyed, it should also be nil'd out on the parent object.
This page is out of date. Refresh to see the latest.
View
1  activerecord/lib/active_record/autosave_association.rb
@@ -402,6 +402,7 @@ def save_belongs_to_association(reflection)
autosave = reflection.options[:autosave]
if autosave && record.marked_for_destruction?
+ self[reflection.foreign_key] = nil
record.destroy
elsif autosave != false
saved = record.save(:validate => !autosave) if record.new_record? || (autosave && record.changed_for_autosave?)
View
16 activerecord/test/cases/nested_attributes_test.rb
@@ -450,6 +450,22 @@ def test_should_destroy_an_existing_record_if_there_is_a_matching_id_and_destroy
end
end
+ def test_should_unset_association_when_an_existing_record_is_destroyed
+ @ship.reload
+ original_pirate_id = @ship.pirate.id
+ @ship.attributes = {:pirate_attributes => {:id => @ship.pirate.id, :_destroy => true}}
+ @ship.save!
+
+ assert_empty Pirate.where(["id = ?", original_pirate_id])
+ assert_nil @ship.pirate_id
+ assert_nil @ship.pirate
+
+ @ship.reload
+ assert_empty Pirate.where(["id = ?", original_pirate_id])
+ assert_nil @ship.pirate_id
+ assert_nil @ship.pirate
+ end
+
def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy
[nil, '0', 0, 'false', false].each do |not_truth|
@ship.update_attributes(:pirate_attributes => { :id => @ship.pirate.id, :_destroy => not_truth })
Something went wrong with that request. Please try again.