Skip to content

Commit 7a06356

Browse files
committed
Bug 1960099 - When a revision has enters the needs-review state with a reviewer group assigned, query Phab for list of group users and update the reviewer
1 parent eb4c8a5 commit 7a06356

File tree

1 file changed

+155
-7
lines changed

1 file changed

+155
-7
lines changed

extensions/PhabBugz/lib/Util.pm

Lines changed: 155 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,17 @@ use Bugzilla::Error;
1717
use Bugzilla::Logging;
1818
use Bugzilla::User;
1919
use Bugzilla::Types qw(:types);
20-
use Bugzilla::Util qw(mojo_user_agent trim);
20+
use Bugzilla::Util qw(mojo_user_agent trim);
2121
use Bugzilla::Extension::PhabBugz::Constants;
2222
use Bugzilla::Extension::PhabBugz::Types qw(:types);
2323

2424
use List::MoreUtils qw(any);
25-
use List::Util qw(first);
25+
use List::Util qw(any first);
2626
use Try::Tiny;
2727
use Type::Params qw( compile );
2828
use Type::Utils;
2929
use Types::Standard qw( :types );
30-
use Mojo::JSON qw(encode_json);
30+
use Mojo::JSON qw(encode_json);
3131

3232
use base qw(Exporter);
3333

@@ -41,6 +41,7 @@ our @EXPORT = qw(
4141
request
4242
set_attachment_approval_flags
4343
set_phab_user
44+
set_reviewer_rotation
4445
);
4546

4647
use constant LEGACY_APPROVAL_MAPPING => {
@@ -188,19 +189,19 @@ sub create_revision_attachment {
188189
# BMO does not contain actual diff content.
189190
my @review_attachments
190191
= grep { is_attachment_phab_revision($_) } @{$bug->attachments};
191-
my $attachment
192-
= first { trim($_->data) eq $revision_uri } @review_attachments;
192+
my $attachment = first { trim($_->data) eq $revision_uri } @review_attachments;
193193

194194

195195
if (!defined $attachment) {
196+
196197
# No attachment is present, so we can now create new one
197198

198199
if (!$timestamp) {
199200
($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
200201
}
201202

202203
# If submitter, then switch to that user when creating attachment
203-
local $submitter->{groups} = [Bugzilla::Group->get_all]; # We need to always be able to add attachment
204+
local $submitter->{groups} = [Bugzilla::Group->get_all]; # We need to always be able to add attachment
204205
my $restore_prev_user = Bugzilla->set_user($submitter, scope_guard => 1);
205206

206207
$attachment = Bugzilla::Attachment->create({
@@ -293,7 +294,7 @@ sub get_attachment_revisions {
293294
}
294295

295296
sub request {
296-
state $check = compile(Str, HashRef, Optional[Bool]);
297+
state $check = compile(Str, HashRef, Optional [Bool]);
297298
my ($method, $data, $no_die) = $check->(@_);
298299
my $request_cache = Bugzilla->request_cache;
299300
my $params = Bugzilla->params;
@@ -332,4 +333,151 @@ sub set_phab_user {
332333
return Bugzilla->set_user($user, scope_guard => 1);
333334
}
334335

336+
sub set_reviewer_rotation {
337+
my ($revision) = @_;
338+
my $rev_identifier = 'D' . $revision->id;
339+
340+
INFO("$rev_identifier: Setting reviewer rotation.");
341+
342+
# Load a fresh version of the revision with Heralds changes.
343+
$revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query(
344+
{phids => [$revision->phid]});
345+
346+
# 1. Find out what the project reviewers and individual reviewers are. If the revision
347+
# has a reviewer group set, normally it ends in "-reviewer-rotation". Normally Herald
348+
# will set this if certain conditions are met. If there are no blocking reviewer groups,
349+
# then do nothing.
350+
my @review_projects;
351+
my @review_users;
352+
353+
# Map of phids to blocking status
354+
my %is_blocking;
355+
356+
foreach my $reviewer (@{$revision->reviews}) {
357+
# Only interested in reviewer rotation groups
358+
if ($reviewer->{is_project} && $reviewer->{user}->name =~ /-reviewer-rotation$/) {
359+
push @review_projects, $reviewer->{user};
360+
$is_blocking{$reviewer->{user}->phid} = $reviewer->{is_blocking} ? 1 : 0;
361+
}
362+
elsif (!$reviewer->{is_project}) {
363+
push @review_users, $reviewer->{user};
364+
$is_blocking{$reviewer->{user}->phid} = $reviewer->{is_blocking} ? 1 : 0;
365+
}
366+
}
367+
368+
if (!@review_projects) {
369+
INFO("$rev_identifier: Reviewer rotation projects not found.");
370+
return;
371+
}
372+
373+
INFO( "$rev_identifier: Reviewer rotation project(s) "
374+
. (join ', ', map { $_->name } @review_projects)
375+
. ' found.');
376+
377+
# 2. Once the blocking reviewer rotation groups are determined, query Phabricator for
378+
# list of group members for each and sort them by user ID descending.
379+
foreach my $project (@review_projects) {
380+
my $project_members
381+
= [sort { $a->id <=> $b->id } @{$project->members}];
382+
383+
# 3. Make sure that none of the individual group members are not already
384+
# set as a blocking reviewer. If so, then remove the blocking group and return.
385+
foreach my $member (@{$project_members}) {
386+
if (any { $_->id == $member->id } @review_users) {
387+
INFO(
388+
"$rev_identifier: Member of blocking reviewer project already set as a reviewer. Removing blocking reviewer project."
389+
);
390+
$revision->remove_reviewer($project->phid);
391+
$revision->update;
392+
next;
393+
}
394+
}
395+
396+
my $dbh = Bugzilla->dbh;
397+
398+
# 4. Find the last selected reviewer for the rotation group
399+
my $user_phid
400+
= $dbh->selectrow_array(
401+
'SELECT user_phid FROM phab_reviewer_rotation WHERE project_phid = ?',
402+
undef, $project->phid);
403+
404+
my $found_reviewer;
405+
my $index = 0;
406+
foreach my $member (@{$project_members}) {
407+
408+
# 5. If there is no member that was the last reviewer, then just pick the first
409+
# member in the list
410+
if (!$user_phid) {
411+
$found_reviewer = $member;
412+
}
413+
414+
# 6. If the member was the last select reviewer, then we need to remove them,
415+
# and pick the next member in the list.
416+
if ($user_phid eq $member->phid) {
417+
418+
# Delete old reviewer
419+
INFO(
420+
"$rev_identifier: Old reviewer " . $member->id . 'deleted and choosing next');
421+
$user_phid = undef;
422+
$dbh->do(
423+
'DELETE FROM phab_reviewer_rotation WHERE project_phid = ? AND user_phid = ?',
424+
undef, $project->phid, $user_phid);
425+
426+
# Select next member in order
427+
$found_reviewer = $project_members->[$index];
428+
$found_reviewer = $project_members->[0] if !$found_reviewer; # Loop back around
429+
}
430+
431+
# 7. Once a potential reviewer has been found, look to see if they can see the bug,
432+
# and they are not set to away (not accepting reviews). If both are are negative,
433+
# we choose the next person in the list.
434+
if ( $found_reviewer
435+
&& $found_reviewer->bugzilla_user->can_see_bug($revision->bug->id)
436+
&& $found_reviewer->bugzilla_user->settings->{block_reviews}->{value} ne 'on')
437+
{
438+
INFO("$rev_identifier: Found new reviewer " . $member->id);
439+
last;
440+
}
441+
else {
442+
$found_reviewer = undef;
443+
}
444+
445+
$index++;
446+
}
447+
448+
if ($found_reviewer) {
449+
450+
# 8. Set the user as a blocking reviewer on the revision.
451+
INFO("$rev_identifier: Adding new reviewer " . $found_reviewer->id);
452+
if ($is_blocking{$project->phid}) {
453+
$revision->add_reviewer('blocking(' . $found_reviewer->phid . ')');
454+
}
455+
else {
456+
$revision->add_reviewer($found_reviewer->phid);
457+
}
458+
459+
# 9. Remove the blocking reviewer group.
460+
INFO("$rev_identifier: Removing reviewer project.");
461+
$revision->remove_reviewer($project->phid);
462+
463+
# 10. Store the data in the phab_reviewer_rotation table so they will be
464+
# next time.
465+
INFO("$rev_identifier: Adding new blocking reviewer to database.");
466+
$dbh->do(
467+
'INSERT INTO phab_reviewer_rotation (project_phid, user_phid) VALUES (?, ?)',
468+
undef,
469+
$project->phid,
470+
$found_reviewer->phid,
471+
);
472+
473+
# 11. Add new blocking reviewer to blocking users list in case they are also
474+
# a member of the next blocking rotation group.
475+
push @review_users, $found_reviewer;
476+
}
477+
}
478+
479+
# 12. Save changes to the revision and return.
480+
$revision->update;
481+
}
482+
335483
1;

0 commit comments

Comments
 (0)