diff options
3 files changed, 50 insertions, 7 deletions
diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 8d1820bc504..9ac561e4bd2 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -11,7 +11,7 @@ class NotificationRecipientService def build_recipients(target, current_user, action:, previous_assignee: nil, skip_current_user: true) custom_action = build_custom_key(action, target) - recipients = target.participants(current_user) + recipients = participants(target, current_user) recipients = add_project_watchers(recipients) recipients = add_custom_notifications(recipients, custom_action) recipients = reject_mention_users(recipients) @@ -86,12 +86,7 @@ class NotificationRecipientService mentioned_users = note.mentioned_users.select { |user| user.can?(ability, subject) } # Add all users participating in the thread (author, assignee, comment authors) - recipients = - if target.respond_to?(:participants) - target.participants(note.author) - else - mentioned_users - end + recipients = participants(target, note.author) || mentioned_users unless note.for_personal_snippet? # Merge project watchers @@ -123,6 +118,14 @@ class NotificationRecipientService protected + # Ensure that if we modify this array, we aren't modifying the memoised + # participants on the target. + def participants(target, user) + return unless target.respond_to?(:participants) + + target.participants(user).dup + end + # Get project/group users with CUSTOM notification level def add_custom_notifications(recipients, action) user_ids = [] diff --git a/changelogs/unreleased/stop-notification-recipient-service-modifying-participants.yml b/changelogs/unreleased/stop-notification-recipient-service-modifying-participants.yml new file mode 100644 index 00000000000..7e66ea4ca8b --- /dev/null +++ b/changelogs/unreleased/stop-notification-recipient-service-modifying-participants.yml @@ -0,0 +1,5 @@ +--- +title: Ensure participants for issues, merge requests, etc. are calculated correctly + when sending notifications +merge_request: +author: diff --git a/spec/services/notification_recipient_service_spec.rb b/spec/services/notification_recipient_service_spec.rb new file mode 100644 index 00000000000..e18e87a8e75 --- /dev/null +++ b/spec/services/notification_recipient_service_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe NotificationRecipientService, services: true do + set(:user) { create(:user) } + set(:project) { create(:empty_project, :public) } + set(:issue) { create(:issue, project: project) } + + set(:watcher) do + watcher = create(:user) + setting = watcher.notification_settings_for(project) + setting.level = :watch + setting.save + + watcher + end + + subject { described_class.new(project) } + + describe '#build_recipients' do + it 'does not modify the participants of the target' do + expect { subject.build_recipients(issue, user, action: :new_issue) }. + not_to change { issue.participants(user) } + end + end + + describe '#build_new_note_recipients' do + set(:note) { create(:note_on_issue, noteable: issue, project: project) } + + it 'does not modify the participants of the target' do + expect { subject.build_new_note_recipients(note) }. + not_to change { note.noteable.participants(note.author) } + end + end + +end |