summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2017-12-01 15:47:28 +0000
committerSean McGivern <sean@gitlab.com>2017-12-04 09:25:39 +0000
commit4b1dd6c02fc2fe2b66a95a05d39c709a4f2922c8 (patch)
tree7b06152657fdc65157802dd00dc48b57875f5557
parente0f84130567dc34edf1ae75fcf595e24991d2fa9 (diff)
downloadgitlab-ce-38862-email-notifications-not-sent-as-expected.tar.gz
Fix watch level for mentions in description38862-email-notifications-not-sent-as-expected
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.
-rw-r--r--app/services/notification_recipient_service.rb11
-rw-r--r--changelogs/unreleased/38862-email-notifications-not-sent-as-expected.yml6
-rw-r--r--spec/services/notification_service_spec.rb28
3 files changed, 40 insertions, 5 deletions
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb
index c9f07c140f7..8caf190f65e 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)
+ 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)
+
add_labels_subscribers
end
end
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 00000000000..6b1b309ab14
--- /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 db5de572b6d..43e2643f709 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