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