Merge branch '326461-fix-moving-epic-list' into 'master'

Fix removing labels when moving an epic between lists

See merge request gitlab-org/gitlab!58145

app/services/boards/base_item_move_service.rb

@@ -69,12 +69,16 @@ module Boards
         if moving_to_list.movable?
           moving_from_list.label_id
         else
-          ::Label.ids_on_board(board.id)
+          board_label_ids
         end
 
       Array(label_ids).compact
     end
 
+    def board_label_ids
+      ::Label.ids_on_board(board.id)
+    end
+
     def move_between_ids(move_params)
       ids = [move_params[:move_after_id], move_params[:move_before_id]]
               .map(&:to_i)

ee/app/models/boards/epic_list.rb

@@ -13,6 +13,7 @@ module Boards
     enum list_type: { backlog: 0, label: 1, closed: 2 }
 
     scope :preload_associated_models, -> { preload(:epic_board, label: :priorities) }
+    scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
 
     alias_method :preferences, :epic_list_user_preferences
 

ee/app/models/ee/label.rb

@@ -10,6 +10,15 @@ module EE
     prepended do
       has_many :epic_board_labels, class_name: 'Boards::EpicBoardLabel', inverse_of: :label
       has_many :epic_lists, class_name: 'Boards::EpicList', inverse_of: :label
+
+      scope :on_epic_board, ->(epic_board_id) do
+        joins(epic_lists: :epic_board).merge(::Boards::EpicList.movable)
+          .where(boards_epic_boards: { id: epic_board_id })
+      end
+
+      def self.ids_on_epic_board(epic_board_id)
+        on_epic_board(epic_board_id).pluck(:label_id)
+      end
     end
 
     def scoped_label?

ee/app/services/boards/epics/move_service.rb

@@ -3,6 +3,8 @@
 module Boards
   module Epics
     class MoveService < Boards::BaseItemMoveService
+      extend ::Gitlab::Utils::Override
+
       private
 
       def update(epic, epic_modification_params)
@@ -13,6 +15,11 @@ module Boards
         @board ||= parent.epic_boards.find(params[:board_id])
       end
 
+      override :board_label_ids
+      def board_label_ids
+        ::Label.ids_on_epic_board(board.id)
+      end
+
       def reposition_parent
         { board_id: board.id }
       end

ee/changelogs/unreleased/326461-fix-moving-epic-list.yml

+---
+title: Fix removing labels when moving an epic between lists
+merge_request: 58145
+author:
+type: fixed

ee/spec/models/ee/label_spec.rb

@@ -3,11 +3,35 @@
 require 'spec_helper'
 
 RSpec.describe Label do
+  let_it_be(:group) { create(:group) }
+
+  let_it_be(:development) { create(:group_label, group: group, name: 'Development') }
+  let_it_be(:testing) { create(:group_label, group: group, name: 'Testing') }
+  let_it_be(:no_board_label) { create(:group_label, group: group, name: 'Feature') }
+
+  let_it_be(:board) { create(:epic_board, group: group) }
+  let_it_be(:list1) { create(:epic_list, epic_board: board, label: development) }
+  let_it_be(:list2) { create(:epic_list, epic_board: board, label: testing) }
+
   describe 'associations' do
     it { is_expected.to have_many(:epic_board_labels).inverse_of(:label) }
     it { is_expected.to have_many(:epic_lists).inverse_of(:label) }
   end
 
+  describe 'scopes' do
+    describe '.on_epic_board' do
+      it 'returns only the board labels' do
+        expect(described_class.on_epic_board(board.id)).to match_array([development, testing])
+      end
+    end
+  end
+
+  describe '#ids_on_epic_board' do
+    it 'returns only the board label ids' do
+      expect(described_class.ids_on_epic_board(board.id)).to match_array([development.id, testing.id])
+    end
+  end
+
   describe '#scoped_label?' do
     context 'with scoped_labels available' do
       before do

ee/spec/services/boards/epics/move_service_spec.rb

@@ -11,10 +11,11 @@ RSpec.describe Boards::Epics::MoveService do
 
     let_it_be(:development) { create(:group_label, group: group, name: 'Development') }
     let_it_be(:testing) { create(:group_label, group: group, name: 'Testing') }
+    let_it_be(:no_board_label) { create(:group_label, group: group, name: 'Feature') }
 
     let_it_be(:backlog) { create(:epic_list, epic_board: board, list_type: :backlog, label: nil) }
-    let_it_be(:list1) { create(:epic_list, epic_board: board, label: development, position: 0) }
-    let_it_be(:list2) { create(:epic_list, epic_board: board, label: testing, position: 1) }
+    let_it_be(:development_list) { create(:epic_list, epic_board: board, label: development, position: 0) }
+    let_it_be(:testing_list) { create(:epic_list, epic_board: board, label: testing, position: 1) }
     let_it_be(:closed) { create(:epic_list, epic_board: board, list_type: :closed, label: nil) }
     let_it_be(:other_board_list) { create(:epic_list, epic_board: other_board, list_type: :closed, label: nil) }
 
@@ -74,7 +75,7 @@ RSpec.describe Boards::Epics::MoveService do
       context 'when moving an epic between lists' do
         context 'when moving the epic from backlog' do
           context 'to a labeled list' do
-            let(:to_list) { list1 }
+            let(:to_list) { development_list }
 
             it 'keeps the epic opened and adds the labels' do
               expect { subject }.not_to change { epic.state }
@@ -100,16 +101,18 @@ RSpec.describe Boards::Epics::MoveService do
 
         context 'when moving the epic from a labeled list' do
           before do
-            epic.labels = [development]
+            epic.labels = [development, no_board_label]
           end
 
-          let(:from_list) { list1 }
+          let(:from_list) { development_list }
 
           context 'to another labeled list' do
-            let(:to_list) { list2 }
+            let(:to_list) { testing_list }
 
             it 'changes the labels' do
-              expect { subject }.to change { epic.reload.labels }.from([development]).to([testing])
+              subject
+
+              expect(epic.labels).to match_array([testing, no_board_label])
             end
           end
 
@@ -119,6 +122,12 @@ RSpec.describe Boards::Epics::MoveService do
             it 'closes the epic' do
               expect { subject }.to change { epic.state }.from('opened').to('closed')
             end
+
+            it 'removes the board labels from the epic' do
+              subject
+
+              expect(epic.labels).to eq([no_board_label])
+            end
           end
         end
       end

spec/models/label_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Label do
+  let_it_be(:project) { create(:project) }
+
   describe 'modules' do
     it { is_expected.to include_module(Referable) }
     it { is_expected.to include_module(Subscribable) }
@@ -44,6 +46,22 @@ RSpec.describe Label do
     end
   end
 
+  describe 'scopes' do
+    describe '.on_board' do
+      let(:board) { create(:board, project: project) }
+      let!(:list1)   { create(:list, board: board, label: development) }
+      let!(:list2)   { create(:list, board: board, label: testing) }
+
+      let!(:development) { create(:label, project: project, name: 'Development') }
+      let!(:testing) { create(:label, project: project, name: 'Testing') }
+      let!(:regression) { create(:label, project: project, name: 'Regression') }
+
+      it 'returns only the board labels' do
+        expect(described_class.on_board(board.id)).to match_array([development, testing])
+      end
+    end
+  end
+
   describe '#color' do
     it 'strips color' do
       label = described_class.new(color: '   #abcdef   ')
@@ -92,9 +110,7 @@ RSpec.describe Label do
   end
 
   describe 'priorization' do
-    subject(:label) { create(:label) }
-
-    let(:project) { label.project }
+    subject(:label) { create(:label, project: project) }
 
     describe '#prioritize!' do
       context 'when label is not prioritized' do