diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index c9f07c140f785202a85b2ac4aa7ee98eae1f303b..3eb8cfcca9be5d581eb1e289e71ed3039aff64fc 100644 --- a/app/services/notification_recipient_service.rb +++ b/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 diff --git a/changelogs/unreleased/38862-email-notifications-not-sent-as-expected.yml b/changelogs/unreleased/38862-email-notifications-not-sent-as-expected.yml new file mode 100644 index 0000000000000000000000000000000000000000..6b1b309ab144c2e4d145e01b42c1f6faf6922a15 --- /dev/null +++ b/changelogs/unreleased/38862-email-notifications-not-sent-as-expected.yml @@ -0,0 +1,6 @@ +--- +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 diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index db5de572b6d7a8916a355a0dc93c30dd0fff4f8f..43e2643f709a654b90005fb307d9af8cd585618d 100644 --- a/spec/services/notification_service_spec.rb +++ b/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