Create directly_addressed Todos when mentioned in beginning of a line

1 files changed, 205 insertions, 1 deletions

diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 13d584a8975..4320365ab57 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -9,7 +9,9 @@ describe TodoService, services: true do
   let(:admin) { create(:admin) }
   let(:john_doe) { create(:user) }
   let(:project) { create(:project) }
-  let(:mentions) { [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') }
+  let(:mentions) { 'FYI: ' + [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') }
+  let(:directly_addressed) { [author, assignee, john_doe, member, guest, non_member, admin].map(&:to_reference).join(' ') }
+  let(:directly_addressed_and_mentioned) { member.to_reference + ", what do you think? cc: " + [guest, admin].map(&:to_reference).join(' ') }
   let(:service) { described_class.new }
 
   before do
@@ -21,8 +23,10 @@ describe TodoService, services: true do
 
   describe 'Issues' do
     let(:issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
+    let(:addressed_issue) { create(:issue, project: project, assignee: john_doe, author: author, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
     let(:unassigned_issue) { create(:issue, project: project, assignee: nil) }
     let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) }
+    let(:addressed_confident_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: directly_addressed) }
 
     describe '#new_issue' do
       it 'creates a todo if assigned' do
@@ -52,6 +56,26 @@ describe TodoService, services: true do
         should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
       end
 
+      it 'creates a directly addressed todo for each valid addressed user' do
+        service.new_issue(addressed_issue, author)
+
+        should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+      end
+
+      it 'creates correct todos for each valid user based on the type of mention' do
+        issue.update(description: directly_addressed_and_mentioned)
+
+        service.new_issue(issue, author)
+
+        should_create_todo(user: member, target: issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: admin, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
+      end
+
       it 'does not create todo if user can not see the issue when issue is confidential' do
         service.new_issue(confidential_issue, john_doe)
 
@@ -63,6 +87,17 @@ describe TodoService, services: true do
         should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
 
+      it 'does not create directly addressed todo if user cannot see the issue when issue is confidential' do
+        service.new_issue(addressed_confident_issue, john_doe)
+
+        should_create_todo(user: assignee, target: addressed_confident_issue, author: john_doe, action: Todo::ASSIGNED)
+        should_create_todo(user: author, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: member, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: admin, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: guest, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: john_doe, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+      end
+
       context 'when a private group is mentioned' do
         let(:group) { create :group, :private }
         let(:project) { create :project, :private, group: group }
@@ -94,12 +129,38 @@ describe TodoService, services: true do
         should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
       end
 
+      it 'creates a todo for each valid user based on the type of mention' do
+        issue.update(description: directly_addressed_and_mentioned)
+
+        service.update_issue(issue, author)
+
+        should_create_todo(user: member, target: issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: guest, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: admin, target: issue, action: Todo::MENTIONED)
+      end
+
+      it 'creates a directly addressed todo for each valid addressed user' do
+        service.update_issue(addressed_issue, author)
+
+        should_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: guest, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+      end
+
       it 'does not create a todo if user was already mentioned' do
         create(:todo, :mentioned, user: member, project: project, target: issue, author: author)
 
         expect { service.update_issue(issue, author) }.not_to change(member.todos, :count)
       end
 
+      it 'does not create a directly addressed todo if user was already mentioned or addressed' do
+        create(:todo, :directly_addressed, user: member, project: project, target: addressed_issue, author: author)
+
+        expect { service.update_issue(addressed_issue, author) }.not_to change(member.todos, :count)
+      end
+
       it 'does not create todo if user can not see the issue when issue is confidential' do
         service.update_issue(confidential_issue, john_doe)
 
@@ -111,6 +172,17 @@ describe TodoService, services: true do
         should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
 
+      it 'does not create a directly addressed todo if user can not see the issue when issue is confidential' do
+        service.update_issue(addressed_confident_issue, john_doe)
+
+        should_create_todo(user: author, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: assignee, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: member, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: admin, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: guest, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: john_doe, target: addressed_confident_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED)
+      end
+
       context 'issues with a task list' do
         it 'does not create todo when tasks are marked as completed' do
           issue.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
@@ -125,6 +197,19 @@ describe TodoService, services: true do
           should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
         end
 
+        it 'does not create directly addressed todo when tasks are marked as completed' do
+          addressed_issue.update(description: "#{directly_addressed}\n- [x] Task 1\n- [x] Task 2\n")
+
+          service.update_issue(addressed_issue, author)
+
+          should_not_create_todo(user: admin, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: assignee, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: author, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: john_doe, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: non_member, target: addressed_issue, action: Todo::DIRECTLY_ADDRESSED)
+        end
+
         it 'does not raise an error when description not change' do
           issue.update(title: 'Sample')
 
@@ -244,8 +329,11 @@ describe TodoService, services: true do
       let!(:second_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) }
       let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee) }
       let(:note) { create(:note, project: project, noteable: issue, author: john_doe, note: mentions) }
+      let(:addressed_note) { create(:note, project: project, noteable: issue, author: john_doe, note: directly_addressed) }
       let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) }
+      let(:addressed_note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: directly_addressed) }
       let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: mentions) }
+      let(:addressed_note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: directly_addressed) }
       let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) }
       let(:system_note) { create(:system_note, project: project, noteable: issue) }
 
@@ -276,6 +364,26 @@ describe TodoService, services: true do
         should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
       end
 
+      it 'creates a todo for each valid user based on the type of mention' do
+        note.update(note: directly_addressed_and_mentioned)
+
+        service.new_note(note, john_doe)
+
+        should_create_todo(user: member, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: note)
+        should_create_todo(user: admin, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
+        should_create_todo(user: guest, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
+      end
+
+      it 'creates a directly addressed todo for each valid addressed user' do
+        service.new_note(addressed_note, john_doe)
+
+        should_create_todo(user: member, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note)
+        should_create_todo(user: guest, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note)
+        should_create_todo(user: author, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note)
+        should_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note)
+        should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note)
+      end
+
       it 'does not create todo if user can not see the issue when leaving a note on a confidential issue' do
         service.new_note(note_on_confidential_issue, john_doe)
 
@@ -287,6 +395,17 @@ describe TodoService, services: true do
         should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
       end
 
+      it 'does not create a directly addressed todo if user can not see the issue when leaving a note on a confidential issue' do
+        service.new_note(addressed_note_on_confidential_issue, john_doe)
+
+        should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue)
+        should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue)
+        should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue)
+        should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue)
+        should_not_create_todo(user: guest, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue)
+        should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue)
+      end
+
       it 'creates a todo for each valid mentioned user when leaving a note on commit' do
         service.new_note(note_on_commit, john_doe)
 
@@ -296,6 +415,15 @@ describe TodoService, services: true do
         should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
       end
 
+      it 'creates a directly addressed todo for each valid mentioned user when leaving a note on commit' do
+        service.new_note(addressed_note_on_commit, john_doe)
+
+        should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
+        should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
+        should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
+        should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
+      end
+
       it 'does not create todo when leaving a note on snippet' do
         should_not_create_any_todo { service.new_note(note_on_project_snippet, john_doe) }
       end
@@ -324,6 +452,7 @@ describe TodoService, services: true do
 
   describe 'Merge Requests' do
     let(:mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: "- [ ] Task 1\n- [ ] Task 2 #{mentions}") }
+    let(:addressed_mr_assigned) { create(:merge_request, source_project: project, author: author, assignee: john_doe, description: "#{directly_addressed}\n- [ ] Task 1\n- [ ] Task 2") }
     let(:mr_unassigned) { create(:merge_request, source_project: project, author: author, assignee: nil) }
 
     describe '#new_merge_request' do
@@ -350,6 +479,25 @@ describe TodoService, services: true do
         should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
       end
+
+      it 'creates a todo for each valid user based on the type of mention' do
+        mr_assigned.update(description: directly_addressed_and_mentioned)
+
+        service.new_merge_request(mr_assigned, author)
+
+        should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED)
+      end
+
+      it 'creates a directly addressed todo for each valid addressed user' do
+        service.new_merge_request(addressed_mr_assigned, author)
+
+        should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+      end
     end
 
     describe '#update_merge_request' do
@@ -363,12 +511,37 @@ describe TodoService, services: true do
         should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
       end
 
+      it 'creates a todo for each valid user based on the type of mention' do
+        mr_assigned.update(description: directly_addressed_and_mentioned)
+
+        service.update_merge_request(mr_assigned, author)
+
+        should_create_todo(user: member, target: mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: admin, target: mr_assigned, action: Todo::MENTIONED)
+      end
+
+      it 'creates a directly addressed todo for each valid addressed user' do
+        service.update_merge_request(addressed_mr_assigned, author)
+
+        should_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+      end
+
       it 'does not create a todo if user was already mentioned' do
         create(:todo, :mentioned, user: member, project: project, target: mr_assigned, author: author)
 
         expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count)
       end
 
+      it 'does not create a directly addressed todo if user was already mentioned or addressed' do
+        create(:todo, :directly_addressed, user: member, project: project, target: addressed_mr_assigned, author: author)
+
+        expect{ service.update_merge_request(addressed_mr_assigned, author) }.not_to change(member.todos, :count)
+      end
+
       context 'with a task list' do
         it 'does not create todo when tasks are marked as completed' do
           mr_assigned.update(description: "- [x] Task 1\n- [X] Task 2 #{mentions}")
@@ -384,6 +557,20 @@ describe TodoService, services: true do
           should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
         end
 
+        it 'does not create directly addressed todo when tasks are marked as completed' do
+          addressed_mr_assigned.update(description: "#{directly_addressed}\n- [x] Task 1\n- [X] Task 2")
+
+          service.update_merge_request(addressed_mr_assigned, author)
+
+          should_not_create_todo(user: admin, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: assignee, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: author, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: john_doe, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: non_member, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+          should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+        end
+
         it 'does not raise an error when description not change' do
           mr_assigned.update(title: 'Sample')
 
@@ -436,6 +623,11 @@ describe TodoService, services: true do
         service.reassigned_merge_request(mr_assigned, author)
         should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
       end
+
+      it 'does not create a directly addressed todo for guests' do
+        service.reassigned_merge_request(addressed_mr_assigned, author)
+        should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+      end
     end
 
     describe '#merge_merge_request' do
@@ -452,6 +644,11 @@ describe TodoService, services: true do
         service.merge_merge_request(mr_assigned, john_doe)
         should_not_create_todo(user: guest, target: mr_assigned, action: Todo::MENTIONED)
       end
+
+      it 'does not create directly addressed todo for guests' do
+        service.merge_merge_request(addressed_mr_assigned, john_doe)
+        should_not_create_todo(user: guest, target: addressed_mr_assigned, action: Todo::DIRECTLY_ADDRESSED)
+      end
     end
 
     describe '#new_award_emoji' do
@@ -509,6 +706,7 @@ describe TodoService, services: true do
     describe '#new_note' do
       let(:mention) { john_doe.to_reference }
       let(:diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") }
+      let(:addressed_diff_note_on_merge_request) { create(:diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "#{mention}, hey!") }
       let(:legacy_diff_note_on_merge_request) { create(:legacy_diff_note_on_merge_request, project: project, noteable: mr_unassigned, author: author, note: "Hey #{mention}") }
 
       it 'creates a todo for mentioned user on new diff note' do
@@ -517,6 +715,12 @@ describe TodoService, services: true do
         should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::MENTIONED, note: diff_note_on_merge_request)
       end
 
+      it 'creates a directly addressed todo for addressed user on new diff note' do
+        service.new_note(addressed_diff_note_on_merge_request, author)
+
+        should_create_todo(user: john_doe, target: mr_unassigned, author: author, action: Todo::DIRECTLY_ADDRESSED, note: addressed_diff_note_on_merge_request)
+      end
+
       it 'creates a todo for mentioned user on legacy diff note' do
         service.new_note(legacy_diff_note_on_merge_request, author)