Merge branch 'mia_backort' into 'master'

Backport of Multiple Assignees feature

See merge request !11089

13 files changed, 165 insertions, 40 deletions

diff --git a/app/services/issuable/bulk_update_service.rb b/app/services/issuable/bulk_update_service.rb
index 60891cbb255..40ff9b8b867 100644
--- a/app/services/issuable/bulk_update_service.rb
+++ b/app/services/issuable/bulk_update_service.rb
@@ -7,10 +7,14 @@ module Issuable
       ids = params.delete(:issuable_ids).split(",")
       items = model_class.where(id: ids)
 
-      %i(state_event milestone_id assignee_id add_label_ids remove_label_ids subscription_event).each do |key|
+      %i(state_event milestone_id assignee_id assignee_ids add_label_ids remove_label_ids subscription_event).each do |key|
         params.delete(key) unless params[key].present?
       end
 
+      if params[:assignee_ids] == [IssuableFinder::NONE.to_s]
+        params[:assignee_ids] = []
+      end
+
       items.each do |issuable|
         next unless can?(current_user, :"update_#{type}", issuable)
 
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index b071a398481..6c2777a8d2d 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -1,11 +1,6 @@
 class IssuableBaseService < BaseService
   private
 
-  def create_assignee_note(issuable)
-    SystemNoteService.change_assignee(
-      issuable, issuable.project, current_user, issuable.assignee)
-  end
-
   def create_milestone_note(issuable)
     SystemNoteService.change_milestone(
       issuable, issuable.project, current_user, issuable.milestone)
@@ -53,6 +48,7 @@ class IssuableBaseService < BaseService
       params.delete(:add_label_ids)
       params.delete(:remove_label_ids)
       params.delete(:label_ids)
+      params.delete(:assignee_ids)
       params.delete(:assignee_id)
       params.delete(:due_date)
     end
@@ -77,7 +73,7 @@ class IssuableBaseService < BaseService
   def assignee_can_read?(issuable, assignee_id)
     new_assignee = User.find_by_id(assignee_id)
 
-    return false unless new_assignee.present?
+    return false unless new_assignee
 
     ability_name = :"read_#{issuable.to_ability_name}"
     resource     = issuable.persisted? ? issuable : project
@@ -207,6 +203,7 @@ class IssuableBaseService < BaseService
     filter_params(issuable)
     old_labels = issuable.labels.to_a
     old_mentioned_users = issuable.mentioned_users.to_a
+    old_assignees = issuable.assignees.to_a
 
     label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
     params[:label_ids] = label_ids if labels_changing?(issuable.label_ids, label_ids)
@@ -222,7 +219,13 @@ class IssuableBaseService < BaseService
           handle_common_system_notes(issuable, old_labels: old_labels)
         end
 
-        handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users)
+        handle_changes(
+          issuable,
+          old_labels: old_labels,
+          old_mentioned_users: old_mentioned_users,
+          old_assignees: old_assignees
+        )
+
         after_update(issuable)
         issuable.create_new_cross_references!(current_user)
         execute_hooks(issuable, 'update')
@@ -272,7 +275,7 @@ class IssuableBaseService < BaseService
     end
   end
 
-  def has_changes?(issuable, old_labels: [])
+  def has_changes?(issuable, old_labels: [], old_assignees: [])
     valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch]
 
     attrs_changed = valid_attrs.any? do |attr|
@@ -281,7 +284,9 @@ class IssuableBaseService < BaseService
 
     labels_changed = issuable.labels != old_labels
 
-    attrs_changed || labels_changed
+    assignees_changed = issuable.assignees != old_assignees
+
+    attrs_changed || labels_changed || assignees_changed
   end
 
   def handle_common_system_notes(issuable, old_labels: [])
diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index ee1b40db718..34199eb5d13 100644
--- a/app/services/issues/base_service.rb
+++ b/app/services/issues/base_service.rb
@@ -9,11 +9,33 @@ module Issues
 
     private
 
+    def create_assignee_note(issue, old_assignees)
+      SystemNoteService.change_issue_assignees(
+        issue, issue.project, current_user, old_assignees)
+    end
+
     def execute_hooks(issue, action = 'open')
       issue_data  = hook_data(issue, action)
       hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks
       issue.project.execute_hooks(issue_data, hooks_scope)
       issue.project.execute_services(issue_data, hooks_scope)
     end
+
+    def filter_assignee(issuable)
+      return if params[:assignee_ids].blank?
+
+      # The number of assignees is limited by one for GitLab CE
+      params[:assignee_ids] = params[:assignee_ids][0, 1]
+
+      assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) }
+
+      if params[:assignee_ids].map(&:to_s) == [IssuableFinder::NONE]
+        params[:assignee_ids] = []
+      elsif assignee_ids.any?
+        params[:assignee_ids] = assignee_ids
+      else
+        params.delete(:assignee_ids)
+      end
+    end
   end
 end
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index b7fe5cb168b..cd9f9a4a16e 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -12,8 +12,12 @@ module Issues
       spam_check(issue, current_user)
     end
 
-    def handle_changes(issue, old_labels: [], old_mentioned_users: [])
-      if has_changes?(issue, old_labels: old_labels)
+    def handle_changes(issue, options)
+      old_labels = options[:old_labels] || []
+      old_mentioned_users = options[:old_mentioned_users] || []
+      old_assignees = options[:old_assignees] || []
+
+      if has_changes?(issue, old_labels: old_labels, old_assignees: old_assignees)
         todo_service.mark_pending_todos_as_done(issue, current_user)
       end
 
@@ -26,9 +30,9 @@ module Issues
         create_milestone_note(issue)
       end
 
-      if issue.previous_changes.include?('assignee_id')
-        create_assignee_note(issue)
-        notification_service.reassigned_issue(issue, current_user)
+      if issue.assignees != old_assignees
+        create_assignee_note(issue, old_assignees)
+        notification_service.reassigned_issue(issue, current_user, old_assignees)
         todo_service.reassigned_issue(issue, current_user)
       end
 
diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb
index 1711be7211c..a85b9465c84 100644
--- a/app/services/members/authorized_destroy_service.rb
+++ b/app/services/members/authorized_destroy_service.rb
@@ -26,15 +26,22 @@ module Members
 
     def unassign_issues_and_merge_requests(member)
       if member.is_a?(GroupMember)
-        IssuesFinder.new(user, group_id: member.source_id, assignee_id: member.user_id).
-          execute.
-          update_all(assignee_id: nil)
+        issue_ids = IssuesFinder.new(user, group_id: member.source_id, assignee_id: member.user_id).
+          execute.pluck(:id)
+
+        IssueAssignee.destroy_all(issue_id: issue_ids, user_id: member.user_id)
+
         MergeRequestsFinder.new(user, group_id: member.source_id, assignee_id: member.user_id).
           execute.
           update_all(assignee_id: nil)
       else
         project = member.source
-        project.issues.opened.assigned_to(member.user).update_all(assignee_id: nil)
+
+        IssueAssignee.destroy_all(
+          user_id: member.user_id,
+          issue_id: project.issues.opened.assigned_to(member.user).select(:id)
+        )
+
         project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil)
         member.user.update_cache_counts
       end
diff --git a/app/services/merge_requests/assign_issues_service.rb b/app/services/merge_requests/assign_issues_service.rb
index 066efa1acc3..8c6c4841020 100644
--- a/app/services/merge_requests/assign_issues_service.rb
+++ b/app/services/merge_requests/assign_issues_service.rb
@@ -4,7 +4,7 @@ module MergeRequests
       @assignable_issues ||= begin
         if current_user == merge_request.author
           closes_issues.select do |issue|
-            !issue.is_a?(ExternalIssue) && !issue.assignee_id? && can?(current_user, :admin_issue, issue)
+            !issue.is_a?(ExternalIssue) && !issue.assignees.present? && can?(current_user, :admin_issue, issue)
           end
         else
           []
@@ -14,7 +14,7 @@ module MergeRequests
 
     def execute
       assignable_issues.each do |issue|
-        Issues::UpdateService.new(issue.project, current_user, assignee_id: current_user.id).execute(issue)
+        Issues::UpdateService.new(issue.project, current_user, assignee_ids: [current_user.id]).execute(issue)
       end
 
       {
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 582d5c47b66..3542a41ac83 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -38,6 +38,11 @@ module MergeRequests
 
     private
 
+    def create_assignee_note(merge_request)
+      SystemNoteService.change_assignee(
+        merge_request, merge_request.project, current_user, merge_request.assignee)
+    end
+
     # Returns all origin and fork merge requests from `@project` satisfying passed arguments.
     def merge_requests_for(source_branch, mr_states: [:opened, :reopened])
       MergeRequest
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index ab7fcf3b6e2..5c843a258fb 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -21,7 +21,10 @@ module MergeRequests
       update(merge_request)
     end
 
-    def handle_changes(merge_request, old_labels: [], old_mentioned_users: [])
+    def handle_changes(merge_request, options)
+      old_labels = options[:old_labels] || []
+      old_mentioned_users = options[:old_mentioned_users] || []
+
       if has_changes?(merge_request, old_labels: old_labels)
         todo_service.mark_pending_todos_as_done(merge_request, current_user)
       end
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index 8bb995158de..988bd0a7cdb 100644
--- a/app/services/notification_recipient_service.rb
+++ b/app/services/notification_recipient_service.rb
@@ -19,9 +19,14 @@ class NotificationRecipientService
     # Re-assign is considered as a mention of the new assignee so we add the
     # new assignee to the list of recipients after we rejected users with
     # the "on mention" notification level
-    if [:reassign_merge_request, :reassign_issue].include?(custom_action)
+    case custom_action
+    when :reassign_merge_request
       recipients << previous_assignee if previous_assignee
       recipients << target.assignee
+    when :reassign_issue
+      previous_assignees = Array(previous_assignee)
+      recipients.concat(previous_assignees)
+      recipients.concat(target.assignees)
     end
 
     recipients = reject_muted_users(recipients)
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 6b186263bd1..c65c66d7150 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -66,8 +66,25 @@ class NotificationService
   #  * issue new assignee if their notification level is not Disabled
   #  * users with custom level checked with "reassign issue"
   #
-  def reassigned_issue(issue, current_user)
-    reassign_resource_email(issue, issue.project, current_user, :reassigned_issue_email)
+  def reassigned_issue(issue, current_user, previous_assignees = [])
+    recipients = NotificationRecipientService.new(issue.project).build_recipients(
+      issue,
+      current_user,
+      action: "reassign",
+      previous_assignee: previous_assignees
+    )
+
+    previous_assignee_ids = previous_assignees.map(&:id)
+
+    recipients.each do |recipient|
+      mailer.send(
+        :reassigned_issue_email,
+        recipient.id,
+        issue.id,
+        previous_assignee_ids,
+        current_user.id
+      ).deliver_later
+    end
   end
 
   # When we add labels to an issue we should send an email to:
@@ -367,10 +384,10 @@ class NotificationService
   end
 
   def previous_record(object, attribute)
-    if object && attribute
-      if object.previous_changes.include?(attribute)
-        object.previous_changes[attribute].first
-      end
+    return unless object && attribute
+
+    if object.previous_changes.include?(attribute)
+      object.previous_changes[attribute].first
     end
   end
 end
diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb
index f1bbc7032d5..a7e13648b54 100644
--- a/app/services/slash_commands/interpret_service.rb
+++ b/app/services/slash_commands/interpret_service.rb
@@ -91,32 +91,47 @@ module SlashCommands
     end
 
     desc 'Assign'
-    explanation do |user|
-      "Assigns #{user.to_reference}." if user
+    explanation do |users|
+      "Assigns #{users.map(&:to_reference).to_sentence}." if users.any?
     end
     params '@user'
     condition do
       current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
     parse_params do |assignee_param|
-      extract_references(assignee_param, :user).first ||
-        User.find_by(username: assignee_param)
+      users = extract_references(assignee_param, :user)
+
+      if users.empty?
+        users = User.where(username: assignee_param.split(' ').map(&:strip))
+      end
+
+      users
     end
-    command :assign do |user|
-      @updates[:assignee_id] = user.id if user
+    command :assign do |users|
+      next if users.empty?
+
+      if issuable.is_a?(Issue)
+        @updates[:assignee_ids] = users.map(&:id)
+      else
+        @updates[:assignee_id] = users.last.id
+      end
     end
 
     desc 'Remove assignee'
     explanation do
-      "Removes assignee #{issuable.assignee.to_reference}."
+      "Removes assignee #{issuable.assignees.first.to_reference}."
     end
     condition do
       issuable.persisted? &&
-        issuable.assignee_id? &&
+        issuable.assignees.any? &&
         current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
     command :unassign do
-      @updates[:assignee_id] = nil
+      if issuable.is_a?(Issue)
+        @updates[:assignee_ids] = []
+      else
+        @updates[:assignee_id] = nil
+      end
     end
 
     desc 'Set milestone'
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index c9e25c7aaa2..fb1f56c9cc6 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -49,6 +49,44 @@ module SystemNoteService
     create_note(NoteSummary.new(noteable, project, author, body, action: 'assignee'))
   end
 
+  # Called when the assignees of an Issue is changed or removed
+  #
+  # issue - Issue object
+  # project  - Project owning noteable
+  # author   - User performing the change
+  # assignees - Users being assigned, or nil
+  #
+  # Example Note text:
+  #
+  #   "removed all assignees"
+  #
+  #   "assigned to @user1 additionally to @user2"
+  #
+  #   "assigned to @user1, @user2 and @user3 and unassigned from @user4 and @user5"
+  #
+  #   "assigned to @user1 and @user2"
+  #
+  # Returns the created Note object
+  def change_issue_assignees(issue, project, author, old_assignees)
+    body =
+      if issue.assignees.any? && old_assignees.any?
+        unassigned_users = old_assignees - issue.assignees
+        added_users = issue.assignees.to_a - old_assignees
+
+        text_parts = []
+        text_parts << "assigned to #{added_users.map(&:to_reference).to_sentence}" if added_users.any?
+        text_parts << "unassigned #{unassigned_users.map(&:to_reference).to_sentence}" if unassigned_users.any?
+
+        text_parts.join(' and ')
+      elsif old_assignees.any?
+        "removed all assignees"
+      elsif issue.assignees.any?
+        "assigned to #{issue.assignees.map(&:to_reference).to_sentence}"
+      end
+
+    create_note(NoteSummary.new(issue, project, author, body, action: 'assignee'))
+  end
+
   # Called when one or more labels on a Noteable are added and/or removed
   #
   # noteable       - Noteable object
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index 8ae61694b50..322c6286365 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -251,9 +251,9 @@ class TodoService
   end
 
   def create_assignment_todo(issuable, author)
-    if issuable.assignee
+    if issuable.assignees.any?
       attributes = attributes_for_todo(issuable.project, issuable, author, Todo::ASSIGNED)
-      create_todos(issuable.assignee, attributes)
+      create_todos(issuable.assignees, attributes)
     end
   end