Commit c2e6bf00 authored by Gary Holtz's avatar Gary Holtz Committed by Thong Kuah

Remove #participant_approvers and associated tests

This also removes automatic email notifications for approvers and
quite a few specs referencing them.
parent a355d1c4
...@@ -58,8 +58,6 @@ module EE ...@@ -58,8 +58,6 @@ module EE
.having("COUNT(users.id) = ?", usernames.size) .having("COUNT(users.id) = ?", usernames.size)
end end
participant :participant_approvers
accepts_nested_attributes_for :approval_rules, allow_destroy: true accepts_nested_attributes_for :approval_rules, allow_destroy: true
scope :order_review_time_desc, -> do scope :order_review_time_desc, -> do
...@@ -164,14 +162,6 @@ module EE ...@@ -164,14 +162,6 @@ module EE
super.concat(positions) super.concat(positions)
end 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 def enabled_reports
{ {
sast: report_type_enabled?(:sast), sast: report_type_enabled?(:sast),
......
---
title: Do not automatically add approvers as participants of each merge request
merge_request: 25546
author:
type: changed
...@@ -92,29 +92,18 @@ describe MergeRequest do ...@@ -92,29 +92,18 @@ describe MergeRequest do
end end
end end
describe '#participant_approvers' do describe '#participants' do
let(:approvers) { create_list(:user, 2) } context 'with approval rule' do
let(:code_owners) { create_list(:user, 2) } before do
approver = create(:approver, target: project)
let!(:regular_rule) { create(:approval_merge_request_rule, merge_request: merge_request, users: approvers) } second_approver = create(:approver, target: project)
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
it 'returns approvers if approval is needed, excluding code owners' do create(:approval_merge_request_rule, merge_request: merge_request, users: [approver.user, second_approver.user])
allow(subject).to receive(:approval_needed?).and_return(true) 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
end end
...@@ -152,27 +141,6 @@ describe MergeRequest do ...@@ -152,27 +141,6 @@ describe MergeRequest do
end end
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 describe '#approvals_before_merge' do
where(:license_value, :db_value, :expected) do where(:license_value, :db_value, :expected) do
true | 5 | 5 true | 5 | 5
......
...@@ -640,10 +640,10 @@ describe EE::NotificationService, :mailer do ...@@ -640,10 +640,10 @@ describe EE::NotificationService, :mailer do
reset_delivered_emails! reset_delivered_emails!
end end
it 'emails the approvers' do it 'does not email the approvers' do
notification.new_merge_request(merge_request, @u_disabled) notification.new_merge_request(merge_request, @u_disabled)
project_approvers.each { |approver| should_email(approver) } project_approvers.each { |approver| should_not_email(approver) }
end end
it 'does not email the approvers when approval is not necessary' do it 'does not email the approvers when approval is not necessary' do
...@@ -666,10 +666,10 @@ describe EE::NotificationService, :mailer do ...@@ -666,10 +666,10 @@ describe EE::NotificationService, :mailer do
reset_delivered_emails! reset_delivered_emails!
end end
it 'emails the MR approvers' do it 'does not email the MR approvers' do
notification.new_merge_request(merge_request, @u_disabled) notification.new_merge_request(merge_request, @u_disabled)
mr_approvers.each { |approver| should_email(approver) } mr_approvers.each { |approver| should_not_email(approver) }
end end
it 'does not email approvers set on the project who are not approvers of this MR' do it 'does not email approvers set on the project who are not approvers of this MR' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment