diff options
| -rw-r--r-- | extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl | 42 | ||||
| -rw-r--r-- | extensions/MyDashboard/web/js/flags.js | 144 | ||||
| -rw-r--r-- | extensions/MyDashboard/web/js/query.js | 7 | ||||
| -rw-r--r-- | extensions/MyDashboard/web/styles/mydashboard.css | 5 | ||||
| -rw-r--r-- | extensions/PhabBugz/lib/Util.pm | 50 | ||||
| -rw-r--r-- | extensions/PhabBugz/lib/WebService.pm | 114 | ||||
| -rw-r--r-- | t/008filter.t | 3 |
7 files changed, 312 insertions, 53 deletions
diff --git a/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl b/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl index 36a8db6f2..a81a5903b 100644 --- a/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl +++ b/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl @@ -105,6 +105,7 @@ <div id="query_container"> <div class="query_heading"></div> <div class="query_description"></div> + <span id="query_loading" class="items_found">Loading...</span> <span id="query_count_refresh" class="bz_default_hidden"> <span class="items_found" id="query_bugs_found">0 [% terms.bugs %] found</span> | <a class="refresh" href="javascript:void(0);" id="query_refresh">Refresh</a> @@ -126,29 +127,28 @@ %] </div> - <div id="requestee_container"> - <div class="query_heading"> - Flags Requested of You + [% BLOCK requests_table %] + <div id="[% name FILTER html %]_container" class="requests"> + <div class="query_heading">[% title FILTER html %]</div> + <span id="[% name FILTER html %]_loading" class="items_found">Loading...</span> + <span id="[% name FILTER html %]_count_refresh" class="bz_default_hidden"> + <span class="items_found" id="[% name FILTER html %]_flags_found">0 reviews found</span> + | <a class="refresh" href="javascript:void(0);" id="[% name FILTER html %]_refresh">Refresh</a> + | <a class="buglist" href="javascript:void(0);" id="[% name FILTER html %]_buglist">Buglist</a> + </span> + <div id="[% name FILTER html %]_table"></div> </div> - <span id="requestee_count_refresh" class="bz_default_hidden"> - <span class="items_found" id="requestee_flags_found">0 flags found</span> - | <a class="refresh" href="javascript:void(0);" id="requestee_refresh">Refresh</a> - | <a class="buglist" href="javascript:void(0);" id="requestee_buglist">Buglist</a> - </span> - <div id="requestee_table"></div> - </div> + [% END %] - <div id="requester_container"> - <div class="query_heading"> - Flags You Have Requested - </div> - <span id="requester_count_refresh" class="bz_default_hidden"> - <span class="items_found" id="requester_flags_found">0 flags found</span> - | <a class="refresh" href="javascript:void(0);" id="requester_refresh">Refresh</a> - | <a class="buglist" href="javascript:void(0);" id="requester_buglist">Buglist</a> - </span> - <div id="requester_table"></div> - </div> + [% ## no-008filter + # requires PhabBugz extension + IF Param('phabricator_enabled'); + PROCESS requests_table name='reviews' title='Reviews Requested of You'; + END; + + PROCESS requests_table name='requestee' title='Flags Requested of You'; + PROCESS requests_table name='requester' title='Flags You Have Requested'; + %] </div> <div style="clear:both;"></div> [% IF user.showmybugslink OR user.queries.size OR user.queries_subscribed.size %] diff --git a/extensions/MyDashboard/web/js/flags.js b/extensions/MyDashboard/web/js/flags.js index 95b256708..b56559ae3 100644 --- a/extensions/MyDashboard/web/js/flags.js +++ b/extensions/MyDashboard/web/js/flags.js @@ -16,15 +16,18 @@ $(function () { // Common var counter = 0; var dataSource = { + reviews: null, requestee: null, requester: null }; var dataTable = { + reviews: null, requestee: null, requester: null }; + var hasReviews = !!document.getElementById('reviews_container'); - var updateFlagTable = function(type) { + var updateRequestsTable = function(type) { if (!type) return; counter = counter + 1; @@ -32,32 +35,40 @@ $(function () { var callback = { success: function(e) { if (e.response) { + Y.one('#' + type + '_loading').addClass('bz_default_hidden'); Y.one('#' + type + '_count_refresh').removeClass('bz_default_hidden'); Y.one("#" + type + "_flags_found").setHTML( - e.response.results.length + ' flags found'); + e.response.results.length + + ' request' + (e.response.results.length == 1 ? '' : 's') + + ' found'); dataTable[type].set('data', e.response.results); } }, failure: function(o) { - if (o.error) { - alert("Failed to load flag list from Bugzilla:\n\n" + o.error.message); + Y.one('#' + type + '_loading').addClass('bz_default_hidden'); + Y.one('#' + type + '_count_refresh').removeClass('bz_default_hidden'); + if (o.error && o.error.message) { + alert("Failed to load requests:\n\n" + o.error.message); } else { - alert("Failed to load flag list from Bugzilla."); + alert("Failed to load requests."); } } }; + var method = type === 'reviews' ? 'PhabBugz.needs_review' : 'MyDashboard.run_flag_query'; var json_object = { version: "1.1", - method: "MyDashboard.run_flag_query", + method: method, id: counter, - params: { type : type, - Bugzilla_api_token : (BUGZILLA.api_token ? BUGZILLA.api_token : '') + params: { + type : type, + Bugzilla_api_token : (BUGZILLA.api_token ? BUGZILLA.api_token : '') } }; var stringified = Y.JSON.stringify(json_object); + Y.one('#' + type + '_loading').removeClass('bz_default_hidden'); Y.one('#' + type + '_count_refresh').addClass('bz_default_hidden'); dataTable[type].set('data', []); @@ -86,14 +97,17 @@ $(function () { }; var bugLinkFormatter = function(o) { + if (!o.data.bug_id) { + return '-'; + } var bug_closed = ""; if (o.data.bug_status == 'RESOLVED' || o.data.bug_status == 'VERIFIED') { bug_closed = "bz_closed"; } - return '<a href="show_bug.cgi?id=' + encodeURIComponent(o.value) + + return '<a href="show_bug.cgi?id=' + encodeURIComponent(o.data.bug_id) + '" target="_blank" ' + 'title="' + Y.Escape.html(o.data.bug_status) + ' - ' + - Y.Escape.html(o.data.bug_summary) + '" class="' + Y.Escape.html(bug_closed) + - '">' + o.value + '</a>'; + Y.Escape.html(o.data.bug_summary) + '" class="' + bug_closed + + '">' + o.data.bug_id + '</a>'; }; var updatedFormatter = function(o) { @@ -124,6 +138,85 @@ $(function () { } }; + var phabAuthorFormatter = function(o) { + return '<span title="' + Y.Escape.html(o.data.author_email) + '">' + + Y.Escape.html(o.data.author_name) + '</span>'; + }; + + var phabRowFormatter = function(o) { + var row = o.cell.ancestor(); + + // space in the 'flags' tables is tight + // render requests as two rows - diff title on first row, columns + // on second + + row.insert( + '<tr class="' + row.getAttribute('class') + '">' + + '<td class="yui3-datatable-cell" colspan="4">' + + '<a href="' + o.data.url + '" target="_blank">' + + Y.Escape.html('D' + o.data.id + ' - ' + o.data.title) + + '</a></td></tr>', + 'before'); + + o.cell.set('text', o.data.status == 'added' ? 'pending' : o.data.status); + + return false; + }; + + // Reviews + if (hasReviews) { + dataSource.reviews = new Y.DataSource.IO({ source: 'jsonrpc.cgi' }); + dataSource.reviews.on('error', function(e) { + console.log(e); + try { + var response = Y.JSON.parse(e.data.responseText); + if (response.error) + e.error.message = response.error.message; + } catch(ex) { + // ignore + } + }); + dataTable.reviews = new Y.DataTable({ + columns: [ + { key: 'author_email', label: 'Requester', sortable: true, + formattter: phabAuthorFormatter, allowHTML: true }, + { key: 'id', label: 'Status', sortable: true, + nodeFormatter: phabRowFormatter, allowHTML: true }, + { key: 'bug_id', label: 'Bug', sortable: true, + formatter: bugLinkFormatter, allowHTML: true }, + { key: 'updated', label: 'Updated', sortable: true, + formatter: updatedFormatter, allowHTML: true } + ], + strings: { + emptyMessage: 'No review requests.', + } + }); + + dataTable.reviews.plug(Y.Plugin.DataTableSort); + + dataTable.reviews.plug(Y.Plugin.DataTableDataSource, { + datasource: dataSource.reviews + }); + + dataSource.reviews.plug(Y.Plugin.DataSourceJSONSchema, { + schema: { + resultListLocator: 'result.result', + resultFields: [ 'author_email', 'author_name', 'bug_id', + 'bug_status', 'bug_summary', 'id', 'status', 'title', + 'updated', 'updated_fancy', 'url' ] + } + }); + + dataTable.reviews.render("#reviews_table"); + + Y.one('#reviews_refresh').on('click', function(e) { + updateRequestsTable('reviews'); + }); + Y.one('#reviews_buglist').on('click', function(e) { + loadBugList('reviews'); + }); + } + // Requestee dataSource.requestee = new Y.DataSource.IO({ source: 'jsonrpc.cgi' }); dataSource.requestee.on('error', function(e) { @@ -146,7 +239,7 @@ $(function () { formatter: updatedFormatter, allowHTML: true } ], strings: { - emptyMessage: 'No flag data found.', + emptyMessage: 'No flags requested of you.', } }); @@ -167,7 +260,7 @@ $(function () { dataTable.requestee.render("#requestee_table"); Y.one('#requestee_refresh').on('click', function(e) { - updateFlagTable('requestee'); + updateRequestsTable('requestee'); }); Y.one('#requestee_buglist').on('click', function(e) { loadBugList('requestee'); @@ -196,7 +289,7 @@ $(function () { formatter: updatedFormatter, allowHTML: true } ], strings: { - emptyMessage: 'No flag data found.', + emptyMessage: 'No requested flags found.', } }); @@ -214,19 +307,24 @@ $(function () { } }); - // Initial load - Y.on("contentready", function (e) { - updateFlagTable("requestee"); - }, "#requestee_table"); - Y.on("contentready", function (e) { - updateFlagTable("requester"); - }, "#requester_table"); - Y.one('#requester_refresh').on('click', function(e) { - updateFlagTable('requester'); + updateRequestsTable('requester'); }); Y.one('#requester_buglist').on('click', function(e) { loadBugList('requester'); }); + + // Initial load + if (hasReviews) { + Y.on("contentready", function (e) { + updateRequestsTable('reviews'); + }, "#reviews_table"); + } + Y.on("contentready", function (e) { + updateRequestsTable("requestee"); + }, "#requestee_table"); + Y.on("contentready", function (e) { + updateRequestsTable("requester"); + }, "#requester_table"); }); }); diff --git a/extensions/MyDashboard/web/js/query.js b/extensions/MyDashboard/web/js/query.js index a95c0be61..e5e0979a1 100644 --- a/extensions/MyDashboard/web/js/query.js +++ b/extensions/MyDashboard/web/js/query.js @@ -77,6 +77,7 @@ $(function() { var bugQueryCallback = { success: function(e) { if (e.response) { + Y.one('#query_loading').addClass('bz_default_hidden'); Y.one('#query_count_refresh').removeClass('bz_default_hidden'); Y.one("#query_container .query_description").setHTML(e.response.meta.description); Y.one("#query_container .query_heading").setHTML(e.response.meta.heading); @@ -100,6 +101,8 @@ $(function() { } }, failure: function(o) { + Y.one('#query_loading').addClass('bz_default_hidden'); + Y.one('#query_count_refresh').removeClass('bz_default_hidden'); if (o.error) { alert("Failed to load bug list from Bugzilla:\n\n" + o.error.message); } else { @@ -114,6 +117,7 @@ $(function() { counter = counter + 1; lastChangesCache = {}; + Y.one('#query_loading').removeClass('bz_default_hidden'); Y.one('#query_count_refresh').addClass('bz_default_hidden'); bugQueryTable.set('data', []); bugQueryTable.render("#query_table"); @@ -173,6 +177,9 @@ $(function() { { key: "bug_status", label: "Status", sortable: true }, { key: "short_desc", label: "Summary", sortable: true }, ], + strings: { + emptyMessage: 'Zarro Boogs found' + } }); var last_changes_source = Y.one('#last-changes-template').getHTML(), diff --git a/extensions/MyDashboard/web/styles/mydashboard.css b/extensions/MyDashboard/web/styles/mydashboard.css index 1011a9143..ef34bb100 100644 --- a/extensions/MyDashboard/web/styles/mydashboard.css +++ b/extensions/MyDashboard/web/styles/mydashboard.css @@ -18,10 +18,13 @@ white-space: nowrap; } +#mydashboard .requests { + margin-bottom: 2em; +} + .query_heading { font-size: 18px; font-weight: 600; - margin: 5px 0; color: rgb(72, 72, 72); } 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', + }, + }, ]; } diff --git a/t/008filter.t b/t/008filter.t index cae1e6880..443fb2b4f 100644 --- a/t/008filter.t +++ b/t/008filter.t @@ -147,6 +147,9 @@ sub directive_ok { $directive =~ s/^\s*//; $directive =~ s/\s*$//; + # Ignore blocks explicitly marked as ok + return 1 if $directive =~ /\b## no-008filter\b/; + # Empty directives are ok; they are usually line break helpers return 1 if $directive eq ''; |