1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84

Index: test/mixin_test.rb
===================================================================
--- test/mixin_test.rb  (revision 7813)
+++ test/mixin_test.rb  (working copy)
@@ -210,7 +210,54 @@
     new2.move_higher
     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
Index: lib/active_record/acts/list.rb
===================================================================
--- lib/active_record/acts/list.rb  (revision 7813)
+++ lib/active_record/acts/list.rb  (working copy)
@@ -63,7 +63,7 @@
             
             #{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 @@
         
         # 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.