Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Update counter cache when doing collection<<(object, …) #3891

Closed
NielsKSchjoedt opened this Issue · 12 comments

7 participants

@NielsKSchjoedt

Hi I'm having trouble with a simple counter cache in rails. I have the following models:

class CarImage < ActiveRecord::Base
  belongs_to :car, :counter_cache => :images_count
end

class Car < ActiveRecord::Base
  has_many :images, :class_name => "CarImage", :dependent => :destroy, :limit => 4
end

The problem occurs when doing this:

car1.images << car2.images

None of the images_count on the two cars are being updated accordingly.

I found this on the topic: http://stackoverflow.com/questions/5758339/how-to-update-counter-cache-when-updating-a-model And I my case the solution would look something like this:

class Car < ActiveRecord::Base
  has_many :images, :class_name => "CarImage", :dependent => :destroy, :limit => 4
  after_save :update_counter, :if => :car_id_changed?

  private

  def update_counter
    new_car = Car.find(car_id)
    Car.increment_counter(:images_count, new_car.id)
    if car_id_was.present?
      old_car = Car.find(car_id_was)
      Car.decrement_counter(:images_count, old_car.id)
    end
  end

end

But - I'm asking myself - Why is this not build into rails at first hand? I can't find anything in the documentation about the issue, and to me using the build-in counter_cache in rails is almost unusable, if its really true, that it only supports create and destroy - not update! Can anyone give a good explanation as to why this is? Is it really necessary for me to build a callback and keep an eye on the relations my self?

BTW. I'm using 3.1

@ahawkins

@NielsKSchjoedt I was very suspicious of your issue. I was pretty sure you wouldn't have to worry about this. So I made a test repo to demonstrate this. According my tests, Rails will track changes on update. The test repo is here: https://github.com/twinturbo/counter_cache_test.

Here is the gist:

require 'test_helper'

class PostTest < ActiveSupport::TestCase
  def tests_creating_a_comments_with_posts_increments_counter
    post = Post.create! :title => 'foo'
    Comment.create! :text => 'foo', :post => post
    post.reload

    assert_equal 1, post.comments_count
  end

  def test_destroy_a_comment_decrements_counter
    post = Post.create :title => 'foo'
    comment = Comment.create! :text => 'foo', :post => post
    post.reload

    assert_equal 1, post.comments_count

    comment.destroy
    post.reload

    assert_equal 0, post.comments_count
  end

  def test_changing_a_comments_post_updates_the_counter
    rails_post = Post.create! :title => 'Rails'
    sinatra_post = Post.create! :title => 'Sinatra'

    comment = rails_post.comments.create! :text => 'foo'

    rails_post.reload

    assert_equal 1, rails_post.comments_count
    assert_equal 0, sinatra_post.comments_count

    comment.post = sinatra_post
    comment.save!

    rails_post.reload
    sinatra_post.reload

    assert_equal 0, rails_post.comments_count
    assert_equal 1, sinatra_post.comments_count
  end
end

All tests pass.

@steveklabnik can you close this one too :D

@rafaelfranca

@twinturbo had you tried with the << operator? If it works too we can close.

@ahawkins

@rafaelfranca bad news dude! Much to my surprise this test case fails:

  def tests_creating_a_comments_with_posts_increments_counter_using_push
    post = Post.create! :title => 'foo'
    post.comments << Comment.create!(:text => 'foo')
    post.reload

    assert_equal 1, post.comments_count
  end

The repo is updated with a new commit.

@rafaelfranca

:bomb: So it is still an issue.

@rafaelfranca

@twinturbo try to fix it if you want, now that you have a test case

@senny senny referenced this issue from a commit in senny/rails
@senny senny test case to reproduce #3891 => passes cc3c966
@senny
Owner

@NielsKSchjoedt can you check this again against the latest version of rails? I tried to reproduce the error in a test case but it passed. (see cc3c966)

@phuongnd08

I have a concern like this:

module HasReferral
  extend ActiveSupport::Concern
  included do
    belongs_to :referrer, class_name: self.name,
      touch: true, inverse_of: :referreds,
      counter_cache: :referreds_count

    has_many :referreds, class_name: self.name,
      foreign_key: :referrer_id
  end
end

which will then be included into User model, constructing the :referrer and :referreds relationship. When I use User.create(referrer: user) then referreds_count is updated properly. But if I use user.referreds << User.create then the referreds_count is not updated. I'm on 3.2.11 now.

@matthewrobertson

@senny I can confirm this is broken in master right now

@matthewrobertson

Seems like this is a regression bug introduced by this commit. I am putting together a patch right now.

@matthewrobertson

@rafaelfranca @senny I have submitted a pull request with a fix for this. Could you please have a look.

@carlosantoniodasilva

Closed by #10292.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.