From ab9c8dbc2d2de013ee448b0191d460114619d04a Mon Sep 17 00:00:00 2001
From: Heinrich Lee Yu <heinrich@gitlab.com>
Date: Mon, 22 Aug 2022 16:57:48 +0800
Subject: [PATCH] Add mutation to move to start / end of board lists
Updates the GraphQL mutation so that we can move to the start or end of
a board list.
Changelog: added
---
.../boards/issues/issue_move_list.rb | 28 ++++++++++-
app/services/boards/base_item_move_service.rb | 22 ++++++++-
app/services/boards/issues/move_service.rb | 4 ++
doc/api/graphql/reference/index.md | 1 +
.../boards/issues/issue_move_list_spec.rb | 46 ++++++++++++++++++-
.../boards/issues/issue_move_list_spec.rb | 14 ++++++
.../issues_move_service_shared_examples.rb | 34 ++++++++++++++
7 files changed, 145 insertions(+), 4 deletions(-)
diff --git a/app/graphql/mutations/boards/issues/issue_move_list.rb b/app/graphql/mutations/boards/issues/issue_move_list.rb
index 14fe9714f99a..e9cae80e5f91 100644
--- a/app/graphql/mutations/boards/issues/issue_move_list.rb
+++ b/app/graphql/mutations/boards/issues/issue_move_list.rb
@@ -38,10 +38,16 @@ class IssueMoveList < Mutations::Issues::Base
required: false,
description: 'ID of issue that should be placed after the current issue.'
+ argument :position_in_list, GraphQL::Types::Int,
+ required: false,
+ description: "Position of issue within the board list. Positions start at 0. "\
+ "Use #{::Boards::Issues::MoveService::LIST_END_POSITION} to move to the end of the list."
+
def ready?(**args)
if move_arguments(args).blank?
raise Gitlab::Graphql::Errors::ArgumentError,
- 'At least one of the arguments fromListId, toListId, afterId or beforeId is required'
+ 'At least one of the arguments ' \
+ 'fromListId, toListId, positionInList, moveAfterId, or moveBeforeId is required'
end
if move_list_arguments(args).one?
@@ -49,6 +55,24 @@ def ready?(**args)
'Both fromListId and toListId must be present'
end
+ if args[:position_in_list].present?
+ if move_list_arguments(args).empty?
+ raise Gitlab::Graphql::Errors::ArgumentError,
+ 'Both fromListId and toListId are required when positionInList is given'
+ end
+
+ if args[:move_before_id].present? || args[:move_after_id].present?
+ raise Gitlab::Graphql::Errors::ArgumentError,
+ 'positionInList is mutually exclusive with any of moveBeforeId or moveAfterId'
+ end
+
+ if args[:position_in_list] != ::Boards::Issues::MoveService::LIST_END_POSITION &&
+ args[:position_in_list] < 0
+ raise Gitlab::Graphql::Errors::ArgumentError,
+ "positionInList must be >= 0 or #{::Boards::Issues::MoveService::LIST_END_POSITION}"
+ end
+ end
+
super
end
@@ -77,7 +101,7 @@ def move_list_arguments(args)
end
def move_arguments(args)
- args.slice(:from_list_id, :to_list_id, :move_after_id, :move_before_id)
+ args.slice(:from_list_id, :to_list_id, :position_in_list, :move_after_id, :move_before_id)
end
def error_for(result)
diff --git a/app/services/boards/base_item_move_service.rb b/app/services/boards/base_item_move_service.rb
index 9d711d83fd27..c9da889c5365 100644
--- a/app/services/boards/base_item_move_service.rb
+++ b/app/services/boards/base_item_move_service.rb
@@ -2,6 +2,8 @@
module Boards
class BaseItemMoveService < Boards::BaseService
+ LIST_END_POSITION = -1
+
def execute(issuable)
issuable_modification_params = issuable_params(issuable)
return if issuable_modification_params.empty?
@@ -32,7 +34,13 @@ def issuable_params(issuable)
)
end
- reposition_ids = move_between_ids(params)
+ move_params = if params[:position_in_list].present?
+ move_params_from_list_position(params[:position_in_list])
+ else
+ params
+ end
+
+ reposition_ids = move_between_ids(move_params)
attrs.merge!(reposition_params(reposition_ids)) if reposition_ids
attrs
@@ -90,6 +98,18 @@ def board_label_ids
::Label.ids_on_board(board.id)
end
+ def move_params_from_list_position(position)
+ if position == LIST_END_POSITION
+ { move_before_id: moving_to_list_items_relation.reverse_order.pick(:id), move_after_id: nil }
+ else
+ item_at_position = moving_to_list_items_relation.offset(position).pick(:id) # rubocop: disable CodeReuse/ActiveRecord
+
+ return move_params_from_list_position(LIST_END_POSITION) if item_at_position.nil?
+
+ { move_before_id: nil, move_after_id: item_at_position }
+ end
+ end
+
def move_between_ids(move_params)
ids = [move_params[:move_before_id], move_params[:move_after_id]]
.map(&:to_i)
diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb
index 90226b9d4e0b..4de4d7c8f69a 100644
--- a/app/services/boards/issues/move_service.rb
+++ b/app/services/boards/issues/move_service.rb
@@ -54,6 +54,10 @@ def board
def update(issue, issue_modification_params)
::Issues::UpdateService.new(project: issue.project, current_user: current_user, params: issue_modification_params).execute(issue)
end
+
+ def moving_to_list_items_relation
+ Boards::Issues::ListService.new(board.resource_parent, current_user, board_id: board.id, id: moving_to_list.id).execute
+ end
end
end
end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 2e77b196068a..089c6e0d766c 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -3038,6 +3038,7 @@ Input type: `IssueMoveListInput`
| <a id="mutationissuemovelistiid"></a>`iid` | [`String!`](#string) | IID of the issue to mutate. |
| <a id="mutationissuemovelistmoveafterid"></a>`moveAfterId` | [`ID`](#id) | ID of issue that should be placed after the current issue. |
| <a id="mutationissuemovelistmovebeforeid"></a>`moveBeforeId` | [`ID`](#id) | ID of issue that should be placed before the current issue. |
+| <a id="mutationissuemovelistpositioninlist"></a>`positionInList` | [`Int`](#int) | Position of issue within the board list. Positions start at 0. Use -1 to move to the end of the list. |
| <a id="mutationissuemovelistprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the issue to mutate is in. |
| <a id="mutationissuemovelisttolistid"></a>`toListId` | [`ID`](#id) | ID of the board list that the issue will be moved to. |
diff --git a/spec/graphql/mutations/boards/issues/issue_move_list_spec.rb b/spec/graphql/mutations/boards/issues/issue_move_list_spec.rb
index 10aed8a1f009..8e9a567f6149 100644
--- a/spec/graphql/mutations/boards/issues/issue_move_list_spec.rb
+++ b/spec/graphql/mutations/boards/issues/issue_move_list_spec.rb
@@ -55,7 +55,7 @@
let(:move_params) { {} }
it 'generates an error' do
- expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, 'At least one of the arguments fromListId, toListId, afterId or beforeId is required') do
+ expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, 'At least one of the arguments fromListId, toListId, positionInList, moveAfterId, or moveBeforeId is required') do
subject
end
end
@@ -71,6 +71,50 @@
end
end
+ context 'when positionInList is given' do
+ let(:move_params) { { from_list_id: list1.id, to_list_id: list2.id, position_in_list: 0 } }
+
+ context 'when fromListId and toListId are missing' do
+ let(:move_params) { { position_in_list: 0 } }
+
+ it 'generates an error' do
+ expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, 'Both fromListId and toListId are required when positionInList is given') do
+ subject
+ end
+ end
+ end
+
+ context 'when move_before_id is also given' do
+ let(:move_params) { { from_list_id: list1.id, to_list_id: list2.id, position_in_list: 0, move_before_id: 1 } }
+
+ it 'generates an error' do
+ expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, 'positionInList is mutually exclusive with any of moveBeforeId or moveAfterId') do
+ subject
+ end
+ end
+ end
+
+ context 'when move_after_id is also given' do
+ let(:move_params) { { from_list_id: list1.id, to_list_id: list2.id, position_in_list: 0, move_after_id: 1 } }
+
+ it 'generates an error' do
+ expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, 'positionInList is mutually exclusive with any of moveBeforeId or moveAfterId') do
+ subject
+ end
+ end
+ end
+
+ context 'when position_in_list is invalid' do
+ let(:move_params) { { from_list_id: list1.id, to_list_id: list2.id, position_in_list: -5 } }
+
+ it 'generates an error' do
+ expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ArgumentError, "positionInList must be >= 0 or #{Boards::Issues::MoveService::LIST_END_POSITION}") do
+ subject
+ end
+ end
+ end
+ end
+
context 'when user have access to resources' do
it 'moves and repositions issue' do
subject
diff --git a/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb b/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb
index 46ec22e7ef84..06093e9f7c2e 100644
--- a/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb
+++ b/spec/requests/api/graphql/mutations/boards/issues/issue_move_list_spec.rb
@@ -100,6 +100,20 @@
expect(response_issue['labels']['edges'][0]['node']['title']).to eq(testing.title)
end
end
+
+ context 'when moving an issue using position_in_list' do
+ let(:issue_move_params) { { from_list_id: list1.id, to_list_id: list2.id, position_in_list: 0 } }
+
+ it 'repositions an issue' do
+ post_graphql_mutation(mutation(params), current_user: current_user)
+
+ expect(response).to have_gitlab_http_status(:success)
+ response_issue = json_response['data'][mutation_result_identifier]['issue']
+ expect(response_issue['iid']).to eq(issue1.iid.to_s)
+ expect(response_issue['labels']['edges'][0]['node']['title']).to eq(testing.title)
+ expect(response_issue['relativePosition']).to be < existing_issue1.relative_position
+ end
+ end
end
context 'when user has no access to resources' do
diff --git a/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb b/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb
index 1638e2fa86ad..162be24fe8f7 100644
--- a/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb
+++ b/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb
@@ -140,6 +140,40 @@
expect(issue2.reload.updated_at.change(usec: 0)).to eq updated_at2.change(usec: 0)
end
+ context 'when moving to a specific list position' do
+ before do
+ [issue1, issue2, issue].each do |issue|
+ issue.move_to_end && issue.save!
+ end
+ end
+
+ it 'moves issue to the top of the list' do
+ described_class.new(parent, user, params.merge({ position_in_list: 0 })).execute(issue)
+
+ expect(issue.relative_position).to be < issue1.relative_position
+ end
+
+ it 'moves issue to a position in the middle of the list' do
+ described_class.new(parent, user, params.merge({ position_in_list: 1 })).execute(issue)
+
+ expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+ end
+
+ it 'moves issue to the bottom of the list' do
+ described_class.new(parent, user, params.merge({ position_in_list: -1 })).execute(issue1)
+
+ expect(issue1.relative_position).to be > issue.relative_position
+ end
+
+ context 'when given position is greater than number of issues in the list' do
+ it 'moves the issue to the bottom of the list' do
+ described_class.new(parent, user, params.merge({ position_in_list: 5 })).execute(issue1)
+
+ expect(issue1.relative_position).to be > issue.relative_position
+ end
+ end
+ end
+
def reorder_issues(params, issues: [])
issues.each do |issue|
issue.move_to_end && issue.save!
--
GitLab