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

Unset association when existing record is destroyed. #5248

Conversation

jcoleman
Copy link
Contributor

@jcoleman jcoleman commented Mar 2, 2012

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.

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.
@isabanin
Copy link

isabanin commented Mar 7, 2012

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
Copy link
Member

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

@jcoleman
Copy link
Contributor Author

@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
Copy link
Member

@jonleighton @tenderlove any thoughts on this?

jonleighton added a commit that referenced this pull request Sep 21, 2012
…an-existing-record-is-destroyed

Unset association when existing record is destroyed.
@jonleighton jonleighton merged commit 834d6da into rails:master Sep 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants