Merge branch 'multiple_assignees_review' into multiple-assignees-fe-sidebar

app/services/issuable_base_service.rb

@@ -48,13 +48,8 @@ class IssuableBaseService < BaseService
       params.delete(:add_label_ids)
       params.delete(:remove_label_ids)
       params.delete(:label_ids)
-
-      if issuable.is_a?(Issue)
-        params.delete(:assignee_ids)
-      else
-        params.delete(:assignee_id)
-      end
-
+      params.delete(:assignee_ids)
+      params.delete(:assignee_id)
       params.delete(:due_date)
     end
 
@@ -280,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|
@@ -289,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: [])

app/services/issues/base_service.rb

@@ -9,9 +9,9 @@ module Issues
 
     private
 
-    def create_assignee_note(issue)
+    def create_assignee_note(issue, old_assignees)
       SystemNoteService.change_issue_assignees(
-        issue, issue.project, current_user, issue.assignees)
+        issue, issue.project, current_user, old_assignees)
     end
 
     def execute_hooks(issue, action = 'open')
@@ -24,7 +24,15 @@ module Issues
     def filter_assignee(issuable)
       return if params[:assignee_ids].to_a.empty?
 
-      params[:assignee_ids] = params[:assignee_ids].select{ |assignee_id| assignee_can_read?(issuable, assignee_id)}
+      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

app/services/issues/update_service.rb

@@ -17,7 +17,7 @@ module Issues
       old_mentioned_users = options[:old_mentioned_users] || []
       old_assignees = options[:old_assignees] || []
 
-      if has_changes?(issue, old_labels: old_labels)
+      if has_changes?(issue, old_labels: old_labels, old_assignees: old_assignees)
         todo_service.mark_pending_todos_as_done(issue, current_user)
       end
 
@@ -31,7 +31,7 @@ module Issues
       end
 
       if issue.assignees != old_assignees
-        create_assignee_note(issue)
+        create_assignee_note(issue, old_assignees)
         notification_service.reassigned_issue(issue, current_user, old_assignees)
         todo_service.reassigned_issue(issue, current_user)
       end

app/services/system_note_service.rb

@@ -67,18 +67,18 @@ module SystemNoteService
   #   "assigned to @user1 and @user2"
   #
   # Returns the created Note object
-  def change_issue_assignees(issue, project, author, assignees)
+  def change_issue_assignees(issue, project, author, old_assignees)
     # TODO: basic implementation, should be improved before merging the MR
     body =
-      if issue.assignees.any? && assignees.any?
-        unassigned_users = issue.assignees - assignees
-        added_users = assignees - issue.assignees
+      if issue.assignees.any? && old_assignees.any?
+        unassigned_users = old_assignees - issue.assignees
+        added_users = issue.assignees.to_a - old_assignees
 
         "assigned to #{added_users.map(&:to_reference).to_sentence} and unassigned #{unassigned_users.map(&:to_reference).to_sentence}"
-      elsif issue.assignees.any?
+      elsif old_assignees.any?
         "removed all assignees"
-      elsif assignees.any?
-        "assigned to #{assignees.map(&:to_reference).to_sentence}"
+      elsif issue.assignees.any?
+        "assigned to #{issue.assignees.map(&:to_reference).to_sentence}"
       end
 
     create_note(noteable: issue, project: project, author: author, note: body)

spec/services/issues/update_service_spec.rb

@@ -165,7 +165,7 @@ describe Issues::UpdateService, services: true do
         non_member        = create(:user)
         original_assignees = issue.assignees
 
-        update_issue(assignee_ids: non_member.id.to_s)
+        update_issue(assignee_ids: [non_member.id])
 
         expect(issue.reload.assignees).to eq(original_assignees)
       end