Added higher_item and lower_item as public methods for acts_as_list #342 [Tobias Luetke]

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

9 files changed, 200 insertions, 81 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 573fa9f618..166bc476da 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Added higher_item and lower_item as public methods for acts_as_list #342 [Tobias Luetke]
+
 * Added respondence to *_before_type_cast for all attributes to return their string-state before they were type casted by the column type.
   This is helpful for getting "100,000" back on a integer-based validation where the value would normally be "100".
 
diff --git a/activerecord/lib/active_record/acts/list.rb b/activerecord/lib/active_record/acts/list.rb
index 626547f005..e8f7f64db1 100644
--- a/activerecord/lib/active_record/acts/list.rb
+++ b/activerecord/lib/active_record/acts/list.rb
@@ -109,6 +109,18 @@ module ActiveRecord
         def last?
           self.send(position_column) == bottom_position_in_list
         end
+        
+        def higher_item
+          self.class.find_first(
+            "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i - 1).to_s}"
+          )
+        end
+
+        def lower_item
+          self.class.find_first(
+            "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i + 1).to_s}"
+          )
+        end
 
         private
         
@@ -123,18 +135,6 @@ module ActiveRecord
         # Overwrite this method to define the scope of the list changes
         def scope_condition() "1" end
 
-        def higher_item
-          self.class.find_first(
-            "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i - 1).to_s}"
-          )
-        end
-
-        def lower_item
-          self.class.find_first(
-            "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i + 1).to_s}"
-          )
-        end
-
         def bottom_position_in_list
           item = bottom_item
           item ? item.send(position_column) : 0
@@ -163,7 +163,7 @@ module ActiveRecord
   
         def increment_positions_on_higher_items
           self.class.update_all(
-            "#{position_column} = (#{position_column} + 1)",  "#{scope_condition} AND #{position_column} < #{send(position_column)}"
+            "#{position_column} = (#{position_column} + 1)",  "#{scope_condition} AND #{position_column} < #{send(position_column).to_i}"
           )
         end
 
diff --git a/activerecord/test/fixtures/db_definitions/mysql.sql b/activerecord/test/fixtures/db_definitions/mysql.sql
index 137fbaa73e..7bd06e6243 100755
--- a/activerecord/test/fixtures/db_definitions/mysql.sql
+++ b/activerecord/test/fixtures/db_definitions/mysql.sql
@@ -102,10 +102,11 @@ CREATE TABLE `mixins` (
   `id` int(11) NOT NULL auto_increment,
   `parent_id` int(11) default NULL,
   `pos` int(11) default NULL,
-  `lft` int(11) default NULL,
-  `rgt` int(11) default NULL,
-  `root_id` int(11) default NULL,  
   `created_at` datetime default NULL,
   `updated_at` datetime default NULL,
+  `lft` int(11) default NULL,
+  `rgt` int(11) default NULL,
+  `root_id` int(11) default NULL,
+  `type` varchar(40) default NULL,
   PRIMARY KEY  (`id`)
 );
diff --git a/activerecord/test/fixtures/db_definitions/postgresql.sql b/activerecord/test/fixtures/db_definitions/postgresql.sql
index 4ca0e9ddc3..305852ba7f 100644
--- a/activerecord/test/fixtures/db_definitions/postgresql.sql
+++ b/activerecord/test/fixtures/db_definitions/postgresql.sql
@@ -118,6 +118,7 @@ CREATE TABLE colnametests (
 CREATE TABLE mixins (
   id serial,
   parent_id integer,
+  type character varying,  
   pos integer,
   lft integer,
   rgt integer,
diff --git a/activerecord/test/fixtures/db_definitions/sqlite.sql b/activerecord/test/fixtures/db_definitions/sqlite.sql
index 832a1a7d31..1e9aed1e1b 100644
--- a/activerecord/test/fixtures/db_definitions/sqlite.sql
+++ b/activerecord/test/fixtures/db_definitions/sqlite.sql
@@ -90,6 +90,7 @@ CREATE TABLE 'colnametests' (
 CREATE TABLE 'mixins' (
   'id' INTEGER NOT NULL PRIMARY KEY,
   'parent_id' INTEGER DEFAULT NULL,
+  'type' VARCHAR(40) DEFAULT NULL,  
   'pos' INTEGER DEFAULT NULL,
   'lft' INTEGER DEFAULT NULL,
   'rgt' INTEGER DEFAULT NULL,
diff --git a/activerecord/test/fixtures/db_definitions/sqlserver.sql b/activerecord/test/fixtures/db_definitions/sqlserver.sql
index 0cc8899945..4152ef59cb 100644
--- a/activerecord/test/fixtures/db_definitions/sqlserver.sql
+++ b/activerecord/test/fixtures/db_definitions/sqlserver.sql
@@ -99,6 +99,7 @@ CREATE TABLE colnametests (
 CREATE TABLE mixins (
   id int NOT NULL IDENTITY(1, 1),
   parent_id int default NULL,
+  type varchar(40) default NULL,  
   pos int default NULL,
   lft int default NULL,
   rgt int default NULL,
diff --git a/activerecord/test/fixtures/mixin.rb b/activerecord/test/fixtures/mixin.rb
index 1b6099901f..b14b0f3132 100644
--- a/activerecord/test/fixtures/mixin.rb
+++ b/activerecord/test/fixtures/mixin.rb
@@ -1,9 +1,12 @@
 class Mixin < ActiveRecord::Base
-  acts_as_tree :foreign_key => "parent_id", :order => "id"
 
 end
 
-class ListMixin < ActiveRecord::Base
+class TreeMixin < Mixin 
+    acts_as_tree :foreign_key => "parent_id", :order => "id"
+end
+
+class ListMixin < Mixin
   acts_as_list :column => "pos", :scope => :parent
 
   def self.table_name() "mixins" end
diff --git a/activerecord/test/fixtures/mixins.yml b/activerecord/test/fixtures/mixins.yml
index c74b1da51b..e326f6675e 100644
--- a/activerecord/test/fixtures/mixins.yml
+++ b/activerecord/test/fixtures/mixins.yml
@@ -1,14 +1,30 @@
-first:
+# tree mixins
+tree_1:
   id: 1
-  pos: 1
+  type: TreeMixin
   parent_id: 0 
   
-second:
+tree_2:
   id: 2
-  pos: 1
+  type: TreeMixin
   parent_id: 1
 
-third:
+tree_3:
   id: 3
-  pos: 2
-  parent_id: 1
\ No newline at end of file
+  type: TreeMixin  
+  parent_id: 2
+
+tree_4:
+  id: 4
+  type: TreeMixin  
+  parent_id: 1
+  
+# List mixins
+
+<% (1..4).each do |counter| %>  
+list_<%= counter %>:
+  id: <%= counter+6 %>
+  pos: <%= counter  %>
+  type: ListMixin
+  parent_id: 5
+<% end %>
\ No newline at end of file
diff --git a/activerecord/test/mixin_test.rb b/activerecord/test/mixin_test.rb
index 3caa5a3813..01b16cc631 100644
--- a/activerecord/test/mixin_test.rb
+++ b/activerecord/test/mixin_test.rb
@@ -3,72 +3,136 @@ require 'active_record/acts/tree'
 require 'active_record/acts/list'
 require 'fixtures/mixin'
 
-
 class ListTest < Test::Unit::TestCase
   fixtures :mixins
   
-  def test_injection
-    l = ListMixin.new("parent_id"=>1)
-    assert_equal "parent_id = 1", l.scope_condition
-    assert_equal "pos", l.position_column
-
-  end
-  
   def test_reordering
     
-    assert_equal [ListMixin.find(2), ListMixin.find(3)], ListMixin.find_all("parent_id=1", "pos")
-    
-    ListMixin.find(2).move_lower
-    
-    assert_equal [ListMixin.find(3), ListMixin.find(2)], ListMixin.find_all("parent_id=1", "pos")
-    
-    ListMixin.find(2).move_higher
+    assert_equal [@mixins['list_1'].find, 
+                  @mixins['list_2'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_4'].find], 
+                  ListMixin.find_all("parent_id=5", "pos")
+                  
+    @mixins['list_2'].find.move_lower
+                  
+    assert_equal [@mixins['list_1'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_2'].find, 
+                  @mixins['list_4'].find], 
+                  ListMixin.find_all("parent_id=5", "pos")                      
+                  
+    @mixins['list_2'].find.move_higher
 
-    assert_equal [ListMixin.find(2), ListMixin.find(3)], ListMixin.find_all("parent_id=1", "pos")
+    assert_equal [@mixins['list_1'].find, 
+                  @mixins['list_2'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_4'].find], 
+                  ListMixin.find_all("parent_id=5", "pos")
     
-  end
+    @mixins['list_1'].find.move_to_bottom
+
+    assert_equal [@mixins['list_2'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_4'].find, 
+                  @mixins['list_1'].find], 
+                  ListMixin.find_all("parent_id=5", "pos")
+
+    @mixins['list_1'].find.move_to_top
 
+    assert_equal [@mixins['list_1'].find, 
+                  @mixins['list_2'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_4'].find],
+                  ListMixin.find_all("parent_id=5", "pos")
+                  
+                  
+    @mixins['list_2'].find.move_to_bottom
+  
+    assert_equal [@mixins['list_1'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_4'].find, 
+                  @mixins['list_2'].find],
+                  ListMixin.find_all("parent_id=5", "pos")                  
+
+    @mixins['list_4'].find.move_to_top
+
+    assert_equal [@mixins['list_4'].find, 
+                  @mixins['list_1'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_2'].find],
+                  ListMixin.find_all("parent_id=5", "pos")                  
+        
+  end
+  
+  def test_next_prev
+    assert_equal @list_2, @list_1.lower_item
+    assert_nil @list_1.higher_item
+    assert_equal @list_3, @list_4.higher_item
+    assert_nil @list_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
-    new = ListMixin.create("parent_id"=>5)
+    new = ListMixin.create("parent_id"=>20)
     assert_equal 1, new.pos
     assert new.first?
     assert new.last?
 
-    new = ListMixin.create("parent_id"=>5)
+    new = ListMixin.create("parent_id"=>20)
     assert_equal 2, new.pos
     assert !new.first?
     assert new.last?
     
-    new = ListMixin.create("parent_id"=>5)
+    new = ListMixin.create("parent_id"=>20)
     assert_equal 3, new.pos    
     assert !new.first?
     assert new.last?
     
-    new = ListMixin.create("parent_id"=>10)
+    new = ListMixin.create("parent_id"=>0)
     assert_equal 1, new.pos
     assert new.first?
     assert new.last?
   end    
   
   def test_delete_middle
-    first = ListMixin.create("parent_id"=>5)
-    assert_equal 1, first.pos
-
-    second = ListMixin.create("parent_id"=>5)
-    assert_equal 2, second.pos
-    
-    third = ListMixin.create("parent_id"=>5)
-    assert_equal 3, third.pos    
     
-    second.destroy
+    assert_equal [@mixins['list_1'].find, 
+                  @mixins['list_2'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_4'].find], 
+                  ListMixin.find_all("parent_id=5", "pos")
+              
+    @mixins['list_2'].find.destroy
     
-    assert_equal 1, @mixins["first"].find.pos
-    assert_equal 2, @mixins["third"].find.pos
+    assert_equal [@mixins['list_1'].find, 
+                  @mixins['list_3'].find, 
+                  @mixins['list_4'].find], 
+                  ListMixin.find_all("parent_id=5", "pos")
+                  
+    assert_equal 1, @mixins['list_1'].find.pos
+    assert_equal 2, @mixins['list_3'].find.pos
+    assert_equal 3, @mixins['list_4'].find.pos
+
+    @mixins['list_1'].find.destroy
+
+    assert_equal [@mixins['list_3'].find, 
+                  @mixins['list_4'].find], 
+                  ListMixin.find_all("parent_id=5", "pos")
+                  
+    assert_equal 1, @mixins['list_3'].find.pos
+    assert_equal 2, @mixins['list_4'].find.pos
     
   end  
 
   def test_with_string_based_scope
-    new = ListWithStringScopeMixin.create("parent_id"=>5)
+    new = ListWithStringScopeMixin.create("parent_id"=>500)
     assert_equal 1, new.pos
     assert new.first?
     assert new.last?
@@ -79,59 +143,89 @@ class TreeTest < Test::Unit::TestCase
   fixtures :mixins
   
   def test_has_child
-    assert_equal true, @first.has_children?
-    assert_equal false, @second.has_children?
-    assert_equal false, @third.has_children?
+    assert_equal true, @tree_1.has_children?
+    assert_equal true, @tree_2.has_children?
+    assert_equal false, @tree_3.has_children?
+    assert_equal false, @tree_4.has_children?
   end
 
   def test_children
-    assert_equal @first.children, [@second, @third]
-    assert_equal @second.children, []
+    assert_equal @tree_1.children, [@tree_2, @tree_4]
+    assert_equal @tree_2.children, [@tree_3]
+    assert_equal @tree_3.children, []
+    assert_equal @tree_4.children, []
   end
 
   def test_parent
-    assert_equal @second.parent, @first
-    assert_equal @second.parent, @third.parent
-    assert_nil @first.parent
+    assert_equal @tree_2.parent, @tree_1
+    assert_equal @tree_2.parent, @tree_4.parent
+    assert_nil @tree_1.parent
+  end
+  
+  def test_delete
+    assert_equal 4, TreeMixin.count
+    @tree_1.destroy
+    assert_equal 0, TreeMixin.count
   end
   
   def test_insert
-    @extra = @first.children.create
+    @extra = @tree_1.children.create
     
     assert @extra
     
-    assert_equal @extra.parent, @first
-    assert_equal [@second, @third, @extra], @first.children
+    assert_equal @extra.parent, @tree_1
+    assert_equal [@tree_2, @tree_4, @extra], @tree_1.children
   end
   
-  def test_delete
-    assert_equal 3, Mixin.count
-    @first.destroy
-    assert_equal 0, Mixin.count
-  end
+
 end
 
 class TouchTest < Test::Unit::TestCase
   fixtures :mixins
   
   def test_update
-    assert_nil @first.updated_at
-    @first.save
-    assert_not_nil @first.updated_at
+    
+    stamped = Mixin.new 
+      
+    assert_nil stamped.updated_at
+    assert_nil stamped.created_at
+    stamped.save
+    assert_not_nil stamped.updated_at
+    assert_not_nil stamped.created_at
   end  
 
   def test_create
-    @obj = Mixin.create({"parent" => @third})
+    @obj = Mixin.create
     assert_not_nil @obj.updated_at
     assert_not_nil @obj.created_at
   end  
+
+  def test_many_updates
+
+    stamped = Mixin.new 
+
+    assert_nil stamped.updated_at
+    assert_nil stamped.created_at
+    stamped.save
+    assert_not_nil stamped.created_at
+    assert_not_nil stamped.updated_at
+    
+    old_updated_at = stamped.updated_at
+
+    sleep 1
+    stamped.save    
+    assert_not_equal stamped.created_at, stamped.updated_at
+    assert_not_equal old_updated_at, stamped.updated_at
+
+  end
+
   
   def test_create_turned_off
     Mixin.record_timestamps = false
 
-    assert_nil @first.updated_at
-    @first.save
-    assert_nil @first.updated_at
+    assert_nil @tree_1.updated_at
+    @tree_1.save
+    assert_nil @tree_1.updated_at
 
     Mixin.record_timestamps = true
   end