Commit e7c64c9d authored by Sean McGivern's avatar Sean McGivern

Fix watch level for mentions in description

For a user with the mention notification level set, the type of their
corresponding NotificationRecipient must be :mention for them to receive an
email.

We set this correctly on notes, but we weren't adding it on new issues or MRs -
perhaps because these users are also participants. But the type of the
NotificationRecipient in that case would be :participant, not mention, so we
have to add the mentioned users manually when creating an issue or MR.

When editing an issue or MR, and there are newly-mentioned users to email, we
still use the :new_issue and :new_merge_request actions, so this works for that
case as well.
parent e0f84130
...@@ -98,6 +98,12 @@ module NotificationRecipientService ...@@ -98,6 +98,12 @@ module NotificationRecipientService
self << [target.participants(user), :participating] self << [target.participants(user), :participating]
end 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 # Get project/group users with CUSTOM notification level
def add_custom_notifications def add_custom_notifications
user_ids = [] user_ids = []
...@@ -227,6 +233,11 @@ module NotificationRecipientService ...@@ -227,6 +233,11 @@ module NotificationRecipientService
add_subscribed_users add_subscribed_users
if [:new_issue, :new_merge_request].include?(custom_action) 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 add_labels_subscribers
end end
end end
...@@ -263,7 +274,7 @@ module NotificationRecipientService ...@@ -263,7 +274,7 @@ module NotificationRecipientService
def build! def build!
# Add all users participating in the thread (author, assignee, comment authors) # Add all users participating in the thread (author, assignee, comment authors)
add_participants(note.author) add_participants(note.author)
self << [note.mentioned_users, :mention] add_mentions(note.author, target: note)
unless note.for_personal_snippet? unless note.for_personal_snippet?
# Merge project watchers # Merge project watchers
......
---
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
...@@ -12,6 +12,8 @@ describe NotificationService, :mailer do ...@@ -12,6 +12,8 @@ describe NotificationService, :mailer do
shared_examples 'notifications for new mentions' do shared_examples 'notifications for new mentions' do
def send_notifications(*new_mentions) def send_notifications(*new_mentions)
mentionable.description = new_mentions.map(&:to_reference).join(' ')
notification.send(notification_method, mentionable, new_mentions, @u_disabled) notification.send(notification_method, mentionable, new_mentions, @u_disabled)
end end
...@@ -20,13 +22,13 @@ describe NotificationService, :mailer do ...@@ -20,13 +22,13 @@ describe NotificationService, :mailer do
should_not_email_anyone should_not_email_anyone
end end
it 'emails new mentions with a watch level higher than participant' do it 'emails new mentions with a watch level higher than mention' do
send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global) send_notifications(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned)
should_only_email(@u_watcher, @u_participant_mentioned, @u_custom_global) should_only_email(@u_watcher, @u_participant_mentioned, @u_custom_global, @u_mentioned)
end end
it 'does not email new mentions with a watch level equal to or less than participant' do it 'does not email new mentions with a watch level equal to or less than mention' do
send_notifications(@u_participating, @u_mentioned) send_notifications(@u_disabled)
should_not_email_anyone should_not_email_anyone
end end
end end
...@@ -509,6 +511,14 @@ describe NotificationService, :mailer do ...@@ -509,6 +511,14 @@ describe NotificationService, :mailer do
should_not_email(issue.assignees.first) should_not_email(issue.assignees.first)
end 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 it "emails the author if they've opted into notifications about their activity" do
issue.author.notified_of_own_activity = true issue.author.notified_of_own_activity = true
...@@ -900,6 +910,14 @@ describe NotificationService, :mailer do ...@@ -900,6 +910,14 @@ describe NotificationService, :mailer do
should_not_email(@u_lazy_participant) should_not_email(@u_lazy_participant)
end 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 it "emails the author if they've opted into notifications about their activity" do
merge_request.author.notified_of_own_activity = true merge_request.author.notified_of_own_activity = true
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment