Merge branch 'approvers_notification_receipients' into 'master'

Fix: When MR approvals are disabled, but approvers were previously assigned

Closes #3104

See merge request !2647

app/models/merge_request.rb

@@ -118,7 +118,7 @@ class MergeRequest < ActiveRecord::Base
   scope :unassigned, -> { where("assignee_id IS NULL") }
   scope :assigned_to, ->(u) { where(assignee_id: u.id)}
 
-  participant :approvers_left
+  participant :participant_approvers
   participant :assignee
 
   after_save :keep_around_commit
@@ -208,6 +208,10 @@ class MergeRequest < ActiveRecord::Base
     author_id == user.id || assignee_id == user.id
   end
 
+  def participant_approvers
+    requires_approve? ? approvers_left : []
+  end
+
   # `from` argument can be a Namespace or Project.
   def to_reference(from = nil, full: false)
     reference = "#{self.class.reference_prefix}#{iid}"

changelogs/unreleased-ee/approvers_notification_receipients.yml

+---
+title: 'Fix: When MR approvals are disabled, but approvers were previously assigned,
+  all approvers receive a notification on every MR'
+merge_request:
+author:

spec/services/notification_service_spec.rb

@@ -932,6 +932,13 @@ describe NotificationService, :mailer do
           project_approvers.each { |approver| should_email(approver) }
         end
 
+        it 'does not email the approvers when approving is disabled' do
+          merge_request.target_project.update_attributes(approvals_before_merge: 0)
+          notification.new_merge_request(merge_request, @u_disabled)
+
+          project_approvers.each { |approver| should_not_email(approver) }
+        end
+
         context 'when the merge request has approvers set' do
           let(:mr_approvers) { create_list(:user, 3) }