Merge branch '35757-move-issues-in-boards-pderichs' into 'master'

Add endpoint to move issues in boards

See merge request gitlab-org/gitlab-ce!30216

2 files changed, 63 insertions, 10 deletions

diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb
index 0dd7500623d..353a9806fd1 100644
--- a/app/controllers/boards/issues_controller.rb
+++ b/app/controllers/boards/issues_controller.rb
@@ -2,16 +2,24 @@
 
 module Boards
   class IssuesController < Boards::ApplicationController
+    # This is the maximum amount of issues which can be moved by one request to
+    # bulk_move for now. This is temporary and might be removed in future by
+    # introducing an alternative (async?) approach.
+    # (related: https://gitlab.com/groups/gitlab-org/-/epics/382)
+    MAX_MOVE_ISSUES_COUNT = 50
+
     include BoardsResponses
     include ControllerWithCrossProjectAccessCheck
 
     requires_cross_project_access if: -> { board&.group_board? }
 
-    before_action :whitelist_query_limiting, only: [:index, :update]
+    before_action :whitelist_query_limiting, only: [:index, :update, :bulk_move]
     before_action :authorize_read_issue, only: [:index]
     before_action :authorize_create_issue, only: [:create]
     before_action :authorize_update_issue, only: [:update]
     skip_before_action :authenticate_user!, only: [:index]
+    before_action :validate_id_list, only: [:bulk_move]
+    before_action :can_move_issues?, only: [:bulk_move]
 
     # rubocop: disable CodeReuse/ActiveRecord
     def index
@@ -46,6 +54,17 @@ module Boards
       end
     end
 
+    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
+    end
+
     def update
       service = Boards::Issues::MoveService.new(board_parent, current_user, move_params)
 
@@ -58,6 +77,10 @@ module Boards
 
     private
 
+    def can_move_issues?
+      head(:forbidden) unless can?(current_user, :admin_issue, board)
+    end
+
     def render_issues(issues, metadata)
       data = { issues: serialize_as_json(issues) }
       data.merge!(metadata)
@@ -90,8 +113,9 @@ module Boards
                    end
     end
 
-    def move_params
-      params.permit(:board_id, :id, :from_list_id, :to_list_id, :move_before_id, :move_after_id)
+    def move_params(multiple = false)
+      id_param = multiple ? :ids : :id
+      params.permit(id_param, :board_id, :from_list_id, :to_list_id, :move_before_id, :move_after_id)
     end
 
     def issue_params
@@ -112,5 +136,10 @@ module Boards
       # Also see https://gitlab.com/gitlab-org/gitlab-ce/issues/42439
       Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42428')
     end
+
+    def validate_id_list
+      head(:bad_request) unless params[:ids].is_a?(Array)
+      head(:unprocessable_entity) if params[:ids].size > MAX_MOVE_ISSUES_COUNT
+    end
   end
 end
diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb
index e27d34dbcab..755d723b9a0 100644
--- a/app/services/boards/issues/move_service.rb
+++ b/app/services/boards/issues/move_service.rb
@@ -4,14 +4,37 @@ module Boards
   module Issues
     class MoveService < Boards::BaseService
       def execute(issue)
-        return false unless can?(current_user, :update_issue, issue)
-        return false if issue_params(issue).empty?
+        issue_modification_params = issue_params(issue)
+        return false if issue_modification_params.empty?
 
-        update(issue)
+        move_single_issue(issue, issue_modification_params)
+      end
+
+      def execute_multiple(issues)
+        return false if issues.empty?
+
+        last_inserted_issue_id = nil
+        issues.map do |issue|
+          issue_modification_params = issue_params(issue)
+          next 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 })
+          end
+
+          last_inserted_issue_id = issue.id
+          move_single_issue(issue, issue_modification_params)
+        end.all?
       end
 
       private
 
+      def move_single_issue(issue, issue_modification_params)
+        return false unless can?(current_user, :update_issue, issue)
+
+        update(issue, issue_modification_params)
+      end
+
       def board
         @board ||= parent.boards.find(params[:board_id])
       end
@@ -33,8 +56,8 @@ module Boards
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      def update(issue)
-        ::Issues::UpdateService.new(issue.project, current_user, issue_params(issue)).execute(issue)
+      def update(issue, issue_modification_params)
+        ::Issues::UpdateService.new(issue.project, current_user, issue_modification_params).execute(issue)
       end
 
       def issue_params(issue)
@@ -48,6 +71,7 @@ module Boards
           )
         end
 
+        move_between_ids = move_between_ids(params)
         if move_between_ids
           attrs[:move_between_ids] = move_between_ids
           attrs[:board_group_id] = board.group&.id
@@ -78,8 +102,8 @@ module Boards
       end
       # rubocop: enable CodeReuse/ActiveRecord
 
-      def move_between_ids
-        ids = [params[:move_after_id], params[:move_before_id]]
+      def move_between_ids(move_params)
+        ids = [move_params[:move_after_id], move_params[:move_before_id]]
                 .map(&:to_i)
                 .map { |m| m.positive? ? m : nil }