Index: extensions/PhabBugz/lib/Revision.pm =================================================================== --- extensions/PhabBugz/lib/Revision.pm +++ extensions/PhabBugz/lib/Revision.pm @@ -27,18 +27,22 @@ # Initialization # ######################### -has id => ( is => 'ro', isa => Int ); -has phid => ( is => 'ro', isa => Str ); -has title => ( is => 'ro', isa => Str ); -has summary => ( is => 'ro', isa => Str ); -has status => ( is => 'ro', isa => Str ); -has creation_ts => ( is => 'ro', isa => Str ); -has modification_ts => ( is => 'ro', isa => Str ); -has author_phid => ( is => 'ro', isa => Str ); -has bug_id => ( is => 'ro', isa => Str ); -has view_policy => ( is => 'ro', isa => Str ); -has edit_policy => ( is => 'ro', isa => Str ); -has subscriber_count => ( is => 'ro', isa => Int ); +has id => ( is => 'ro', isa => Int ); +has phid => ( is => 'ro', isa => Str ); +has title => ( is => 'ro', isa => Str ); +has summary => ( is => 'ro', isa => Str ); +has status => ( is => 'ro', isa => Str ); +has creation_ts => ( is => 'ro', isa => Str ); +has modification_ts => ( is => 'ro', isa => Str ); +has author_phid => ( is => 'ro', isa => Str ); +has diff_phid => ( is => 'ro', isa => Str ); +has repository_phid => ( is => 'ro', isa => Maybe [Str]); +has is_draft => ( is => 'ro', isa => Bool | JSONBool); +has hold_as_draft => ( is => 'ro', isa => Bool | JSONBool); +has bug_id => ( is => 'ro', isa => Str ); +has view_policy => ( is => 'ro', isa => Str ); +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 reviews => ( is => 'lazy', isa => ArrayRef [ Dict [ user => PhabUser, status => Str ] ] ); @@ -64,11 +68,21 @@ ] ); has projects_raw => ( - is => 'ro', + is => 'ro', isa => Dict [ projectPHIDs => ArrayRef [Str] ] ); +has reviewers_extra_raw => ( + is => 'ro', + isa => ArrayRef [ + Dict [ + reviewerPHID => Str, + voidedPHID => Maybe [Str], + diffPHID => Maybe [Str] + ] + ] +); sub new_from_query { my ( $class, $params ) = @_; @@ -76,9 +90,10 @@ my $data = { queryKey => 'all', attachments => { - projects => 1, - reviewers => 1, - subscribers => 1 + 'projects' => 1, + 'reviewers' => 1, + 'subscribers' => 1, + 'reviewers-extra' => 1, }, constraints => $params }; @@ -102,24 +117,32 @@ sub BUILDARGS { my ( $class, $params ) = @_; - $params->{title} = $params->{fields}->{title}; - $params->{summary} = $params->{fields}->{summary}; - $params->{status} = $params->{fields}->{status}->{value}; - $params->{creation_ts} = $params->{fields}->{dateCreated}; - $params->{modification_ts} = $params->{fields}->{dateModified}; - $params->{author_phid} = $params->{fields}->{authorPHID}; - $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->{subscribers_raw} = $params->{attachments}->{subscribers}; - $params->{projects_raw} = $params->{attachments}->{projects}; + $params->{title} = $params->{fields}->{title}; + $params->{summary} = $params->{fields}->{summary}; + $params->{status} = $params->{fields}->{status}->{value}; + $params->{creation_ts} = $params->{fields}->{dateCreated}; + $params->{modification_ts} = $params->{fields}->{dateModified}; + $params->{author_phid} = $params->{fields}->{authorPHID}; + $params->{diff_phid} = $params->{fields}->{diffPHID}; + $params->{repository_phid} = $params->{fields}->{repositoryPHID}; + $params->{is_draft} = $params->{fields}->{isDraft}; + $params->{hold_as_draft} = $params->{fields}->{holdAsDraft}; + $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->{subscribers_raw} = $params->{attachments}->{subscribers}; + $params->{projects_raw} = $params->{attachments}->{projects}; + $params->{reviewers_extra_raw} = $params->{attachments}->{reviewers_extra}->{reviewers_extra} // []; $params->{subscriber_count} = $params->{attachments}->{subscribers}->{subscriberCount}; delete $params->{fields}; delete $params->{attachments}; + # FIXME: Remove debugging + use Data::Dumper; print STDERR Dumper $params; + return $params; } @@ -313,14 +336,21 @@ } ); - return [ - map { - { - user => $_, - status => $by_phid{ $_->phid }{status}, + my @reviewers; + foreach my $user (@{ $users }) { + my $reviewer_data = { + user => $user, + status => $by_phid{ $_->phid }{status} + }; + foreach my $reviewer_extra (@{ $self->reviewers_extra_raw }) { + if ($reviewer_extra->{reviewerPHID} eq $user->phid) { + $reviewer_data->{diff_phid} = $reviewer_extra->{diffPHID}; } - } @$users - ]; + } + push @reviewers, $reviewer_data; + } + + return \@reviewers; } sub _build_subscribers { Index: extensions/PhabBugz/lib/WebService.pm =================================================================== --- extensions/PhabBugz/lib/WebService.pm +++ extensions/PhabBugz/lib/WebService.pm @@ -136,96 +136,103 @@ } sub bug_revisions { - my ( $self, $params ) = @_; - - $self->_check_phabricator(); - - my $user = Bugzilla->login(LOGIN_REQUIRED); - - # Validate that a bug id and user id are provided - ThrowUserError('phabricator_invalid_request_params') - unless $params->{bug_id}; - - # Validate that the user can see the bug itself - my $bug = Bugzilla::Bug->check( { id => $params->{bug_id}, cache => 1 } ); - - my @revision_ids; - foreach my $attachment ( @{ $bug->attachments } ) { - next if $attachment->contenttype ne PHAB_CONTENT_TYPE; - my ($revision_id) = ( $attachment->filename =~ PHAB_ATTACHMENT_PATTERN ); - next if !$revision_id; - push @revision_ids, int $revision_id; + my ($self, $params) = @_; + + $self->_check_phabricator(); + + my $user = Bugzilla->login(LOGIN_REQUIRED); + + # Validate that a bug id and user id are provided + ThrowUserError('phabricator_invalid_request_params') unless $params->{bug_id}; + + # Validate that the user can see the bug itself + my $bug = Bugzilla::Bug->check({id => $params->{bug_id}, cache => 1}); + + my @revision_ids; + foreach my $attachment (@{$bug->attachments}) { + next if $attachment->contenttype ne PHAB_CONTENT_TYPE; + my ($revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN); + next if !$revision_id; + push @revision_ids, int $revision_id; + } + + my $response = request( + 'differential.revision.search', + { + attachments => { + 'projects' => 1, + 'reviewers' => 1, + 'subscribers' => 1, + 'reviewers-extra' => 1, + }, + constraints => {ids => \@revision_ids,}, + order => 'newest', } - - my $response = request( - 'differential.revision.search', - { - attachments => { - 'projects' => 1, - 'reviewers' => 1, - 'subscribers' => 1, - 'reviewers-extra' => 1, - }, - constraints => { - ids => \@revision_ids, - }, - order => 'newest', - } - ); - - ThrowCodeError( 'phabricator_api_error', { reason => 'Malformed Response' } ) - unless exists $response->{result}{data}; - - my $revision_status_map = { - 'abandoned' => 'Abandoned', - 'accepted' => 'Accepted', - 'changes-planned' => 'Changes Planned', - 'needs-review' => 'Needs Review', - 'needs-revision' => 'Needs Revision', + ); + + ThrowCodeError('phabricator_api_error', {reason => 'Malformed Response'}) + unless exists $response->{result}{data}; + + my $revision_status_map = { + 'abandoned' => 'Abandoned', + 'accepted' => 'Accepted', + 'changes-planned' => 'Changes Planned', + 'needs-review' => 'Needs Review', + 'needs-revision' => 'Needs Revision', + }; + + my $review_status_map = { + 'accepted' => 'Accepted', + 'accepted-prior' => 'Accepted Prior Diff', + 'added' => 'Review Requested', + 'blocking' => 'Blocking Review', + 'rejected' => 'Requested Changes', + 'resigned' => 'Resigned' + }; + + my @revisions; + foreach my $revision (@{$response->{result}{data}}) { + my $revision_obj = Bugzilla::Extension::PhabBugz::Revision->new($revision); + my $revision_data = { + id => 'D' . $revision_obj->id, + author => $revision_obj->author->name, + status => $revision_obj->status, + long_status => $revision_status_map->{$revision_obj->status} + || $revision_obj->status }; - my $review_status_map = { - accepted => 'Accepted', - added => 'Review Requested', - blocking => 'Blocking Review', - rejected => 'Requested Changes', - resigned => 'Resigned' - }; + my @reviews; + foreach my $review (@{$revision_obj->reviews}) { + my $review_data = {user => $review->{user}->name}; + if ( $review->{status} eq 'accepted' + && $review->{diff_phid} ne $revision->diff_phid) + { + $review_data->{status} = 'accepted-prior'; + $review_data->{long_status} = $review_status_map->{'accepted-prior'}; + } + else { + $review_data->{status} = $review->{status}; + $review_data->{long_status} + = $review_status_map->{$review->{status}} || $review->{status}; + } + push @reviews, $review_data; + } + $revision_data->{reviews} = \@reviews; - my @revisions; - foreach my $revision ( @{ $response->{result}{data} } ) { - my $revision_obj = Bugzilla::Extension::PhabBugz::Revision->new($revision); - my $revision_data = { - id => 'D' . $revision_obj->id, - author => $revision_obj->author->name, - status => $revision_obj->status, - long_status => $revision_status_map->{$revision_obj->status} || $revision_obj->status - }; - - my @reviews; - foreach my $review ( @{ $revision_obj->reviews } ) { - push @reviews, { - user => $review->{user}->name, - status => $review->{status}, - long_status => $review_status_map->{$review->{status}} || $review->{status} - }; - } - $revision_data->{reviews} = \@reviews; - - if ( $revision_obj->view_policy ne 'public' ) { - $revision_data->{title} = '(secured)'; - } - else { - $revision_data->{title} = $revision_obj->title; - } - - push @revisions, $revision_data; + if ($revision_obj->view_policy ne 'public') { + $revision_data->{title} = '(secured)'; } + else { + $revision_data->{title} = $revision_obj->title; + } + + push @revisions, $revision_data; + } - # sort by revision id - @revisions = sort { $a->{id} cmp $b->{id} } @revisions; + # sort by revision id + @revisions = sort { $a->{id} cmp $b->{id} } @revisions; - return { revisions => \@revisions }; + return {revisions => \@revisions}; } sub rest_resources {