problem writing to a counter_cache with update_attribute

[Stephen Bannasch]

I added a counter_cache to an existing parent-child model pair and
wanted to set all the values for the counter_cache but can't do it
using update_attribute (or a number of other methods).

Should I create a bug report?

I created a simple test app in 2.2.1 to isolate the problem.

# create the learner and learner_session models and migrations:

script/generate model learner name:string learner_session_id:integer
script/generate model learner_session learner_id:integer
rake db:migrate

# add the associations to the models:

class Learner < ActiveRecord::Base
has_many :learner_sessions
end

class LearnerSession < ActiveRecord::Base
belongs_to :learner
end

# create a learner and 2 learner sessions without a counter_cache

$ script/console

>> l = Learner.create(:name => "stephen")
=> #<Learner id: 1, name: "stephen", learner_session_id: nil>

>> l.learner_sessions.create
=> #<LearnerSession id: 1, learner_id: 1>

>> l.learner_sessions.create
=> #<LearnerSession id: 2, learner_id: 1>

>> l.learner_sessions.count
=> 2

# now run this migration:

class AddCounterCacheToLearners < ActiveRecord::Migration
def self.up
add_column :learners, :learner_sessions_count, :integer, :default => 0
# Learner.reset_column_information
end

def self.down
remove_column :learners, :learner_sessions_count
end
end

# and add the counter_cache to the belongs_to association

class LearnerSession < ActiveRecord::Base
belongs_to :learner, :counter_cache => true
end

# back into script/console

$ script/console

>> l = Learner.find(:first)
=> #<Learner id: 1, name: "stephen", learner_session_id: nil,
learner_sessions_count: 0>

# there are still 2 learner sessions:

>> l.learner_sessions.count
=> 2

# but the learner_sessions_count is not set yet:

>> l.learner_sessions_count
=> 0

# update the learner_sessions_count:

>> l.update_attribute(:learner_sessions_count, 2)
=> true

# it is written to the model object:

>> l
=> #<Learner id: 1, name: "stephen", learner_session_id: nil,
learner_sessions_count: 2>

>> l.learner_sessions_count
=> 2

# but not to the database

>> l = Learner.find(:first)
=> #<Learner id: 1, name: "stephen", learner_session_id: nil,
learner_sessions_count: 0>

[Frederick Cheung]

On 10 Oct 2008, at 21:17, Stephen Bannasch wrote:

>
> I added a counter_cache to an existing parent-child model pair and
> wanted to set all the values for the counter_cache but can't do it
> using update_attribute (or a number of other methods).
>
> Should I create a bug report?
>

From the docs (http://api.rubyonrails.com/classes/ActiveRecord/Associations/ClassMethods.html
):
> Note: Specifying a counter cache will add it to that model‘s list of
> readonly attributes using attr_readonly.

Fred
>

[Duncan Beevers]

I'm sure you read Frederick's response about the counter_cache being readonly.

The way to properly populate these during a migration is to redeclare the Learner class as an inner-class of the Migration itself, specifying only minimal functionality.

class AddCounterCacheToLearners < ActiveRecord::Migration

  class Learner < ActiveRecord::Base

    def reset_column_information

      # re-implement reset_column_information functionality here

    end

  end

  def self.up
    add_column :learners, :learner_sessions_count, :integer, :default => 0

    Learner.reset_column_information
  end

  def self.down
    remove_column :learners, :learner_sessions_count
  end
end

On Fri, Oct 10, 2008 at 1:17 PM, Stephen Bannasch <stephen....@deanbrook.org> wrote:

I added a counter_cache to an existing parent-child model pair and
wanted to set all the values for the counter_cache but can't do it
using update_attribute (or a number of other methods).

Should I create a bug report?

[Stephen Bannasch]

Thanks Frederic and Duncan,

FYI: here's the complete migration for the simple test case that adds
a counter_cache AND sets the value.

class AddCounterCacheToLearners < ActiveRecord::Migration
class Learner < ActiveRecord::Base

has_many :learner_sessions

def reset_column_information
# re-implement reset_column_information functionality here

generated_methods.each { |name| undef_method(name) }
@column_names = @columns = @columns_hash = @content_columns =
@dynamic_methods_hash = @generated_methods = @inheritance_column = nil
end
end

def self.up
add_column :learners, :learner_sessions_count, :integer, :default => 0
Learner.reset_column_information

Learner.find(:all).each do |learner|
count = learner.learner_sessions.count
learner.update_attribute(:learner_sessions_count, count)
end

[Damian Janowski]

On Sat, Oct 11, 2008 at 1:25 AM, Stephen Bannasch
<stephen....@deanbrook.org> wrote:
>
> Thanks Frederic and Duncan,
>
> FYI: here's the complete migration for the simple test case that adds
> a counter_cache AND sets the value.

Or you could use http://rails.lighthouseapp.com/projects/8994/tickets/228 :-)

[Stephen Bannasch]

Thanks for that pointer Damian. I didnm't know about the class method Model.update_counters.

I've simplified my migration as follows:

class AddCounterCacheToLearners < ActiveRecord::Migration
def self.up
add_column :learners, :learner_sessions_count, :integer, :default => 0

Learner.reset_column_information
Learner.find(:all).each do |learner|

change_in_count = learner.learner_sessions.count - learner.learner_sessions_count
Learner.update_counters(learner.id, :learner_sessions_count => change_in_count)
end
end

def self.down
remove_column :learners, :learner_sessions_count
end
end

While this is better than my previous solution it still seems a bit ugly.

I like your proposed patch but it's marked as "wontfix".

In my simplified migration above this section:

Learner.reset_column_information
Learner.find(:all).each do |learner|

change_in_count = learner.learner_sessions.count - learner.learner_sessions_count
Learner.update_counters(learner.id, :learner_sessions_count => change_in_count)
end

is equivalent to the suggestions at the end of your patch discussion for a class method something like this:

Learner.update_counter_cache

[Trevor Turk]

On Oct 13, 2:39 pm, Stephen Bannasch <stephen.banna...@deanbrook.org>
wrote:

> I like your proposed patch but it's marked as "wontfix".

> http://rails.lighthouseapp.com/projects/8994/tickets/228

I like some of the ideas here, and I've been annoyed by this issue
quite a few times. Did a new ticket get created, or was a consensus
about an approach reached?

- Trevor

[Damian Janowski]

No consensus :-/

I think the resolution was "we don't want yet another migration helper" (?).

[Pratik]

I like the idea of adding a class method like
Model.reset_counter_cache(:column_name)

I don't think there is a new ticket.

--
Cheers!
- Pratik
http://m.onkey.org

[Trevor Turk]

On Oct 14, 3:21 pm, Pratik <pratikn...@gmail.com> wrote:
> I like the idea of adding a class method like
> Model.reset_counter_cache(:column_name)
>
> I don't think there is a new ticket.

I like that as well. I made a new ticket:

http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1211-reset_counter_cache

This is probably something I can work on, unless somebody else is
chomping at the bit.