Merge branch '341562-support-position-in-issue-create-mutation' into 'master'

Add `move_before_id` and `move_after_id` to issue creation mutation

See merge request gitlab-org/gitlab!79397

app/graphql/mutations/issues/create.rb

@@ -51,6 +51,14 @@ module Mutations
                required: false,
                description: 'Array of user IDs to assign to the issue.'
 
+      argument :move_before_id, ::Types::GlobalIDType[::Issue],
+               required: false,
+               description: 'Global ID of issue that should be placed before the current issue.'
+
+      argument :move_after_id, ::Types::GlobalIDType[::Issue],
+               required: false,
+               description: 'Global ID of issue that should be placed after the current issue.'
+
       field :issue,
             Types::IssueType,
             null: true,
@@ -93,6 +101,13 @@ module Mutations
         params[:assignee_ids] &&= params[:assignee_ids].map { |assignee_id| assignee_id&.model_id }
         params[:label_ids] &&= params[:label_ids].map { |label_id| label_id&.model_id }
 
+        if params[:move_before_id].present? || params[:move_after_id].present?
+          params[:move_between_ids] = [
+            params.delete(:move_before_id)&.model_id,
+            params.delete(:move_after_id)&.model_id
+          ]
+        end
+
         params
       end
 

app/services/boards/base_item_move_service.rb

@@ -91,7 +91,7 @@ module Boards
     end
 
     def move_between_ids(move_params)
-      ids = [move_params[:move_after_id], move_params[:move_before_id]]
+      ids = [move_params[:move_before_id], move_params[:move_after_id]]
               .map(&:to_i)
               .map { |m| m > 0 ? m : nil }
 

app/services/issuable_base_service.rb

@@ -494,7 +494,7 @@ class IssuableBaseService < ::BaseProjectService
   def handle_move_between_ids(issuable_position)
     return unless params[:move_between_ids]
 
-    after_id, before_id = params.delete(:move_between_ids)
+    before_id, after_id = params.delete(:move_between_ids)
 
     positioning_scope = issuable_position.class.relative_positioning_query_base(issuable_position)
 

app/services/issues/base_service.rb

@@ -2,6 +2,7 @@
 
 module Issues
   class BaseService < ::IssuableBaseService
+    extend ::Gitlab::Utils::Override
     include IncidentManagement::UsageData
     include IssueTypeHelpers
 
@@ -61,6 +62,23 @@ module Issues
       issue.system_note_timestamp = params[:created_at] || params[:updated_at]
     end
 
+    override :handle_move_between_ids
+    def handle_move_between_ids(issue)
+      issue.check_repositioning_allowed! if params[:move_between_ids]
+
+      super
+
+      rebalance_if_needed(issue)
+    end
+
+    def issuable_for_positioning(id, positioning_scope)
+      return unless id
+
+      issue = positioning_scope.find(id)
+
+      issue if can?(current_user, :update_issue, issue)
+    end
+
     def create_assignee_note(issue, old_assignees)
       SystemNoteService.change_issuable_assignees(
         issue, issue.project, current_user, old_assignees)

app/services/issues/create_service.rb

@@ -21,6 +21,8 @@ module Issues
     def execute(skip_system_notes: false)
       @issue = @build_service.execute
 
+      handle_move_between_ids(@issue)
+
       filter_resolve_discussion_params
 
       create(@issue, skip_system_notes: skip_system_notes)

app/services/issues/reorder_service.rb

@@ -21,7 +21,7 @@ module Issues
 
     def move_between_ids
       strong_memoize(:move_between_ids) do
-        ids = [params[:move_after_id], params[:move_before_id]]
+        ids = [params[:move_before_id], params[:move_after_id]]
                 .map(&:to_i)
                 .map { |m| m > 0 ? m : nil }
 

app/services/issues/update_service.rb

@@ -2,8 +2,6 @@
 
 module Issues
   class UpdateService < Issues::BaseService
-    extend ::Gitlab::Utils::Override
-
     # NOTE: For Issues::UpdateService, we default the spam_params to nil, because spam_checking is not
     # necessary in many cases, and we don't want to require every caller to explicitly pass it as nil
     # to disable spam checking.
@@ -92,14 +90,6 @@ module Issues
       todo_service.update_issue(issuable, current_user)
     end
 
-    def handle_move_between_ids(issue)
-      issue.check_repositioning_allowed! if params[:move_between_ids]
-
-      super
-
-      rebalance_if_needed(issue)
-    end
-
     # rubocop: disable CodeReuse/ActiveRecord
     def change_issue_duplicate(issue)
       canonical_issue_id = params.delete(:canonical_issue_id)
@@ -217,14 +207,6 @@ module Issues
       ).execute
     end
 
-    def issuable_for_positioning(id, positioning_scope)
-      return unless id
-
-      issue = positioning_scope.find(id)
-
-      issue if can?(current_user, :update_issue, issue)
-    end
-
     def create_confidentiality_note(issue)
       SystemNoteService.change_issue_confidentiality(issue, issue.project, current_user)
     end

doc/api/graphql/reference/index.md

@@ -1420,6 +1420,8 @@ Input type: `CreateIssueInput`
 | <a id="mutationcreateissuelocked"></a>`locked` | [`Boolean`](#boolean) | Indicates discussion is locked on the issue. |
 | <a id="mutationcreateissuemergerequesttoresolvediscussionsof"></a>`mergeRequestToResolveDiscussionsOf` | [`MergeRequestID`](#mergerequestid) | IID of a merge request for which to resolve discussions. |
 | <a id="mutationcreateissuemilestoneid"></a>`milestoneId` | [`MilestoneID`](#milestoneid) | ID of the milestone to assign to the issue. On update milestone will be removed if set to null. |
+| <a id="mutationcreateissuemoveafterid"></a>`moveAfterId` | [`IssueID`](#issueid) | Global ID of issue that should be placed after the current issue. |
+| <a id="mutationcreateissuemovebeforeid"></a>`moveBeforeId` | [`IssueID`](#issueid) | Global ID of issue that should be placed before the current issue. |
 | <a id="mutationcreateissueprojectpath"></a>`projectPath` | [`ID!`](#id) | Project full path the issue is associated with. |
 | <a id="mutationcreateissuetitle"></a>`title` | [`String!`](#string) | Title of the issue. |
 | <a id="mutationcreateissuetype"></a>`type` | [`IssueType`](#issuetype) | Type of the issue. |

ee/spec/services/epics/update_service_spec.rb

@@ -116,7 +116,7 @@ RSpec.describe Epics::UpdateService do
 
       shared_examples 'board repositioning' do
         context 'when moving between 2 epics on the board' do
-          subject { update_epic(move_between_ids: [epic1.id, epic2.id], board_id: board.id, list_id: list.id, board_group: group) }
+          subject { update_epic(move_between_ids: [epic2.id, epic1.id], board_id: board.id, list_id: list.id, board_group: group) }
 
           it 'moves the epic correctly' do
             subject
@@ -130,7 +130,7 @@ RSpec.describe Epics::UpdateService do
 
         context 'when moving the epic to the end' do
           it 'moves the epic correctly' do
-            update_epic(move_between_ids: [nil, epic2.id], board_id: board.id, list_id: list.id, board_group: group)
+            update_epic(move_between_ids: [epic2.id, nil], board_id: board.id, list_id: list.id, board_group: group)
 
             expect(position(epic)).to be > position(epic2)
           end
@@ -147,7 +147,7 @@ RSpec.describe Epics::UpdateService do
 
         context 'when moving beetween 2 epics on the board' do
           it 'keeps epic3 on top of the board' do
-            update_epic(move_between_ids: [epic1.id, epic2.id], board_id: board.id, list_id: list.id, board_group: group)
+            update_epic(move_between_ids: [epic2.id, epic1.id], board_id: board.id, list_id: list.id, board_group: group)
 
             expect(position(epic3)).to be < position(epic2)
             expect(position(epic3)).to be < position(epic1)
@@ -160,7 +160,7 @@ RSpec.describe Epics::UpdateService do
           end
 
           it 'moves the epic correctly' do
-            update_epic(move_between_ids: [epic3.id, nil], board_id: board.id, list_id: list.id, board_group: group)
+            update_epic(move_between_ids: [nil, epic3.id], board_id: board.id, list_id: list.id, board_group: group)
 
             expect(epic_position.reload.relative_position).to be < epic3_position.relative_position
           end
@@ -168,7 +168,7 @@ RSpec.describe Epics::UpdateService do
 
         context 'when moving the epic to the end' do
           it 'keeps epic3 on top of the board' do
-            update_epic(move_between_ids: [epic1.id, epic2.id], board_id: board.id, list_id: list.id, board_group: group)
+            update_epic(move_between_ids: [epic1.id, nil], board_id: board.id, list_id: list.id, board_group: group)
 
             expect(position(epic3)).to be < position(epic2)
             expect(position(epic3)).to be < position(epic1)
@@ -211,7 +211,7 @@ RSpec.describe Epics::UpdateService do
           let_it_be_with_reload(:epic2_position) { create(:epic_board_position, epic: epic2, epic_board: board, relative_position: 30) }
           let_it_be_with_reload(:epic_position) { create(:epic_board_position, epic: epic, epic_board: board, relative_position: 10) }
 
-          subject { update_epic(move_between_ids: [epic1.id, epic2.id], board_id: board.id, list_id: list.id, board_group: group) }
+          subject { update_epic(move_between_ids: [epic2.id, epic1.id], board_id: board.id, list_id: list.id, board_group: group) }
 
           it 'moves the epic correctly' do
             subject

spec/requests/api/graphql/mutations/issues/create_spec.rb

@@ -52,5 +52,22 @@ RSpec.describe 'Create an issue' do
     it_behaves_like 'has spam protection' do
       let(:mutation_class) { ::Mutations::Issues::Create }
     end
+
+    context 'when position params are provided' do
+      let(:existing_issue) { create(:issue, project: project, relative_position: 50) }
+
+      before do
+        input.merge!(
+          move_after_id: existing_issue.to_global_id.to_s
+        )
+      end
+
+      it 'sets the correct position' do
+        post_graphql_mutation(mutation, current_user: current_user)
+
+        expect(response).to have_gitlab_http_status(:success)
+        expect(mutation_response['issue']['relativePosition']).to be < existing_issue.relative_position
+      end
+    end
   end
 end

spec/services/issues/create_service_spec.rb

@@ -92,6 +92,23 @@ RSpec.describe Issues::CreateService do
         end
       end
 
+      context 'when setting a position' do
+        let(:issue_before) { create(:issue, project: project, relative_position: 10) }
+        let(:issue_after) { create(:issue, project: project, relative_position: 50) }
+
+        before do
+          project.add_reporter(user)
+
+          opts.merge!(move_between_ids: [issue_before.id, issue_after.id])
+        end
+
+        it 'sets the correct relative position' do
+          expect(issue).to be_persisted
+          expect(issue.relative_position).to be_present
+          expect(issue.relative_position).to be_between(issue_before.relative_position, issue_after.relative_position)
+        end
+      end
+
       it_behaves_like 'not an incident issue'
 
       context 'when issue is incident type' do