Send notification to all participants when MR is merged.

3 files changed, 16 insertions, 41 deletions

diff --git a/CHANGELOG b/CHANGELOG
index c03e5053d17..9e0d96c5b7d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -19,6 +19,7 @@ v 7.14.0 (unreleased)
   - Add fetch command to the MR page
   - Fix bug causing Bitbucket importer to crash when OAuth application had been removed.
   - Add fetch command to the MR page.
+  - Send notification to all participants when MR is merged.
 
 v 7.13.2
   - Fix randomly failed spec
diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb
index 312b56eb87b..3735a136365 100644
--- a/app/services/notification_service.rb
+++ b/app/services/notification_service.rb
@@ -70,12 +70,6 @@ class NotificationService
     reassign_resource_email(merge_request, merge_request.target_project, current_user, 'reassigned_merge_request_email')
   end
 
-  # When we close a merge request we should send next emails:
-  #
-  #  * merge_request author if their notification level is not Disabled
-  #  * merge_request assignee if their notification level is not Disabled
-  #  * project team members with notification level higher then Participating
-  #
   def close_mr(merge_request, current_user)
     close_resource_email(merge_request, merge_request.target_project, current_user, 'closed_merge_request_email')
   end
@@ -84,26 +78,8 @@ class NotificationService
     reopen_resource_email(issue, issue.project, current_user, 'issue_status_changed_email', 'reopened')
   end
 
-  # When we merge a merge request we should send next emails:
-  #
-  #  * merge_request author if their notification level is not Disabled
-  #  * merge_request assignee if their notification level is not Disabled
-  #  * project team members with notification level higher then Participating
-  #
   def merge_mr(merge_request, current_user)
-    recipients = [merge_request.author, merge_request.assignee]
-
-    recipients = add_project_watchers(recipients, merge_request.target_project)
-    recipients = reject_muted_users(recipients, merge_request.target_project)
-
-    recipients = add_subscribed_users(recipients, merge_request)
-    recipients = reject_unsubscribed_users(recipients, merge_request)
-
-    recipients.delete(current_user)
-
-    recipients.each do |recipient|
-      mailer.merged_merge_request_email(recipient.id, merge_request.id, current_user.id)
-    end
+    close_resource_email(merge_request, merge_request.target_project, current_user, 'merged_merge_request_email')
   end
 
   def reopen_mr(merge_request, current_user)
@@ -364,8 +340,7 @@ class NotificationService
   end
   
   def new_resource_email(target, project, method)
-    recipients = build_recipients(target, project)
-    recipients.delete(target.author)
+    recipients = build_recipients(target, project, target.author)
 
     recipients.each do |recipient|
       mailer.send(method, recipient.id, target.id)
@@ -373,8 +348,7 @@ class NotificationService
   end
 
   def close_resource_email(target, project, current_user, method)
-    recipients = build_recipients(target, project)
-    recipients.delete(current_user)
+    recipients = build_recipients(target, project, current_user)
 
     recipients.each do |recipient|
       mailer.send(method, recipient.id, target.id, current_user.id)
@@ -383,8 +357,7 @@ class NotificationService
 
   def reassign_resource_email(target, project, current_user, method)
     assignee_id_was = previous_record(target, "assignee_id")
-    recipients = build_recipients(target, project)
-    recipients.delete(current_user)
+    recipients = build_recipients(target, project, current_user)
 
     recipients.each do |recipient|
       mailer.send(method, recipient.id, target.id, assignee_id_was, current_user.id)
@@ -392,21 +365,15 @@ class NotificationService
   end
 
   def reopen_resource_email(target, project, current_user, method, status)
-    recipients = build_recipients(target, project)
-    recipients.delete(current_user)
+    recipients = build_recipients(target, project, 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
+  def build_recipients(target, project, current_user)
+    recipients = target.participants(current_user)
 
     recipients = add_project_watchers(recipients, project)
     recipients = reject_mention_users(recipients, project)
@@ -415,6 +382,8 @@ class NotificationService
     recipients = add_subscribed_users(recipients, target)
     recipients = reject_unsubscribed_users(recipients, target)
 
+    recipients.delete(current_user)
+
     recipients
   end
 
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 253e5823499..9da6c9dc949 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -300,7 +300,7 @@ describe NotificationService do
 
   describe 'Merge Requests' do
     let(:project) { create(:project, :public) }
-    let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user) }
+    let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' }
 
     before do
       build_team(merge_request.target_project)
@@ -311,6 +311,7 @@ describe NotificationService do
       it do
         should_email(merge_request.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)
         notification.new_merge_request(merge_request, @u_disabled)
@@ -329,6 +330,7 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_email(@subscriber.id)
         should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
@@ -349,6 +351,7 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_email(@subscriber.id)
         should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
@@ -369,6 +372,7 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_email(@subscriber.id)
         should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)
@@ -389,6 +393,7 @@ describe NotificationService do
       it do
         should_email(merge_request.assignee_id)
         should_email(@u_watcher.id)
+        should_email(@u_participant_mentioned.id)
         should_email(@subscriber.id)
         should_not_email(@unsubscriber.id)
         should_not_email(@u_participating.id)