Skip to content

Commit 757e436

Browse files
miloopsjeremy
authored andcommitted
Fix: counter_cache should decrement on deleting associated records.
[#1195 state:committed] Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
1 parent 4130e13 commit 757e436

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

activerecord/lib/active_record/associations/association_collection.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@ def delete(*records)
139139

140140
records.each do |record|
141141
@target.delete(record)
142+
if respond_to?(:cached_counter_attribute_name) && @owner[cached_counter_attribute_name]
143+
@owner.class.decrement_counter(cached_counter_attribute_name, @owner.send(@owner.class.primary_key))
144+
end
142145
callback(:after_remove, record)
143146
end
144147
end

activerecord/test/cases/associations/has_many_associations_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,18 @@ def test_deleting
536536
assert_equal 0, companies(:first_firm).clients_of_firm(true).size
537537
end
538538

539+
def test_deleting_updates_counter_cache
540+
post = Post.first
541+
542+
post.comments.delete(post.comments.first)
543+
post.reload
544+
assert_equal post.comments(true).size, post.comments_count
545+
546+
post.comments.delete(post.comments.first)
547+
post.reload
548+
assert_equal 0, post.comments_count
549+
end
550+
539551
def test_deleting_before_save
540552
new_firm = Firm.new("name" => "A New Firm, Inc.")
541553
new_client = new_firm.clients_of_firm.build("name" => "Another Client")
@@ -589,6 +601,14 @@ def test_clearing_an_association_collection
589601
end
590602
end
591603

604+
def test_clearing_updates_counter_cache
605+
post = Post.first
606+
607+
post.comments.clear
608+
post.reload
609+
assert_equal 0, post.comments_count
610+
end
611+
592612
def test_clearing_a_dependent_association_collection
593613
firm = companies(:first_firm)
594614
client_id = firm.dependent_clients_of_firm.first.id

activerecord/test/cases/associations/has_many_through_associations_test.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22
require 'models/post'
33
require 'models/person'
44
require 'models/reader'
5+
require 'models/comment'
6+
require 'models/tag'
7+
require 'models/tagging'
8+
require 'models/author'
59

610
class HasManyThroughAssociationsTest < ActiveRecord::TestCase
711
fixtures :posts, :readers, :people
@@ -81,6 +85,17 @@ def test_delete_association
8185
assert posts(:welcome).reload.people(true).empty?
8286
end
8387

88+
def test_deleting_updates_counter_cache
89+
taggable = Tagging.first.taggable
90+
taggable.taggings.push(Tagging.new)
91+
taggable.reload
92+
assert_equal 1, taggable.taggings_count
93+
94+
taggable.taggings.delete(taggable.taggings.first)
95+
taggable.reload
96+
assert_equal 0, taggable.taggings_count
97+
end
98+
8499
def test_replace_association
85100
assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)}
86101

0 commit comments

Comments
 (0)