#5568 2.3.9 destroy_all destroys loaded records that do not match the scope on associations - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5568 ✓stale

2.3.9 destroy_all destroys loaded records that do not match the scope on associations

Reported by Will Bryant | September 7th, 2010 @ 10:32 AM

Release 2.3.9's commit https://github.com/rails/rails/commit/b41c3ba154c2038ecc7b230693662... introduced a nasty bug that causes some_association.some_scope.destroy_all to destroy any records that were loaded in some_association, even if they do not match some_scope.

For example, see the attached test case, where I create some records on an association, and then destroy_all on a scope of that association.

In ActiveRecord::Associations::AssociationCollection#load_target, the new code adds back all the records in @target, which are then returned in the target array to destroy_all, which then destroys them.

This didn't previously happen as that method previously only added back in any new records. So I have attached a patch that solves this particular case by only adding back the new records. This restores the prior behavior in this respect

One could argue though that this whole method needs a rewrite to have clearer semantics with respect to scopes.

Comments and changes to this ticket

You flagged this item as spam.
Will Bryant September 7th, 2010 @ 10:39 AM
- no changes were found...
- 0001-don-t-add-non-new-records-back-to-the-target-array-a.patch 3.2 KB
You flagged this item as spam.
Will Bryant September 7th, 2010 @ 10:41 AM
- Assigned user set to “Pratik”
Pratik, would you mind taking a look at this? Hoping you know a lot more about the original patch than me :).
Jeff Kreeftmeijer October 10th, 2010 @ 09:02 AM
- Tag changed from “activerecord regression destroy_all load_target has_many named_scope” to “activerecord, destroy_all, has_many, named_scope, patch, regression”
- Importance changed from “” to “Low”
You flagged this item as spam.
Gonçalo Silva October 18th, 2010 @ 05:14 PM
Just confirmed this behaviour on Rails 2.3.9.
Santiago Pastorino February 9th, 2011 @ 12:31 AM
- State changed from “new” to “open”
This issue has been automatically marked as stale because it has not been commented on for at least three months.

The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
Santiago Pastorino February 9th, 2011 @ 12:31 AM
- State changed from “open” to “stale”