Order board lists by list_type, and position

3 files changed, 17 insertions, 29 deletions

diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb
index 42ab93ad8c4..151bb67bea1 100644
--- a/app/services/boards/lists/create_service.rb
+++ b/app/services/boards/lists/create_service.rb
@@ -19,10 +19,7 @@ module Boards
       attr_reader :board, :params
 
       def find_next_position
-        return 0 unless board.lists.any?
-
-        records = board.lists.where.not(list_type: List.list_types[:done])
-        records.maximum(:position).to_i + 1
+        board.lists.label.maximum(:position).to_i + 1
       end
 
       def create_list_at(position)
@@ -30,8 +27,8 @@ module Boards
       end
 
       def increment_higher_lists(position)
-        board.lists.where('position >= ?', position)
-             .update_all('position = position + 1')
+        board.lists.label.where('position >= ?', position)
+                         .update_all('position = position + 1')
       end
     end
   end
diff --git a/app/services/boards/lists/destroy_service.rb b/app/services/boards/lists/destroy_service.rb
index ba097942aa6..4e0b3012775 100644
--- a/app/services/boards/lists/destroy_service.rb
+++ b/app/services/boards/lists/destroy_service.rb
@@ -10,7 +10,7 @@ module Boards
         return false unless list.label?
 
         list.with_lock do
-          reorder_higher_lists
+          decrement_higher_lists
           remove_list
         end
       end
@@ -23,9 +23,9 @@ module Boards
         @list ||= board.lists.find(params[:list_id])
       end
 
-      def reorder_higher_lists
-        board.lists.where('position > ?',  list.position)
-             .update_all('position = position - 1')
+      def decrement_higher_lists
+        board.lists.label.where('position > ?',  list.position)
+                   .update_all('position = position - 1')
       end
 
       def remove_list
diff --git a/app/services/boards/lists/move_service.rb b/app/services/boards/lists/move_service.rb
index f6705b5afce..0f2cb7f4b8b 100644
--- a/app/services/boards/lists/move_service.rb
+++ b/app/services/boards/lists/move_service.rb
@@ -8,7 +8,7 @@ module Boards
 
       def execute
         return false unless list.label?
-        return false if invalid_position?
+        return false unless valid_move?
 
         list.with_lock do
           reorder_intermediate_lists
@@ -24,18 +24,9 @@ module Boards
         @list ||= board.lists.find(params[:list_id])
       end
 
-      def invalid_position?
-        return true if new_position.blank?
-
-        [old_position, first_position, last_position].include?(new_position)
-      end
-
-      def first_position
-        board.lists.first.try(:position)
-      end
-
-      def last_position
-        board.lists.last.try(:position)
+      def valid_move?
+        new_position.present? && new_position != old_position &&
+          new_position >= 0 && new_position <= board.lists.label.size
       end
 
       def old_position
@@ -55,15 +46,15 @@ module Boards
       end
 
       def decrement_intermediate_lists
-        board.lists.where('position > ?',  old_position)
-             .where('position <= ?', new_position)
-             .update_all('position = position - 1')
+        board.lists.label.where('position > ?',  old_position)
+                         .where('position <= ?', new_position)
+                         .update_all('position = position - 1')
       end
 
       def increment_intermediate_lists
-        board.lists.where('position >= ?', new_position)
-                   .where('position < ?',  old_position)
-                   .update_all('position = position + 1')
+        board.lists.label.where('position >= ?', new_position)
+                         .where('position < ?',  old_position)
+                         .update_all('position = position + 1')
       end
 
       def update_list_position