Bug 1482145 - refactor revison reviewers to be a list of revsion reviews

2 files changed, 21 insertions, 48 deletions

diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm
index 4d2732f94..2be96153e 100644
--- a/extensions/PhabBugz/lib/Feed.pm
+++ b/extensions/PhabBugz/lib/Feed.pm
@@ -497,31 +497,15 @@ sub process_revision_change {
 
     # REVIEWER STATUSES
 
-    my (@accepted_phids, @denied_phids, @accepted_user_ids, @denied_user_ids);
-    foreach my $reviewer (@{ $revision->reviewers }) {
-        push(@accepted_phids, $reviewer->phid) if $reviewer->{phab_review_status} eq 'accepted';
-        push(@denied_phids, $reviewer->phid) if $reviewer->{phab_review_status} eq 'rejected';
+    my (@accepted, @denied);
+    foreach my $review (@{ $revision->reviews }) {
+        push @accepted, $review->{user} if $review->{status} eq 'accepted';
+        push @denied,   $review->{user} if $review->{status} eq 'rejected';
     }
 
-    if ( @accepted_phids ) {
-        my $phab_users = Bugzilla::Extension::PhabBugz::User->match(
-          {
-            phids => \@accepted_phids
-          }
-        );
-        @accepted_user_ids = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @$phab_users;
-    }
-
-    if ( @denied_phids ) {
-        my $phab_users = Bugzilla::Extension::PhabBugz::User->match(
-          {
-            phids => \@denied_phids
-          }
-        );
-        @denied_user_ids = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @$phab_users;
-    }
-
-    my %reviewers_hash =  map { $_->name => 1 } @{ $revision->reviewers };
+    my @accepted_user_ids = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @accepted;
+    my @denied_user_ids   = map { $_->bugzilla_user->id } grep { defined $_->bugzilla_user } @denied;
+    my %reviewers_hash    = map { $_->{user}->name => 1 } @{ $revision->reviews };
 
     foreach my $attachment (@attachments) {
         my ($attach_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN);
diff --git a/extensions/PhabBugz/lib/Revision.pm b/extensions/PhabBugz/lib/Revision.pm
index 295713aaf..6ad906829 100644
--- a/extensions/PhabBugz/lib/Revision.pm
+++ b/extensions/PhabBugz/lib/Revision.pm
@@ -41,7 +41,7 @@ has edit_policy      => ( is => 'ro',   isa => Str );
 has subscriber_count => ( is => 'ro',   isa => Int );
 has bug              => ( is => 'lazy', isa => Object );
 has author           => ( is => 'lazy', isa => Object );
-has reviewers        => ( is => 'lazy', isa => ArrayRef [PhabUser] );
+has reviews          => ( is => 'lazy', isa => ArrayRef [ Dict [ user => PhabUser, status => Str ] ] );
 has subscribers      => ( is => 'lazy', isa => ArrayRef [PhabUser] );
 has projects         => ( is => 'lazy', isa => ArrayRef [Project] );
 has reviewers_raw => (
@@ -111,7 +111,7 @@ sub BUILDARGS {
     $params->{bug_id}           = $params->{fields}->{'bugzilla.bug-id'};
     $params->{view_policy}      = $params->{fields}->{policy}->{view};
     $params->{edit_policy}      = $params->{fields}->{policy}->{edit};
-    $params->{reviewers_raw}    = $params->{attachments}->{reviewers}->{reviewers};
+    $params->{reviewers_raw}    = $params->{attachments}->{reviewers}->{reviewers} // [];
     $params->{subscribers_raw}  = $params->{attachments}->{subscribers};
     $params->{projects_raw}     = $params->{attachments}->{projects};
     $params->{subscriber_count} =
@@ -303,35 +303,24 @@ sub _build_author {
     }
 }
 
-sub _build_reviewers {
+sub _build_reviews {
     my ($self) = @_;
 
-    return $self->{reviewers} if $self->{reviewers};
-    return [] unless $self->reviewers_raw;
-
-    my @phids;
-    foreach my $reviewer ( @{ $self->reviewers_raw } ) {
-        push @phids, $reviewer->{reviewerPHID};
-    }
-
-    return [] unless @phids;
-
+    my %by_phid = map { $_->{reviewerPHID} => $_ } @{ $self->reviewers_raw };
     my $users = Bugzilla::Extension::PhabBugz::User->match(
-      {
-        phids => \@phids
-      }
+        {
+            phids => [keys %by_phid]
+        }
     );
 
-    foreach my $user (@$users) {
-        foreach my $reviewer_data ( @{ $self->reviewers_raw } ) {
-            if ( $reviewer_data->{reviewerPHID} eq $user->phid ) {
-                $user->{phab_review_status} = $reviewer_data->{status};
-                last;
+    return [
+        map {
+            {
+                user => $_,
+                status => $by_phid{ $_->phid }{status},
             }
-        }
-    }
-
-    return $self->{reviewers} = $users;
+        } @$users
+    ];
 }
 
 sub _build_subscribers {