Order board lists by list_type, and position

6 files changed, 24 insertions, 36 deletions

diff --git a/app/models/board.rb b/app/models/board.rb
index d44df497e78..d6358fb15e8 100644
--- a/app/models/board.rb
+++ b/app/models/board.rb
@@ -1,7 +1,7 @@
 class Board < ActiveRecord::Base
   belongs_to :project
 
-  has_many :lists, dependent: :destroy
+  has_many :lists, -> { order(:list_type, :position) }, dependent: :destroy
 
   validates :project, presence: true
 end
diff --git a/app/models/list.rb b/app/models/list.rb
index 7b347c2255b..38cf2050527 100644
--- a/app/models/list.rb
+++ b/app/models/list.rb
@@ -2,9 +2,9 @@ class List < ActiveRecord::Base
   belongs_to :board
   belongs_to :label
 
-  enum list_type: { label: 0, backlog: 1, done: 2 }
+  enum list_type: { backlog: 0, label: 1, done: 2 }
 
-  validates :board, :list_type, :position, presence: true
-  validates :label, presence: true, if: :label?
-  validates :position, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
+  validates :board, :list_type, presence: true
+  validates :label, :position, presence: true, if: :label?
+  validates :position, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, if: :label?
 end
diff --git a/app/services/boards/create_service.rb b/app/services/boards/create_service.rb
index 44e937af8c6..f1eef870925 100644
--- a/app/services/boards/create_service.rb
+++ b/app/services/boards/create_service.rb
@@ -15,8 +15,8 @@ module Boards
 
     def create_board!
       project.create_board
-      project.board.lists.create(list_type: :backlog, position: 0)
-      project.board.lists.create(list_type: :done, position: 1)
+      project.board.lists.create(list_type: :backlog)
+      project.board.lists.create(list_type: :done)
     end
   end
 end
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