Merge branch 'issue-be_331613' into 'master'

Return error message when moving issue to not authorized board assignee list

See merge request gitlab-org/gitlab!66101

app/graphql/mutations/boards/issues/issue_move_list.rb

@@ -56,11 +56,11 @@ module Mutations
           issue = authorized_find!(project_path: project_path, iid: iid)
           move_params = { id: issue.id, board_id: board.id }.merge(move_arguments(args))
 
-          move_issue(board, issue, move_params)
+          result = move_issue(board, issue, move_params)
 
           {
             issue: issue.reset,
-            errors: issue.errors.full_messages
+            errors: error_for(result)
           }
         end
 
@@ -79,6 +79,12 @@ module Mutations
         def move_arguments(args)
           args.slice(:from_list_id, :to_list_id, :move_after_id, :move_before_id)
         end
+
+        def error_for(result)
+          return [] unless result.error?
+
+          [result.message]
+        end
       end
     end
   end

app/services/boards/base_item_move_service.rb

@@ -4,13 +4,23 @@ module Boards
   class BaseItemMoveService < Boards::BaseService
     def execute(issuable)
       issuable_modification_params = issuable_params(issuable)
-      return false if issuable_modification_params.empty?
+      return if issuable_modification_params.empty?
 
-      move_single_issuable(issuable, issuable_modification_params)
+      return unless move_single_issuable(issuable, issuable_modification_params)
+
+      success(issuable)
     end
 
     private
 
+    def success(issuable)
+      ServiceResponse.success(payload: { issuable: issuable })
+    end
+
+    def error(issuable, message)
+      ServiceResponse.error(payload: { issuable: issuable }, message: message)
+    end
+
     def issuable_params(issuable)
       attrs = {}
 

ee/app/graphql/ee/mutations/boards/issues/issue_move_list.rb

@@ -20,11 +20,11 @@ module EE
           def move_issue(board, issue, move_params)
             super
           rescue ::Issues::BaseService::EpicAssignmentError => e
-            issue.errors.add(:epic_issue, e.message)
+            ServiceResponse.error(message: e.message)
           rescue ::Gitlab::Access::AccessDeniedError
-            issue.errors.add(:base, 'You are not allowed to move the issue')
+            ServiceResponse.error(message: 'You are not allowed to move the issue')
           rescue ActiveRecord::RecordNotFound
-            issue.errors.add(:base, 'Resource not found')
+            ServiceResponse.error(message: 'Resource not found')
           end
 
           override :move_arguments

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

@@ -6,6 +6,15 @@ module EE
       module MoveService
         extend ::Gitlab::Utils::Override
 
+        override :execute
+        def execute(issue)
+          unless assignee_authorized_for?(issue)
+            return error(issue, 'Not authorized to assign issue to list user')
+          end
+
+          super
+        end
+
         override :issuable_params
         def issuable_params(issue)
           args = super
@@ -20,6 +29,12 @@ module EE
           args.merge(list_movement_args(issue))
         end
 
+        def assignee_authorized_for?(issue)
+          return true unless moving_to_list&.assignee?
+
+          Ability.allowed?(moving_to_list.user, :read_issue, issue)
+        end
+
         def both_are_list_type?(type)
           return false unless moving_from_list.list_type == type
 

ee/spec/graphql/ee/mutations/boards/issues/issue_move_list_spec.rb

@@ -54,5 +54,19 @@ RSpec.describe Mutations::Boards::Issues::IssueMoveList do
         expect(issue1.relative_position).to eq(3)
       end
     end
+
+    context 'when user cannot be assigned to issue' do
+      before do
+        stub_licensed_features(board_assignee_lists: true)
+      end
+
+      it 'returns error on result' do
+        params[:to_list_id] = create(:user_list, board: board, position: 2).id
+
+        result = mutation.resolve(**params)
+
+        expect(result[:errors]).to eq(['Not authorized to assign issue to list user'])
+      end
+    end
   end
 end

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

@@ -259,6 +259,17 @@ RSpec.describe Boards::Issues::MoveService, services: true do
         expect(issue.assignees).to contain_exactly(user)
         expect(issue.milestone).to eq(milestone1)
       end
+
+      context 'when cannot assign to target list user' do
+        it 'returns error' do
+          random_list = create(:user_list, board: board1, position: 2)
+          params = { board_id: board1.id, from_list_id: user_list1.id, to_list_id: random_list.id }
+
+          result = described_class.new(parent, user, params).execute(issue)
+
+          expect(result[:status]).to eq(:error)
+        end
+      end
     end
   end
 

spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb

@@ -100,8 +100,8 @@ RSpec.shared_examples 'issues move service' do |group|
       create(:labeled_issue, project: project, labels: [bug, development], assignees: [assignee])
     end
 
-    it 'returns false' do
-      expect(described_class.new(parent, user, params).execute(issue)).to eq false
+    it 'returns nil' do
+      expect(described_class.new(parent, user, params).execute(issue)).to be_nil
     end
 
     it 'keeps issues labels' do