Commit 4425026d authored by Stan Hu's avatar Stan Hu

Only exclude commit authors in merge request approvers

With https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/9007, anyone
who rebases or amends a commit in a merge request will be tagged as a
committer, even if there were no changes. This is surprising behavior
and prevents reviewers from even touching a merge request.

Instead, we loosen the restriction to look only at the commit authors.

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/9787
parent 016c4c1b
......@@ -20,8 +20,8 @@ class CommitCollection
commits.each(&block)
end
def committers
emails = without_merge_commits.map(&:committer_email).uniq
def authors
emails = without_merge_commits.map(&:author_email).uniq
User.by_any_email(emails)
end
......
......@@ -288,12 +288,12 @@ class MergeRequest < ActiveRecord::Base
work_in_progress?(title) ? title : "WIP: #{title}"
end
def committers
@committers ||= commits.committers
def commit_authors
@commit_authors ||= commits.authors
end
def authors
User.from_union([committers, User.where(id: self.author_id)])
User.from_union([commit_authors, User.where(id: self.author_id)])
end
# Verifies if title has changed not taking into account WIP prefix
......
......@@ -52,8 +52,8 @@ A group can also be added as an approver. [In the future](https://gitlab.com/git
group approvers will be restricted.
If a user is added as an individual approver and is also part of a group approver,
then that user is just counted once. The merge request author and users that have committed
to the merge request do not count as eligible approvers,
then that user is just counted once. The merge request author and users who have authored
commits in the merge request do not count as eligible approvers,
unless [self-approval] is explicitly enabled on the project settings.
### Implicit approvers
......
......@@ -12,26 +12,26 @@ describe CommitCollection do
end
end
describe '.committers' do
describe '.authors' do
it 'returns a relation of users when users are found' do
user = create(:user, email: commit.committer_email.upcase)
user = create(:user, email: commit.author_email.upcase)
collection = described_class.new(project, [commit])
expect(collection.committers).to contain_exactly(user)
expect(collection.authors).to contain_exactly(user)
end
it 'returns empty array when committers cannot be found' do
it 'returns empty array when authors cannot be found' do
collection = described_class.new(project, [commit])
expect(collection.committers).to be_empty
expect(collection.authors).to be_empty
end
it 'excludes authors of merge commits' do
commit = project.commit("60ecb67744cb56576c30214ff52294f8ce2def98")
create(:user, email: commit.committer_email.upcase)
create(:user, email: commit.author_email.upcase)
collection = described_class.new(project, [commit])
expect(collection.committers).to be_empty
expect(collection.authors).to be_empty
end
end
......
......@@ -1024,23 +1024,23 @@ describe MergeRequest do
end
end
describe '#committers' do
it 'returns all the committers of every commit in the merge request' do
users = subject.commits.map(&:committer_email).uniq.map do |email|
describe '#commit_authors' do
it 'returns all the authors of every commit in the merge request' do
users = subject.commits.map(&:author_email).uniq.map do |email|
create(:user, email: email)
end
expect(subject.committers).to match_array(users)
expect(subject.commit_authors).to match_array(users)
end
it 'returns an empty array if no committer is associated with a user' do
expect(subject.committers).to be_empty
it 'returns an empty array if no author is associated with a user' do
expect(subject.commit_authors).to be_empty
end
end
describe '#authors' do
it 'returns a list with all the committers in the merge request and author' do
users = subject.commits.map(&:committer_email).uniq.map do |email|
it 'returns a list with all the commit authors in the merge request and author' do
users = subject.commits.map(&:author_email).uniq.map do |email|
create(:user, email: email)
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