acts_as_list plays nicely with inheritance by remembering the class which declared it. Closes #2811.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2964 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 196 insertions, 12 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index aac1c856f0..8dd92a1e8b 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* acts_as_list plays nicely with inheritance by remembering the class which declared it.  #2811 [rephorm@rephorm.com]
+
 * Fix sqlite adaptor's detection of missing dbfile or database declaration. [Nicholas Seckar]
 
 * Fixed acts_as_list for definitions without an explicit :order #2803 [jonathan@bluewire.net.nz]
diff --git a/activerecord/lib/active_record/acts/list.rb b/activerecord/lib/active_record/acts/list.rb
index 551c41fb6a..560c74dfff 100644
--- a/activerecord/lib/active_record/acts/list.rb
+++ b/activerecord/lib/active_record/acts/list.rb
@@ -54,6 +54,10 @@ module ActiveRecord
           class_eval <<-EOV
             include ActiveRecord::Acts::List::InstanceMethods
 
+            def acts_as_list_class
+              #{self.name}
+            end
+            
             def position_column
               '#{configuration[:column]}'
             end
@@ -78,7 +82,7 @@ module ActiveRecord
         def move_lower
           return unless lower_item
 
-          self.class.transaction do
+          acts_as_list_class.transaction do
             lower_item.decrement_position
             increment_position
           end
@@ -87,7 +91,7 @@ module ActiveRecord
         def move_higher
           return unless higher_item
 
-          self.class.transaction do
+          acts_as_list_class.transaction do
             higher_item.increment_position
             decrement_position
           end
@@ -95,7 +99,7 @@ module ActiveRecord
         
         def move_to_bottom
           return unless in_list?
-          self.class.transaction do
+          acts_as_list_class.transaction do
             decrement_positions_on_lower_items
             assume_bottom_position
           end
@@ -103,7 +107,7 @@ module ActiveRecord
         
         def move_to_top
           return unless in_list?
-          self.class.transaction do
+          acts_as_list_class.transaction do
             increment_positions_on_higher_items
             assume_top_position
           end
@@ -135,14 +139,14 @@ module ActiveRecord
         
         def higher_item
           return nil unless in_list?
-          self.class.find(:first, :conditions =>
+          acts_as_list_class.find(:first, :conditions =>
             "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i - 1).to_s}"
           )
         end
 
         def lower_item
           return nil unless in_list?
-          self.class.find(:first, :conditions =>
+          acts_as_list_class.find(:first, :conditions =>
             "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i + 1).to_s}"
           )
         end
@@ -171,7 +175,7 @@ module ActiveRecord
           def bottom_item(except = nil)
             conditions = scope_condition
             conditions = "#{conditions} AND id != #{except.id}" if except
-            self.class.find(:first, :conditions => conditions, :order => "#{position_column} DESC")
+            acts_as_list_class.find(:first, :conditions => conditions, :order => "#{position_column} DESC")
           end
 
           def assume_bottom_position
@@ -184,7 +188,7 @@ module ActiveRecord
 
           # This has the effect of moving all the higher items up one.
           def decrement_positions_on_higher_items(position)
-            self.class.update_all(
+            acts_as_list_class.update_all(
               "#{position_column} = (#{position_column} - 1)", "#{scope_condition} AND #{position_column} <= #{position}"
             )
           end
@@ -192,7 +196,7 @@ module ActiveRecord
           # This has the effect of moving all the lower items up one.
           def decrement_positions_on_lower_items
             return unless in_list?
-            self.class.update_all(
+            acts_as_list_class.update_all(
               "#{position_column} = (#{position_column} - 1)", "#{scope_condition} AND #{position_column} > #{send(position_column).to_i}"
             )
           end
@@ -200,20 +204,20 @@ module ActiveRecord
           # This has the effect of moving all the higher items down one.
           def increment_positions_on_higher_items
             return unless in_list?
-            self.class.update_all(
+            acts_as_list_class.update_all(
               "#{position_column} = (#{position_column} + 1)", "#{scope_condition} AND #{position_column} < #{send(position_column).to_i}"
             )
           end
 
           # This has the effect of moving all the lower items down one.
           def increment_positions_on_lower_items(position)
-            self.class.update_all(
+            acts_as_list_class.update_all(
               "#{position_column} = (#{position_column} + 1)", "#{scope_condition} AND #{position_column} >= #{position}"
            )
           end
 
           def increment_positions_on_all_items
-            self.class.update_all(
+            acts_as_list_class.update_all(
               "#{position_column} = (#{position_column} + 1)",  "#{scope_condition}"
             )
           end
diff --git a/activerecord/test/fixtures/mixin.rb b/activerecord/test/fixtures/mixin.rb
index a0a92c4f64..0411c0d87e 100644
--- a/activerecord/test/fixtures/mixin.rb
+++ b/activerecord/test/fixtures/mixin.rb
@@ -16,6 +16,12 @@ class ListMixin < Mixin
   def self.table_name() "mixins" end
 end
 
+class ListMixinSub1 < ListMixin
+end
+
+class ListMixinSub2 < ListMixin
+end
+
 
 class ListWithStringScopeMixin < ActiveRecord::Base
   acts_as_list :column => "pos", :scope => 'parent_id = #{parent_id}'
diff --git a/activerecord/test/fixtures/mixins.yml b/activerecord/test/fixtures/mixins.yml
index 7b20965512..cb21349cd0 100644
--- a/activerecord/test/fixtures/mixins.yml
+++ b/activerecord/test/fixtures/mixins.yml
@@ -78,3 +78,12 @@ tree_<%= set[0] %>:
   root_id: 42
 
 <% end %>
+
+# subclasses of list items
+<% (1..4).each do |i| %>
+list_sub_<%= i %>:
+  id: <%= i + 5000 %>
+  pos: <%= i %>
+  parent_id: 5000
+  type: <%= (i % 2 == 1) ? ListMixinSub1 : ListMixinSub2 %> 
+<% end %>
diff --git a/activerecord/test/mixin_test.rb b/activerecord/test/mixin_test.rb
index 82b2a143fd..4a59b649dd 100644
--- a/activerecord/test/mixin_test.rb
+++ b/activerecord/test/mixin_test.rb
@@ -195,6 +195,7 @@ class ListTest < Test::Unit::TestCase
     new2.move_higher
     assert_equal [new2, new1, new3], ListMixin.find(:all, :conditions => 'parent_id IS NULL', :order => 'pos')
   end
+
 end
 
 class TreeTest < Test::Unit::TestCase
@@ -353,3 +354,165 @@ class TouchTest < Test::Unit::TestCase
   end
 
 end
+
+
+class ListSubTest < Test::Unit::TestCase
+  fixtures :mixins
+ 
+  def test_reordering
+    
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_2), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_4)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+                  
+    mixins(:list_sub_2).move_lower
+                  
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_2), 
+                  mixins(:list_sub_4)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+                  
+    mixins(:list_sub_2).move_higher
+
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_2), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_4)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+    
+    mixins(:list_sub_1).move_to_bottom
+
+    assert_equal [mixins(:list_sub_2), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_4), 
+                  mixins(:list_sub_1)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+
+    mixins(:list_sub_1).move_to_top
+
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_2), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_4)],
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+                  
+                  
+    mixins(:list_sub_2).move_to_bottom
+  
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_4), 
+                  mixins(:list_sub_2)],
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+
+    mixins(:list_sub_4).move_to_top
+
+    assert_equal [mixins(:list_sub_4), 
+                  mixins(:list_sub_1), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_2)],
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+        
+  end
+
+  def test_move_to_bottom_with_next_to_last_item
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_2), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_4)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+
+    mixins(:list_sub_3).move_to_bottom
+
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_2), 
+                  mixins(:list_sub_4), 
+                  mixins(:list_sub_3)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+  end
+  
+  def test_next_prev
+    assert_equal mixins(:list_sub_2), mixins(:list_sub_1).lower_item
+    assert_nil mixins(:list_sub_1).higher_item
+    assert_equal mixins(:list_sub_3), mixins(:list_sub_4).higher_item
+    assert_nil mixins(:list_sub_4).lower_item
+  end
+  
+  
+  def test_injection
+    item = ListMixin.new("parent_id"=>1)
+    assert_equal "parent_id = 1", item.scope_condition
+    assert_equal "pos", item.position_column
+  end  
+  
+
+  def test_insert_at
+    new = ListMixin.create("parent_id" => 20)
+    assert_equal 1, new.pos
+
+	 new = ListMixinSub1.create("parent_id" => 20)
+	 assert_equal 2, new.pos
+	
+	 new = ListMixinSub2.create("parent_id" => 20)
+	 assert_equal 3, new.pos
+
+	 new4 = ListMixin.create("parent_id" => 20)
+	 assert_equal 4, new4.pos
+
+	 new4.insert_at(3)
+	 assert_equal 3, new4.pos
+
+	 new.reload
+	 assert_equal 4, new.pos
+    
+    new.insert_at(2)
+    assert_equal 2, new.pos
+
+    new4.reload
+    assert_equal 4, new4.pos
+
+    new5 = ListMixinSub1.create("parent_id" => 20)
+    assert_equal 5, new5.pos
+
+    new5.insert_at(1)
+    assert_equal 1, new5.pos
+
+    new4.reload
+    assert_equal 5, new4.pos
+  end
+  
+  def test_delete_middle
+    
+    assert_equal [mixins(:list_sub_1), 
+                  mixins(:list_sub_2), 
+                  mixins(:list_sub_3), 
+                  mixins(:list_sub_4)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+              
+    mixins(:list_sub_2).destroy
+    
+    assert_equal [mixins(:list_sub_1, :reload), 
+                  mixins(:list_sub_3, :reload), 
+                  mixins(:list_sub_4, :reload)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+                  
+    assert_equal 1, mixins(:list_sub_1).pos
+    assert_equal 2, mixins(:list_sub_3).pos
+    assert_equal 3, mixins(:list_sub_4).pos
+
+    mixins(:list_sub_1).destroy
+
+    assert_equal [mixins(:list_sub_3, :reload), 
+                  mixins(:list_sub_4, :reload)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos')
+                  
+    assert_equal 1, mixins(:list_sub_3).pos
+    assert_equal 2, mixins(:list_sub_4).pos
+    
+  end  
+
+end
+