diff options
| author | Sean McGivern <sean@gitlab.com> | 2017-06-28 11:44:32 +0100 |
|---|---|---|
| committer | Sean McGivern <sean@gitlab.com> | 2017-06-28 12:14:44 +0100 |
| commit | 60bd2ae372a9cab09990a6d2d5522ad3aeed8a40 (patch) | |
| tree | 6720ff89f951c050458299b24af98efa95a8e2b3 /app/services | |
| parent | 18a7fa550d2ab9ab4c20709a9fa0a0a75e3bf3c6 (diff) | |
| download | gitlab-ce-60bd2ae372a9cab09990a6d2d5522ad3aeed8a40.tar.gz | |
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.
Diffstat (limited to 'app/services')
| -rw-r--r-- | app/services/notification_recipient_service.rb | 17 |
1 files changed, 10 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 = [] |