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.

4 files changed, 47 insertions, 16 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..f8a25d814ed 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 }
@@ -200,12 +200,21 @@ describe Boards::IssuesController 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 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 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