diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2019-07-16 15:53:43 +0000 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2019-07-16 15:53:43 +0000 |
commit | cca71da16ac12d3df787de3f3a0cd60d30649a1d (patch) | |
tree | 3e997f4694fc9ddf815cb0ef7287225594a79ab0 | |
parent | f97a73fa39b48b6c3c770d609fcd9584d17221da (diff) | |
parent | ca089d91f6313b32a6381aef36583e83869f4017 (diff) | |
download | gitlab-ce-cca71da16ac12d3df787de3f3a0cd60d30649a1d.tar.gz |
Merge branch '35757-add-result-to-request' into 'master'
Add response to move multiple issues request
See merge request gitlab-org/gitlab-ce!30400
-rw-r--r-- | app/controllers/boards/issues_controller.rb | 7 | ||||
-rw-r--r-- | app/services/boards/issues/move_service.rb | 39 | ||||
-rw-r--r-- | spec/controllers/boards/issues_controller_spec.rb | 16 | ||||
-rw-r--r-- | spec/services/boards/issues/move_service_spec.rb | 4 |
4 files changed, 51 insertions, 15 deletions
diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 353a9806fd1..90528f75ffd 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -58,11 +58,8 @@ module Boards 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 755d723b9a0..00ce27db7c8 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -11,26 +11,51 @@ module Boards 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 246d6f9e0f9..0db58fbefc1 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -160,7 +160,7 @@ describe Boards::IssuesController do 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 } @@ -196,6 +196,20 @@ describe Boards::IssuesController do sign_in(signed_in_user) end + it 'responds as expected' do + 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 match_array(move_issues_params[:ids]) + end + end + it 'moves issues as expected' do put :bulk_move, params: move_issues_params expect(response).to have_gitlab_http_status(expected_status) diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb index 1bfb5602df2..cf84ec8fd4c 100644 --- a/spec/services/boards/issues/move_service_spec.rb +++ b/spec/services/boards/issues/move_service_spec.rb @@ -68,8 +68,8 @@ describe Boards::Issues::MoveService do 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 |