From 867057e89a8f309cedcc1affbe00923fd8f8b3c2 Mon Sep 17 00:00:00 2001 From: byron jones Date: Sat, 21 Apr 2018 01:18:53 +0800 Subject: Bug 1440828 - Phabricator review requests should show up on the BMO dashboard --- extensions/PhabBugz/lib/Util.pm | 50 ++++++++++++++- extensions/PhabBugz/lib/WebService.pm | 114 ++++++++++++++++++++++++++++++++-- 2 files changed, 156 insertions(+), 8 deletions(-) (limited to 'extensions/PhabBugz') diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm index a640f52a1..0f9351285 100644 --- a/extensions/PhabBugz/lib/Util.pm +++ b/extensions/PhabBugz/lib/Util.pm @@ -21,20 +21,22 @@ use Bugzilla::Extension::PhabBugz::Constants; use JSON::XS qw(encode_json decode_json); use List::Util qw(first); use LWP::UserAgent; +use Taint::Util qw(untaint); use base qw(Exporter); -our @EXPORT = qw( +our @EXPORT_OK = qw( add_comment_to_revision add_security_sync_comments - create_revision_attachment create_private_revision_policy create_project + create_revision_attachment edit_revision_policy get_attachment_revisions get_bug_role_phids get_members_by_bmo_id get_members_by_phid + get_needs_review get_phab_bmo_ids get_project_phid get_revisions_by_ids @@ -490,7 +492,12 @@ sub request { if $response->is_error; my $result; - my $result_ok = eval { $result = decode_json( $response->content); 1 }; + my $result_ok = eval { + my $content = $response->content; + untaint($content); + $result = decode_json( $content ); + 1; + }; if (!$result_ok || $result->{error_code}) { ThrowCodeError('phabricator_api_error', { reason => 'JSON decode failure' }) if !$result_ok; @@ -548,4 +555,41 @@ sub add_security_sync_comments { Bugzilla->set_user($old_user); } +sub get_needs_review { + my ($user) = @_; + $user //= Bugzilla->user; + return unless $user->id; + + my $ids = get_members_by_bmo_id([$user]); + return [] unless @$ids; + my $phid_user = $ids->[0]; + + my $diffs = request( + 'differential.revision.search', + { + attachments => { + reviewers => 1, + }, + constraints => { + reviewerPHIDs => [$phid_user], + statuses => [qw( needs-review )], + }, + order => 'newest', + } + ); + ThrowCodeError('phabricator_api_error', { reason => 'Malformed Response' }) + unless exists $diffs->{result}{data}; + + # extract this reviewer's status from 'attachments' + my @result; + foreach my $diff (@{ $diffs->{result}{data} }) { + my $attachments = delete $diff->{attachments}; + my $reviewers = $attachments->{reviewers}{reviewers}; + my $review = first { $_->{reviewerPHID} eq $phid_user } @$reviewers; + $diff->{fields}{review_status} = $review->{status}; + push @result, $diff; + } + return \@result; +} + 1; diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm index 5b6310de6..adf127a1f 100644 --- a/extensions/PhabBugz/lib/WebService.pm +++ b/extensions/PhabBugz/lib/WebService.pm @@ -20,34 +20,42 @@ use Bugzilla::Constants; use Bugzilla::Error; use Bugzilla::Extension::Push::Util qw(is_public); use Bugzilla::User; -use Bugzilla::Util qw(detaint_natural); +use Bugzilla::Util qw(detaint_natural datetime_from time_ago); use Bugzilla::WebService::Constants; use Bugzilla::Extension::PhabBugz::Constants; use Bugzilla::Extension::PhabBugz::Util qw( add_security_sync_comments - create_revision_attachment create_private_revision_policy + create_revision_attachment edit_revision_policy get_bug_role_phids + get_phab_bmo_ids + get_needs_review get_project_phid get_revisions_by_ids + get_security_sync_groups intersect is_attachment_phab_revision make_revision_public request - get_security_sync_groups ); -use List::Util qw(first); +use DateTime (); +use List::Util qw(first uniq); use List::MoreUtils qw(any); use MIME::Base64 qw(decode_base64); +use constant READ_ONLY => qw( + needs_review +); + use constant PUBLIC_METHODS => qw( check_user_permission_for_bug obsolete_attachments revision update_reviewer_statuses + needs_review ); sub revision { @@ -291,6 +299,96 @@ sub obsolete_attachments { return { result => \@updated_attach_ids }; } +sub needs_review { + my ($self, $params) = @_; + ThrowUserError('phabricator_not_enabled') + unless Bugzilla->params->{phabricator_enabled}; + my $user = Bugzilla->login(LOGIN_REQUIRED); + my $dbh = Bugzilla->dbh; + + my $reviews = get_needs_review(); + + # map author phids to bugzilla users + my $author_id_map = get_phab_bmo_ids({ + phids => [ + uniq + grep { defined } + map { $_->{fields}{authorPHID} } + @$reviews + ] + }); + my %author_phab_to_id = map { $_->{phid} => $_->{id} } @$author_id_map; + my $author_users = Bugzilla::User->new_from_list([ map { $_->{id} } @$author_id_map ]); + my %author_id_to_user = map { $_->id => $_ } @$author_users; + + # bug data + my $visible_bugs = $user->visible_bugs([ + uniq + grep { $_ } + map { $_->{fields}{'bugzilla.bug-id'} } + @$reviews + ]); + + # get all bug statuses and summaries in a single query to avoid creation of + # many bug objects + my %bugs; + if (@$visible_bugs) { + #<<< + my $bug_rows =$dbh->selectall_arrayref( + 'SELECT bug_id, bug_status, short_desc ' . + ' FROM bugs ' . + ' WHERE bug_id IN (' . join(',', ('?') x @$visible_bugs) . ')', + { Slice => {} }, + @$visible_bugs + ); + #>>> + %bugs = map { $_->{bug_id} => $_ } @$bug_rows; + } + + # build result + my $datetime_now = DateTime->now(time_zone => $user->timezone); + my @result; + foreach my $review (@$reviews) { + my $review_flat = { + id => $review->{id}, + status => $review->{fields}{review_status}, + title => $review->{fields}{title}, + url => Bugzilla->params->{phabricator_base_uri} . 'D' . $review->{id}, + }; + + # show date in user's timezone + my $datetime = DateTime->from_epoch( + epoch => $review->{fields}{dateModified}, + time_zone => 'UTC' + ); + $datetime->set_time_zone($user->timezone); + $review_flat->{updated} = $datetime->strftime('%Y-%m-%d %T %Z'); + $review_flat->{updated_fancy} = time_ago($datetime, $datetime_now); + + # review requester + if (my $author = $author_id_to_user{$author_phab_to_id{ $review->{fields}{authorPHID} }}) { + $review_flat->{author_name} = $author->name; + $review_flat->{author_email} = $author->email; + } + else { + $review_flat->{author_name} = 'anonymous'; + $review_flat->{author_email} = 'anonymous'; + } + + # referenced bug + if (my $bug_id = $review->{fields}{'bugzilla.bug-id'}) { + my $bug = $bugs{$bug_id}; + $review_flat->{bug_id} = $bug_id; + $review_flat->{bug_status} = $bug->{bug_status}; + $review_flat->{bug_summary} = $bug->{short_desc}; + } + + push @result, $review_flat; + } + + return { result => \@result }; +} + sub _phabricator_precheck { my ($user) = @_; @@ -334,7 +432,13 @@ sub rest_resources { PUT => { method => 'obsolete_attachments', } - } + }, + # Review requests + qw{^/phabbugz/needs_review$}, { + GET => { + method => 'needs_review', + }, + }, ]; } -- cgit v1.2.3-24-g4f1b