From 60bd2ae372a9cab09990a6d2d5522ad3aeed8a40 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Wed, 28 Jun 2017 11:44:32 +0100 Subject: Ensure NotificationRecipientService doesn't modify participants Even though it does modify the participants of the notification target in some cases, this should have been safe, as different workers are responsible for creating the notifications for each target. However, this is at best confusing, and we should ensure we don't do that. --- app/services/notification_recipient_service.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'app/services') 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 = [] -- cgit v1.2.1