Commit cef127e1 authored by Jacopo's avatar Jacopo

Excludes MR author from Review roulette

Excludes MR author from gitlab_ui and single_codebase Review roulette
results.
parent 8ade1f87
---
title: Excludes MR author from Review roulette
merge_request: 28886
author: Jacopo Beschi @jacopo-beschi
type: fixed
FRONTEND_MAINTAINERS = %w[filipa iamphill psimyn sarahghp mishunov].freeze
UX_MAINTAINERS = %w[tauriedavis rverissimo].freeze
NO_REVIEWER = 'No reviewer available'.freeze
def mention_single_codebase_approvers
frontend_maintainers = %w(@filipa @iamphill @psimyn @sarahghp @mishunov)
ux_maintainers = %w(@tauriedavis @rverissimo)
canonical_branch_name =
roulette.canonical_branch_name(gitlab.mr_json['source_branch'])
random = roulette.new_random(canonical_branch_name)
frontend_maintainers = helper.new_teammates(FRONTEND_MAINTAINERS)
ux_maintainers = helper.new_teammates(UX_MAINTAINERS)
rows = []
users = []
if gitlab.mr_labels.include?('frontend')
frontend_maintainer = frontend_maintainers.sample
frontend_maintainer =
roulette.spin_for_person(frontend_maintainers, random: random)
rows << "| ~frontend | `#{frontend_maintainer}`"
users << frontend_maintainer
rows << "| ~frontend | #{frontend_maintainer&.markdown_name || NO_REVIEWER}"
end
if gitlab.mr_labels.include?('UX')
ux_maintainers = ux_maintainers.sample
ux_maintainers =
roulette.spin_for_person(ux_maintainers, random: random)
rows << "| ~UX | `#{ux_maintainers}`"
users << ux_maintainers
rows << "| ~UX | #{ux_maintainers&.markdown_name || NO_REVIEWER}"
end
if rows.empty?
backup_maintainer = frontend_maintainers.sample
rows << "| ~frontend / ~UX | `#{backup_maintainer}`"
users << backup_maintainer
rows << "| ~frontend / ~UX | #{backup_maintainer.markdown_name}"
end
markdown(<<~MARKDOWN.strip)
......
......@@ -31,6 +31,9 @@ Please consider creating a merge request to
for them.
MARKDOWN
NO_REVIEWER = 'No reviewer available'.freeze
NO_MAINTAINER = 'No maintainer available'.freeze
def spin_for_category(team, project, category, branch_name)
random = roulette.new_random(branch_name)
labels = gitlab.mr_labels
......@@ -49,7 +52,7 @@ def spin_for_category(team, project, category, branch_name)
reviewer = roulette.spin_for_person(reviewers + traintainers + traintainers, random: random)
maintainer = roulette.spin_for_person(maintainers, random: random)
"| #{helper.label_for_category(category)} | #{reviewer&.markdown_name} | #{maintainer&.markdown_name} |"
"| #{helper.label_for_category(category)} | #{reviewer&.markdown_name || NO_REVIEWER} | #{maintainer&.markdown_name || NO_MAINTAINER} |"
end
def build_list(items)
......@@ -85,9 +88,6 @@ if changes.any? && !gitlab.mr_labels.include?('single codebase') && !gitlab.mr_l
[]
end
# Exclude the MR author from the team for selection purposes
team.delete_if { |teammate| teammate.username == gitlab.mr_author }
project = helper.project_name
unknown = changes.fetch(:unknown, [])
......
def new_teammates(usernames)
usernames.map { |u| ::Gitlab::Danger::Teammate.new('username' => u) }
end
FRONTEND_MAINTAINERS = %w[filipa iamphill].freeze
BACKEND_MAINTAINERS = %w[rspeicher rymai yorickpeterse godfat].freeze
NO_REVIEWER = 'No reviewer available'.freeze
def mention_single_codebase_approvers
canonical_branch_name =
......@@ -8,8 +8,8 @@ def mention_single_codebase_approvers
random = roulette.new_random(canonical_branch_name)
frontend_maintainers = new_teammates(%w[filipa iamphill])
backend_maintainers = new_teammates(%w[rspeicher rymai yorickpeterse godfat])
frontend_maintainers = helper.new_teammates(FRONTEND_MAINTAINERS)
backend_maintainers = helper.new_teammates(BACKEND_MAINTAINERS)
rows = []
......@@ -17,14 +17,14 @@ def mention_single_codebase_approvers
frontend_maintainer =
roulette.spin_for_person(frontend_maintainers, random: random)
rows << "| ~frontend | #{frontend_maintainer.markdown_name}"
rows << "| ~frontend | #{frontend_maintainer&.markdown_name || NO_REVIEWER}"
end
if gitlab.mr_labels.include?('backend')
backend_maintainer =
roulette.spin_for_person(backend_maintainers, random: random)
rows << "| ~backend | #{backend_maintainer.markdown_name}"
rows << "| ~backend | #{backend_maintainer&.markdown_name || NO_REVIEWER}"
end
if rows.empty?
......
......@@ -124,6 +124,10 @@ module Gitlab
%r{\.(md|txt)\z} => :none, # To reinstate roulette for documentation, set to `:docs`.
%r{\.js\z} => :frontend
}.freeze
def new_teammates(usernames)
usernames.map { |u| Gitlab::Danger::Teammate.new('username' => u) }
end
end
end
end
......@@ -45,22 +45,20 @@ module Gitlab
# Known issue: If someone is rejected due to OOO, and then becomes not OOO, the
# selection will change on next spin
def spin_for_person(people, random:)
person = nil
people = people.dup
people.size.times do
person = people.sample(random: random)
people.shuffle(random: random)
.find(&method(:valid_person?))
end
break person unless out_of_office?(person)
private
people -= [person]
def valid_person?(person)
!mr_author?(person) && !out_of_office?(person)
end
person
def mr_author?(person)
person.username == gitlab.mr_author
end
private
def out_of_office?(person)
username = CGI.escape(person.username)
api_endpoint = "https://gitlab.com/api/v4/users/#{username}/status"
......
......@@ -202,4 +202,14 @@ describe Gitlab::Danger::Helper do
it { is_expected.to eq(expected_label) }
end
end
describe '#new_teammates' do
it 'returns an array of Teammate' do
usernames = %w[filipa iamphil]
teammates = helper.new_teammates(usernames)
expect(teammates.map(&:username)).to eq(usernames)
end
end
end
......@@ -98,4 +98,47 @@ describe Gitlab::Danger::Roulette do
is_expected.to contain_exactly(ce_teammate_matcher)
end
end
describe '#spin_for_person' do
let(:person1) { Gitlab::Danger::Teammate.new('username' => 'rymai') }
let(:person2) { Gitlab::Danger::Teammate.new('username' => 'godfat') }
let(:author) { Gitlab::Danger::Teammate.new('username' => 'filipa') }
let(:ooo) { Gitlab::Danger::Teammate.new('username' => 'jacopo-beschi') }
before do
stub_person_message(person1, 'making GitLab magic')
stub_person_message(person2, 'making GitLab magic')
stub_person_message(ooo, 'OOO till 15th')
# we don't stub Filipa, as she is the author and
# we should not fire request checking for her
allow(subject).to receive_message_chain(:gitlab, :mr_author).and_return(author.username)
end
it 'returns a random person' do
persons = [person1, person2]
selected = subject.spin_for_person(persons, random: Random.new)
expect(selected.username).to be_in(persons.map(&:username))
end
it 'excludes OOO persons' do
expect(subject.spin_for_person([ooo], random: Random.new)).to be_nil
end
it 'excludes mr.author' do
expect(subject.spin_for_person([author], random: Random.new)).to be_nil
end
private
def stub_person_message(person, message)
body = { message: message }.to_json
WebMock
.stub_request(:get, "https://gitlab.com/api/v4/users/#{person.username}/status")
.to_return(body: body)
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