Remove all revelant through records.

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.

(Previously the records were removed in the database, but only one was
removed from the in-memory target array.)

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]

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)

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