Support position params in issue creation mutation

This allows specifying the position of an issue upon creation. This is
going to be used on issue boards where we want to position the issue at
the top of the list upon creation.

Without this param, newly created issues are added to the end by
default.

Changelog: added

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