ActiveRecord / acts as list / position is set upon saving and does not get updated when you change to a different scope
From WhyNotWiki
At first I thought this might be a bug, but now I see that it behaves this way by design (and, well, I can't really think of a better way to have implemented with it), so it's really just a [caveat (category)] to be aware of...
Please see http://svn.tylerrick.com/public/rails/examples/trees/ordered_trees/test/moving_to_different_parent_test.rb, as it demonstrates this behavior better than my explanation here...
Contents |
[edit] Step 1: Write an automated test to demonstrate my problem
This works as expected:
class Person < ActiveRecord::Base
has_many :children, :class_name => name, :foreign_key => 'parent_id', :order => 'position', :dependent => :destroy
acts_as_list :scope => :parent_id
end
class HasManyTest < Test::Unit::TestCase
def test_position_after_adding_to_children
dad = Person.create(:name => "Dad")
assert_equal 1, dad.position
assert_equal 0, dad.children.size
puts '-------------------------- about to create bob'
bob = Person.new(:name => "Bob")
puts '-------------------------- about to add bob to dad.children...'
dad.children << bob
assert_equal dad.id, bob.parent_id
assert_equal 1, bob.position
end
end
bottom_item() scope_condition(): returning 'parent_id IS NULL' bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id IS NULL", :order => "position DESC") bottom_item(): returning nil -------------------------- about to create bob -------------------------- about to add bob to dad.children... bottom_item() scope_condition(): returning 'parent_id = 1' bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id = 1", :order => "position DESC") bottom_item(): returning nil
But this:
puts '-------------------------- about to create bob'
bob = Person.create(:name => "Bob") # create instead of new
puts '-------------------------- about to add bob to dad.children...'
dad.children << bob
...does not (perhaps, depending on what you expected to happen)...
bottom_item()
scope_condition(): returning 'parent_id IS NULL'
bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id IS NULL", :order => "position DESC")
bottom_item(): returning nil
-------------------------- about to create bob
bottom_item()
scope_condition(): returning 'parent_id IS NULL'
bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id IS NULL", :order => "position DESC")
bottom_item(): returning #<Person:0xb783c950 @attributes={"name"=>"Dad", "id"=>"1", "parent_id"=>nil, "position"=>"1"}>
-------------------------- about to add bob to dad.children...
F
Finished in 0.032971 seconds.
1) Failure:
test_position_after_adding_to_children(HasManyTest) [has_many_test.rb:25]:
<1> expected but was
<2>.
Neither does this (because new+save is the same as create):
puts '-------------------------- about to create bob'
bob = Person.new(:name => "Bob")
bob.save!
puts '-------------------------- about to add bob to dad.children...'
dad.children << bob
[edit] Step 2: Refine
A little bit more verbose with the debug output...
bob = Person.new(:name => "Bob")
Dad: scope_condition(): returning 'parent_id IS NULL' Dad: bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id IS NULL", :order => "position DESC") Dad: bottom_item(): returning nil Dad: bottom_position_in_list(): returning 0 Dad: add_to_list_bottom(): setting self[position] = 0 + 1 -------------------------- about to create bob -------------------------- about to add bob to dad.children... Bob: scope_condition(): returning 'parent_id = 1' Bob: bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id = 1", :order => "position DESC") Bob: bottom_item(): returning nil (This makes sense... There are no other items with parent_id = 1. So we expect our new item to assume position = 1, which it does...) Bob: bottom_position_in_list(): returning 0 Bob: add_to_list_bottom(): setting self[position] = 0 + 1
But this... (Note the comments)
bob = Person.create(:name => "Bob")
Dad: scope_condition(): returning 'parent_id IS NULL'
Dad: bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id IS NULL", :order => "position DESC")
Dad: bottom_item(): returning nil
Dad: bottom_position_in_list(): returning 0
Dad: add_to_list_bottom(): setting self[position] = 0 + 1
-------------------------- about to create bob
Bob: scope_condition(): returning 'parent_id IS NULL'
(!!! There's our problem! At the time it is created, parent_id is nil, so it is going to assume the position right below Dad: position 2!)
Bob: bottom_item(): acts_as_list_class.find(:first, :conditions => "parent_id IS NULL", :order => "position DESC")
Bob: bottom_item(): returning #<Person:0xb77d8a7c @attributes={"name"=>"Dad", "id"=>"1", "parent_id"=>nil, "position"=>"1"}>
Bob: bottom_position_in_list(): returning 1
Bob: add_to_list_bottom(): setting self[position] = 1 + 1
-------------------------- about to add bob to dad.children...
So that actually may be a reasonable behavior -- it has to choose a position at the time it is created, and at the time it is created, it has no parent_id, so the :scope => :parent_id has no effect.
It has to assume some position at the time it is saved, and since it doesn't yet have a parent, its scope is "parent IS NULL" and it assumes (the position of (the last object at that same scope: dad): 1) + 1: 2.
It leads me to wonder, though: when we change its parent_id (with dad.children << bob), shouldn't it recalculate a new position within the new scope?
... ?
[edit] Step 3: Making it work?
Here are my new expectations...
def test_position_after_adding_to_children
puts '-------------------------- about to create dad'
dad = Person.create(:name => "Dad")
assert_equal 1, dad.position
assert_equal nil, dad.parent_id
puts '-------------------------- about to create bob'
bob = Person.create(:name => "Bob")
assert_equal 2, bob.position
assert_equal nil, bob.parent_id
puts '-------------------------- about to add bob to dad.children...'
dad.children << bob
assert_equal dad.id, bob.parent_id
assert_equal 1, bob.position
end
The question is... Should I be expecting that to work (maybe there's a "move_to_new_parent" method that I should be using instead?)?
And if I need to monkey patch things to make it work, where would be the best place to hook in that code? It looks like I have these callbacks available, for example:
- before_validation
- before_validation_on_update
- after_validation
- after_validation_on_update
- before_save
- before_update
- after_update
- after_save
[edit] Step 4: Making it work by means of ActiveRecord's callback mechanism?
# /usr/lib/ruby/gems/1.8/gems/activerecord-1.15.3/lib/active_record/acts/list.rb
before_validation :report_my_parent
after_validation :report_my_parent
before_save :report_my_parent
before_update :report_my_parent
after_update :report_my_parent
after_save :report_my_parent
puts '-------------------------- about to add bob to dad.children...'
puts "parent_id = #{bob.parent_id.inspect}"
dad.children << bob
-------------------------- about to add bob to dad.children... parent_id = nil callback(before_validation) Bob: report_my_parent: parent_id = 1 callback(before_validation_on_update) callback(after_validation) Bob: report_my_parent: parent_id = 1 callback(after_validation_on_update) callback(before_save) Bob: report_my_parent: parent_id = 1 callback(before_update) Bob: report_my_parent: parent_id = 1 callback(after_update) Bob: report_my_parent: parent_id = 1 callback(after_save) Bob: report_my_parent: parent_id = 1
All right, so parent_id is set well in advance of any of the callbacks... But when? And how?
[edit] Step 5: When does parent_id get set?
Well, I thought this might tell me something...
require 'unroller'
class HasManyTest < Test::Unit::TestCase
def test_position_after_adding_to_children
...
Unroller::trace :exclude_classes => [Class, Module, Object, /ActiveSupport/, /Inflector/, /Monitor/],
:exclude_methods => [:assert_valid_keys, :name, /deprecat/],
:show_locals => true, :interactive => true do
dad.children << bob
end
...
end
end
...but it was a bit too verbose and I have to admit, I didn't get much out of it.
[edit] Step 6: Forget that
I'm giving up on trying to make it update position when you change scope... (I still think it should do that automatically though...)
Maybe I'll just try to use new() everywhere and only save the model after we've assigned it a parent_id (such as by doing << ...)...
Would that be doable? Or is there some special case where I'd have to / want to save first and then assign it a parent?
In that case, how would we say "move to the end of this new list", for example?
It looks like BetterNestedSet provides methods like move_to_child_of, which is just what I need. I think I'll switch to using BetterNestedSet instead of acts_as_tree + acts_as_list.
