From cfb84c6899369ceeac12511e39693c5ed12f3b21 Mon Sep 17 00:00:00 2001 From: Kohei Yoshino Date: Sun, 4 Mar 2018 19:00:54 -0500 Subject: Bug 1429344 - Review requests in requests dropdown should link to MozReview or GitHub instead of Bugzilla details page --- extensions/Review/web/js/badge.js | 43 +++++++++++++++++++++++------ request.cgi | 12 ++++---- template/en/default/global/header.html.tmpl | 1 + template/en/default/request/queue.json.tmpl | 9 +++--- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/extensions/Review/web/js/badge.js b/extensions/Review/web/js/badge.js index d104ce5ae..4ae135407 100644 --- a/extensions/Review/web/js/badge.js +++ b/extensions/Review/web/js/badge.js @@ -70,8 +70,8 @@ Bugzilla.Review.Badge = class Badge { // Sort requests from new to old, then group reviews/feedbacks asked by the same person in the same bug _requests.reverse().forEach(_req => { - const dup_index = requests.findIndex(req => req.requester === _req.requester - && req.bug_id === _req.bug_id && req.type === _req.type && req.attach_id && _req.attach_id); + const dup_index = requests.findIndex(req => req.requester === _req.requester && req.type === _req.type + && req.bug_id === _req.bug_id && req.attach_mimetype === _req.attach_mimetype); if (dup_index > -1) { requests[dup_index].dup_count++; @@ -86,17 +86,14 @@ Bugzilla.Review.Badge = class Badge { const $li = document.createElement('li'); const [, name, email] = req.requester.match(/^(?:(.*)\s<)?(.+?)>?$/); const pretty_name = name ? name.replace(/([\[\(<‹].*?[›>\)\]]|\:[\w\-]+|\s+\-\s+.*)/g, '').trim() : email; - const link = req.attach_id && req.dup_count === 1 - ? `attachment.cgi?id=${req.attach_id}&action=edit` : `show_bug.cgi?id=${req.bug_id}`; + const [link, attach_label] = this.get_link(req); $li.setAttribute('role', 'none'); - $li.innerHTML = `` + `` + `` + ``; @@ -107,6 +104,36 @@ Bugzilla.Review.Badge = class Badge { $ul.appendChild($fragment); $ul.hidden = false; } + + /** + * Get the link to a request as well as the label of any attachment. It could be the direct link to the attachment + * unless multiple requests are grouped. + * @param {Object} req - A request object. + * @returns {Array} The result including the link and attachment label. + */ + get_link(req) { + const dup = req.dup_count > 1; + const splinter_base = BUGZILLA.param.splinter_base; + const x_types = ['github-pull-request', 'review-board-request', 'phabricator-request', 'google-doc']; + const is_patch = req.attach_ispatch; + const [is_ghpr, is_rbr, is_phr, is_gdoc] = x_types.map(type => req.attach_mimetype === `text/x-${type}`); + const is_redirect = is_ghpr || is_rbr || is_phr || is_gdoc; + const is_file = req.attach_id && !is_patch && !is_redirect; + + const link = (is_patch && !dup && splinter_base) + ? `${splinter_base}&bug=${req.bug_id}&attachment=${req.attach_id}` + : (is_redirect && !dup) ? `attachment.cgi?id=${req.attach_id}` // external redirect + : ((is_patch || is_file) && !dup) ? `attachment.cgi?id=${req.attach_id}&action=edit` + : `show_bug.cgi?id=${req.bug_id}`; + + const attach_label = (is_patch || is_rbr || is_phr) ? (dup ? `${req.dup_count} patches` : 'a patch') + : is_ghpr ? (dup ? `${req.dup_count} pull requests` : 'a pull request') + : is_gdoc ? (dup ? `${req.dup_count} Google Docs` : 'a Google Doc') + : is_file ? (dup ? `${req.dup_count} files` : 'a file') + : undefined; + + return [link, attach_label]; + } } window.addEventListener('DOMContentLoaded', () => new Bugzilla.Review.Badge(), { once: true }); diff --git a/request.cgi b/request.cgi index 0d1dd138b..9f5d249cc 100755 --- a/request.cgi +++ b/request.cgi @@ -102,6 +102,7 @@ sub queue { requesters.realname, requesters.login_name, requestees.realname, requestees.login_name, COUNT(privs.group_id), " . $dbh->sql_date_format('flags.modification_date', '%Y.%m.%d %H:%i') . ", + attachments.mimetype, attachments.ispatch, bugs.bug_status, bugs.priority, @@ -248,7 +249,7 @@ sub queue { requesters.login_name, requestees.realname, requestees.login_name, flags.modification_date, attachments.ispatch cclist_accessible, bugs.reporter, bugs.reporter_accessible, - bugs.assigned_to, attachments.ispatch'); + bugs.assigned_to, attachments.mimetype'); # Group the records, in other words order them by the group column # so the loop in the display template can break them up into separate @@ -291,10 +292,11 @@ sub queue { 'requestee' => ($data->[11] ? "$data->[11] <$data->[12]>" : $data->[12]) , 'restricted' => $data->[13] ? 1 : 0, 'created' => $data->[14], - 'ispatch' => $data->[15], - 'bug_status' => $data->[16], - 'priority' => $data->[17], - 'bug_severity' => $data->[18], + 'attach_mimetype' => $data->[15], + 'attach_ispatch' => $data->[16], + 'bug_status' => $data->[17], + 'priority' => $data->[18], + 'bug_severity' => $data->[19], }; push(@requests, $request); } diff --git a/template/en/default/global/header.html.tmpl b/template/en/default/global/header.html.tmpl index 428354233..ded28d186 100644 --- a/template/en/default/global/header.html.tmpl +++ b/template/en/default/global/header.html.tmpl @@ -107,6 +107,7 @@ [%- js_BUGZILLA = { param => { maxusermatches => Param('maxusermatches'), + splinter_base => Param('splinter_base'), }, constant => { COMMENT_COLS => constants.COMMENT_COLS, diff --git a/template/en/default/request/queue.json.tmpl b/template/en/default/request/queue.json.tmpl index 7f86072b4..50638de9e 100644 --- a/template/en/default/request/queue.json.tmpl +++ b/template/en/default/request/queue.json.tmpl @@ -6,10 +6,9 @@ # defined by the Mozilla Public License, v. 2.0. #%] [% RAWPERL %] -my @display_columns = ( - "requester", "requestee", "type", "status", "bug_id", "bug_summary", - "attach_id", "attach_summary", "ispatch", "created", "category", "restricted" -); +my @display_columns = ('requester', 'requestee', 'type', 'created', 'category', + 'restricted', 'bug_id', 'bug_summary', 'attach_id', + 'attach_summary', 'attach_mimetype', 'attach_ispatch'); my $requests = $stash->get('requests'); my $time_filter = $context->filter('time', [ '%Y-%m-%dT%H:%M:%SZ', 'UTC' ]); my $mail_filter = $context->filter('email'); @@ -28,7 +27,7 @@ foreach my $request (@$requests) { elsif ( $column =~ /_id$/ ) { $val = $request->{$column} ? 0 + $request->{$column} : undef; } - elsif ( $column =~ /^is/ or $column eq 'restricted' ) { + elsif ( $column =~ /^(restricted|attach_ispatch)$/ ) { $val = $request->{$column} ? \1 : \0; } else { -- cgit v1.2.3-24-g4f1b