Commit f2eed8e8 authored by Mark Chao's avatar Mark Chao

Port "revent commit authors from self approvaling MR"

https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9007
parent e45bbba6
...@@ -20,15 +20,12 @@ class ApprovalState ...@@ -20,15 +20,12 @@ class ApprovalState
# Excludes the author if 'self-approval' isn't explicitly enabled on project settings. # Excludes the author if 'self-approval' isn't explicitly enabled on project settings.
def self.filter_author(users, merge_request) def self.filter_author(users, merge_request)
return users unless merge_request.author_id
return users if merge_request.target_project.merge_requests_author_approval? return users if merge_request.target_project.merge_requests_author_approval?
if users.is_a?(ActiveRecord::Relation) && !users.loaded? if users.is_a?(ActiveRecord::Relation) && !users.loaded?
users.where.not(id: merge_request.author_id) users.where.not(id: merge_request.authors)
else else
users.dup users - merge_request.authors
users.delete(merge_request.author)
users
end end
end end
...@@ -106,7 +103,7 @@ class ApprovalState ...@@ -106,7 +103,7 @@ class ApprovalState
# That is, they're included/excluded from that list accordingly. # That is, they're included/excluded from that list accordingly.
return true if unactioned_approvers.include?(user) return true if unactioned_approvers.include?(user)
# We can safely unauthorize authors if it reaches this guard clause. # We can safely unauthorize authors if it reaches this guard clause.
return false if user == merge_request.author return false if merge_request.authors.include?(user)
return false unless user.can?(:update_merge_request, merge_request) return false unless user.can?(:update_merge_request, merge_request)
any_approver_allowed? && merge_request.approvals.where(user: user).empty? any_approver_allowed? && merge_request.approvals.where(user: user).empty?
......
...@@ -27,15 +27,17 @@ describe ApprovalState do ...@@ -27,15 +27,17 @@ describe ApprovalState do
shared_examples 'filtering author' do shared_examples 'filtering author' do
before do before do
allow(merge_request).to receive(:authors).and_return([merge_request.author, create(:user, username: 'commiter')])
project.update(merge_requests_author_approval: merge_requests_author_approval) project.update(merge_requests_author_approval: merge_requests_author_approval)
create_rule(users: [merge_request.author]) create_rule(users: merge_request.authors)
end end
context 'when self approval is disabled' do context 'when self approval is disabled' do
let(:merge_requests_author_approval) { false } let(:merge_requests_author_approval) { false }
it 'excludes author' do it 'excludes authors' do
expect(results).not_to include(merge_request.author) expect(results).not_to include(*merge_request.authors)
end end
end end
...@@ -43,7 +45,7 @@ describe ApprovalState do ...@@ -43,7 +45,7 @@ describe ApprovalState do
let(:merge_requests_author_approval) { true } let(:merge_requests_author_approval) { true }
it 'includes author' do it 'includes author' do
expect(results).to include(merge_request.author) expect(results).to include(*merge_request.authors)
end end
end end
end end
......
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