From ea5da30326a0515f98d7276c9cff239232cafed4 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 26 Aug 2015 17:23:22 -0700 Subject: [PATCH] Don't notify users without access to the project when they are (accidentally) mentioned in a note. --- CHANGELOG | 1 + app/services/notification_service.rb | 11 ++++++++--- spec/services/notification_service_spec.rb | 11 +++++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 9593c16c9cb..bf3b7bb8b01 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,7 @@ v 8.0.0 (unreleased) - Move dashboard activity to separate page - Improve performance of git blame - Limit content width to 1200px for most of pages to improve readability on big screens + - Don't notify users without access to the project when they are (accidentally) mentioned in a note. v 7.14.1 - Improve abuse reports management from admin area diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 3735a136365..e294b23bc23 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -107,12 +107,17 @@ class NotificationService 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) participants = if target.respond_to?(:participants) target.participants(note.author) else - note.mentioned_users + mentioned_users end recipients = recipients.concat(participants) @@ -120,8 +125,8 @@ class NotificationService recipients = add_project_watchers(recipients, note.project) # Reject users with Mention notification level, except those mentioned in _this_ note. - recipients = reject_mention_users(recipients - note.mentioned_users, note.project) - recipients = recipients + note.mentioned_users + recipients = reject_mention_users(recipients - mentioned_users, note.project) + recipients = recipients + mentioned_users recipients = reject_muted_users(recipients, note.project) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 9da6c9dc949..8865335d0d1 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -31,13 +31,16 @@ describe NotificationService do describe 'Notes' 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(: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 build_team(note.project) + project.team << [issue.author, :master] + project.team << [issue.assignee, :master] + project.team << [note.author, :master] end describe :new_note do @@ -53,6 +56,7 @@ describe NotificationService do should_not_email(@u_participating.id) should_not_email(@u_disabled.id) should_not_email(@unsubscriber.id) + should_not_email(@u_outsider_mentioned) notification.new_note(note) end @@ -444,12 +448,15 @@ describe NotificationService do @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_MENTION) @u_committer = create(:user, username: 'committer') @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_participating, :master] + project.team << [@u_participant_mentioned, :master] project.team << [@u_disabled, :master] project.team << [@u_mentioned, :master] project.team << [@u_committer, :master] + project.team << [@u_not_mentioned, :master] end def add_users_with_subscription(project, issuable)