Skip to content
Browse files

Make sure acts_as_list's remove_from_list and in_list? play nicely wi…

…th one another. Closes #8822 [mikel]

git-svn-id: http://svn-commit.rubyonrails.org/rails/branches/1-2-stable@7815 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 5e281e9 commit 1d5d050c70e27c05ef96d5eadcdffab465b89a0b @NZKoz NZKoz committed Oct 9, 2007
Showing with 52 additions and 2 deletions.
  1. +5 −2 activerecord/lib/active_record/acts/list.rb
  2. +47 −0 activerecord/test/mixin_test.rb
View
7 activerecord/lib/active_record/acts/list.rb
@@ -63,7 +63,7 @@ def position_column
#{scope_condition_method}
- after_destroy :remove_from_list
+ before_destroy :remove_from_list
before_create :add_to_list_bottom
EOV
end
@@ -121,7 +121,10 @@ def move_to_top
# Removes the item from the list.
def remove_from_list
- decrement_positions_on_lower_items if in_list?
+ if in_list?
+ decrement_positions_on_lower_items
+ update_attribute position_column, nil
+ end
end
# Increase the position of this item without adjusting the rest of the list.
View
47 activerecord/test/mixin_test.rb
@@ -211,6 +211,53 @@ def test_nil_scope
assert_equal [new2, new1, new3], ListMixin.find(:all, :conditions => 'parent_id IS NULL', :order => 'pos')
end
+ def test_remove_from_list_should_then_fail_in_list?
+ assert_equal true, mixins(:list_1).in_list?
+ mixins(:list_1).remove_from_list
+ assert_equal false, mixins(:list_1).in_list?
+ end
+
+ def test_remove_from_list_should_set_position_to_nil
+ 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_2).remove_from_list
+
+ assert_equal [mixins(:list_2, :reload),
+ mixins(:list_1, :reload),
+ mixins(:list_3, :reload),
+ mixins(:list_4, :reload)],
+ ListMixin.find(:all, :conditions => 'parent_id = 5', :order => 'pos')
+
+ assert_equal 1, mixins(:list_1).pos
+ assert_equal nil, mixins(:list_2).pos
+ assert_equal 2, mixins(:list_3).pos
+ assert_equal 3, mixins(:list_4).pos
+ end
+
+ def test_remove_before_destroy_does_not_shift_lower_items_twice
+ 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_2).remove_from_list
+ mixins(:list_2).destroy
+
+ assert_equal [mixins(:list_1, :reload),
+ mixins(:list_3, :reload),
+ mixins(:list_4, :reload)],
+ ListMixin.find(:all, :conditions => 'parent_id = 5', :order => 'pos')
+
+ assert_equal 1, mixins(:list_1).pos
+ assert_equal 2, mixins(:list_3).pos
+ assert_equal 3, mixins(:list_4).pos
+ end
+
end
class TreeTest < Test::Unit::TestCase

0 comments on commit 1d5d050

Please sign in to comment.
Something went wrong with that request. Please try again.