Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix reset_counters crashing on has_many :through associations. #7822

Merged
merged 1 commit into from

3 participants

@lulalala

Currently when a counter cache column name is specified in the intermediate model of a has_many :through association, the reset_counters() method is unable to get the model where the column resides in. The resulting error is:

NoMethodError: undefined method `counter_cache_column' for nil:NilClass

The correct model reference has to be accessed using the additional through_reflection object.

@rafaelfranca
Owner

Could you add a CHANGELOG entry?

@rafaelfranca rafaelfranca commented on the diff
activerecord/lib/active_record/counter_cache.rb
@@ -22,8 +22,13 @@ def reset_counters(id, *counters)
counters.each do |association|
has_many_association = reflect_on_association(association.to_sym)
- foreign_key = has_many_association.foreign_key.to_s
- child_class = has_many_association.klass
+ if has_many_association.is_a? ActiveRecord::Reflection::ThroughReflection
+ foreign_key = has_many_association.through_reflection.foreign_key
@rafaelfranca Owner

Why we are not calling to_s in this foreign_key?

@lulalala
lulalala added a note

Thanks, I missed it. Let me update it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lulalala lulalala Fix reset_counters() crashing on has_many :through associations.
The counter column name in the intermediate model need to be access
via the through reflection.
6e56a03
@lulalala

@rafaelfranca Thanks, I have updated the code and the CHANGELOG.

@rafaelfranca rafaelfranca merged commit 2bc1ac7 into rails:master
@rafaelfranca
Owner

Thanks

@artemave

Can anyone please have a look at backport of this for 3-2 stable? #9156 Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 2, 2012
  1. @lulalala

    Fix reset_counters() crashing on has_many :through associations.

    lulalala authored
    The counter column name in the intermediate model need to be access
    via the through reflection.
This page is out of date. Refresh to see the latest.
View
5 activerecord/CHANGELOG.md
@@ -1,5 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* Fix `reset_counters` crashing on `has_many :through` associations.
+ Fix #7822.
+
+ *lulalala*
+
* Support for partial inserts.
When inserting new records, only the fields which have been changed
View
9 activerecord/lib/active_record/counter_cache.rb
@@ -22,8 +22,13 @@ def reset_counters(id, *counters)
counters.each do |association|
has_many_association = reflect_on_association(association.to_sym)
- foreign_key = has_many_association.foreign_key.to_s
- child_class = has_many_association.klass
+ if has_many_association.is_a? ActiveRecord::Reflection::ThroughReflection
+ foreign_key = has_many_association.through_reflection.foreign_key.to_s
@rafaelfranca Owner

Why we are not calling to_s in this foreign_key?

@lulalala
lulalala added a note

Thanks, I missed it. Let me update it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ child_class = has_many_association.through_reflection.klass
+ else
+ foreign_key = has_many_association.foreign_key.to_s
+ child_class = has_many_association.klass
+ end
belongs_to = child_class.reflect_on_all_associations(:belongs_to)
reflection = belongs_to.find { |e| e.foreign_key.to_s == foreign_key && e.options[:counter_cache].present? }
counter_name = reflection.counter_cache_column
View
15 activerecord/test/cases/counter_cache_test.rb
@@ -10,9 +10,12 @@
require 'models/dog_lover'
require 'models/person'
require 'models/friendship'
+require 'models/subscriber'
+require 'models/subscription'
+require 'models/book'
class CounterCacheTest < ActiveRecord::TestCase
- fixtures :topics, :categories, :categorizations, :cars, :dogs, :dog_lovers, :people, :friendships
+ fixtures :topics, :categories, :categorizations, :cars, :dogs, :dog_lovers, :people, :friendships, :subscribers, :subscriptions, :books
class ::SpecialTopic < ::Topic
has_many :special_replies, :foreign_key => 'parent_id'
@@ -118,4 +121,14 @@ class ::SpecialReply < ::Reply
Person.reset_counters(michael.id, :followers)
end
end
+
+ test "reset counter of has_many :through association" do
+ subscriber = subscribers('second')
+ Subscriber.reset_counters(subscriber.id, 'books')
+ Subscriber.increment_counter('books_count', subscriber.id)
+
+ assert_difference 'subscriber.reload.books_count', -1 do
+ Subscriber.reset_counters(subscriber.id, 'books')
+ end
+ end
end
View
2  activerecord/test/models/subscription.rb
@@ -1,4 +1,4 @@
class Subscription < ActiveRecord::Base
- belongs_to :subscriber
+ belongs_to :subscriber, :counter_cache => :books_count
belongs_to :book
end
View
1  activerecord/test/schema/schema.rb
@@ -621,6 +621,7 @@ def create_table(*args, &block)
create_table :subscribers, :force => true, :id => false do |t|
t.string :nick, :null => false
t.string :name
+ t.column :books_count, :integer, :null => false, :default => 0
end
add_index :subscribers, :nick, :unique => true
Something went wrong with that request. Please try again.