Merge branch '2451-fix-mentions-in-issue-updates' into 'master'

Send notification emails when users are newly mentioned in issue or MR edits

## What does this MR do?

Introduces "new mention in issue" and "new mention in MR" email notifications.  Editing a Mentionable title or description and adding a mention to a user who was not previously mentioned will now send them a notification email, following usual permissions for doing so.

## Why was this MR needed?

Issues & MRs may be edited to include mentions to new people. We don't currently send out email notifications of these edits to anyone, although they do create TODOs. This brings email notifications into parity with TODOs.

## What are the relevant issue numbers?

Closes #2451

See merge request !5800

5 files changed, 90 insertions, 0 deletions

diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 088c3d48bf7..0313f424463 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -319,5 +319,10 @@ describe Issues::UpdateService, services: true do
         end
       end
     end
+
+    context 'updating mentions' do
+      let(:mentionable) { issue }
+      include_examples 'updating mentions', Issues::UpdateService
+    end
   end
 end
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 283a336afd9..6dfeb581975 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -226,6 +226,11 @@ describe MergeRequests::UpdateService, services: true do
       end
     end
 
+    context 'updating mentions' do
+      let(:mentionable) { merge_request }
+      include_examples 'updating mentions', MergeRequests::UpdateService
+    end
+
     context 'when MergeRequest has tasks' do
       before { update_merge_request({ description: "- [ ] Task 1\n- [ ] Task 2" }) }
 
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 92b441c28ca..62c97e09288 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -9,6 +9,28 @@ describe NotificationService, services: true do
     end
   end
 
+  shared_examples 'notifications for new mentions' do
+    def send_notifications(*new_mentions)
+      reset_delivered_emails!
+      notification.send(notification_method, mentionable, new_mentions, @u_disabled)
+    end
+
+    it 'sends no emails when no new mentions are present' do
+      send_notifications
+      expect(ActionMailer::Base.deliveries).to be_empty
+    end
+
+    it 'emails new mentions with a watch level higher than participant' do
+      send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global)
+      should_only_email(@u_watcher, @u_participant_mentioned, @u_custom_global)
+    end
+
+    it 'does not email new mentions with a watch level equal to or less than participant' do
+      send_notifications(@u_participating, @u_mentioned)
+      expect(ActionMailer::Base.deliveries).to be_empty
+    end
+  end
+
   describe 'Keys' do
     describe '#new_key' do
       let!(:key) { create(:personal_key) }
@@ -399,6 +421,13 @@ describe NotificationService, services: true do
       end
     end
 
+    describe '#new_mentions_in_issue' do
+      let(:notification_method) { :new_mentions_in_issue }
+      let(:mentionable) { issue }
+
+      include_examples 'notifications for new mentions'
+    end
+
     describe '#reassigned_issue' do
       before do
         update_custom_notification(:reassign_issue, @u_guest_custom, project)
@@ -700,6 +729,8 @@ describe NotificationService, services: true do
     before do
       build_team(merge_request.target_project)
       add_users_with_subscription(merge_request.target_project, merge_request)
+      update_custom_notification(:new_merge_request, @u_guest_custom, project)
+      update_custom_notification(:new_merge_request, @u_custom_global)
       ActionMailer::Base.deliveries.clear
     end
 
@@ -763,6 +794,13 @@ describe NotificationService, services: true do
       end
     end
 
+    describe '#new_mentions_in_merge_request' do
+      let(:notification_method) { :new_mentions_in_merge_request }
+      let(:mentionable) { merge_request }
+
+      include_examples 'notifications for new mentions'
+    end
+
     describe '#reassigned_merge_request' do
       before do
         update_custom_notification(:reassign_merge_request, @u_guest_custom, project)
diff --git a/spec/support/email_helpers.rb b/spec/support/email_helpers.rb
index a85ab22ce36..0bfc4685532 100644
--- a/spec/support/email_helpers.rb
+++ b/spec/support/email_helpers.rb
@@ -3,6 +3,16 @@ module EmailHelpers
     ActionMailer::Base.deliveries.map(&:to).flatten.count(user.email) == 1
   end
 
+  def reset_delivered_emails!
+    ActionMailer::Base.deliveries.clear
+  end
+
+  def should_only_email(*users)
+    users.each {|user| should_email(user) }
+    recipients = ActionMailer::Base.deliveries.flat_map(&:to)
+    expect(recipients.count).to eq(users.count)
+  end
+
   def should_email(user)
     expect(sent_to_user?(user)).to be_truthy
   end
diff --git a/spec/support/updating_mentions_shared_examples.rb b/spec/support/updating_mentions_shared_examples.rb
new file mode 100644
index 00000000000..e0c59a5c280
--- /dev/null
+++ b/spec/support/updating_mentions_shared_examples.rb
@@ -0,0 +1,32 @@
+RSpec.shared_examples 'updating mentions' do |service_class|
+  let(:mentioned_user) { create(:user) }
+  let(:service_class) { service_class }
+
+  before { project.team << [mentioned_user, :developer] }
+
+  def update_mentionable(opts)
+    reset_delivered_emails!
+
+    perform_enqueued_jobs do
+      service_class.new(project, user, opts).execute(mentionable)
+    end
+
+    mentionable.reload
+  end
+
+  context 'in title' do
+    before { update_mentionable(title: mentioned_user.to_reference) }
+
+    it 'emails only the newly-mentioned user' do
+      should_only_email(mentioned_user)
+    end
+  end
+
+  context 'in description' do
+    before { update_mentionable(description: mentioned_user.to_reference) }
+
+    it 'emails only the newly-mentioned user' do
+      should_only_email(mentioned_user)
+    end
+  end
+end