From 1ebef4aae55adb5e17a76b92aea74467e49130bf Mon Sep 17 00:00:00 2001
From: Patrick Derichs <pderichs@gitlab.com>
Date: Fri, 5 Jul 2019 12:50:20 +0200
Subject: [PATCH] Add result to MoveService#execute_multiple
It adds a hash response which includes
the count, success state and the moved
issues itself so the caller has additional
information about the result of the
process.
---
app/controllers/boards/issues_controller.rb | 7 +---
app/services/boards/issues/move_service.rb | 39 +++++++++++++++----
.../boards/issues_controller_spec.rb | 13 ++++++-
.../boards/issues/move_service_spec.rb | 4 +-
4 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb
index 353a9806fd121..90528f75ffd6a 100644
--- a/app/controllers/boards/issues_controller.rb
+++ b/app/controllers/boards/issues_controller.rb
@@ -58,11 +58,8 @@ def bulk_move
service = Boards::Issues::MoveService.new(board_parent, current_user, move_params(true))
issues = Issue.find(params[:ids])
- if service.execute_multiple(issues)
- head :ok
- else
- head :unprocessable_entity
- end
+
+ render json: service.execute_multiple(issues)
end
def update
diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb
index 755d723b9a003..00ce27db7c80c 100644
--- a/app/services/boards/issues/move_service.rb
+++ b/app/services/boards/issues/move_service.rb
@@ -11,26 +11,51 @@ def execute(issue)
end
def execute_multiple(issues)
- return false if issues.empty?
+ return execute_multiple_empty_result if issues.empty?
+ handled_issues = []
last_inserted_issue_id = nil
- issues.map do |issue|
+ count = issues.each.inject(0) do |moved_count, issue|
issue_modification_params = issue_params(issue)
- next if issue_modification_params.empty?
+ next moved_count if issue_modification_params.empty?
if last_inserted_issue_id
- issue_modification_params[:move_between_ids] = move_between_ids({ move_after_id: nil, move_before_id: last_inserted_issue_id })
+ issue_modification_params[:move_between_ids] = move_below(last_inserted_issue_id)
end
last_inserted_issue_id = issue.id
- move_single_issue(issue, issue_modification_params)
- end.all?
+ handled_issue = move_single_issue(issue, issue_modification_params)
+ handled_issues << present_issue_entity(handled_issue) if handled_issue
+ handled_issue && handled_issue.valid? ? moved_count + 1 : moved_count
+ end
+
+ {
+ count: count,
+ success: count == issues.size,
+ issues: handled_issues
+ }
end
private
+ def present_issue_entity(issue)
+ ::API::Entities::Issue.represent(issue)
+ end
+
+ def execute_multiple_empty_result
+ @execute_multiple_empty_result ||= {
+ count: 0,
+ success: false,
+ issues: []
+ }
+ end
+
+ def move_below(id)
+ move_between_ids({ move_after_id: nil, move_before_id: id })
+ end
+
def move_single_issue(issue, issue_modification_params)
- return false unless can?(current_user, :update_issue, issue)
+ return unless can?(current_user, :update_issue, issue)
update(issue, issue_modification_params)
end
diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb
index 246d6f9e0f992..f8a25d814edd8 100644
--- a/spec/controllers/boards/issues_controller_spec.rb
+++ b/spec/controllers/boards/issues_controller_spec.rb
@@ -160,7 +160,7 @@
end
end
- describe 'PUT move_multiple' do
+ describe 'PUT bulk_move' do
let(:todo) { create(:group_label, group: group, name: 'Todo') }
let(:development) { create(:group_label, group: group, name: 'Development') }
let(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user }
@@ -200,12 +200,21 @@
put :bulk_move, params: move_issues_params
expect(response).to have_gitlab_http_status(expected_status)
+ if expected_status == 200
+ expect(json_response).to include(
+ 'count' => move_issues_params[:ids].size,
+ 'success' => true
+ )
+
+ expect(json_response['issues'].pluck('id')).to include(*move_issues_params[:ids])
+ end
+
list_issues user: requesting_user, board: board, list: list2
expect(response).to have_gitlab_http_status(200)
expect(response).to match_response_schema('entities/issue_boards')
- responded_issues = json_response['issues']
+ responded_issues = JSON.parse(response.body)['issues']
expect(responded_issues.length).to eq expected_issue_count
ids_in_order = responded_issues.pluck('id')
diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb
index 1bfb5602df2c0..cf84ec8fd4c18 100644
--- a/spec/services/boards/issues/move_service_spec.rb
+++ b/spec/services/boards/issues/move_service_spec.rb
@@ -68,8 +68,8 @@
project.add_developer(user)
end
- it 'returns false if list of issues is empty' do
- expect(described_class.new(group, user, params).execute_multiple([])).to eq(false)
+ it 'returns the expected result if list of issues is empty' do
+ expect(described_class.new(group, user, params).execute_multiple([])).to eq({ count: 0, success: false, issues: [] })
end
context 'moving multiple issues' do
--
GitLab