Extract EE-specific lines from NotificationService

app/services/notification_service.rb

@@ -136,7 +136,8 @@ class NotificationService
   #  * project team members with notification level higher then Participating
   #  * watchers of the mr's labels
   #  * users with custom level checked with "new merge request"
-  #  * approvers of the merge request
+  #
+  # In EE, approvers of the merge request are also included
   #
   def new_merge_request(merge_request, current_user)
     new_resource_email(merge_request, :new_merge_request_email)
@@ -215,14 +216,6 @@ class NotificationService
     reopen_resource_email(issue, current_user, :issue_status_changed_email, 'reopened')
   end
 
-  # When we add approvers to a merge request we should send an email to:
-  #
-  #  * the new approvers
-  #
-  def add_merge_request_approvers(merge_request, new_approvers, current_user)
-    add_mr_approvers_email(merge_request, new_approvers, current_user)
-  end
-
   def merge_mr(merge_request, current_user)
     close_resource_email(
       merge_request,
@@ -241,14 +234,6 @@ class NotificationService
     )
   end
 
-  def approve_mr(merge_request, current_user)
-    approve_mr_email(merge_request, merge_request.target_project, current_user)
-  end
-
-  def unapprove_mr(merge_request, current_user)
-    unapprove_mr_email(merge_request, merge_request.target_project, current_user)
-  end
-
   def resolve_all_discussions(merge_request, current_user)
     recipients = NotificationRecipientService.build_recipients(
       merge_request,
@@ -525,30 +510,6 @@ class NotificationService
     end
   end
 
-  def approve_mr_email(merge_request, project, current_user)
-    recipients = NotificationRecipientService.build_recipients(merge_request, current_user, action: 'approve')
-
-    recipients.each do |recipient|
-      mailer.approved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later
-    end
-  end
-
-  def unapprove_mr_email(merge_request, project, current_user)
-    recipients = NotificationRecipientService.build_recipients(merge_request, current_user, action: 'unapprove')
-
-    recipients.each do |recipient|
-      mailer.unapproved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later
-    end
-  end
-
-  def add_mr_approvers_email(merge_request, approvers, current_user)
-    approvers.each do |approver|
-      recipient = approver.user
-
-      mailer.add_merge_request_approver_email(recipient.id, merge_request.id, current_user.id).deliver_later
-    end
-  end
-
   def mailer
     Notify
   end

ee/app/services/ee/notification_service.rb

@@ -2,25 +2,28 @@ require 'ee/gitlab/service_desk'
 
 module EE
   module NotificationService
-    # override
-    def send_new_note_notifications(note)
-      super
-      send_service_desk_notification(note)
-    end
+    extend ::Gitlab::Utils::Override
 
-    def send_service_desk_notification(note)
-      return unless EE::Gitlab::ServiceDesk.enabled?
-      return unless note.noteable_type == 'Issue'
+    # When we add approvers to a merge request we should send an email to:
+    #
+    #  * the new approvers
+    #
+    def add_merge_request_approvers(merge_request, new_approvers, current_user)
+      add_mr_approvers_email(merge_request, new_approvers, current_user)
+    end
 
-      issue = note.noteable
-      support_bot = ::User.support_bot
+    def approve_mr(merge_request, current_user)
+      approve_mr_email(merge_request, merge_request.target_project, current_user)
+    end
 
-      return unless issue.service_desk_reply_to.present?
-      return unless issue.project.service_desk_enabled?
-      return if note.author == support_bot
-      return unless issue.subscribed?(support_bot, issue.project)
+    def unapprove_mr(merge_request, current_user)
+      unapprove_mr_email(merge_request, merge_request.target_project, current_user)
+    end
 
-      mailer.service_desk_new_note_email(issue.id, note.id).deliver_later
+    override :send_new_note_notifications
+    def send_new_note_notifications(note)
+      super
+      send_service_desk_notification(note)
     end
 
     def mirror_was_hard_failed(project)
@@ -38,5 +41,46 @@ module EE
     def project_mirror_user_changed(new_mirror_user, deleted_user_name, project)
       mailer.project_mirror_user_changed_email(new_mirror_user.id, deleted_user_name, project.id).deliver_later
     end
+
+    private
+
+    def add_mr_approvers_email(merge_request, approvers, current_user)
+      approvers.each do |approver|
+        recipient = approver.user
+
+        mailer.add_merge_request_approver_email(recipient.id, merge_request.id, current_user.id).deliver_later
+      end
+    end
+
+    def approve_mr_email(merge_request, project, current_user)
+      recipients = NotificationRecipientService.build_recipients(merge_request, current_user, action: 'approve')
+
+      recipients.each do |recipient|
+        mailer.approved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later
+      end
+    end
+
+    def unapprove_mr_email(merge_request, project, current_user)
+      recipients = NotificationRecipientService.build_recipients(merge_request, current_user, action: 'unapprove')
+
+      recipients.each do |recipient|
+        mailer.unapproved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later
+      end
+    end
+
+    def send_service_desk_notification(note)
+      return unless EE::Gitlab::ServiceDesk.enabled?
+      return unless note.noteable_type == 'Issue'
+
+      issue = note.noteable
+      support_bot = ::User.support_bot
+
+      return unless issue.service_desk_reply_to.present?
+      return unless issue.project.service_desk_enabled?
+      return if note.author == support_bot
+      return unless issue.subscribed?(support_bot, issue.project)
+
+      mailer.service_desk_new_note_email(issue.id, note.id).deliver_later
+    end
   end
 end

ee/spec/services/ee/notification_service_spec.rb

@@ -62,7 +62,7 @@ describe EE::NotificationService, :mailer do
     end
 
     def execute!
-      subject.send_service_desk_notification(note)
+      subject.new_note(note)
     end
 
     def self.it_should_email!
@@ -105,7 +105,7 @@ describe EE::NotificationService, :mailer do
 
       context 'when the license doesn\'t allow service desk' do
         before do
-          expect(EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
+          allow(::EE::Gitlab::ServiceDesk).to receive(:enabled?).and_return(false)
         end
 
         it_should_not_email!