Merge branch '6653-extract-ee-specific-files-lines-for-discussion-app-services' into 'master'

Resolve "Extract EE specific files/lines for Discussion app/services"

Closes #6653

See merge request gitlab-org/gitlab-ee!6392

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

app/services/system_note_service.rb

@@ -527,111 +527,6 @@ module SystemNoteService
     create_note(NoteSummary.new(noteable, project, author, body, action: 'moved'))
   end
 
-  #
-  # noteable     - Noteable object
-  # noteable_ref - Referenced noteable object
-  # user         - User performing reference
-  #
-  # Example Note text:
-  #
-  #   "marked this issue as related to gitlab-ce#9001"
-  #
-  # Returns the created Note object
-  def relate_issue(noteable, noteable_ref, user)
-    body = "marked this issue as related to #{noteable_ref.to_reference(noteable.project)}"
-
-    create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'relate'))
-  end
-
-  #
-  # noteable     - Noteable object
-  # noteable_ref - Referenced noteable object
-  # user         - User performing reference
-  #
-  # Example Note text:
-  #
-  #   "removed the relation with gitlab-ce#9001"
-  #
-  # Returns the created Note object
-  def unrelate_issue(noteable, noteable_ref, user)
-    body = "removed the relation with #{noteable_ref.to_reference(noteable.project)}"
-
-    create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'unrelate'))
-  end
-
-  def epic_issue(epic, issue, user, type)
-    return unless validate_epic_issue_action_type(type)
-
-    action = type == :added ? 'epic_issue_added' : 'epic_issue_removed'
-
-    body = "#{type} issue #{issue.to_reference(epic.group)}"
-
-    create_note(NoteSummary.new(epic, nil, user, body, action: action))
-  end
-
-  def epic_issue_moved(from_epic, issue, to_epic, user)
-    epic_issue_moved_act(from_epic, issue, to_epic, user, verb: 'added', direction: 'from')
-    epic_issue_moved_act(to_epic, issue, from_epic, user, verb: 'moved', direction: 'to')
-  end
-
-  def epic_issue_moved_act(subject_epic, issue, object_epic, user, verb:, direction:)
-    action = 'epic_issue_moved'
-
-    body = "#{verb} issue #{issue.to_reference(subject_epic.group)} #{direction}" \
-      " epic #{subject_epic.to_reference(object_epic.group)}"
-
-    create_note(NoteSummary.new(object_epic, nil, user, body, action: action))
-  end
-
-  def issue_on_epic(issue, epic, user, type)
-    return unless validate_epic_issue_action_type(type)
-
-    if type == :added
-      direction = 'to'
-      action = 'issue_added_to_epic'
-    else
-      direction = 'from'
-      action = 'issue_removed_from_epic'
-    end
-
-    body = "#{type} #{direction} epic #{epic.to_reference(issue.project)}"
-
-    create_note(NoteSummary.new(issue, issue.project, user, body, action: action))
-  end
-
-  def issue_epic_change(issue, epic, user)
-    body = "changed epic to #{epic.to_reference(issue.project)}"
-    action = 'issue_changed_epic'
-
-    create_note(NoteSummary.new(issue, issue.project, user, body, action: action))
-  end
-
-  def validate_epic_issue_action_type(type)
-    [:added, :removed].include?(type)
-  end
-
-  # Called when the merge request is approved by user
-  #
-  # noteable - Noteable object
-  # user     - User performing approve
-  #
-  # Example Note text:
-  #
-  #   "approved this merge request"
-  #
-  # Returns the created Note object
-  def approve_mr(noteable, user)
-    body = "approved this merge request"
-
-    create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'approved'))
-  end
-
-  def unapprove_mr(noteable, user)
-    body = "unapproved this merge request"
-
-    create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'unapproved'))
-  end
-
   # Called when a Noteable has been marked as a duplicate of another Issue
   #
   # noteable        - Noteable object

app/services/todo_service.rb

@@ -108,14 +108,6 @@ class TodoService
     end
   end
 
-  # When new approvers are added for a merge request:
-  #
-  #  * create a todo for those users to approve the MR
-  #
-  def add_merge_request_approvers(merge_request, approvers)
-    create_approval_required_todos(merge_request, approvers, merge_request.author)
-  end
-
   # When a new commit is pushed to a merge request we should:
   #
   #  * mark all pending todos related to the merge request for that user as done
@@ -237,11 +229,6 @@ class TodoService
 
   def new_issuable(issuable, author)
     create_assignment_todo(issuable, author)
-
-    if issuable.is_a?(MergeRequest)
-      create_approval_required_todos(issuable, issuable.overall_approvers, author)
-    end
-
     create_mention_todos(issuable.project, issuable, author)
   end
 
@@ -287,11 +274,6 @@ class TodoService
     create_todos(mentioned_users, attributes)
   end
 
-  def create_approval_required_todos(merge_request, approvers, author)
-    attributes = attributes_for_todo(merge_request.project, merge_request, author, Todo::APPROVAL_REQUIRED)
-    create_todos(approvers.map(&:user), attributes)
-  end
-
   def create_build_failed_todo(merge_request, todo_author)
     attributes = attributes_for_todo(merge_request.project, merge_request, todo_author, Todo::BUILD_FAILED)
     create_todos(todo_author, attributes)

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/app/services/ee/system_note_service.rb

@@ -6,6 +6,111 @@ module EE
   module SystemNoteService
     extend self
 
+    #
+    # noteable     - Noteable object
+    # noteable_ref - Referenced noteable object
+    # user         - User performing reference
+    #
+    # Example Note text:
+    #
+    #   "marked this issue as related to gitlab-ce#9001"
+    #
+    # Returns the created Note object
+    def relate_issue(noteable, noteable_ref, user)
+      body = "marked this issue as related to #{noteable_ref.to_reference(noteable.project)}"
+
+      create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'relate'))
+    end
+
+    #
+    # noteable     - Noteable object
+    # noteable_ref - Referenced noteable object
+    # user         - User performing reference
+    #
+    # Example Note text:
+    #
+    #   "removed the relation with gitlab-ce#9001"
+    #
+    # Returns the created Note object
+    def unrelate_issue(noteable, noteable_ref, user)
+      body = "removed the relation with #{noteable_ref.to_reference(noteable.project)}"
+
+      create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'unrelate'))
+    end
+
+    def epic_issue(epic, issue, user, type)
+      return unless validate_epic_issue_action_type(type)
+
+      action = type == :added ? 'epic_issue_added' : 'epic_issue_removed'
+
+      body = "#{type} issue #{issue.to_reference(epic.group)}"
+
+      create_note(NoteSummary.new(epic, nil, user, body, action: action))
+    end
+
+    def epic_issue_moved(from_epic, issue, to_epic, user)
+      epic_issue_moved_act(from_epic, issue, to_epic, user, verb: 'added', direction: 'from')
+      epic_issue_moved_act(to_epic, issue, from_epic, user, verb: 'moved', direction: 'to')
+    end
+
+    def epic_issue_moved_act(subject_epic, issue, object_epic, user, verb:, direction:)
+      action = 'epic_issue_moved'
+
+      body = "#{verb} issue #{issue.to_reference(subject_epic.group)} #{direction}" \
+             " epic #{subject_epic.to_reference(object_epic.group)}"
+
+      create_note(NoteSummary.new(object_epic, nil, user, body, action: action))
+    end
+
+    def issue_on_epic(issue, epic, user, type)
+      return unless validate_epic_issue_action_type(type)
+
+      if type == :added
+        direction = 'to'
+        action = 'issue_added_to_epic'
+      else
+        direction = 'from'
+        action = 'issue_removed_from_epic'
+      end
+
+      body = "#{type} #{direction} epic #{epic.to_reference(issue.project)}"
+
+      create_note(NoteSummary.new(issue, issue.project, user, body, action: action))
+    end
+
+    def issue_epic_change(issue, epic, user)
+      body = "changed epic to #{epic.to_reference(issue.project)}"
+      action = 'issue_changed_epic'
+
+      create_note(NoteSummary.new(issue, issue.project, user, body, action: action))
+    end
+
+    def validate_epic_issue_action_type(type)
+      [:added, :removed].include?(type)
+    end
+
+    # Called when the merge request is approved by user
+    #
+    # noteable - Noteable object
+    # user     - User performing approve
+    #
+    # Example Note text:
+    #
+    #   "approved this merge request"
+    #
+    # Returns the created Note object
+    def approve_mr(noteable, user)
+      body = "approved this merge request"
+
+      create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'approved'))
+    end
+
+    def unapprove_mr(noteable, user)
+      body = "unapproved this merge request"
+
+      create_note(NoteSummary.new(noteable, noteable.project, user, body, action: 'unapproved'))
+    end
+
     # Called when the weight of a Noteable is changed
     #
     # noteable   - Noteable object
@@ -19,7 +124,6 @@ module EE
     #   "changed weight to 4"
     #
     # Returns the created Note object
-
     def change_weight_note(noteable, project, author)
       body = noteable.weight ? "changed weight to **#{noteable.weight}**" : 'removed the weight'
       create_note(NoteSummary.new(noteable, project, author, body, action: 'weight'))

ee/app/services/ee/todo_service.rb

@@ -2,6 +2,23 @@ module EE
   module TodoService
     extend ::Gitlab::Utils::Override
 
+    # When new approvers are added for a merge request:
+    #
+    #  * create a todo for those users to approve the MR
+    #
+    def add_merge_request_approvers(merge_request, approvers)
+      create_approval_required_todos(merge_request, approvers, merge_request.author)
+    end
+
+    override :new_issuable
+    def new_issuable(issuable, author)
+      if issuable.is_a?(MergeRequest)
+        create_approval_required_todos(issuable, issuable.overall_approvers, author)
+      end
+
+      super
+    end
+
     def new_epic(epic, current_user)
       create_mention_todos(nil, epic, current_user)
     end
@@ -10,6 +27,8 @@ module EE
       create_mention_todos(nil, epic, current_user, nil, skip_users)
     end
 
+    private
+
     override :attributes_for_target
     def attributes_for_target(target)
       attributes = super
@@ -20,5 +39,10 @@ module EE
 
       attributes
     end
+
+    def create_approval_required_todos(merge_request, approvers, author)
+      attributes = attributes_for_todo(merge_request.project, merge_request, author, ::Todo::APPROVAL_REQUIRED)
+      create_todos(approvers.map(&:user), attributes)
+    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!