Fix position when no serial positions

lib/acts_as_list/active_record/acts/list.rb

@@ -174,8 +174,12 @@ def move_lower
           return unless lower_item
 
           acts_as_list_class.transaction do
-            lower_item.decrement_position
-            increment_position
+            if lower_item.send(position_column) != self.send(position_column)
+              swap_positions(lower_item, self)
+            else
+              lower_item.decrement_position
+              increment_position
+            end
           end
         end
 
@@ -184,8 +188,12 @@ def move_higher
           return unless higher_item
 
           acts_as_list_class.transaction do
-            higher_item.increment_position
-            decrement_position
+            if higher_item.send(position_column) != self.send(position_column)
+              swap_positions(higher_item, self)
+            else
+              higher_item.increment_position
+              decrement_position
+            end
           end
         end
 
@@ -236,16 +244,14 @@ def decrement_position
           set_list_position(self.send(position_column).to_i - 1)
         end
 
-        # Return +true+ if this object is the first in the list.
         def first?
           return false unless in_list?
-          self.send(position_column) == acts_as_list_top
+          !higher_item
         end
 
-        # Return +true+ if this object is the last in the list.
         def last?
           return false unless in_list?
-          self.send(position_column) == bottom_position_in_list
+          !lower_item
         end
 
         # Return the next higher item in the list.
@@ -261,9 +267,8 @@ def higher_items(limit=nil)
           position_value = send(position_column)
           acts_as_list_list.
             where("#{quoted_position_column_with_table_name} < ?", position_value).
-            where("#{quoted_position_column_with_table_name} >= ?", position_value - limit).
-            limit(limit).
-            order("#{quoted_position_column_with_table_name} ASC")
+            order("#{quoted_position_column_with_table_name} DESC").
+            limit(limit)
         end
 
         # Return the next lower item in the list.
@@ -279,9 +284,8 @@ def lower_items(limit=nil)
           position_value = send(position_column)
           acts_as_list_list.
             where("#{quoted_position_column_with_table_name} > ?", position_value).
-            where("#{quoted_position_column_with_table_name} <= ?", position_value + limit).
-            limit(limit).
-            order("#{quoted_position_column_with_table_name} ASC")
+            order("#{quoted_position_column_with_table_name} ASC").
+            limit(limit)
         end
 
         # Test if this record is in a list
@@ -308,6 +312,12 @@ def set_list_position(new_position)
         end
 
         private
+
+          def swap_positions(item1, item2)
+            item1.set_list_position(item2.send(position_column))
+            item2.set_list_position(item1.send("#{position_column}_was"))
+          end
+
           def acts_as_list_list
             acts_as_list_class.unscoped do
               acts_as_list_class.where(scope_condition)

test/shared_list_sub.rb

@@ -43,6 +43,32 @@ def test_next_prev
       assert_nil ListMixin.where(id: 4).first.lower_item
     end
 
+    def test_next_prev_not_regular_sequence
+      ListMixin.all.each do |item|
+        item.update_attributes(pos: item.pos * 5)
+      end
+
+      assert_equal [1, 2, 3, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id)
+
+      ListMixin.where(id: 2).first.move_lower
+      assert_equal [1, 3, 2, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id)
+
+      ListMixin.where(id: 2).first.move_higher
+      assert_equal [1, 2, 3, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id)
+
+      ListMixin.where(id: 1).first.move_to_bottom
+      assert_equal [2, 3, 4, 1], ListMixin.where(parent_id: 5000).order('pos').map(&:id)
+
+      ListMixin.where(id: 1).first.move_to_top
+      assert_equal [1, 2, 3, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id)
+
+      ListMixin.where(id: 2).first.move_to_bottom
+      assert_equal [1, 3, 4, 2], ListMixin.where(parent_id: 5000).order('pos').map(&:id)
+
+      ListMixin.where(id: 4).first.move_to_top
+      assert_equal [4, 1, 3, 2], ListMixin.where(parent_id: 5000).order('pos').map(&:id)
+    end
+
     def test_next_prev_groups
       li1 = ListMixin.where(id: 1).first
       li2 = ListMixin.where(id: 2).first
@@ -53,9 +79,9 @@ def test_next_prev_groups
       assert_equal [li2, li3], li1.lower_items(2)
       assert_equal [], li4.lower_items
 
-      assert_equal [li1, li2], li3.higher_items
+      assert_equal [li2, li1], li3.higher_items
       assert_equal [li1], li2.higher_items
-      assert_equal [li2, li3], li4.higher_items(2)
+      assert_equal [li3, li2], li4.higher_items(2)
       assert_equal [], li1.higher_items
     end
 

test/test_joined_list.rb

@@ -51,7 +51,7 @@ def test_higher_items
     item1 = Item.create section: section
     item2 = Item.create section: section
     item3 = Item.create section: section
-    assert_equal item3.higher_items.visible, [item1, item2]
+    assert_equal item3.higher_items.visible, [item2, item1]
   end
 
   def test_lower_items