Merge branch 'stop-notification-recipient-service-modifying-participants' into 'master'

Ensure NotificationRecipientService doesn't modify participants

See merge request !12517

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 = []