Merge branch '3642-assignees-todo-fix' into 'master'

Don't create todos for old issue assignees

Closes #3642

See merge request gitlab-org/gitlab-ee!3064

app/services/issues/update_service.rb

@@ -34,7 +34,7 @@ module Issues
       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)
+        todo_service.reassigned_issue(issue, current_user, old_assignees)
       end
 
       if issue.previous_changes.include?('confidential')

app/services/todo_service.rb

@@ -43,8 +43,8 @@ class TodoService
   #
   #  * create a pending todo for new assignee if issue is assigned
   #
-  def reassigned_issue(issue, current_user)
-    create_assignment_todo(issue, current_user)
+  def reassigned_issue(issue, current_user, old_assignees = [])
+    create_assignment_todo(issue, current_user, old_assignees)
   end
 
   # When create a merge request we should:
@@ -267,10 +267,11 @@ class TodoService
     create_mention_todos(project, target, author, note, skip_users)
   end
 
-  def create_assignment_todo(issuable, author)
+  def create_assignment_todo(issuable, author, old_assignees = [])
     if issuable.assignees.any?
+      assignees = issuable.assignees - old_assignees
       attributes = attributes_for_todo(issuable.project, issuable, author, Todo::ASSIGNED)
-      create_todos(issuable.assignees, attributes)
+      create_todos(assignees, attributes)
     end
   end
 

changelogs/unreleased-ee/3642-assignees-todo-fix.yml

+---
+title: Don't create todos for old issue assignees
+merge_request:
+author:
+type: fixed

spec/services/issues/update_service_spec.rb

@@ -257,6 +257,30 @@ describe Issues::UpdateService, :mailer do
         end
       end
 
+      context 'when a new assignee added' do
+        subject { update_issue(assignees: issue.assignees + [user2]) }
+
+        it 'creates only 1 new todo' do
+          expect { subject }.to change { Todo.count }.by(1)
+        end
+
+        it 'creates a todo for new assignee' do
+          subject
+
+          attributes = {
+            project: project,
+            author: user,
+            user: user2,
+            target_id: issue.id,
+            target_type: issue.class.name,
+            action: Todo::ASSIGNED,
+            state: :pending
+          }
+
+          expect(Todo.where(attributes).count).to eq(1)
+        end
+      end
+
       context 'when the milestone change' do
         it 'marks todos as done' do
           update_issue(milestone: create(:milestone))