Merge branch 'mass-change-comments' into 'master'

Send notifications and leave system comments when bulk updating issues.

Resolves https://dev.gitlab.org/gitlab/gitlabhq/issues/1885.

See merge request !1646

CHANGELOG

@@ -29,6 +29,7 @@ v 7.9.0 (unreleased)
   - Add Bitbucket importer.
   - Support referencing issues to a project whose name starts with a digit
   - Condense commits already in target branch when updating merge request source branch.
+  - Send notifications and leave system comments when bulk updating issues.
 
 v 7.8.2
   - Fix service migration issue when upgrading from versions prior to 7.3

app/controllers/projects/issues_controller.rb

@@ -93,7 +93,7 @@ class Projects::IssuesController < Projects::ApplicationController
   end
 
   def bulk_update
-    result = Issues::BulkUpdateService.new(project, current_user, params).execute
+    result = Issues::BulkUpdateService.new(project, current_user, bulk_update_params).execute
     redirect_to :back, notice: "#{result[:count]} issues updated"
   end
 
@@ -141,4 +141,13 @@ class Projects::IssuesController < Projects::ApplicationController
       :milestone_id, :state_event, :task_num, label_ids: []
     )
   end
+
+  def bulk_update_params
+    params.require(:update).permit(
+      :issues_ids,
+      :assignee_id,
+      :milestone_id,
+      :state_event
+    )
+  end
 end

app/services/issues/bulk_update_service.rb

 module Issues
   class BulkUpdateService < BaseService
     def execute
-      update_data = params[:update]
+      issues_ids   = params.delete(:issues_ids).split(",")
+      issue_params = params
 
-      issues_ids   = update_data[:issues_ids].split(",")
-      milestone_id = update_data[:milestone_id]
-      assignee_id  = update_data[:assignee_id]
-      status       = update_data[:status]
-
-      new_state = nil
-
-      if status.present?
-        if status == 'closed'
-          new_state = :close
-        else
-          new_state = :reopen
-        end
-      end
-
-      opts = {}
-      opts[:milestone_id] = milestone_id if milestone_id.present?
-      opts[:assignee_id] = assignee_id if assignee_id.present?
+      issue_params.delete(:state_event) unless issue_params[:state_event].present?
+      issue_params.delete(:milestone_id) unless issue_params[:milestone_id].present?
+      issue_params.delete(:assignee_id) unless issue_params[:assignee_id].present?
 
       issues = Issue.where(id: issues_ids)
-      issues = issues.select { |issue| can?(current_user, :modify_issue, issue) }
-
       issues.each do |issue|
-        issue.update_attributes(opts)
-        issue.send new_state if new_state
+        next unless can?(current_user, :modify_issue, issue)
+
+        Issues::UpdateService.new(issue.project, current_user, issue_params).execute(issue)
       end
 
       {
-        count: issues.count,
-        success: !issues.count.zero?
+        count:    issues.count,
+        success:  !issues.count.zero?
       }
     end
   end

app/views/projects/issues/index.html.haml

@@ -25,11 +25,11 @@
   .clearfix
     .issues_bulk_update.hide
       = form_tag bulk_update_namespace_project_issues_path(@project.namespace, @project), method: :post  do
-        = select_tag('update[status]', options_for_select([['Open', 'open'], ['Closed', 'closed']]), prompt: "Status")
+        = select_tag('update[state_event]', options_for_select([['Open', 'reopen'], ['Closed', 'close']]), prompt: "Status")
         = project_users_select_tag('update[assignee_id]', placeholder: 'Assignee')
         = select_tag('update[milestone_id]', bulk_update_milestone_options, prompt: "Milestone")
         = hidden_field_tag 'update[issues_ids]', []
-        = hidden_field_tag :status, params[:status]
+        = hidden_field_tag :state_event, params[:state_event]
         = button_tag "Update issues", class: "btn update_selected_issues btn-save"
 
 .issues-holder

spec/services/issues/bulk_update_service_spec.rb

@@ -21,10 +21,8 @@ describe Issues::BulkUpdateService do
         create(:issue, project: @project)
       end
       @params = {
-        update: {
-          status: 'closed',
-          issues_ids: @issues.map(&:id)
-        }
+        state_event: 'close',
+        issues_ids: @issues.map(&:id)
       }
     end
 
@@ -46,10 +44,8 @@ describe Issues::BulkUpdateService do
         create(:closed_issue, project: @project)
       end
       @params = {
-        update: {
-          status: 'reopen',
-          issues_ids: @issues.map(&:id)
-        }
+        state_event: 'reopen',
+        issues_ids: @issues.map(&:id)
       }
     end
 
@@ -69,10 +65,8 @@ describe Issues::BulkUpdateService do
     before do
       @new_assignee = create :user
       @params = {
-        update: {
-          issues_ids: [issue.id],
-          assignee_id: @new_assignee.id
-        }
+        issues_ids: [issue.id],
+        assignee_id: @new_assignee.id
       }
     end
 
@@ -88,7 +82,7 @@ describe Issues::BulkUpdateService do
       @project.issues.first.update_attribute(:assignee, @new_assignee)
       expect(@project.issues.first.assignee).not_to be_nil
 
-      @params[:update][:assignee_id] = -1
+      @params[:assignee_id] = -1
 
       Issues::BulkUpdateService.new(@project, @user, @params).execute
       expect(@project.issues.first.assignee).to be_nil
@@ -98,7 +92,7 @@ describe Issues::BulkUpdateService do
       @project.issues.first.update_attribute(:assignee, @new_assignee)
       expect(@project.issues.first.assignee).not_to be_nil
 
-      @params[:update][:assignee_id] = ''
+      @params[:assignee_id] = ''
 
       Issues::BulkUpdateService.new(@project, @user, @params).execute
       expect(@project.issues.first.assignee).not_to be_nil
@@ -110,10 +104,8 @@ describe Issues::BulkUpdateService do
     before do
       @milestone = create :milestone
       @params = {
-        update: {
-          issues_ids: [issue.id],
-          milestone_id: @milestone.id
-        }
+        issues_ids: [issue.id],
+        milestone_id: @milestone.id
       }
     end