Commit 7cb8aed6 authored by Sean McGivern's avatar Sean McGivern

Fallback to current user when suggesting approvers

A new MR won't have an author necessarily, so use the current user in
that case.
parent 216a4823
...@@ -707,7 +707,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController ...@@ -707,7 +707,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
def set_suggested_approvers def set_suggested_approvers
if @merge_request.requires_approve? if @merge_request.requires_approve?
@suggested_approvers = Gitlab::AuthorityAnalyzer.new( @suggested_approvers = Gitlab::AuthorityAnalyzer.new(
@merge_request @merge_request,
@merge_request.author || current_user
).calculate(@merge_request.approvals_required) ).calculate(@merge_request.approvals_required)
end end
end end
......
...@@ -2,8 +2,9 @@ module Gitlab ...@@ -2,8 +2,9 @@ module Gitlab
class AuthorityAnalyzer class AuthorityAnalyzer
COMMITS_TO_CONSIDER = 25 COMMITS_TO_CONSIDER = 25
def initialize(merge_request) def initialize(merge_request, skip_user)
@merge_request = merge_request @merge_request = merge_request
@skip_user = skip_user
@users = Hash.new(0) @users = Hash.new(0)
end end
...@@ -20,7 +21,7 @@ module Gitlab ...@@ -20,7 +21,7 @@ module Gitlab
@repo = @merge_request.target_project.repository @repo = @merge_request.target_project.repository
@repo.commits(@merge_request.target_branch, path: list_of_involved_files, limit: COMMITS_TO_CONSIDER).each do |commit| @repo.commits(@merge_request.target_branch, path: list_of_involved_files, limit: COMMITS_TO_CONSIDER).each do |commit|
if commit.author && commit.author != @merge_request.author if commit.author && commit.author != @skip_user
@users[commit.author] += 1 @users[commit.author] += 1
end end
end end
......
...@@ -6,7 +6,7 @@ describe Gitlab::AuthorityAnalyzer, lib: true do ...@@ -6,7 +6,7 @@ describe Gitlab::AuthorityAnalyzer, lib: true do
let(:author) { create(:user) } let(:author) { create(:user) }
let(:user_a) { create(:user) } let(:user_a) { create(:user) }
let(:user_b) { create(:user) } let(:user_b) { create(:user) }
let(:merge_request) { create(:merge_request, target_project: project, source_project: project, author: author) } let(:merge_request) { create(:merge_request, target_project: project, source_project: project) }
let(:files) { [double(:file, deleted_file: true, old_path: 'foo')] } let(:files) { [double(:file, deleted_file: true, old_path: 'foo')] }
let(:commits) do let(:commits) do
...@@ -19,7 +19,7 @@ describe Gitlab::AuthorityAnalyzer, lib: true do ...@@ -19,7 +19,7 @@ describe Gitlab::AuthorityAnalyzer, lib: true do
] ]
end end
let(:approvers) { Gitlab::AuthorityAnalyzer.new(merge_request).calculate(number_of_approvers) } let(:approvers) { Gitlab::AuthorityAnalyzer.new(merge_request, author).calculate(number_of_approvers) }
before do before do
merge_request.compare = double(:compare, raw_diffs: files) merge_request.compare = double(:compare, raw_diffs: files)
......
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