Does not allow Backlog/Done lists to be moved

app/services/boards/lists/move_service.rb

@@ -7,8 +7,11 @@ module Boards
       end
 
       def execute
+        return false unless list.label?
         return false if new_position.blank?
         return false if new_position == old_position
+        return false if new_position == first_position
+        return false if new_position == last_position
 
         list.with_lock do
           reorder_intermediate_lists
@@ -24,6 +27,14 @@ module Boards
         @list ||= board.lists.find(params[:list_id])
       end
 
+      def first_position
+        board.lists.first.try(:position)
+      end
+
+      def last_position
+        board.lists.last.try(:position)
+      end
+
       def old_position
         @old_position ||= list.position
       end
@@ -34,16 +45,24 @@ module Boards
 
       def reorder_intermediate_lists
         if old_position < new_position
-          board.lists.where('position > ?',  old_position)
-                     .where('position <= ?', new_position)
-                     .update_all('position = position - 1')
+          decrement_intermediate_lists
         else
-          board.lists.where('position >= ?', new_position)
-                     .where('position < ?',  old_position)
-                     .update_all('position = position + 1')
+          increment_intermediate_lists
         end
       end
 
+      def decrement_intermediate_lists
+        board.lists.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')
+      end
+
       def update_list_position
         list.update_attribute(:position, new_position)
       end

spec/services/boards/lists/move_service_spec.rb

@@ -4,73 +4,81 @@ describe Boards::Lists::MoveService, services: true do
   describe '#execute' do
     let(:project) { create(:project_with_board) }
     let(:board)   { project.board }
+    let!(:list1)  { create(:backlog_list, board: board, position: 1) }
+    let!(:list2)  { create(:label_list,   board: board, position: 2) }
+    let!(:list3)  { create(:label_list,   board: board, position: 3) }
+    let!(:list4)  { create(:label_list,   board: board, position: 4) }
+    let!(:list5)  { create(:label_list,   board: board, position: 5) }
+    let!(:list6)  { create(:done_list,    board: board, position: 6) }
 
-    it 'keeps position of lists when new position is nil' do
-      list1 = create(:list, board: board, position: 1)
-      list2 = create(:list, board: board, position: 2)
-      list3 = create(:list, board: board, position: 3)
-      list4 = create(:list, board: board, position: 4)
-      list5 = create(:list, board: board, position: 5)
-
-      service = described_class.new(project, { list_id: list2.id, position: nil })
-
-      expect(service.execute).to eq false
-      expect(list1.reload.position).to eq 1
-      expect(list2.reload.position).to eq 2
-      expect(list3.reload.position).to eq 3
-      expect(list4.reload.position).to eq 4
-      expect(list5.reload.position).to eq 5
-    end
+    context 'when list type is set to label' do
+      it 'keeps position of lists when new position is nil' do
+        service = described_class.new(project, { list_id: list2.id, position: nil })
+
+        service.execute
+
+        expect(positions_of_lists).to eq [1, 2, 3, 4, 5, 6]
+      end
+
+      it 'keeps position of lists when new positon is equal to old position' do
+        service = described_class.new(project, { list_id: list2.id, position: 2 })
+
+        service.execute
+
+        expect(positions_of_lists).to eq [1, 2, 3, 4, 5, 6]
+      end
+
+      it 'keeps position of lists when new positon is equal to first position' do
+        service = described_class.new(project, { list_id: list3.id, position: 1 })
+
+        service.execute
+
+        expect(positions_of_lists).to eq [1, 2, 3, 4, 5, 6]
+      end
+
+      it 'keeps position of lists when new positon is equal to last position' do
+        service = described_class.new(project, { list_id: list3.id, position: 6 })
+
+        service.execute
+
+        expect(positions_of_lists).to eq [1, 2, 3, 4, 5, 6]
+      end
 
-    it 'keeps position of lists when new positon is equal to old position' do
-      list1 = create(:list, board: board, position: 1)
-      list2 = create(:list, board: board, position: 2)
-      list3 = create(:list, board: board, position: 3)
-      list4 = create(:list, board: board, position: 4)
-      list5 = create(:list, board: board, position: 5)
-
-      service = described_class.new(project, { list_id: list2.id, position: 2 })
-
-      expect(service.execute).to eq false
-      expect(list1.reload.position).to eq 1
-      expect(list2.reload.position).to eq 2
-      expect(list3.reload.position).to eq 3
-      expect(list4.reload.position).to eq 4
-      expect(list5.reload.position).to eq 5
+      it 'decrements position of intermediate lists when new position is greater than old position' do
+        service = described_class.new(project, { list_id: list2.id, position: 5 })
+
+        service.execute
+
+        expect(positions_of_lists).to eq [1, 5, 2, 3, 4, 6]
+      end
+
+      it 'increments position of intermediate lists when when new position is lower than old position' do
+        service = described_class.new(project, { list_id: list5.id, position: 2 })
+
+        service.execute
+
+        expect(positions_of_lists).to eq [1, 3, 4, 5, 2, 6]
+      end
     end
 
-    it 'decrements position of intermediate lists when new position is greater than old position' do
-      list1 = create(:list, board: board, position: 1)
-      list2 = create(:list, board: board, position: 2)
-      list3 = create(:list, board: board, position: 3)
-      list4 = create(:list, board: board, position: 4)
-      list5 = create(:list, board: board, position: 5)
-
-      service = described_class.new(project, { list_id: list2.id, position: 5 })
-
-      expect(service.execute).to eq true
-      expect(list1.reload.position).to eq 1
-      expect(list2.reload.position).to eq 5
-      expect(list3.reload.position).to eq 2
-      expect(list4.reload.position).to eq 3
-      expect(list5.reload.position).to eq 4
+    it 'keeps position of lists when list type is backlog' do
+      service = described_class.new(project, { list_id: list1.id, position: 2 })
+
+      service.execute
+
+      expect(positions_of_lists).to eq [1, 2, 3, 4, 5, 6]
     end
 
-    it 'increments position of intermediate lists when when new position is lower than old position' do
-      list1 = create(:list, board: board, position: 1)
-      list2 = create(:list, board: board, position: 2)
-      list3 = create(:list, board: board, position: 3)
-      list4 = create(:list, board: board, position: 4)
-      list5 = create(:list, board: board, position: 5)
-
-      service = described_class.new(project, { list_id: list5.id, position: 2 })
-
-      expect(service.execute).to eq true
-      expect(list1.reload.position).to eq 1
-      expect(list2.reload.position).to eq 3
-      expect(list3.reload.position).to eq 4
-      expect(list4.reload.position).to eq 5
-      expect(list5.reload.position).to eq 2
+    it 'keeps position of lists when list type is done' do
+      service = described_class.new(project, { list_id: list6.id, position: 2 })
+
+      service.execute
+
+      expect(positions_of_lists).to eq [1, 2, 3, 4, 5, 6]
     end
   end
+
+  def positions_of_lists
+    (1..6).map { |index| send("list#{index}").reload.position }
+  end
 end