Respect Backlog/Done positions when creating a new list

2 files changed, 57 insertions, 11 deletions

diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb
index c6b74148ee1..42ab93ad8c4 100644
--- a/app/services/boards/lists/create_service.rb
+++ b/app/services/boards/lists/create_service.rb
@@ -7,15 +7,31 @@ module Boards
       end
 
       def execute
-        board.lists.create(params.merge(position: position))
+        List.transaction do
+          position = find_next_position
+          increment_higher_lists(position)
+          create_list_at(position)
+        end
       end
 
       private
 
       attr_reader :board, :params
 
-      def position
-        board.lists.size
+      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
+      end
+
+      def create_list_at(position)
+        board.lists.create(params.merge(list_type: :label, position: position))
+      end
+
+      def increment_higher_lists(position)
+        board.lists.where('position >= ?', position)
+             .update_all('position = position + 1')
       end
     end
   end
diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb
index f173b0e60fd..09cef364a3b 100644
--- a/spec/services/boards/lists/create_service_spec.rb
+++ b/spec/services/boards/lists/create_service_spec.rb
@@ -6,19 +6,49 @@ describe Boards::Lists::CreateService, services: true do
     let(:board)   { project.board }
     let(:label)   { create(:label, name: 'in-progress') }
 
-    it 'creates a new list for board' do
-      service = described_class.new(project, label_id: label.id)
+    subject(:service) { described_class.new(project, label_id: label.id) }
 
-      expect { service.execute }.to change(board.lists, :count).by(1)
+    context 'when board lists is empty' do
+      it 'creates a new list at begginning of the list' do
+        list = service.execute
+
+        expect(list.position).to eq 0
+      end
+    end
+
+    context 'when board lists has a backlog list' do
+      it 'creates a new list at end of the list' do
+        create(:backlog_list, board: board, position: 0)
+
+        list = service.execute
+
+        expect(list.position).to eq 1
+      end
+    end
+
+    context 'when board lists are only labels lists' do
+      it 'creates a new list at end of the list' do
+        create_list(:label_list, 2, board: board)
+
+        list = described_class.new(project, label_id: label.id).execute
+
+        expect(list.position).to eq 3
+      end
     end
 
-    it 'inserts the list to the end of lists' do
-      create_list(:list, 2, board: board)
-      service = described_class.new(project, label_id: label.id)
+    context 'when board lists has a done list' do
+      it 'creates a new list before' do
+        list1 = create(:backlog_list, board: board, position: 1)
+        list2 = create(:label_list, board: board, position: 2)
+        list3 = create(:done_list, board: board, position: 3)
 
-      list = service.execute
+        list = described_class.new(project, label_id: label.id).execute
 
-      expect(list.position).to eq 2
+        expect(list.position).to eq 3
+        expect(list1.reload.position).to eq 1
+        expect(list2.reload.position).to eq 2
+        expect(list3.reload.position).to eq 4
+      end
     end
   end
 end