Commit 4cb3b71d authored by Mario de la Ossa's avatar Mario de la Ossa

Always notify new mentions even if explicitly unsubscribed

parent feb95ce3
...@@ -35,7 +35,8 @@ class NotificationRecipient ...@@ -35,7 +35,8 @@ class NotificationRecipient
# check this last because it's expensive # check this last because it's expensive
# nobody should receive notifications if they've specifically unsubscribed # nobody should receive notifications if they've specifically unsubscribed
return false if unsubscribed? # except if they were mentioned.
return false if @type != :mention && unsubscribed?
true true
end end
......
---
title: Send @mention notifications even if a user has explicitly unsubscribed from
item
merge_request:
author:
type: added
...@@ -34,6 +34,12 @@ describe NotificationService, :mailer do ...@@ -34,6 +34,12 @@ describe NotificationService, :mailer do
should_not_email_anyone should_not_email_anyone
end end
it 'emails new mentions despite being unsubscribed' do
send_notifications(@unsubscribed_mentioned)
should_only_email(@unsubscribed_mentioned)
end
it 'sends the proper notification reason header' do it 'sends the proper notification reason header' do
send_notifications(@u_watcher) send_notifications(@u_watcher)
should_only_email(@u_watcher) should_only_email(@u_watcher)
...@@ -122,7 +128,7 @@ describe NotificationService, :mailer do ...@@ -122,7 +128,7 @@ describe NotificationService, :mailer do
let(:project) { create(:project, :private) } let(:project) { create(:project, :private) }
let(:issue) { create(:issue, project: project, assignees: [assignee]) } let(:issue) { create(:issue, project: project, assignees: [assignee]) }
let(:mentioned_issue) { create(:issue, assignees: issue.assignees) } let(:mentioned_issue) { create(:issue, assignees: issue.assignees) }
let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced, @outsider also') } let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced, @unsubscribed_mentioned and @outsider also') }
before do before do
build_team(note.project) build_team(note.project)
...@@ -150,7 +156,7 @@ describe NotificationService, :mailer do ...@@ -150,7 +156,7 @@ describe NotificationService, :mailer do
add_users_with_subscription(note.project, issue) add_users_with_subscription(note.project, issue)
reset_delivered_emails! reset_delivered_emails!
expect(SentNotification).to receive(:record).with(issue, any_args).exactly(9).times expect(SentNotification).to receive(:record).with(issue, any_args).exactly(10).times
notification.new_note(note) notification.new_note(note)
...@@ -163,6 +169,7 @@ describe NotificationService, :mailer do ...@@ -163,6 +169,7 @@ describe NotificationService, :mailer do
should_email(@watcher_and_subscriber) should_email(@watcher_and_subscriber)
should_email(@subscribed_participant) should_email(@subscribed_participant)
should_email(@u_custom_off) should_email(@u_custom_off)
should_email(@unsubscribed_mentioned)
should_not_email(@u_guest_custom) should_not_email(@u_guest_custom)
should_not_email(@u_guest_watcher) should_not_email(@u_guest_watcher)
should_not_email(note.author) should_not_email(note.author)
...@@ -279,6 +286,7 @@ describe NotificationService, :mailer do ...@@ -279,6 +286,7 @@ describe NotificationService, :mailer do
before do before do
build_team(note.project) build_team(note.project)
note.project.add_master(note.author) note.project.add_master(note.author)
add_users_with_subscription(note.project, issue)
reset_delivered_emails! reset_delivered_emails!
end end
...@@ -286,6 +294,9 @@ describe NotificationService, :mailer do ...@@ -286,6 +294,9 @@ describe NotificationService, :mailer do
it 'notifies the team members' do it 'notifies the team members' do
notification.new_note(note) notification.new_note(note)
# Make sure @unsubscribed_mentioned is part of the team
expect(note.project.team.members).to include(@unsubscribed_mentioned)
# Notify all team members # Notify all team members
note.project.team.members.each do |member| note.project.team.members.each do |member|
# User with disabled notification should not be notified # User with disabled notification should not be notified
...@@ -486,7 +497,7 @@ describe NotificationService, :mailer do ...@@ -486,7 +497,7 @@ describe NotificationService, :mailer do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:project, :public, namespace: group) } let(:project) { create(:project, :public, namespace: group) }
let(:another_project) { create(:project, :public, namespace: group) } let(:another_project) { create(:project, :public, namespace: group) }
let(:issue) { create :issue, project: project, assignees: [assignee], description: 'cc @participant' } let(:issue) { create :issue, project: project, assignees: [assignee], description: 'cc @participant @unsubscribed_mentioned' }
before do before do
build_team(issue.project) build_team(issue.project)
...@@ -510,6 +521,7 @@ describe NotificationService, :mailer do ...@@ -510,6 +521,7 @@ describe NotificationService, :mailer do
should_email(@u_participant_mentioned) should_email(@u_participant_mentioned)
should_email(@g_global_watcher) should_email(@g_global_watcher)
should_email(@g_watcher) should_email(@g_watcher)
should_email(@unsubscribed_mentioned)
should_not_email(@u_mentioned) should_not_email(@u_mentioned)
should_not_email(@u_participating) should_not_email(@u_participating)
should_not_email(@u_disabled) should_not_email(@u_disabled)
...@@ -1823,6 +1835,7 @@ describe NotificationService, :mailer do ...@@ -1823,6 +1835,7 @@ describe NotificationService, :mailer do
def add_users_with_subscription(project, issuable) def add_users_with_subscription(project, issuable)
@subscriber = create :user @subscriber = create :user
@unsubscriber = create :user @unsubscriber = create :user
@unsubscribed_mentioned = create :user, username: 'unsubscribed_mentioned'
@subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating) @subscribed_participant = create_global_setting_for(create(:user, username: 'subscribed_participant'), :participating)
@watcher_and_subscriber = create_global_setting_for(create(:user), :watch) @watcher_and_subscriber = create_global_setting_for(create(:user), :watch)
...@@ -1830,7 +1843,9 @@ describe NotificationService, :mailer do ...@@ -1830,7 +1843,9 @@ describe NotificationService, :mailer do
project.add_master(@subscriber) project.add_master(@subscriber)
project.add_master(@unsubscriber) project.add_master(@unsubscriber)
project.add_master(@watcher_and_subscriber) project.add_master(@watcher_and_subscriber)
project.add_master(@unsubscribed_mentioned)
issuable.subscriptions.create(user: @unsubscribed_mentioned, project: project, subscribed: false)
issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true) issuable.subscriptions.create(user: @subscriber, project: project, subscribed: true)
issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true) issuable.subscriptions.create(user: @subscribed_participant, project: project, subscribed: true)
issuable.subscriptions.create(user: @unsubscriber, project: project, subscribed: false) issuable.subscriptions.create(user: @unsubscriber, project: project, subscribed: false)
......
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