Rails core - Oct 2008 - problem writing to a counter_cache with update_attribute

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>


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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.bannasch@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?
>
> 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>
>
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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
   end

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


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Sat, Oct 11, 2008 at 1:25 AM, Stephen Bannasch
<stephen.bannasch@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 :-)

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

At 7:47 PM -0300 10/12/08, Damian Janowski wrote:
>On Sat, Oct 11, 2008 at 1:25 AM, Stephen Bannasch
><stephen.bannasch@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
:-)
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

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

On Tue, Oct 14, 2008 at 4:44 PM, Trevor Turk <trevorturk@gmail.com>
wrote:
>
> 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?
No consensus :-/

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

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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.

On Tue, Oct 14, 2008 at 9:50 PM, Damian Janowski
<damian.janowski@gmail.com> wrote:
>
> On Tue, Oct 14, 2008 at 4:44 PM, Trevor Turk <trevorturk@gmail.com>
wrote:
>>
>> 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?
>
> No consensus :-/
>
> I think the resolution was "we don''t want yet another
migration helper" (?).
>
> >
>


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

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

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.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---