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

Author: dklawren

extensions/PhabBugz/Extension.pm

@@ -118,6 +118,13 @@ sub db_schema_abstract_schema {
     ],
     INDEXES => [phabbugz_idx => {FIELDS => ['name'], TYPE => 'UNIQUE',},],
   };
+  $args->{'schema'}->{'phab_reviewer_rotation'} = {
+    FIELDS => [
+      id            => {TYPE => 'INTSERIAL',    NOTNULL => 1, PRIMARYKEY => 1,},
+      project_phid  => {TYPE => 'VARCHAR(255)', NOTNULL => 1,},
+      user_phid     => {TYPE => 'VARCHAR(255)', NOTNULL => 1,},
+    ]
+  };
 }
 
 sub install_filesystem {

extensions/PhabBugz/lib/Feed.pm

@@ -42,6 +42,7 @@ use Bugzilla::Extension::PhabBugz::Util qw(
   request
   set_attachment_approval_flags
   set_phab_user
+  set_reviewer_rotation
 );
 
 has 'is_daemon' => (is => 'rw', default => 0);
@@ -726,6 +727,10 @@ sub process_revision_change {
   $bug->update($timestamp);
   $revision->update();
 
+  ### Phabricator Reviewer Rotation
+  # This should happen after all of the above changes and Herald has had a chance to run.
+  set_reviewer_rotation($revision);
+
   # Email changes for this revisions bug and also for any other
   # bugs that previously had these revision attachments
   foreach my $bug_id ($revision->bug_id, keys %other_bugs) {

extensions/PhabBugz/lib/Revision.pm

@@ -57,8 +57,17 @@ has bug              => (is => 'lazy', isa => Object);
 has uplift_request   => (is => 'ro',   isa => Maybe[ArrayRef | Dict [ slurpy Any ]]);
 has author           => (is => 'lazy', isa => Object);
 has repository       => (is => 'lazy', isa => Maybe[PhabRepo]);
-has reviews =>
-  (is => 'lazy', isa => ArrayRef [Dict [user => PhabUser | PhabProject, status => Str]]);
+has reviews => (
+  is  => 'lazy',
+  isa => ArrayRef [
+    Dict [
+      user        => PhabUser | PhabProject,
+      status      => Str,
+      is_blocking => Bool,
+      is_project  => Bool
+    ]
+  ]
+);
 has subscribers => (is => 'lazy', isa => ArrayRef [PhabUser]);
 has projects    => (is => 'lazy', isa => ArrayRef [Project]);
 has reviewers_raw => (
@@ -87,6 +96,7 @@ has reviewers_extra_raw => (
     Dict [reviewerPHID => Str, voidedPHID => Maybe [Str], diffPHID => Maybe [Str]]
   ]
 );
+has stack_graph => (is => 'lazy', isa => Tuple[ArrayRef, ArrayRef]);
 
 sub new_from_query {
   my ($class, $params) = @_;
@@ -299,15 +309,6 @@ sub update {
   return $result;
 }
 
-#########################
-#      Accessors        #
-#########################
-
-sub secured_title {
-  my ($self) = @_;
-  return $self->is_private ? '(secure)' : $self->title;
-}
-
 #########################
 #      Builders         #
 #########################
@@ -334,26 +335,26 @@ sub _build_author {
 sub _build_reviews {
   my ($self) = @_;
 
-  my %by_phid = map { $_->{reviewerPHID} => $_ } @{$self->reviewers_raw};
-  my @users;
-  foreach my $phid (keys %by_phid) {
-    if ($phid =~ /^PHID-PROJ/) {
-      push(@users,
-        Bugzilla::Extension::PhabBugz::Project->new_from_query({phids => [$phid]}));
+  my @reviewers;
+  foreach my $raw (@{$self->reviewers_raw}) {
+    my $reviewer_data = {
+      is_blocking => ($raw->{isBlocking} ? 1 : 0),
+      is_project  => 0,
+      status      => $raw->{status},
+    };
+
+    my $reviewer_phid = $raw->{reviewerPHID};
+    if ($reviewer_phid =~ /^PHID-PROJ/) {
+      $reviewer_data->{user} = Bugzilla::Extension::PhabBugz::Project->new_from_query({phids => [$reviewer_phid]});
+      $reviewer_data->{is_project} = 1;
     }
-    elsif ($phid =~ /^PHID-USER/) {
-      push(@users,
-        Bugzilla::Extension::PhabBugz::User->new_from_query({phids => [$phid]}));
+    elsif ($reviewer_phid =~ /^PHID-USER/) {
+      $reviewer_data->{user} = Bugzilla::Extension::PhabBugz::User->new_from_query({phids => [$reviewer_phid]});
     }
-  }
-
-  my @reviewers;
-  foreach my $user (@users) {
-    my $reviewer_data = {user => $user, status => $by_phid{$user->phid}{status}};
 
     # Set to accepted-prior if the diffs reviewer are different and the reviewer status is accepted
     foreach my $reviewer_extra (@{$self->reviewers_extra_raw}) {
-      if ($reviewer_extra->{reviewerPHID} eq $user->phid) {
+      if ($reviewer_extra->{reviewerPHID} eq $reviewer_phid) {
         if ($reviewer_extra->{diffPHID}) {
           if ( $reviewer_data->{status} eq 'accepted'
             && $reviewer_extra->{diffPHID} ne $self->diff_phid)
@@ -422,6 +423,22 @@ sub _build_repository {
   return undef;
 }
 
+sub _build_stack_graph {
+  my ($self) = @_;
+
+  my @phids = keys %{$self->stack_graph_raw};
+  my @edges = ();
+
+  foreach my $node (@phids) {
+    my $predecessors = $self->stack_graph_raw->{$node};
+    foreach my $predecessor (@{$predecessors}) {
+      push @edges, [$node, $predecessor];
+    }
+  }
+
+  return (\@phids, \@edges);
+}
+
 #########################
 #       Mutators        #
 #########################
@@ -512,8 +529,6 @@ sub make_private {
   $self->set_policy('view', $new_policy->phid);
   $self->set_policy('edit', $new_policy->phid);
 
-  $self->{view_policy} = $new_policy->phid;
-
   return $self;
 }
 
@@ -528,8 +543,6 @@ sub make_public {
   $self->set_policy('view', 'public');
   $self->set_policy('edit', ($editbugs ? $editbugs->phid : 'users'));
 
-  $self->{view_policy} = 'public';
-
   my @current_group_projects
     = grep { $_->name =~ /^(bmo-.*|secure-revision)$/ } @{$self->projects};
   foreach my $project (@current_group_projects) {
@@ -539,11 +552,6 @@ sub make_public {
   return $self;
 }
 
-sub is_private {
-  my ($self) = @_;
-  return $self->{view_policy} eq 'public' ? 0 : 1;
-}
-
 sub set_private_project_tags {
   my ($self, $project_names) = @_;