Merge branch '616-do-not-add-approvers-as-participants-of-each-merge-request' into 'master'

Do not add approvers as participants of each merge request

Closes #616

See merge request gitlab-org/gitlab!25546

ee/app/models/ee/merge_request.rb

@@ -58,8 +58,6 @@ module EE
           .having("COUNT(users.id) = ?", usernames.size)
       end
 
-      participant :participant_approvers
-
       accepts_nested_attributes_for :approval_rules, allow_destroy: true
 
       scope :order_review_time_desc, -> do
@@ -164,14 +162,6 @@ module EE
       super.concat(positions)
     end
 
-    def participant_approvers
-      strong_memoize(:participant_approvers) do
-        next [] unless approval_needed?
-
-        approval_state.filtered_approvers(code_owner: false, unactioned: true)
-      end
-    end
-
     def enabled_reports
       {
         sast: report_type_enabled?(:sast),

ee/changelogs/unreleased/616-do-not-add-approvers-as-participants-of-each-merge-request.yml

+---
+title: Do not automatically add approvers as participants of each merge request
+merge_request: 25546
+author:
+type: changed

ee/spec/models/merge_request_spec.rb

@@ -92,29 +92,18 @@ describe MergeRequest do
     end
   end
 
-  describe '#participant_approvers' do
-    let(:approvers) { create_list(:user, 2) }
-    let(:code_owners) { create_list(:user, 2) }
-
-    let!(:regular_rule) { create(:approval_merge_request_rule, merge_request: merge_request, users: approvers) }
-    let!(:code_owner_rule) { create(:code_owner_rule, merge_request: merge_request, users: code_owners) }
-
-    let!(:approval) { create(:approval, merge_request: merge_request, user: approvers.last) }
-
-    before do
-      allow(subject).to receive(:code_owners).and_return(code_owners)
-    end
-
-    it 'returns empty array if approval not needed' do
-      allow(subject).to receive(:approval_needed?).and_return(false)
-
-      expect(subject.participant_approvers).to be_empty
-    end
+  describe '#participants' do
+    context 'with approval rule' do
+      before do
+        approver = create(:approver, target: project)
+        second_approver = create(:approver, target: project)
 
-    it 'returns approvers if approval is needed, excluding code owners' do
-      allow(subject).to receive(:approval_needed?).and_return(true)
+        create(:approval_merge_request_rule, merge_request: merge_request, users: [approver.user, second_approver.user])
+      end
 
-      expect(subject.participant_approvers).to contain_exactly(approvers.first)
+      it 'returns only the author as a participant' do
+        expect(subject.participants).to contain_exactly(subject.author)
+      end
     end
   end
 
@@ -152,27 +141,6 @@ describe MergeRequest do
     end
   end
 
-  describe '#participant_approvers with approval_rules disabled' do
-    let!(:approver) { create(:approver, target: project) }
-    let(:code_owners) { [double(:code_owner)] }
-
-    before do
-      allow(subject).to receive(:code_owners).and_return(code_owners)
-    end
-
-    it 'returns empty array if approval not needed' do
-      allow(subject).to receive(:approval_needed?).and_return(false)
-
-      expect(subject.participant_approvers).to eq([])
-    end
-
-    it 'returns approvers if approval is needed, excluding code owners' do
-      allow(subject).to receive(:approval_needed?).and_return(true)
-
-      expect(subject.participant_approvers).to eq([approver.user])
-    end
-  end
-
   describe '#approvals_before_merge' do
     where(:license_value, :db_value, :expected) do
       true  | 5   | 5

ee/spec/services/ee/notification_service_spec.rb

@@ -640,10 +640,10 @@ describe EE::NotificationService, :mailer do
           reset_delivered_emails!
         end
 
-        it 'emails the approvers' do
+        it 'does not email the approvers' do
           notification.new_merge_request(merge_request, @u_disabled)
 
-          project_approvers.each { |approver| should_email(approver) }
+          project_approvers.each { |approver| should_not_email(approver) }
         end
 
         it 'does not email the approvers when approval is not necessary' do
@@ -666,10 +666,10 @@ describe EE::NotificationService, :mailer do
             reset_delivered_emails!
           end
 
-          it 'emails the MR approvers' do
+          it 'does not email the MR approvers' do
             notification.new_merge_request(merge_request, @u_disabled)
 
-            mr_approvers.each { |approver| should_email(approver) }
+            mr_approvers.each { |approver| should_not_email(approver) }
           end
 
           it 'does not email approvers set on the project who are not approvers of this MR' do