diff options
3 files changed, 29 insertions, 9 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index fac9ad1188..203b110e98 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -39,6 +39,11 @@ *Rails 3.1.2 (unreleased)* +* If a record is removed from a has_many :through, all of the join records relating to that + record should also be removed from the through association's target. + + [Jon Leighton] + * Fix adding multiple instances of the same record to a has_many :through. [GH #3425] [Jon Leighton] diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index d82d041b69..6ba405ba4c 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -79,12 +79,6 @@ module ActiveRecord @through_records.delete(record.object_id) end - def through_record(record) - attributes = construct_join_attributes(record) - candidates = Array.wrap(through_association.target) - candidates.find { |c| c.attributes.slice(*attributes.keys) == attributes } - end - def build_record(attributes, options = {}) ensure_not_nested @@ -145,14 +139,20 @@ module ActiveRecord update_counter(-count) end + def through_records_for(record) + attributes = construct_join_attributes(record) + candidates = Array.wrap(through_association.target) + candidates.find_all { |c| c.attributes.slice(*attributes.keys) == attributes } + end + def delete_through_records(through, records) records.each do |record| - through_record = through_record(record) + through_records = through_records_for(record) if through_reflection.macro == :has_many - through.target.delete(through_record) + through_records.each { |r| through.target.delete(r) } else - through.target = nil if through.target == through_record + through.target = nil if through_records.include?(through.target) end @through_records.delete(record.object_id) diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 115e06144e..7a6aba6a6b 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -77,6 +77,21 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase end end + def test_add_two_instance_and_then_deleting + post = posts(:thinking) + person = people(:david) + + post.people << person + post.people << person + + counts = ['post.people.count', 'post.people.to_a.count', 'post.readers.count', 'post.readers.to_a.count'] + assert_difference counts, -2 do + post.people.delete(person) + end + + assert !post.people.reload.include?(person) + end + def test_associating_new assert_queries(1) { posts(:thinking) } new_person = nil # so block binding catches it |