Add issue/mr participants to reasign events

Also refactor NotificationService a bit

3 files changed, 22 insertions, 39 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 6af250cb395..72ca2b529d5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,7 +3,7 @@ Note: The upcoming release contains empty lines to reduce the number of merge co
 v 7.8.0
   - Replace highlight.js with rouge-fork rugments (Stefan Tatschner)
   - Make project search case insensitive (Hannes Rosenögger)
-  - Include issue/mr participants in list of recipients for close/reopen emails
+  - Include issue/mr participants in list of recipients for reassign/close/reopen emails
   - Expose description in groups API
   - 
   - 
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 87366b65725..2fc63b9f4b7 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -314,15 +314,7 @@ class NotificationService
   end
 
   def new_resource_email(target, project, method)
-    if target.respond_to?(:participants)
-      recipients = target.participants
-    else
-      recipients = []
-    end
-
-    recipients = reject_muted_users(recipients, project)
-    recipients = reject_mention_users(recipients, project)
-    recipients = recipients.concat(project_watchers(project)).uniq
+    recipients = build_recipients(target, project)
     recipients.delete(target.author)
 
     recipients.each do |recipient|
@@ -331,16 +323,7 @@ class NotificationService
   end
 
   def close_resource_email(target, project, current_user, method)
-    participants =
-      if target.respond_to?(:participants)
-        target.participants
-      else
-        [target.author, target.assignee]
-      end
-
-    recipients = reject_muted_users(participants, project)
-    recipients = reject_mention_users(recipients, project)
-    recipients = recipients.concat(project_watchers(project)).uniq
+    recipients = build_recipients(target, project)
     recipients.delete(current_user)
 
     recipients.each do |recipient|
@@ -350,17 +333,7 @@ class NotificationService
 
   def reassign_resource_email(target, project, current_user, method)
     assignee_id_was = previous_record(target, "assignee_id")
-
-    recipients = User.where(id: [target.assignee_id, assignee_id_was])
-
-    # Add watchers to email list
-    recipients = recipients.concat(project_watchers(project))
-
-    # reject users with disabled notifications
-    recipients = reject_muted_users(recipients, project)
-    recipients = reject_mention_users(recipients, project)
-
-    # Reject me from recipients if I reassign an item
+    recipients = build_recipients(target, project)
     recipients.delete(current_user)
 
     recipients.each do |recipient|
@@ -369,21 +342,26 @@ class NotificationService
   end
 
   def reopen_resource_email(target, project, current_user, method, status)
-    participants =
+    recipients = build_recipients(target, project)
+    recipients.delete(current_user)
+
+    recipients.each do |recipient|
+      mailer.send(method, recipient.id, target.id, status, current_user.id)
+    end
+  end
+
+  def build_recipients(target, project)
+    recipients =
       if target.respond_to?(:participants)
         target.participants
       else
         [target.author, target.assignee]
       end
 
-    recipients = reject_muted_users(participants, project)
+    recipients = reject_muted_users(recipients, project)
     recipients = reject_mention_users(recipients, project)
     recipients = recipients.concat(project_watchers(project)).uniq
-    recipients.delete(current_user)
-
-    recipients.each do |recipient|
-      mailer.send(method, recipient.id, target.id, status, current_user.id)
-    end
+    recipients
   end
 
   def mailer
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index e305536f7ee..2ba1e3372b9 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -187,7 +187,7 @@ describe NotificationService do
   end
 
   describe 'Issues' do
-    let(:issue) { create :issue, assignee: create(:user) }
+    let(:issue) { create :issue, assignee: create(:user), description: 'cc @participant' }
 
     before do
       build_team(issue.project)
@@ -197,6 +197,7 @@ describe NotificationService do
       it do
         should_email(issue.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_not_email(@u_mentioned.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
@@ -222,6 +223,7 @@ describe NotificationService do
       it 'should email new assignee' do
         should_email(issue.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
 
@@ -242,6 +244,7 @@ describe NotificationService do
         should_email(issue.assignee_id)
         should_email(issue.author_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
 
@@ -262,6 +265,7 @@ describe NotificationService do
         should_email(issue.assignee_id)
         should_email(issue.author_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_not_email(@u_participating.id)
         should_not_email(@u_disabled.id)
 
@@ -404,6 +408,7 @@ describe NotificationService do
   def build_team(project)
     @u_watcher = create(:user, notification_level: Notification::N_WATCH)
     @u_participating = create(:user, notification_level: Notification::N_PARTICIPATING)
+    @u_participant_mentioned = create(:user, username: 'participant', notification_level: Notification::N_PARTICIPATING)
     @u_disabled = create(:user, notification_level: Notification::N_DISABLED)
     @u_mentioned = create(:user, username: 'mention', notification_level: Notification::N_MENTION)
     @u_committer = create(:user, username: 'committer')