aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG5
-rw-r--r--activerecord/lib/active_record/associations/has_many_through_association.rb18
-rw-r--r--activerecord/test/cases/associations/has_many_through_associations_test.rb15
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