Fix acts_as_list such that moving next-to-last item to the bottom does not result in duplicate positions

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

3 files changed, 25 insertions, 5 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 1eaa5517d0..018972d954 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fix acts_as_list so that moving next-to-last item to the bottom does not result in duplicate item positions
+
 * Fixed incompatibility in DB2 adapter with the new limit/offset approach #1718 [Maik Schmidt]
 
 * Added :select option to find which can specify a different value than the default *, like find(:all, :select => "first_name, last_name"), if you either only want to select part of the columns or exclude columns otherwise included from a join #1338 [Stefan Kaes]
diff --git a/activerecord/lib/active_record/acts/list.rb b/activerecord/lib/active_record/acts/list.rb
index 29b5a0a58e..f3cb3496d8 100644
--- a/activerecord/lib/active_record/acts/list.rb
+++ b/activerecord/lib/active_record/acts/list.rb
@@ -163,17 +163,19 @@ module ActiveRecord
           # Overwrite this method to define the scope of the list changes
           def scope_condition() "1" end
 
-          def bottom_position_in_list
-            item = bottom_item
+          def bottom_position_in_list(except = nil)
+            item = bottom_item(except)
             item ? item.send(position_column) : 0
           end
 
-          def bottom_item
-            self.class.find(:first, :conditions => scope_condition, :order => "#{position_column} DESC")
+          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")
           end
 
           def assume_bottom_position
-            update_attribute(position_column, bottom_position_in_list.to_i + 1) unless last?
+            update_attribute(position_column, bottom_position_in_list(self).to_i + 1)
           end
   
           def assume_top_position
diff --git a/activerecord/test/mixin_test.rb b/activerecord/test/mixin_test.rb
index 96e6f7442f..df03bcd9a7 100644
--- a/activerecord/test/mixin_test.rb
+++ b/activerecord/test/mixin_test.rb
@@ -65,6 +65,22 @@ class ListTest < Test::Unit::TestCase
                   ListMixin.find(:all, :conditions => 'parent_id = 5', :order => 'pos')
         
   end
+
+  def test_move_to_bottom_with_next_to_last_item
+    assert_equal [mixins(:list_1), 
+                  mixins(:list_2), 
+                  mixins(:list_3), 
+                  mixins(:list_4)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5', :order => 'pos')
+
+    mixins(:list_3).move_to_bottom
+
+    assert_equal [mixins(:list_1), 
+                  mixins(:list_2), 
+                  mixins(:list_4), 
+                  mixins(:list_3)], 
+                  ListMixin.find(:all, :conditions => 'parent_id = 5', :order => 'pos')
+  end
   
   def test_next_prev
     assert_equal mixins(:list_2), mixins(:list_1).lower_item