Merge branch '38862-email-notifications-not-sent-as-expected' into 'master'

app/services/notification_recipient_service.rb

@@ -98,6 +98,12 @@ module NotificationRecipientService
         self << [target.participants(user), :participating]
       end
 
+      def add_mentions(user, target:)
+        return unless target.respond_to?(:mentioned_users)
+
+        self << [target.mentioned_users(user), :mention]
+      end
+
       # Get project/group users with CUSTOM notification level
       def add_custom_notifications
         user_ids = []
@@ -227,6 +233,11 @@ module NotificationRecipientService
         add_subscribed_users
 
         if [:new_issue, :new_merge_request].include?(custom_action)
+          # These will all be participants as well, but adding with the :mention
+          # type ensures that users with the mention notification level will
+          # receive them, too.
+          add_mentions(current_user, target: target)
+
           add_labels_subscribers
         end
       end
@@ -263,7 +274,7 @@ module NotificationRecipientService
       def build!
         # Add all users participating in the thread (author, assignee, comment authors)
         add_participants(note.author)
-        self << [note.mentioned_users, :mention]
+        add_mentions(note.author, target: note)
 
         unless note.for_personal_snippet?
           # Merge project watchers

changelogs/unreleased/38862-email-notifications-not-sent-as-expected.yml

+---
+title: Fix sending notification emails to users with the mention level set who were
+  mentioned in an issue or merge request description
+merge_request:
+author:
+type: fixed

spec/services/notification_service_spec.rb

@@ -12,6 +12,8 @@ describe NotificationService, :mailer do
 
   shared_examples 'notifications for new mentions' do
     def send_notifications(*new_mentions)
+      mentionable.description = new_mentions.map(&:to_reference).join(' ')
+
       notification.send(notification_method, mentionable, new_mentions, @u_disabled)
     end
 
@@ -20,13 +22,13 @@ describe NotificationService, :mailer do
       should_not_email_anyone
     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)
+    it 'emails new mentions with a watch level higher than mention' do
+      send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned)
+      should_only_email(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned)
     end
 
-    it 'does not email new mentions with a watch level equal to or less than participant' do
-      send_notifications(@u_participating, @u_mentioned)
+    it 'does not email new mentions with a watch level equal to or less than mention' do
+      send_notifications(@u_disabled)
       should_not_email_anyone
     end
   end
@@ -509,6 +511,14 @@ describe NotificationService, :mailer do
         should_not_email(issue.assignees.first)
       end
 
+      it "emails any mentioned users with the mention level" do
+        issue.description = @u_mentioned.to_reference
+
+        notification.new_issue(issue, @u_disabled)
+
+        should_email(@u_mentioned)
+      end
+
       it "emails the author if they've opted into notifications about their activity" do
         issue.author.notified_of_own_activity = true
 
@@ -900,6 +910,14 @@ describe NotificationService, :mailer do
         should_not_email(@u_lazy_participant)
       end
 
+      it "emails any mentioned users with the mention level" do
+        merge_request.description = @u_mentioned.to_reference
+
+        notification.new_merge_request(merge_request, @u_disabled)
+
+        should_email(@u_mentioned)
+      end
+
       it "emails the author if they've opted into notifications about their activity" do
         merge_request.author.notified_of_own_activity = true