Commit 8c0565b7 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'dont-notify-users-without-project-access' into 'master'

Don't notify users without access to the project when they are (accidentally) mentioned in a note.

Addresses #2366.

See merge request !1216
parents c388f3db ea5da303
...@@ -25,6 +25,7 @@ v 8.0.0 (unreleased) ...@@ -25,6 +25,7 @@ v 8.0.0 (unreleased)
- Bring more UI consistency in way how projects, snippets and groups lists are rendered - Bring more UI consistency in way how projects, snippets and groups lists are rendered
- Make all profiles public - Make all profiles public
- Fixed login failure when extern_uid changes (Joel Koglin) - Fixed login failure when extern_uid changes (Joel Koglin)
- Don't notify users without access to the project when they are (accidentally) mentioned in a note.
v 7.14.1 v 7.14.1
- Improve abuse reports management from admin area - Improve abuse reports management from admin area
......
...@@ -107,12 +107,17 @@ class NotificationService ...@@ -107,12 +107,17 @@ class NotificationService
recipients = [] recipients = []
mentioned_users = note.mentioned_users
mentioned_users.select! do |user|
user.can?(:read_project, note.project)
end
# Add all users participating in the thread (author, assignee, comment authors) # Add all users participating in the thread (author, assignee, comment authors)
participants = participants =
if target.respond_to?(:participants) if target.respond_to?(:participants)
target.participants(note.author) target.participants(note.author)
else else
note.mentioned_users mentioned_users
end end
recipients = recipients.concat(participants) recipients = recipients.concat(participants)
...@@ -120,8 +125,8 @@ class NotificationService ...@@ -120,8 +125,8 @@ class NotificationService
recipients = add_project_watchers(recipients, note.project) recipients = add_project_watchers(recipients, note.project)
# Reject users with Mention notification level, except those mentioned in _this_ note. # Reject users with Mention notification level, except those mentioned in _this_ note.
recipients = reject_mention_users(recipients - note.mentioned_users, note.project) recipients = reject_mention_users(recipients - mentioned_users, note.project)
recipients = recipients + note.mentioned_users recipients = recipients + mentioned_users
recipients = reject_muted_users(recipients, note.project) recipients = reject_muted_users(recipients, note.project)
......
...@@ -31,13 +31,16 @@ describe NotificationService do ...@@ -31,13 +31,16 @@ describe NotificationService do
describe 'Notes' do describe 'Notes' do
context 'issue note' do context 'issue note' do
let(:project) { create(:empty_project, :public) } let(:project) { create(:empty_project, :private) }
let(:issue) { create(:issue, project: project, assignee: create(:user)) } let(:issue) { create(:issue, project: project, assignee: create(:user)) }
let(:mentioned_issue) { create(:issue, assignee: issue.assignee) } let(:mentioned_issue) { create(:issue, assignee: issue.assignee) }
let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced') } let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@mention referenced, @outsider also') }
before do before do
build_team(note.project) build_team(note.project)
project.team << [issue.author, :master]
project.team << [issue.assignee, :master]
project.team << [note.author, :master]
end end
describe :new_note do describe :new_note do
...@@ -53,6 +56,7 @@ describe NotificationService do ...@@ -53,6 +56,7 @@ describe NotificationService do
should_not_email(@u_participating.id) should_not_email(@u_participating.id)
should_not_email(@u_disabled.id) should_not_email(@u_disabled.id)
should_not_email(@unsubscriber.id) should_not_email(@unsubscriber.id)
should_not_email(@u_outsider_mentioned)
notification.new_note(note) notification.new_note(note)
end end
...@@ -444,12 +448,15 @@ describe NotificationService do ...@@ -444,12 +448,15 @@ describe NotificationService do
@u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_MENTION) @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_MENTION)
@u_committer = create(:user, username: 'committer') @u_committer = create(:user, username: 'committer')
@u_not_mentioned = create(:user, username: 'regular', notification_level: Notification::N_PARTICIPATING) @u_not_mentioned = create(:user, username: 'regular', notification_level: Notification::N_PARTICIPATING)
@u_outsider_mentioned = create(:user, username: 'outsider')
project.team << [@u_watcher, :master] project.team << [@u_watcher, :master]
project.team << [@u_participating, :master] project.team << [@u_participating, :master]
project.team << [@u_participant_mentioned, :master]
project.team << [@u_disabled, :master] project.team << [@u_disabled, :master]
project.team << [@u_mentioned, :master] project.team << [@u_mentioned, :master]
project.team << [@u_committer, :master] project.team << [@u_committer, :master]
project.team << [@u_not_mentioned, :master]
end end
def add_users_with_subscription(project, issuable) def add_users_with_subscription(project, issuable)
......
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