From 757e4364dc3f808f0002a6c8cb03531e69e2f356 Mon Sep 17 00:00:00 2001
From: Emilio Tagua <miloops@gmail.com>
Date: Tue, 2 Dec 2008 14:12:09 -0300
Subject: [PATCH] Fix: counter_cache should decrement on deleting associated
 records.

[#1195 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
---
 .../associations/association_collection.rb    |  3 +++
 .../has_many_associations_test.rb             | 20 +++++++++++++++++++
 .../has_many_through_associations_test.rb     | 15 ++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index ae409c9042a86..dc8e792e81102 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -139,6 +139,9 @@ def delete(*records)
           
           records.each do |record|
             @target.delete(record)
+            if respond_to?(:cached_counter_attribute_name) && @owner[cached_counter_attribute_name]
+              @owner.class.decrement_counter(cached_counter_attribute_name, @owner.send(@owner.class.primary_key))
+            end
             callback(:after_remove, record)
           end
         end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 5704df8a43382..f1bd168ea7536 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -536,6 +536,18 @@ def test_deleting
     assert_equal 0, companies(:first_firm).clients_of_firm(true).size
   end
 
+  def test_deleting_updates_counter_cache
+    post = Post.first
+
+    post.comments.delete(post.comments.first)
+    post.reload
+    assert_equal post.comments(true).size, post.comments_count
+
+    post.comments.delete(post.comments.first)
+    post.reload
+    assert_equal 0, post.comments_count
+  end
+
   def test_deleting_before_save
     new_firm = Firm.new("name" => "A New Firm, Inc.")
     new_client = new_firm.clients_of_firm.build("name" => "Another Client")
@@ -589,6 +601,14 @@ def test_clearing_an_association_collection
     end
   end
 
+  def test_clearing_updates_counter_cache
+    post = Post.first
+
+    post.comments.clear
+    post.reload
+    assert_equal 0, post.comments_count
+  end
+
   def test_clearing_a_dependent_association_collection
     firm = companies(:first_firm)
     client_id = firm.dependent_clients_of_firm.first.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 652db3b9fbca7..77108b8232a42 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -2,6 +2,10 @@
 require 'models/post'
 require 'models/person'
 require 'models/reader'
+require 'models/comment'
+require 'models/tag'
+require 'models/tagging'
+require 'models/author'
 
 class HasManyThroughAssociationsTest < ActiveRecord::TestCase
   fixtures :posts, :readers, :people
@@ -81,6 +85,17 @@ def test_delete_association
     assert posts(:welcome).reload.people(true).empty?
   end
 
+  def test_deleting_updates_counter_cache
+    taggable = Tagging.first.taggable
+    taggable.taggings.push(Tagging.new)
+    taggable.reload
+    assert_equal 1, taggable.taggings_count
+
+    taggable.taggings.delete(taggable.taggings.first)
+    taggable.reload
+    assert_equal 0, taggable.taggings_count
+  end
+
   def test_replace_association
     assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)}