Merge branch 'fix-mentions-on-confidential-issues-for-non-members' into 'master'

Mentions on confidential issues doesn't create todos for non-members

Closes #14569 

See merge request !3374

1 files changed, 56 insertions, 21 deletions

diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index b4728807b8b..82b7fbfa816 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -2,22 +2,25 @@ require 'spec_helper'
 
 describe TodoService, services: true do
   let(:author) { create(:user) }
-  let(:john_doe) { create(:user, username: 'john_doe') }
-  let(:michael) { create(:user, username: 'michael') }
-  let(:stranger) { create(:user, username: 'stranger') }
+  let(:assignee) { create(:user) }
+  let(:non_member) { create(:user) }
+  let(:member) { create(:user) }
+  let(:admin) { create(:admin) }
+  let(:john_doe) { create(:user) }
   let(:project) { create(:project) }
-  let(:mentions) { [author.to_reference, john_doe.to_reference, michael.to_reference, stranger.to_reference].join(' ') }
+  let(:mentions) { [author, assignee, john_doe, member, non_member, admin].map(&:to_reference).join(' ') }
   let(:service) { described_class.new }
 
   before do
     project.team << [author, :developer]
+    project.team << [member, :developer]
     project.team << [john_doe, :developer]
-    project.team << [michael, :developer]
   end
 
   describe 'Issues' do
     let(:issue) { create(:issue, project: project, assignee: john_doe, author: author, description: mentions) }
     let(:unassigned_issue) { create(:issue, project: project, assignee: nil) }
+    let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignee: assignee, description: mentions) }
 
     describe '#new_issue' do
       it 'creates a todo if assigned' do
@@ -37,10 +40,20 @@ describe TodoService, services: true do
       it 'creates a todo for each valid mentioned user' do
         service.new_issue(issue, author)
 
-        should_create_todo(user: michael, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
-        should_not_create_todo(user: stranger, target: issue, action: Todo::MENTIONED)
+        should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
+      end
+
+      it 'does not create todo for non project members when issue is confidential' do
+        service.new_issue(confidential_issue, john_doe)
+
+        should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::ASSIGNED)
+        should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
     end
 
@@ -48,16 +61,26 @@ describe TodoService, services: true do
       it 'creates a todo for each valid mentioned user' do
         service.update_issue(issue, author)
 
-        should_create_todo(user: michael, target: issue, action: Todo::MENTIONED)
+        should_create_todo(user: member, target: issue, action: Todo::MENTIONED)
         should_create_todo(user: john_doe, target: issue, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: issue, action: Todo::MENTIONED)
-        should_not_create_todo(user: stranger, target: issue, action: Todo::MENTIONED)
+        should_not_create_todo(user: non_member, target: issue, action: Todo::MENTIONED)
       end
 
       it 'does not create a todo if user was already mentioned' do
-        create(:todo, :mentioned, user: michael, project: project, target: issue, author: author)
+        create(:todo, :mentioned, user: member, project: project, target: issue, author: author)
 
-        expect { service.update_issue(issue, author) }.not_to change(michael.todos, :count)
+        expect { service.update_issue(issue, author) }.not_to change(member.todos, :count)
+      end
+
+      it 'does not create todo for non project members when issue is confidential' do
+        service.update_issue(confidential_issue, john_doe)
+
+        should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
+        should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED)
       end
     end
 
@@ -109,8 +132,10 @@ describe TodoService, services: true do
     describe '#new_note' do
       let!(:first_todo) { create(:todo, :assigned, user: john_doe, project: project, target: issue, author: author) }
       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(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) }
+      let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: mentions) }
       let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) }
       let(:award_note) { create(:note, :award, project: project, noteable: issue, author: john_doe, note: 'thumbsup') }
       let(:system_note) { create(:system_note, project: project, noteable: issue) }
@@ -142,19 +167,29 @@ describe TodoService, services: true do
       it 'creates a todo for each valid mentioned user' do
         service.new_note(note, john_doe)
 
-        should_create_todo(user: michael, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
+        should_create_todo(user: member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_create_todo(user: author, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
         should_not_create_todo(user: john_doe, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
-        should_not_create_todo(user: stranger, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
+        should_not_create_todo(user: non_member, target: issue, author: john_doe, action: Todo::MENTIONED, note: note)
+      end
+
+      it 'does not create todo for non project members when leaving a note on a confidential issue' do
+        service.new_note(note_on_confidential_issue, john_doe)
+
+        should_create_todo(user: author, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
+        should_create_todo(user: assignee, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
+        should_create_todo(user: member, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
+        should_create_todo(user: admin, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: note_on_confidential_issue)
+        should_not_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::MENTIONED, note: 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)
 
-        should_create_todo(user: michael, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
+        should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
         should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
         should_not_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
-        should_not_create_todo(user: stranger, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
+        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 'does not create todo when leaving a note on snippet' do
@@ -185,10 +220,10 @@ describe TodoService, services: true do
       it 'creates a todo for each valid mentioned user' do
         service.new_merge_request(mr_assigned, author)
 
-        should_create_todo(user: michael, target: mr_assigned, action: Todo::MENTIONED)
+        should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
-        should_not_create_todo(user: stranger, target: mr_assigned, action: Todo::MENTIONED)
+        should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
       end
     end
 
@@ -196,16 +231,16 @@ describe TodoService, services: true do
       it 'creates a todo for each valid mentioned user' do
         service.update_merge_request(mr_assigned, author)
 
-        should_create_todo(user: michael, target: mr_assigned, action: Todo::MENTIONED)
+        should_create_todo(user: member, target: mr_assigned, action: Todo::MENTIONED)
         should_create_todo(user: john_doe, target: mr_assigned, action: Todo::MENTIONED)
         should_not_create_todo(user: author, target: mr_assigned, action: Todo::MENTIONED)
-        should_not_create_todo(user: stranger, target: mr_assigned, action: Todo::MENTIONED)
+        should_not_create_todo(user: non_member, target: mr_assigned, action: Todo::MENTIONED)
       end
 
       it 'does not create a todo if user was already mentioned' do
-        create(:todo, :mentioned, user: michael, project: project, target: mr_assigned, author: author)
+        create(:todo, :mentioned, user: member, project: project, target: mr_assigned, author: author)
 
-        expect { service.update_merge_request(mr_assigned, author) }.not_to change(michael.todos, :count)
+        expect { service.update_merge_request(mr_assigned, author) }.not_to change(member.todos, :count)
       end
     end