diff options
author | Dylan William Hardison <dylan@hardison.net> | 2018-07-08 06:59:55 +0200 |
---|---|---|
committer | Dylan William Hardison <dylan@hardison.net> | 2018-07-08 06:59:55 +0200 |
commit | be1f92450788dc89280c9e04a4bf983b5d7fac54 (patch) | |
tree | b5513fd846597fe5f152177dbbda88dca08fdf5f /extensions | |
parent | 9bbd8d368598046c47964ee043620621b6c3634b (diff) | |
parent | 446a08b30b0dbaac9f2b88e0a5cad410f0446140 (diff) | |
download | bugzilla-be1f92450788dc89280c9e04a4bf983b5d7fac54.tar.gz bugzilla-be1f92450788dc89280c9e04a4bf983b5d7fac54.tar.xz |
Merge remote-tracking branch 'bmo/master'
Diffstat (limited to 'extensions')
53 files changed, 1429 insertions, 1371 deletions
diff --git a/extensions/BMO/Extension.pm b/extensions/BMO/Extension.pm index f23ab723f..d2e62eccd 100644 --- a/extensions/BMO/Extension.pm +++ b/extensions/BMO/Extension.pm @@ -1213,7 +1213,7 @@ sub _attachment_fetch_github_pr_diff { warn "Github fetch error: $pr_diff, " . $response->status_line; return "Error retrieving Github pull request diff for " . $self->data; } - return $response->content; + return $response->decoded_content; } # redirect automatically to github urls diff --git a/extensions/BMO/lib/Data.pm b/extensions/BMO/lib/Data.pm index 4df05581c..30a32c95e 100644 --- a/extensions/BMO/lib/Data.pm +++ b/extensions/BMO/lib/Data.pm @@ -109,12 +109,11 @@ tie(%$cf_visible_in_products, "Tie::IxHash", "Calendar" => [], "Composer" => [], "Core" => [], + "DevTools" => [], "Directory" => [], "External Software Affecting Firefox" => [], "Firefox" => [], "Firefox for Android" => [], - "Firefox for Metro" => [], - "Firefox OS" => [], "JSS" => [], "MailNews Core" => [], "Mozilla Labs" => [], @@ -133,6 +132,7 @@ tie(%$cf_visible_in_products, "Tie::IxHash", "Testing" => [], "Thunderbird" => [], "Toolkit" => [], + "WebExtensions" => [], }, qr/^cf_due_date$/ => { "bugzilla.mozilla.org" => [], diff --git a/extensions/BMO/template/en/default/account/create.html.tmpl b/extensions/BMO/template/en/default/account/create.html.tmpl index 59ff822bd..fa897e0e8 100644 --- a/extensions/BMO/template/en/default/account/create.html.tmpl +++ b/extensions/BMO/template/en/default/account/create.html.tmpl @@ -156,10 +156,12 @@ function onSubmit() { </tr> <tr> <td colspan="2"> + <input type="checkbox" id="etiquette" value="agreed"> + <label for="etiquette"> I have read <a href="page.cgi?id=etiquette.html">[% terms.Bugzilla %] Etiquette</a> and the <a href="https://www.mozilla.org/about/governance/policies/participation/">Mozilla Community Participation Guidelines</a> and agree to abide by them. - <input type="checkbox" id="etiquette" value="agreed"> + </label> </td> </tr> <tr> diff --git a/extensions/BMO/template/en/default/bug/create/comment-third-party-apps.txt.tmpl b/extensions/BMO/template/en/default/bug/create/comment-third-party-apps.txt.tmpl deleted file mode 100644 index 7a30902f2..000000000 --- a/extensions/BMO/template/en/default/bug/create/comment-third-party-apps.txt.tmpl +++ /dev/null @@ -1,22 +0,0 @@ -[%# This Source Code Form is subject to the terms of the Mozilla Public - # License, v. 2.0. If a copy of the MPL was not distributed with this - # file, You can obtain one at http://mozilla.org/MPL/2.0/. - # - # This Source Code Form is "Incompatible With Secondary Licenses", as - # defined by the Mozilla Public License, v. 2.0. - #%] - -[% USE Bugzilla %] -[% cgi = Bugzilla.cgi %] - -> Detailed description of the issue or enhancement request -[%+ cgi.param('desc') %] - -> Is this issue reproducible? -[%+ cgi.param('reproducible') %] - -> Device Information -[%+ cgi.param('device_info') %] - -> Mozilla Reference Device -[%+ cgi.param('reference_device') %] diff --git a/extensions/BMO/template/en/default/bug/create/create-dev-engagement-event.html.tmpl b/extensions/BMO/template/en/default/bug/create/create-dev-engagement-event.html.tmpl index 1197952fe..5478de00f 100644 --- a/extensions/BMO/template/en/default/bug/create/create-dev-engagement-event.html.tmpl +++ b/extensions/BMO/template/en/default/bug/create/create-dev-engagement-event.html.tmpl @@ -142,7 +142,7 @@ <option value="Yes">Yes</option> <option value="No">No</option> </select> - <div id="developer_event_warning" class="bz_default_hidden"> + <div id="developer_event_warning" class="warning bz_default_hidden"> The Developer Events Team only participates in developer events. Form submission has been disabled. </div> diff --git a/extensions/BMO/template/en/default/bug/create/create-third-party-apps.html.tmpl b/extensions/BMO/template/en/default/bug/create/create-third-party-apps.html.tmpl deleted file mode 100644 index b6173e6e4..000000000 --- a/extensions/BMO/template/en/default/bug/create/create-third-party-apps.html.tmpl +++ /dev/null @@ -1,191 +0,0 @@ -[%# This Source Code Form is subject to the terms of the Mozilla Public - # License, v. 2.0. If a copy of the MPL was not distributed with this - # file, You can obtain one at http://mozilla.org/MPL/2.0/. - # - # This Source Code Form is "Incompatible With Secondary Licenses", as - # defined by the Mozilla Public License, v. 2.0. - #%] - -[% PROCESS global/variables.none.tmpl %] - -[% inline_css = BLOCK %] -#third_party_form { - width: 60%; -} -#third_party_form .required:after { - content: " *"; - color: red; -} -#third_party_form .field_label { - text-align: left; - font-weight: bold; -} -#third_party_form .field_desc, -#third_party_form .head_desc { - word-wrap: normal; -} -#third_party_form .head_desc { - font-size: 1.25em; - padding-bottom: .5em; -} -#third_party_form .form_section { - margin-bottom: 1em; - padding-left: 2em; -} -.yui-calcontainer { - z-index: 2; -} -[% END %] - -[% inline_javascript = BLOCK %] - function validateAndSubmit() { - var alert_text = ''; - $('label.required').each(function () { - var id = $(this).attr('for'); - if (id && !isFilledOut(id)) { - var desc = $(this).text() || id; - alert_text += "Please select or enter a value for" + - desc.replace(/[\r\n]+/, "").replace(/\s+/g, " ") + "\n"; - } - }); - if (alert_text != '') { - alert(alert_text); - return false; - } - return true; - } -[% END %] - -[% PROCESS global/header.html.tmpl - title = "Third Party Applications Issue Form" - style = inline_css - style_urls = [ 'skins/standard/enter_bug.css', - 'skins/standard/attachment.css' ] - javascript = inline_javascript - javascript_urls = [ 'js/attachment.js', - 'js/field.js', - 'js/util.js', - 'extensions/BMO/web/js/form_validate.js' ] -%] - -<h2>Third Party Applications Issue Form</h2> - -<form method="post" action="post_bug.cgi" id="third_party_form" - class="enter_bug_form" enctype="multipart/form-data" - onSubmit="return validateAndSubmit();"> -<input type="hidden" name="format" value="third-party-apps"> -<input type="hidden" name="product" value="Marketplace"> -<input type="hidden" name="component" value="3rd Party Applications"> -<input type="hidden" name="rep_platform" value="All"> -<input type="hidden" name="op_sys" value="All"> -<input type="hidden" name="priority" value="--"> -<input type="hidden" name="version" value="unspecified"> -<input type="hidden" name="comment" id="comment" value=""> -<input type="hidden" name="status_whiteboard" id="status_whiteboard" value=""> -<input type="hidden" name="contenttypemethod" value="autodetect"> -<input type="hidden" name="token" value="[% token FILTER html %]"> - -<div class="form_section"> - <label for="short_desc" class="field_label required"> - Summary - </label> - <div class="field_desc"> - Please enter the name of the application in brackets and a short summary of - the issue. (ex. [App Name] Fails to launch at startup. - </div> - <input type="text" name="short_desc" id="short_desc" size="40" class="wide"> -</div> - -<div class="form_section"> - <label for="bug_file_loc" class="field_label required"> - Marketplace App URL - </label> - <div class="field_desc"> - Please copy the App listing page URL from Marketplace. - </div> - <input type="text" name="bug_file_loc" id="bug_file_loc" size="40" class="wide"> -</div> - -<div class="form_section"> - <label for="desc" class="field_label required"> - Detailed description of the issue or enhancement request - </label> - <div class="field_desc"> - Please enter a description of the issue with as much detail as possible. - </div> - <textarea name="desc" id="desc" rows="10" cols="60" class="wide"></textarea> -</div> - -<div class="form_section"> - <label for="reproducible" class="field_label required"> - Is this issue reproducible? - </label> - <div class="field_desc"> - Please let us know if this issue is reproducible, steps to reproduction, and - how often the issue presents itself. - </div> - <textarea name="reproducible" id="reproducible" rows="10" cols="60" class="wide"></textarea> -</div> - -<div class="form_section"> - <label for="device_info" class="field_label required"> - Device Information - </label> - <div class="field_desc"> - Please let us know what device you were using when you experienced this - issue. For FirefoxOS devices; please include the device make, model, and OS - version at a minimum. RAM, Chipset, Screen size, and buildID is preferred - if that information is available. (You can find BuildID by using this nav - path on your FirefoxOS device; Settings—>Device info—>More info) - </div> - <textarea name="device_info" id="device_info" rows="10" cols="60" class="wide"></textarea> -</div> - -<div class="form_section"> - <label for="reference_device" class="field_label required"> - Mozilla Reference Device - </label> - <div class="field_desc"> - Have you tried to reproduce this issue on a Mozilla reference device? If - yes, please post your results, which device you were using, and OS version. - </div> - <textarea name="reference_device" id="reference_device" rows="10" cols="60" class="wide"></textarea> -</div> - -<div class="form_section"> - <label class="field_label"> - Have additional materials? - </label> - <div class="field_desc"> - Please attach any additional information that will help us understand and - diagnose this issue. Screenshots, testing documents, etc. - </div> - <table> - <tr> - <td> - <label for="description" class="field_label"> - Description - </label> - </td> - <td> - <input type="text" name="description" id="description" size="40" class="wide"> - </td> - </tr> - <tr> - <td> - <label for="data" class="field_label"> - Filename - </label> - </th> - <td> - <input type="file" id="data" name="data" size="60"> - </td> - </tr> - </table> -</div> - -<input type="submit" id="commit" value="Submit Request"> - -</form> - -[% PROCESS global/footer.html.tmpl %] diff --git a/extensions/BMO/template/en/default/bug/create/created-fxos-betaprogram.html.tmpl b/extensions/BMO/template/en/default/bug/create/created-fxos-betaprogram.html.tmpl deleted file mode 100644 index 145c976cd..000000000 --- a/extensions/BMO/template/en/default/bug/create/created-fxos-betaprogram.html.tmpl +++ /dev/null @@ -1,30 +0,0 @@ -[%# This Source Code Form is subject to the terms of the Mozilla Public - # License, v. 2.0. If a copy of the MPL was not distributed with this - # file, You can obtain one at http://mozilla.org/MPL/2.0/. - # - # This Source Code Form is "Incompatible With Secondary Licenses", as - # defined by the Mozilla Public License, v. 2.0. - #%] - -[% PROCESS global/variables.none.tmpl %] - -[% PROCESS global/header.html.tmpl - title = "Firefox OS Beta Bug Submission" -%] - -<h1>Thank you!</h1> - -<p> - Thank you for submitting feedback about Firefox OS. -</p> - -<p> - We'll link any [% terms.bugs %] we file (or are already filed) as a result of - this feedback to this report so you can be notified about their progress. -</p> - -<p style="font-size: x-small"> - Reference: <a href="show_bug.cgi?id=[% id FILTER uri %]">#[% id FILTER html %]</a> -</p> - -[% PROCESS global/footer.html.tmpl %] diff --git a/extensions/BMO/template/en/default/bug/create/custom_forms.none.tmpl b/extensions/BMO/template/en/default/bug/create/custom_forms.none.tmpl index f0ddf0229..7b588f765 100644 --- a/extensions/BMO/template/en/default/bug/create/custom_forms.none.tmpl +++ b/extensions/BMO/template/en/default/bug/create/custom_forms.none.tmpl @@ -152,16 +152,6 @@ custom_forms = { title => "Internet Public Policy Issue", }, ], - "Marketplace" => [ - { - link => "form.fxos.preload.app", - title => "Firefox OS Pre-load App", - }, - { - link => "form.third.party", - title => "Third Party Applications Issue Form", - }, - ], "Data Compliance" => [ { link => "form.data.compliance", diff --git a/extensions/BMO/template/en/default/email/bugmail.html.tmpl b/extensions/BMO/template/en/default/email/bugmail.html.tmpl index d5cb3af9a..0b08e4a86 100644 --- a/extensions/BMO/template/en/default/email/bugmail.html.tmpl +++ b/extensions/BMO/template/en/default/email/bugmail.html.tmpl @@ -111,7 +111,10 @@ [% END %] [% END %] </ul> + Configure your email settings at + <a href="[% urlbase FILTER none %]userprefs.cgi?tab=email">[% urlbase FILTER none %]userprefs.cgi?tab=email</a>. </div> + <div itemscope itemtype="http://schema.org/EmailMessage"> <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction"> [%# Filtering of the URL param is not required & would break the URL when the comment anchor is set %] diff --git a/extensions/BMO/template/en/default/global/choose-product.html.tmpl b/extensions/BMO/template/en/default/global/choose-product.html.tmpl index 4329b716a..679d812e1 100644 --- a/extensions/BMO/template/en/default/global/choose-product.html.tmpl +++ b/extensions/BMO/template/en/default/global/choose-product.html.tmpl @@ -91,10 +91,22 @@ icon="firefox_ios.png" %] [% INCLUDE easyproduct + name="DevTools" + icon="devtools.png" + %] + [% INCLUDE easyproduct + name="WebExtensions" + icon="webextensions.png" + %] + [% INCLUDE easyproduct name="Toolkit" icon="component.png" %] [% INCLUDE easyproduct + name="Mozilla Localizations" + icon="localization.png" + %] + [% INCLUDE easyproduct name="Thunderbird" icon="thunderbird.png" %] @@ -103,10 +115,6 @@ icon="seamonkey.png" %] [% INCLUDE easyproduct - name="Mozilla Localizations" - icon="localization.png" - %] - [% INCLUDE easyproduct name="Data Platform and Tools" icon="sync.png" %] @@ -173,12 +181,14 @@ </section> </div> +[% IF NOT is_describe %] <div id="guided"> <a href="enter_bug.cgi?format=guided"> <img src="extensions/BMO/web/images/guided.png" width="16" height="16"> Switch to the [% terms.Bugzilla %] Helper</a> | <a href="page.cgi?id=custom_forms.html">Custom [% terms.bug %] entry forms</a> </div> +[% END %] [% PROCESS global/footer.html.tmpl %] diff --git a/extensions/BMO/template/en/default/hook/attachment/createformcontents-mimetypes.html.tmpl b/extensions/BMO/template/en/default/hook/attachment/createformcontents-mimetypes.html.tmpl deleted file mode 100644 index 3dc727b87..000000000 --- a/extensions/BMO/template/en/default/hook/attachment/createformcontents-mimetypes.html.tmpl +++ /dev/null @@ -1,2 +0,0 @@ -[% mimetypes.push({type => "image/svg+xml", desc => "SVG image"}) %] -[% mimetypes.push({type => "application/vnd.mozilla.xul+xml", desc => "XUL"}) %]
\ No newline at end of file diff --git a/extensions/BMO/template/en/default/pages/attachment_bounty_form.html.tmpl b/extensions/BMO/template/en/default/pages/attachment_bounty_form.html.tmpl index faf32aa36..0f7966097 100644 --- a/extensions/BMO/template/en/default/pages/attachment_bounty_form.html.tmpl +++ b/extensions/BMO/template/en/default/pages/attachment_bounty_form.html.tmpl @@ -211,12 +211,12 @@ function validateAndSubmit() { </div> <div class="form_section"> - <label for="credit_2" class="field_label">Credit</label> + <label for="credit_2" class="field_label">Credit Twitter</label> <input type="text" name="credit_2" id="credit_2" size="80" value="[% form.credit.1 FILTER html %]"> </div> <div class="form_section"> - <label for="credit_3" class="field_label">Credit</label> + <label for="credit_3" class="field_label">Credit URL</label> <input type="text" name="credit_3" id="credit_3" size="80" value="[% form.credit.2 FILTER html %]"> </div> diff --git a/extensions/BMO/web/producticons/devtools.png b/extensions/BMO/web/producticons/devtools.png Binary files differnew file mode 100644 index 000000000..9800f654b --- /dev/null +++ b/extensions/BMO/web/producticons/devtools.png diff --git a/extensions/BMO/web/producticons/webextensions.png b/extensions/BMO/web/producticons/webextensions.png Binary files differnew file mode 100644 index 000000000..7e5503d97 --- /dev/null +++ b/extensions/BMO/web/producticons/webextensions.png diff --git a/extensions/BugModal/template/en/default/bug_modal/header.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/header.html.tmpl index 38b6b572f..b9a42caf3 100644 --- a/extensions/BugModal/template/en/default/bug_modal/header.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/header.html.tmpl @@ -83,12 +83,7 @@ [%# update last-visited %] [% IF user.id && user.is_involved_in_bug(bug) %] - $(function() { - bugzilla_ajax({ - url: 'rest/bug_user_last_visit/[% bug.id FILTER none %]', - type: 'POST' - }); - }); + document.addEventListener('DOMContentLoaded', () => show_new_changes_indicator(), { once: true }); [% END %] [%# expose useful data to js %] @@ -100,6 +95,7 @@ is_insider: [% user.is_insider ? "true" : "false" %], is_timetracker: [% user.is_timetracker ? "true" : "false" %], can_tag: [% user.can_tag_comments ? "true" : "false" %], + timezone: '[% user.timezone.name FILTER js %]', settings: { quote_replies: '[% user.settings.quote_replies.value FILTER js %]', zoom_textareas: [% user.settings.zoom_textareas.value == "on" ? "true" : "false" %], diff --git a/extensions/BugModal/template/en/default/bug_modal/user.html.tmpl b/extensions/BugModal/template/en/default/bug_modal/user.html.tmpl index cd05d053f..9eda7b936 100644 --- a/extensions/BugModal/template/en/default/bug_modal/user.html.tmpl +++ b/extensions/BugModal/template/en/default/bug_modal/user.html.tmpl @@ -46,7 +46,7 @@ END; href="mailto:[% u.email FILTER html %]" data-user-email="[% u.email FILTER html %]" data-user-id="[% u.id FILTER html %]" - data-show-edit="[% user.in_group('editusers') || user.bless_groups.size > 0 ? 1 : 0 %]" + data-show-edit="[% user.in_group('editusers') || user.in_group('disableusers') || user.bless_groups.size > 0 ? 1 : 0 %]" title="[% u.identity FILTER html %]" [% ELSE %] href="user_profile?user_id=[% u.id FILTER none %]" diff --git a/extensions/BugModal/web/bug_modal.css b/extensions/BugModal/web/bug_modal.css index 65aeec86d..a8c469ad6 100644 --- a/extensions/BugModal/web/bug_modal.css +++ b/extensions/BugModal/web/bug_modal.css @@ -153,6 +153,12 @@ a.activity-ref { font-weight: normal; padding-right: 5px; color: #666; + opacity: 1; + transition: all .2s; +} + +.module-spinner[aria-expanded="true"] ~ .module-subtitle { + opacity: 0; } .module .fields-lhs { @@ -532,6 +538,43 @@ td.flag-requestee { text-align: right; } +.new-changes-link { + margin: 8px 0; + border-radius: 4px; + padding: 4px; + font-size: 12px; + text-align: center; + color: #FFF; + background: #277AC1; + cursor: pointer; +} + +.new-changes-separator { + position: relative; + margin: 16px -8px; + height: 0; + border-top: 1px solid #C00; + -moz-user-select: none; + -webkit-user-select: none; + user-select: none; +} + +.new-changes-separator span { + display: inline-block; + position: absolute; + top: -10px; + right: 16px; + border: 1px solid #CCC; + border-radius: 4px; + padding: 0 4px; + height: 16px; + font-size: 10px; + line-height: 16px; + text-transform: uppercase; + color: #C00; + background-color: #FFF; +} + .change-set { clear: both; -webkit-box-shadow: 1px 1px 5px rgba(0, 0, 0, 0.1); @@ -626,6 +669,11 @@ body.platform-Win32 .comment-text, body.platform-Win64 .comment-text { width: 99% !important; } +.comment-text.bz_private { + color: darkred; + border: 1px dashed darkred; +} + .comment-tags { padding: 0 8px 2px 8px !important; } diff --git a/extensions/BugModal/web/bug_modal.js b/extensions/BugModal/web/bug_modal.js index c6abb5d7a..4a770e66c 100644 --- a/extensions/BugModal/web/bug_modal.js +++ b/extensions/BugModal/web/bug_modal.js @@ -96,6 +96,54 @@ $(function() { $('#editing').val(''); } + function saveBugComment(text) { + if (text.length < 1) return clearSavedBugComment(); + if (text.length > 65535) return; + let key = `bug-modal-saved-comment-${BUGZILLA.bug_id}`; + let value = { + text: text, + savedAt: Date.now() + }; + localStorage.setItem(key, JSON.stringify(value)); + } + + function clearSavedBugComment() { + let key = `bug-modal-saved-comment-${BUGZILLA.bug_id}`; + localStorage.removeItem(key); + } + + function restoreSavedBugComment() { + expireSavedComments(); + let key = `bug-modal-saved-comment-${BUGZILLA.bug_id}`; + let value = JSON.parse(localStorage.getItem(key)); + if (value){ + let commentBox = document.querySelector("textarea#comment"); + commentBox.value = value['text']; + if (BUGZILLA.user.settings.autosize_comments) { + autosize.update(commentBox); + } + } + } + + function expireSavedComments() { + const AGE_THRESHOLD = 7 * 24 * 60 * 60 * 1000; // 7 days in milliseconds. + let expiredKeys = []; + for (let i = 0; i < localStorage.length; i++) { + let key = localStorage.key(i); + if (key.match(/^bug-modal-saved-comment-/)) { + let value = JSON.parse(localStorage.getItem(key)); + let savedAt = value['savedAt'] || 0; + let age = Date.now() - savedAt; + if (age < 0 || age > AGE_THRESHOLD) { + expiredKeys.push(key); + } + } + } + expiredKeys.forEach((key) => { + localStorage.removeItem(key); + }); + } + // expand/colapse module $('.module-latch') .click(function(event) { @@ -309,7 +357,9 @@ $(function() { // execCommand("copy") only works on selected text $('#clip-container').show(); $('#clip').val(clipboardSummary()).select(); - document.execCommand("copy"); + $('#floating-message-text') + .text(document.execCommand("copy") ? 'Bug summary copied!' : 'Couldn’t copy bug summary'); + $('#floating-message').fadeIn(250).delay(2500).fadeOut(); $('#clip-container').hide(); }); } @@ -509,6 +559,8 @@ $(function() { keywords = data.keywords; $('#keywords') .devbridgeAutocomplete({ + appendTo: $('#main-inner'), + forceFixPosition: true, lookup: function(query, done) { query = query.toLowerCase(); var matchStart = @@ -586,6 +638,8 @@ $(function() { .toArray() .join(' ') ); + + clearSavedBugComment(); }) .attr('disabled', false); @@ -1272,9 +1326,18 @@ $(function() { } }); + // Save comments in progress + $('#comment') + .on('input', function(event) { + saveBugComment(event.target.value); + }); + // finally switch to edit mode if we navigate back to a page that was editing $(window).on('pageshow', restoreEditMode); + $(window).on('pageshow', restoreSavedBugComment); + $(window).on('focus', restoreSavedBugComment); restoreEditMode(); + restoreSavedBugComment(); }); function confirmUnsafeURL(url) { @@ -1283,6 +1346,93 @@ function confirmUnsafeURL(url) { 'The full URL is:\n\n' + url + '\n\nContinue?'); } +function show_new_changes_indicator() { + const url = `rest/bug_user_last_visit/${BUGZILLA.bug_id}`; + + // Get the last visited timestamp + bugzilla_ajax({ url }, data => { + // Save the current timestamp + bugzilla_ajax({ url, type: 'POST' }); + + if (!data[0] || !data[0].last_visit_ts) { + return; + } + + const last_visit_ts = new Date(data[0].last_visit_ts); + const new_changes = [...document.querySelectorAll('main .change-set')].filter($change => { + // Exclude hidden CC changes + return $change.clientHeight > 0 && + new Date($change.querySelector('[data-time]').getAttribute('data-time') * 1000) > last_visit_ts; + }); + + if (new_changes.length === 0) { + return; + } + + const now = new Date(); + const date_locale = document.querySelector('html').lang; + const date_options = { + year: 'numeric', + month: 'long', + day: 'numeric', + hour: 'numeric', + minute: 'numeric', + hour12: false, + timeZone: BUGZILLA.user.timezone, + timeZoneName: 'short', + }; + + if (last_visit_ts.getFullYear() === now.getFullYear()) { + delete date_options.year; + + if (last_visit_ts.getMonth() === now.getMonth() && last_visit_ts.getDate() === now.getDate()) { + delete date_options.month; + delete date_options.day; + } + } + + const $link = document.createElement('div'); + const $separator = document.createElement('div'); + const comments_count = new_changes.filter($change => !!$change.querySelector('.comment')).length; + const changes_count = new_changes.length - comments_count; + const date_attr = last_visit_ts.toISOString(); + const date_label = last_visit_ts.toLocaleString(date_locale, date_options); + + // Insert a link + $link.className = 'new-changes-link'; + $link.innerHTML = + (c => c === 0 ? '' : (c === 1 ? `${c} new comment` : `${c} new comments`))(comments_count) + + (comments_count > 0 && changes_count > 0 ? ', ' : '') + + (c => c === 0 ? '' : (c === 1 ? `${c} new change` : `${c} new changes`))(changes_count) + + ` since <time datetime="${date_attr}">${date_label}</time>`; + $link.addEventListener('click', () => { + $link.remove(); + scroll_element_into_view($separator); + }, { once: true }); + document.querySelector('#changeform').insertAdjacentElement('beforebegin', $link); + + // Insert a separator + $separator.className = 'new-changes-separator'; + $separator.innerHTML = '<span>New</span>'; + new_changes[0].insertAdjacentElement('beforebegin', $separator); + + // Remove the link once the separator goes into the viewport + if ('IntersectionObserver' in window) { + const observer = new IntersectionObserver(entries => entries.forEach(entry => { + if (entry.intersectionRatio > 0) { + observer.unobserve($separator); + $link.remove(); + } + }), { root: document.querySelector('#bugzilla-body') }); + + observer.observe($separator); + } + + // TODO: Enable auto-scroll once the modal page layout is optimized + // scroll_element_into_view($separator); + }); +} + // fix url after bug creation/update if (history && history.replaceState) { var href = document.location.href; diff --git a/extensions/BugModal/web/comments.js b/extensions/BugModal/web/comments.js index 04894506e..85c5a2368 100644 --- a/extensions/BugModal/web/comments.js +++ b/extensions/BugModal/web/comments.js @@ -355,6 +355,8 @@ $(function() { $('#ctag-add') .devbridgeAutocomplete({ + appendTo: $('#main-inner'), + forceFixPosition: true, serviceUrl: function(query) { return 'rest/bug/comment/tags/' + encodeURIComponent(query); }, diff --git a/extensions/BugModal/web/common_bug_modal.js b/extensions/BugModal/web/common_bug_modal.js index dc91824f6..6cd658045 100644 --- a/extensions/BugModal/web/common_bug_modal.js +++ b/extensions/BugModal/web/common_bug_modal.js @@ -509,6 +509,8 @@ $(function() { keywords = data.keywords; $('#keywords') .devbridgeAutocomplete({ + appendTo: $('#main-inner'), + forceFixPosition: true, lookup: function(query, done) { query = query.toLowerCase(); var matchStart = diff --git a/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl b/extensions/MyDashboard/template/en/default/pages/mydashboard.html.tmpl index 36a8db6f2..5c372db3c 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,29 @@ %] </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_light %]</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'); + title = '<a href="' _ Param('phabricator_base_uri') _ '">Phabricator</a> Reviews Requested of You'; + PROCESS requests_table name='reviews' title=title; + 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..425e42e57 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,83 @@ $(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: '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 +237,7 @@ $(function () { formatter: updatedFormatter, allowHTML: true } ], strings: { - emptyMessage: 'No flag data found.', + emptyMessage: 'No flags requested of you.', } }); @@ -167,7 +258,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 +287,7 @@ $(function () { formatter: updatedFormatter, allowHTML: true } ], strings: { - emptyMessage: 'No flag data found.', + emptyMessage: 'No requested flags found.', } }); @@ -214,19 +305,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/OpenGraph/template/en/default/hook/global/header-start.html.tmpl b/extensions/OpenGraph/template/en/default/hook/global/header-start.html.tmpl index 2d7c3379f..51c388d42 100644 --- a/extensions/OpenGraph/template/en/default/hook/global/header-start.html.tmpl +++ b/extensions/OpenGraph/template/en/default/hook/global/header-start.html.tmpl @@ -13,6 +13,9 @@ [% IF bug %] <meta property="og:description" content="[% bug.bug_status FILTER html %] ([% bug.assigned_to.login FILTER email FILTER html %]) in [% bug.product FILTER html %] - [% bug.component FILTER html %]. Last updated [% bug.delta_ts FILTER time('%Y-%m-%d') %]."> -[% ELSE %] -<meta property="og:image" content="[% urlbase FILTER none %]extensions/OpenGraph/web/moz-social-bw-rgb-1200x1200.png"> +[% ELSIF error_message %] +<meta property="og:description" content="[% error_message FILTER txt FILTER html %]"> +[% END %] +[% IF og_image %] +<meta property="og:image" content="[% urlbase FILTER none %][% og_image FILTER html %]"> [% END %] diff --git a/extensions/OrangeFactor/Extension.pm b/extensions/OrangeFactor/Extension.pm index ab2f1d749..56dd5dc6e 100644 --- a/extensions/OrangeFactor/Extension.pm +++ b/extensions/OrangeFactor/Extension.pm @@ -17,6 +17,8 @@ use Bugzilla::User::Setting; use Bugzilla::Constants; use Bugzilla::Attachment; +use DateTime; + our $VERSION = '1.0'; sub template_before_process { @@ -38,6 +40,8 @@ sub template_before_process { my $bug = exists $vars->{'bugs'} ? $vars->{'bugs'}[0] : $vars->{'bug'}; if ($bug && grep($_->name eq 'intermittent-failure', @{ $bug->keyword_objects })) { $vars->{'orange_factor'} = 1; + $vars->{'date_start'} = ( DateTime->now() - DateTime::Duration->new( days => 7 ) )->ymd(); + $vars->{'date_end'} = DateTime->now->ymd(); } } diff --git a/extensions/OrangeFactor/template/en/default/hook/bug/edit-after_custom_fields.html.tmpl b/extensions/OrangeFactor/template/en/default/hook/bug/edit-after_custom_fields.html.tmpl index a41188a63..1eedab479 100644 --- a/extensions/OrangeFactor/template/en/default/hook/bug/edit-after_custom_fields.html.tmpl +++ b/extensions/OrangeFactor/template/en/default/hook/bug/edit-after_custom_fields.html.tmpl @@ -17,9 +17,14 @@ <td> [% IF cgi.user_agent.match('(?i)gecko') %] <canvas id="orange-graph" class="bz_default_hidden"></canvas> - <span id="orange-count"></span> + <span id="orange-count" + style="display:none;" + data-date-start="[% date_start FILTER html %]" + data-date-end="[% date_end FILTER html %]" + data-bug-id="[% bug.bug_id FILTER html %]"> + </span> [% END %] - (<a href="https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=[% bug.bug_id FILTER uri %]" + (<a href="https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?startday=[% date_start FILTER uri %]&endday=[% date_end FILTER uri %]&tree=trunk&bug=[% bug.bug_id FILTER uri %]" title="Click to load Orange Factor page for this [% terms.bug %]">link</a>) </td> </tr> diff --git a/extensions/OrangeFactor/template/en/default/hook/bug_modal/edit-details_rhs.html.tmpl b/extensions/OrangeFactor/template/en/default/hook/bug_modal/edit-details_rhs.html.tmpl index 30cd65cd2..d6ee5f127 100644 --- a/extensions/OrangeFactor/template/en/default/hook/bug_modal/edit-details_rhs.html.tmpl +++ b/extensions/OrangeFactor/template/en/default/hook/bug_modal/edit-details_rhs.html.tmpl @@ -16,10 +16,15 @@ %] [% IF cgi.user_agent.match('(?i)gecko') %] <canvas id="orange-graph" style="display:none;"></canvas> - <span id="orange-count" style="display:none;"></span> + <span id="orange-count" + style="display:none;" + data-date-start="[% date_start FILTER html %]" + data-date-end="[% date_end FILTER html %]" + data-bug-id="[% bug.bug_id FILTER html %]"> + </span> [% END %] <span id="orange-link"> - (<a href="https://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=[% bug.bug_id FILTER uri %]" + (<a href="https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?startday=[% date_start FILTER uri %]&endday=[% date_end FILTER uri %]&tree=trunk&bug=[% bug.bug_id FILTER uri %]" title="Click to load Orange Factor page for this [% terms.bug %]">link</a>) </span> [% END %] diff --git a/extensions/OrangeFactor/web/js/orange_factor.js b/extensions/OrangeFactor/web/js/orange_factor.js index fa9411cf8..78fbb5eb3 100644 --- a/extensions/OrangeFactor/web/js/orange_factor.js +++ b/extensions/OrangeFactor/web/js/orange_factor.js @@ -8,21 +8,17 @@ $(function() { 'use strict'; - var dayMs = 24 * 60 * 60 * 1000; - var limit = 7; function getOrangeCount(data) { - data = data.oranges; - var total = 0, - days = [], - date = getCurrentDateMs() - limit * dayMs; - for(var i = 0; i < limit; i++) { - var iso = dateString(new Date(date)); - var count = data[iso] ? data[iso].orangecount : 0; - days.push(count); - total += count; - date += dayMs; - } + let days = []; + let total = 0; + + data.forEach(entry => { + let failureCount = entry["failure_count"]; + days.push(failureCount); + total += failureCount; + }); + displayGraph(days); displayCount(total); } @@ -53,39 +49,26 @@ $(function() { $('#orange-count').text(count + ' failures on trunk in the past week'); } - function dateString(date) { - function norm(part) { - return JSON.stringify(part).length == 2 ? part : '0' + part; - } - return date.getFullYear() - + "-" + norm(date.getMonth() + 1) - + "-" + norm(date.getDate()); - } - - function getCurrentDateMs() { - var d = new Date; - return d.getTime(); - }; - function orangify() { - $('#orange-count') - .text('Loading...') - .show(); - var bugId = document.forms['changeform'].id.value; - var request = { + let $orangeCount = $('#orange-count'); + let queryParams = $.param({ + bug: $orangeCount.data('bug-id'), + startday: $orangeCount.data('date-start'), + endday: $orangeCount.data('date-end'), + tree: 'trunk' + }); + let request = { dataType: "json", - xhrFields: { - withCredentials: true - }, - url: "https://brasstacks.mozilla.com/orangefactor/api/count?" + - "bugid=" + encodeURIComponent(bugId) + "&tree=trunk" + url: `https://treeherder.mozilla.org/api/failurecount/?${queryParams}` }; + + $orangeCount.text('Loading...').show(); $.ajax(request) .done(function(data) { getOrangeCount(data); }) .fail(function() { - $('#orange-count').text('Please sign into OrangeFactor first'); + $orangeCount.text('Unable to load OrangeFactor at this time.'); }); } diff --git a/extensions/PhabBugz/Extension.pm b/extensions/PhabBugz/Extension.pm index ee96901a2..c857c60ab 100644 --- a/extensions/PhabBugz/Extension.pm +++ b/extensions/PhabBugz/Extension.pm @@ -18,11 +18,6 @@ use Bugzilla::Extension::PhabBugz::Feed; our $VERSION = '0.01'; -BEGIN { - *Bugzilla::User::phab_phid = sub { return $_[0]->{phab_phid}; }; - *Bugzilla::User::phab_review_status = sub { return $_[0]->{phab_review_status}; }; -} - sub config_add_panels { my ($self, $args) = @_; my $modules = $args->{panel_modules}; @@ -85,7 +80,7 @@ sub install_filesystem { my $files = $args->{'files'}; my $extensionsdir = bz_locations()->{'extensionsdir'}; - my $scriptname = $extensionsdir . "/PhabBugz/bin/phabbugzd.pl"; + my $scriptname = $extensionsdir . "/PhabBugz/bin/phabbugz_feed.pl"; $files->{$scriptname} = { perms => Bugzilla::Install::Filesystem::WS_EXECUTE diff --git a/extensions/PhabBugz/bin/update_project_members.pl b/extensions/PhabBugz/bin/update_project_members.pl deleted file mode 100755 index fe62170a6..000000000 --- a/extensions/PhabBugz/bin/update_project_members.pl +++ /dev/null @@ -1,99 +0,0 @@ -#!/usr/bin/perl - -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. -# -# This Source Code Form is "Incompatible With Secondary Licenses", as -# defined by the Mozilla Public License, v. 2.0. - -use 5.10.1; -use strict; -use warnings; - -use lib qw(. lib local/lib/perl5); - -use Bugzilla; -BEGIN { Bugzilla->extensions() } - -use Bugzilla::Constants; -use Bugzilla::Error; -use Bugzilla::Group; - -use Bugzilla::Extension::PhabBugz::Project; -use Bugzilla::Extension::PhabBugz::Util qw( - get_phab_bmo_ids -); - -Bugzilla->usage_mode(USAGE_MODE_CMDLINE); - -my ($phab_uri, $phab_sync_groups); - -if (!Bugzilla->params->{phabricator_enabled}) { - exit; -} - -# Sanity checks -unless ($phab_uri = Bugzilla->params->{phabricator_base_uri}) { - ThrowUserError('invalid_phabricator_uri'); -} - -unless ($phab_sync_groups = Bugzilla->params->{phabricator_sync_groups}) { - ThrowUserError('invalid_phabricator_sync_groups'); -} - -# Loop through each group and perform the following: -# -# 1. Load flattened list of group members -# 2. Check to see if Phab project exists for 'bmo-<group_name>' -# 3. Create if does not exist with locked down policy. -# 4. Set project members to exact list -# 5. Profit - -my $sync_groups = Bugzilla::Group->match({ name => [ split('[,\s]+', $phab_sync_groups) ] }); - -foreach my $group (@$sync_groups) { - # Create group project if one does not yet exist - my $phab_project_name = 'bmo-' . $group->name; - my $project = Bugzilla::Extension::PhabBugz::Project->new_from_query({ - name => $phab_project_name - }); - if (!$project) { - my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({ - name => 'secure-revision' - }); - $project = Bugzilla::Extension::PhabBugz::Project->create({ - name => $phab_project_name, - description => 'BMO Security Group for ' . $group->name, - view_policy => $secure_revision->phid, - edit_policy => $secure_revision->phid, - join_policy => $secure_revision->phid - }); - } - - if (my @group_members = get_group_members($group)) { - $project->set_members(\@group_members); - $project->update(); - } -} - -sub get_group_members { - my ($group) = @_; - my $group_obj = ref $group ? $group : Bugzilla::Group->check({ name => $group }); - my $members_all = $group_obj->members_complete(); - my %users; - foreach my $name (keys %$members_all) { - foreach my $user (@{ $members_all->{$name} }) { - $users{$user->id} = $user; - } - } - - # Look up the phab ids for these users - my $phab_users = get_phab_bmo_ids({ ids => [ keys %users ] }); - foreach my $phab_user (@{ $phab_users }) { - $users{$phab_user->{id}}->{phab_phid} = $phab_user->{phid}; - } - - # We only need users who have accounts in phabricator - return grep { $_->phab_phid } values %users; -}
\ No newline at end of file diff --git a/extensions/PhabBugz/lib/Config.pm b/extensions/PhabBugz/lib/Config.pm index 85ba37e74..d4b71430b 100644 --- a/extensions/PhabBugz/lib/Config.pm +++ b/extensions/PhabBugz/lib/Config.pm @@ -31,11 +31,6 @@ sub get_param_list { checker => \&check_urlbase }, { - name => 'phabricator_sync_groups', - type => 't', - default => '', - }, - { name => 'phabricator_api_key', type => 't', default => '', diff --git a/extensions/PhabBugz/lib/Constants.pm b/extensions/PhabBugz/lib/Constants.pm index 754130f0b..1692f8fb9 100644 --- a/extensions/PhabBugz/lib/Constants.pm +++ b/extensions/PhabBugz/lib/Constants.pm @@ -16,12 +16,16 @@ our @EXPORT = qw( PHAB_AUTOMATION_USER PHAB_ATTACHMENT_PATTERN PHAB_CONTENT_TYPE - PHAB_POLL_SECONDS + PHAB_FEED_POLL_SECONDS + PHAB_USER_POLL_SECONDS + PHAB_GROUP_POLL_SECONDS ); use constant PHAB_ATTACHMENT_PATTERN => qr/^phabricator-D(\d+)/; use constant PHAB_AUTOMATION_USER => 'phab-bot@bmo.tld'; use constant PHAB_CONTENT_TYPE => 'text/x-phabricator-request'; -use constant PHAB_POLL_SECONDS => 5; +use constant PHAB_FEED_POLL_SECONDS => $ENV{PHAB_FEED_POLL} // 5; +use constant PHAB_USER_POLL_SECONDS => $ENV{PHAB_USER_POLL} // 60; +use constant PHAB_GROUP_POLL_SECONDS => $ENV{PHAB_GROUP_POLL} // 300; 1; diff --git a/extensions/PhabBugz/lib/Feed.pm b/extensions/PhabBugz/lib/Feed.pm index 074ecc0f9..c46d36c13 100644 --- a/extensions/PhabBugz/lib/Feed.pm +++ b/extensions/PhabBugz/lib/Feed.pm @@ -9,14 +9,21 @@ package Bugzilla::Extension::PhabBugz::Feed; use 5.10.1; +use IO::Async::Timer::Periodic; +use IO::Async::Loop; use List::Util qw(first); use List::MoreUtils qw(any); use Moo; +use Scalar::Util qw(blessed); +use Try::Tiny; -use Bugzilla::Logging; use Bugzilla::Constants; +use Bugzilla::Error; +use Bugzilla::Field; +use Bugzilla::Logging; +use Bugzilla::Mailer; use Bugzilla::Search; -use Bugzilla::Util qw(diff_arrays with_writable_database with_readonly_database); +use Bugzilla::Util qw(diff_arrays format_time with_writable_database with_readonly_database); use Bugzilla::Extension::PhabBugz::Constants; use Bugzilla::Extension::PhabBugz::Policy; @@ -25,13 +32,9 @@ use Bugzilla::Extension::PhabBugz::User; use Bugzilla::Extension::PhabBugz::Util qw( add_security_sync_comments create_revision_attachment - edit_revision_policy get_bug_role_phids - get_phab_bmo_ids - get_project_phid get_security_sync_groups is_attachment_phab_revision - make_revision_public request set_phab_user ); @@ -40,38 +43,85 @@ has 'is_daemon' => ( is => 'rw', default => 0 ); sub start { my ($self) = @_; - while (1) { - my $ok = eval { - if (Bugzilla->params->{phabricator_enabled}) { + + # Query for new revisions or changes + my $feed_timer = IO::Async::Timer::Periodic->new( + first_interval => 0, + interval => PHAB_FEED_POLL_SECONDS, + reschedule => 'drift', + on_tick => sub { + try{ $self->feed_query(); - Bugzilla->_cleanup(); } - 1; - }; - ERROR( $@ // "unknown exception" ) unless $ok; - sleep(PHAB_POLL_SECONDS); - } + catch { + FATAL($_); + }; + Bugzilla->_cleanup(); + }, + ); + + # Query for new users + my $user_timer = IO::Async::Timer::Periodic->new( + first_interval => 0, + interval => PHAB_USER_POLL_SECONDS, + reschedule => 'drift', + on_tick => sub { + try{ + $self->user_query(); + } + catch { + FATAL($_); + }; + Bugzilla->_cleanup(); + }, + ); + + # Update project membership in Phabricator based on Bugzilla groups + my $group_timer = IO::Async::Timer::Periodic->new( + first_interval => 0, + interval => PHAB_GROUP_POLL_SECONDS, + reschedule => 'drift', + on_tick => sub { + try{ + $self->group_query(); + } + catch { + FATAL($_); + }; + Bugzilla->_cleanup(); + }, + ); + + my $loop = IO::Async::Loop->new; + $loop->add($feed_timer); + $loop->add($user_timer); + $loop->add($group_timer); + $feed_timer->start; + $user_timer->start; + $group_timer->start; + $loop->run; } sub feed_query { my ($self) = @_; - my $dbh = Bugzilla->dbh; + + local Bugzilla::Logging->fields->{type} = 'FEED'; # Ensure Phabricator syncing is enabled if (!Bugzilla->params->{phabricator_enabled}) { - INFO("PHABRICATOR SYNC DISABLED"); + WARN("PHABRICATOR SYNC DISABLED"); return; } # PROCESS NEW FEED TRANSACTIONS - INFO("FEED: Fetching new transactions"); + INFO("Fetching new stories"); my $story_last_id = $self->get_last_id('feed'); # Check for new transctions (stories) my $new_stories = $self->new_stories($story_last_id); - INFO("FEED: No new stories") unless @$new_stories; + INFO("No new stories") unless @$new_stories; # Process each story foreach my $story_data (@$new_stories) { @@ -81,25 +131,29 @@ sub feed_query { my $object_phid = $story_data->{objectPHID}; my $story_text = $story_data->{text}; - DEBUG("STORY ID: $story_id"); - DEBUG("STORY PHID: $story_phid"); - DEBUG("AUTHOR PHID: $author_phid"); - DEBUG("OBJECT PHID: $object_phid"); + TRACE("STORY ID: $story_id"); + TRACE("STORY PHID: $story_phid"); + TRACE("AUTHOR PHID: $author_phid"); + TRACE("OBJECT PHID: $object_phid"); INFO("STORY TEXT: $story_text"); # Only interested in changes to revisions for now. if ($object_phid !~ /^PHID-DREV/) { - DEBUG("SKIPPING: Not a revision change"); + INFO("SKIPPING: Not a revision change"); $self->save_last_id($story_id, 'feed'); next; } # Skip changes done by phab-bot user - my $phab_users = get_phab_bmo_ids({ phids => [$author_phid] }); - if (@$phab_users) { - my $user = Bugzilla::User->new({ id => $phab_users->[0]->{id}, cache => 1 }); - if ($user->login eq PHAB_AUTOMATION_USER) { - DEBUG("SKIPPING: Change made by phabricator user"); + my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query( + { + phids => [ $author_phid ] + } + ); + + if ($phab_user && $phab_user->bugzilla_id) { + if ($phab_user->bugzilla_user->login eq PHAB_AUTOMATION_USER) { + INFO("SKIPPING: Change made by phabricator user"); $self->save_last_id($story_id, 'feed'); next; } @@ -111,15 +165,69 @@ sub feed_query { $self->save_last_id($story_id, 'feed'); } + # Process any build targets as well. + my $dbh = Bugzilla->dbh; + + INFO("Checking for revisions in draft mode"); + my $build_targets = $dbh->selectall_arrayref( + "SELECT name, value FROM phabbugz WHERE name LIKE 'build_target_%'", + { Slice => {} } + ); + + my $delete_build_target = $dbh->prepare( + "DELETE FROM phabbugz WHERE name = ? AND VALUE = ?" + ); + + foreach my $target (@$build_targets) { + my ($revision_id) = ($target->{name} =~ /^build_target_(\d+)$/); + my $build_target = $target->{value}; + + next unless $revision_id && $build_target; + + INFO("Processing revision $revision_id with build target $build_target"); + + my $revision = + Bugzilla::Extension::PhabBugz::Revision->new_from_query( + { + ids => [ int($revision_id) ] + } + ); + + with_writable_database { + $self->process_revision_change($revision, " created D" . $revision->id); + }; + + # Set the build target to a passing status to + # allow the revision to exit draft state + request( 'harbormaster.sendmessage', { + buildTargetPHID => $build_target, + type => 'pass' + } ); + + $delete_build_target->execute($target->{name}, $target->{value}); + } +} + +sub user_query { + my ( $self ) = @_; + + local Bugzilla::Logging->fields->{type} = 'USERS'; + + # Ensure Phabricator syncing is enabled + if (!Bugzilla->params->{phabricator_enabled}) { + WARN("PHABRICATOR SYNC DISABLED"); + return; + } + # PROCESS NEW USERS - INFO("FEED: Fetching new users"); + INFO("Fetching new users"); my $user_last_id = $self->get_last_id('user'); # Check for new users my $new_users = $self->new_users($user_last_id); - INFO("FEED: No new users") unless @$new_users; + INFO("No new users") unless @$new_users; # Process each new user foreach my $user_data (@$new_users) { @@ -128,10 +236,10 @@ sub feed_query { my $user_realname = $user_data->{fields}{realName}; my $object_phid = $user_data->{phid}; - DEBUG("USER ID: $user_id"); - DEBUG("USER LOGIN: $user_login"); - DEBUG("USER REALNAME: $user_realname"); - DEBUG("OBJECT PHID: $object_phid"); + TRACE("ID: $user_id"); + TRACE("LOGIN: $user_login"); + TRACE("REALNAME: $user_realname"); + TRACE("OBJECT PHID: $object_phid"); with_readonly_database { $self->process_new_user($user_data); @@ -140,26 +248,110 @@ sub feed_query { } } +sub group_query { + my ($self) = @_; + + local Bugzilla::Logging->fields->{type} = 'GROUPS'; + + # Ensure Phabricator syncing is enabled + if ( !Bugzilla->params->{phabricator_enabled} ) { + WARN("PHABRICATOR SYNC DISABLED"); + return; + } + + # PROCESS SECURITY GROUPS + + INFO("Updating group memberships"); + + # Loop through each group and perform the following: + # + # 1. Load flattened list of group members + # 2. Check to see if Phab project exists for 'bmo-<group_name>' + # 3. Create if does not exist with locked down policy. + # 4. Set project members to exact list including phab-bot user + # 5. Profit + + my $sync_groups = Bugzilla::Group->match( { isactive => 1, isbuggroup => 1 } ); + + # Load phab-bot Phabricator user to add as a member of each project group later + my $phab_bmo_user = Bugzilla::User->new( { name => PHAB_AUTOMATION_USER, cache => 1 } ); + my $phab_user = + Bugzilla::Extension::PhabBugz::User->new_from_query( + { + ids => [ $phab_bmo_user->id ] + } + ); + + # secure-revision project that will be used for bmo group projects + my $secure_revision = + Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + name => 'secure-revision' + } + ); + + foreach my $group (@$sync_groups) { + # Create group project if one does not yet exist + my $phab_project_name = 'bmo-' . $group->name; + my $project = + Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + name => $phab_project_name + } + ); + + if ( !$project ) { + INFO("Project $phab_project_name not found. Creating."); + $project = Bugzilla::Extension::PhabBugz::Project->create( + { + name => $phab_project_name, + description => 'BMO Security Group for ' . $group->name, + view_policy => $secure_revision->phid, + edit_policy => $secure_revision->phid, + join_policy => $secure_revision->phid + } + ); + } + else { + # Make sure that the group project permissions are set properly + INFO("Updating permissions on $phab_project_name"); + $project->set_policy( 'view', $secure_revision->phid ); + $project->set_policy( 'edit', $secure_revision->phid ); + $project->set_policy( 'join', $secure_revision->phid ); + } + + # Make sure phab-bot also a member of the new project group so that it can + # make policy changes to the private revisions + INFO("Setting project members for " . $project->name); + my $set_members = $self->get_group_members( $group ); + push @$set_members, $phab_user unless grep $_->phid eq $phab_user->phid, @$set_members; + $project->set_members( $set_members ); + $project->update(); + } +} + sub process_revision_change { my ($self, $revision_phid, $story_text) = @_; # Load the revision from Phabricator - my $revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] }); + my $revision = + blessed $revision_phid + ? $revision_phid + : Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] }); # NO BUG ID if (!$revision->bug_id) { if ($story_text =~ /\s+created\s+D\d+/) { # If new revision and bug id was omitted, make revision public - DEBUG("No bug associated with new revision. Marking public."); - $revision->set_policy('view', 'public'); - $revision->set_policy('edit', 'users'); + INFO("No bug associated with new revision. Marking public."); + $revision->make_public(); $revision->update(); INFO("SUCCESS"); return; } else { - DEBUG("SKIPPING: No bug associated with revision change"); + INFO("SKIPPING: No bug associated with revision change"); return; } } @@ -180,11 +372,8 @@ sub process_revision_change { # If bug is public then remove privacy policy if (!@{ $bug->groups_in }) { - DEBUG('Bug is public so setting view/edit public'); - $revision->set_policy('view', 'public'); - $revision->set_policy('edit', 'users'); - my $secure_project_phid = get_project_phid('secure-revision'); - $revision->remove_project($secure_project_phid); + INFO('Bug is public so setting view/edit public'); + $revision->make_public(); } # else bug is private. else { @@ -193,54 +382,51 @@ sub process_revision_change { # If bug privacy groups do not have any matching synchronized groups, # then leave revision private and it will have be dealt with manually. if (!@set_groups) { - DEBUG('No matching groups. Adding comments to bug and revision'); + INFO('No matching groups. Adding comments to bug and revision'); add_security_sync_comments([$revision], $bug); } # Otherwise, we create a new custom policy containing the project # groups that are mapped to bugzilla groups. else { - my @set_projects = map { "bmo-" . $_ } @set_groups; + my $set_project_names = [ map { "bmo-" . $_ } @set_groups ]; # If current policy projects matches what we want to set, then # we leave the current policy alone. my $current_policy; if ($revision->view_policy =~ /^PHID-PLCY/) { - DEBUG("Loading current policy: " . $revision->view_policy); + INFO("Loading current policy: " . $revision->view_policy); $current_policy = Bugzilla::Extension::PhabBugz::Policy->new_from_query({ phids => [ $revision->view_policy ]}); - my $current_projects = $current_policy->rule_projects; - DEBUG("Current policy projects: " . join(", ", @$current_projects)); - my ($added, $removed) = diff_arrays($current_projects, \@set_projects); + my $current_project_names = [ map { $_->name } @{ $current_policy->rule_projects } ]; + INFO("Current policy projects: " . join(", ", @$current_project_names)); + my ($added, $removed) = diff_arrays($current_project_names, $set_project_names); if (@$added || @$removed) { - DEBUG('Project groups do not match. Need new custom policy'); - $current_policy= undef; + INFO('Project groups do not match. Need new custom policy'); + $current_policy = undef; } else { - DEBUG('Project groups match. Leaving current policy as-is'); + INFO('Project groups match. Leaving current policy as-is'); } } if (!$current_policy) { - DEBUG("Creating new custom policy: " . join(", ", @set_projects)); - my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects); - $revision->set_policy('view', $new_policy->phid); - $revision->set_policy('edit', $new_policy->phid); + INFO("Creating new custom policy: " . join(", ", @$set_project_names)); + $revision->make_private($set_project_names); } - my $secure_project_phid = get_project_phid('secure-revision'); - $revision->add_project($secure_project_phid); + # Subscriber list of the private revision should always match + # the bug roles such as assignee, qa contact, and cc members. + my $subscribers = get_bug_role_phids($bug); + $revision->set_subscribers($subscribers); } - - # Subscriber list of the private revision should always match - # the bug roles such as assignee, qa contact, and cc members. - my $subscribers = get_bug_role_phids($bug); - $revision->set_subscribers($subscribers); } my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()"); - my $attachment = create_revision_attachment($bug, $revision, $timestamp); - + INFO('Checking for revision attachment'); + my $attachment = create_revision_attachment($bug, $revision, $timestamp, $revision->author->bugzilla_user); + INFO('Attachment ' . $attachment->id . ' created or already exists.'); + # ATTACHMENT OBSOLETES # fixup attachments on current bug @@ -252,11 +438,11 @@ sub process_revision_change { next if $attach_revision_id != $revision->id; my $make_obsolete = $revision->status eq 'abandoned' ? 1 : 0; - DEBUG('Updating obsolete status on attachmment ' . $attachment->id); + INFO('Updating obsolete status on attachmment ' . $attachment->id); $attachment->set_is_obsolete($make_obsolete); if ($revision->title ne $attachment->description) { - DEBUG('Updating description on attachment ' . $attachment->id); + INFO('Updating description on attachment ' . $attachment->id); $attachment->set_description($revision->title); } @@ -272,8 +458,8 @@ sub process_revision_change { }); foreach my $attachment (@$other_attachments) { $other_bugs{$attachment->bug_id}++; - DEBUG('Updating obsolete status on attachment ' . - $attachment->id . " for bug " . $attachment->bug_id); + INFO('Updating obsolete status on attachment ' . + $attachment->id . " for bug " . $attachment->bug_id); $attachment->set_is_obsolete(1); $attachment->update($timestamp); } @@ -281,17 +467,28 @@ sub process_revision_change { # REVIEWER STATUSES my (@accepted_phids, @denied_phids, @accepted_user_ids, @denied_user_ids); - unless ($revision->status eq 'changes-planned' || $revision->status eq 'needs-review') { - foreach my $reviewer (@{ $revision->reviewers }) { - push(@accepted_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'accepted'; - push(@denied_phids, $reviewer->phab_phid) if $reviewer->phab_review_status eq 'rejected'; - } + 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 $phab_users = get_phab_bmo_ids({ phids => \@accepted_phids }); - @accepted_user_ids = map { $_->{id} } @$phab_users; - $phab_users = get_phab_bmo_ids({ phids => \@denied_phids }); - @denied_user_ids = map { $_->{id} } @$phab_users; + 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 }; @@ -299,19 +496,22 @@ sub process_revision_change { my ($attach_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN); next if $revision->id != $attach_revision_id; - # Clear old flags if no longer accepted + # Clear old accepted review flags if no longer accepted my (@denied_flags, @new_flags, @removed_flags, %accepted_done, $flag_type); foreach my $flag (@{ $attachment->flags }) { next if $flag->type->name ne 'review'; $flag_type = $flag->type if $flag->type->is_active; + next if $flag->status ne '+'; if (any { $flag->setter->id == $_ } @denied_user_ids) { + INFO('Denying review flag set by ' . $flag->setter->name); push(@denied_flags, { id => $flag->id, setter => $flag->setter, status => 'X' }); } if (any { $flag->setter->id == $_ } @accepted_user_ids) { + INFO('Skipping as review+ already set by ' . $flag->setter->name); $accepted_done{$flag->setter->id}++; } - if ($flag->status eq '+' - && !any { $flag->setter->id == $_ } (@accepted_user_ids, @denied_user_ids)) { + if (!any { $flag->setter->id == $_ } (@accepted_user_ids, @denied_user_ids)) { + INFO('Clearing review+ flag set by ' . $flag->setter->name); push(@removed_flags, { id => $flag->id, setter => $flag->setter, status => 'X' }); } } @@ -322,6 +522,7 @@ sub process_revision_change { foreach my $user_id (@accepted_user_ids) { next if $accepted_done{$user_id}; my $user = Bugzilla::User->check({ id => $user_id, cache => 1 }); + INFO('Setting new review+ flag for ' . $user->name); push(@new_flags, { type_id => $flag_type->id, setter => $user, status => '+' }); } @@ -344,6 +545,7 @@ sub process_revision_change { if ($comment) { $comment .= "\n" . Bugzilla->params->{phabricator_base_uri} . "D" . $revision->id; + INFO("Flag comment: $comment"); # Add transaction_id as anchor if one present # $comment .= "#" . $params->{transaction_id} if $params->{transaction_id}; $bug->add_comment($comment, { @@ -380,7 +582,7 @@ sub process_new_user { my $phab_user = Bugzilla::Extension::PhabBugz::User->new($user_data); if (!$phab_user->bugzilla_id) { - DEBUG("SKIPPING: No bugzilla id associated with user"); + WARN("SKIPPING: No bugzilla id associated with user"); return; } @@ -389,6 +591,55 @@ sub process_new_user { # Pre setup before querying DB my $old_user = set_phab_user(); + # CHECK AND WARN FOR POSSIBLE USERNAME SQUATTING + INFO("Checking for username squatters"); + my $dbh = Bugzilla->dbh; + my $regexp = $dbh->quote( ":?:" . quotemeta($phab_user->name) . "[[:>:]]" ); + my $results = $dbh->selectall_arrayref( " + SELECT userid, login_name, realname + FROM profiles + WHERE userid != ? AND " . $dbh->sql_regexp( 'realname', $regexp ), + { Slice => {} }, + $bug_user->id ); + if (@$results) { + # The email client will display the Date: header in the desired timezone, + # so we can always use UTC here. + my $timestamp = Bugzilla->dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)'); + $timestamp = format_time($timestamp, '%a, %d %b %Y %T %z', 'UTC'); + + foreach my $row (@$results) { + WARN( + 'Possible username squatter: ', + 'phab user login: ' . $phab_user->name, + ' phab user realname: ' . $phab_user->realname, + ' bugzilla user id: ' . $row->{userid}, + ' bugzilla login: ' . $row->{login_name}, + ' bugzilla realname: ' . $row->{realname} + ); + + my $vars = { + date => $timestamp, + phab_user_login => $phab_user->name, + phab_user_realname => $phab_user->realname, + bugzilla_userid => $bug_user->id, + bugzilla_login => $bug_user->login, + bugzilla_realname => $bug_user->name, + squat_userid => $row->{userid}, + squat_login => $row->{login_name}, + squat_realname => $row->{realname} + }; + + my $message; + my $template = Bugzilla->template; + $template->process("admin/email/squatter-alert.txt.tmpl", $vars, \$message) + || ThrowTemplateError($template->error()); + + MessageToMTA($message); + } + } + + # ADD SUBSCRIBERS TO REVSISIONS FOR CURRENT PRIVATE BUGS + my $params = { f3 => 'OP', j3 => 'OR', @@ -432,8 +683,10 @@ sub process_new_user { # the first value of each row should be the bug id my @bug_ids = map { shift @$_ } @$data; + INFO("Updating subscriber values for old private bugs"); + foreach my $bug_id (@bug_ids) { - DEBUG("Processing bug $bug_id"); + INFO("Processing bug $bug_id"); my $bug = Bugzilla::Bug->new({ id => $bug_id, cache => 1 }); @@ -442,7 +695,8 @@ sub process_new_user { foreach my $attachment (@attachments) { my ($revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN); - DEBUG("Processing revision D$revision_id"); + + INFO("Processing revision D$revision_id"); my $revision = Bugzilla::Extension::PhabBugz::Revision->new_from_query( { ids => [ int($revision_id) ] }); @@ -450,7 +704,7 @@ sub process_new_user { $revision->add_subscriber($phab_user->phid); $revision->update(); - DEBUG("Revision $revision_id updated"); + INFO("Revision $revision_id updated"); } } @@ -467,7 +721,9 @@ sub new_stories { my ( $self, $after ) = @_; my $data = { view => 'text' }; $data->{after} = $after if $after; + my $result = request( 'feed.query_id', $data ); + unless ( ref $result->{result}{data} eq 'ARRAY' && @{ $result->{result}{data} } ) { @@ -487,7 +743,9 @@ sub new_users { } }; $data->{before} = $after if $after; + my $result = request( 'user.search', $data ); + unless ( ref $result->{result}{data} eq 'ARRAY' && @{ $result->{result}{data} } ) { @@ -504,7 +762,7 @@ sub get_last_id { my $last_id = Bugzilla->dbh->selectrow_array( " SELECT value FROM phabbugz WHERE name = ?", undef, $type_full ); $last_id ||= 0; - DEBUG( "QUERY " . uc($type_full) . ": $last_id" ); + TRACE(uc($type_full) . ": $last_id" ); return $last_id; } @@ -513,9 +771,34 @@ sub save_last_id { # Store the largest last key so we can start from there in the next session my $type_full = $type . "_last_id"; - DEBUG( "UPDATING " . uc($type_full) . ": $last_id" ); + TRACE("UPDATING " . uc($type_full) . ": $last_id" ); Bugzilla->dbh->do( "REPLACE INTO phabbugz (name, value) VALUES (?, ?)", undef, $type_full, $last_id ); } +sub get_group_members { + my ( $self, $group ) = @_; + + my $group_obj = + ref $group ? $group : Bugzilla::Group->check( { name => $group, cache => 1 } ); + + my $flat_list = join(',', + @{ Bugzilla::Group->flatten_group_membership( $group_obj->id ) } ); + + my $user_query = " + SELECT DISTINCT profiles.userid + FROM profiles, user_group_map AS ugm + WHERE ugm.user_id = profiles.userid + AND ugm.isbless = 0 + AND ugm.group_id IN($flat_list)"; + my $user_ids = Bugzilla->dbh->selectcol_arrayref($user_query); + + # Return matching users in Phabricator + return Bugzilla::Extension::PhabBugz::User->match( + { + ids => $user_ids + } + ); +} + 1; diff --git a/extensions/PhabBugz/lib/Policy.pm b/extensions/PhabBugz/lib/Policy.pm index 8162ac52c..a86c83036 100644 --- a/extensions/PhabBugz/lib/Policy.pm +++ b/extensions/PhabBugz/lib/Policy.pm @@ -41,7 +41,7 @@ has 'rules' => ( has 'rule_projects' => ( is => 'lazy', - isa => ArrayRef[Str], + isa => ArrayRef[Object], ); # { @@ -88,7 +88,7 @@ sub new_from_query { } sub create { - my ($class, $project_names) = @_; + my ($class, $projects) = @_; my $data = { objectType => 'DREV', @@ -97,23 +97,19 @@ sub create { { action => 'allow', rule => 'PhabricatorSubscriptionsSubscribersPolicyRule', + }, + { + action => 'allow', + rule => 'PhabricatorDifferentialReviewersPolicyRule' } ] }; - if (@$project_names) { - my $project_phids = []; - foreach my $project_name (@$project_names) { - my $project = Bugzilla::Extension::PhabBugz::Project->new_from_query({ name => $project_name }); - push @$project_phids, $project->phid if $project; - } - - ThrowUserError('invalid_phabricator_sync_groups') unless @$project_phids; - + if (@$projects) { push @{ $data->{policy} }, { action => 'allow', - rule => 'PhabricatorProjectsPolicyRule', - value => $project_phids, + rule => 'PhabricatorProjectsAllPolicyRule', + value => [ map { $_->phid } @$projects ], }; } else { @@ -131,11 +127,9 @@ sub _build_rule_projects { my ($self) = @_; return [] unless $self->rules; - my $rule = first { $_->{rule} eq 'PhabricatorProjectsPolicyRule'} @{ $self->rules }; + my $rule = first { $_->{rule} =~ /PhabricatorProjects(?:All)?PolicyRule/ } @{ $self->rules }; return [] unless $rule; return [ - map { $_->name } - grep { $_ } map { Bugzilla::Extension::PhabBugz::Project->new_from_query( { phids => [$_] } ) } @{ $rule->{value} } ]; diff --git a/extensions/PhabBugz/lib/Project.pm b/extensions/PhabBugz/lib/Project.pm index b0babc58b..c52e1a661 100644 --- a/extensions/PhabBugz/lib/Project.pm +++ b/extensions/PhabBugz/lib/Project.pm @@ -9,15 +9,14 @@ package Bugzilla::Extension::PhabBugz::Project; use 5.10.1; use Moo; +use Scalar::Util qw(blessed); use Types::Standard -all; use Type::Utils; use Bugzilla::Error; use Bugzilla::Util qw(trim); -use Bugzilla::Extension::PhabBugz::Util qw( - request - get_phab_bmo_ids -); +use Bugzilla::Extension::PhabBugz::User; +use Bugzilla::Extension::PhabBugz::Util qw(request); ######################### # Initialization # @@ -47,7 +46,18 @@ sub new_from_query { my $result = request( 'project.search', $data ); if ( exists $result->{result}{data} && @{ $result->{result}{data} } ) { - return $class->new( $result->{result}{data}[0] ); + # If name is used as a query param, we need to loop through and look + # for exact match as Conduit will tokenize the name instead of doing + # exact string match :( If name is not used, then return first one. + if ( exists $params->{name} ) { + foreach my $item ( @{ $result->{result}{data} } ) { + next if $item->{fields}{name} ne $params->{name}; + return $class->new($item); + } + } + else { + return $class->new( $result->{result}{data}[0] ); + } } } @@ -157,7 +167,7 @@ sub create { my $result = request( 'project.edit', $data ); return $class->new_from_query( - { phids => $result->{result}{object}{phid} } ); + { phids => [ $result->{result}{object}{phid} ] } ); } sub update { @@ -270,20 +280,20 @@ sub set_description { sub add_member { my ( $self, $member ) = @_; $self->{add_members} ||= []; - my $member_phid = blessed $member ? $member->phab_phid : $member; + my $member_phid = blessed $member ? $member->phid : $member; push( @{ $self->{add_members} }, $member_phid ); } sub remove_member { my ( $self, $member ) = @_; $self->{remove_members} ||= []; - my $member_phid = blessed $member ? $member->phab_phid : $member; + my $member_phid = blessed $member ? $member->phid : $member; push( @{ $self->{remove_members} }, $member_phid ); } sub set_members { my ( $self, $members ) = @_; - $self->{set_members} = [ map { $_->phab_phid } @$members ]; + $self->{set_members} = [ map { blessed $_ ? $_->phid : $_ } @$members ]; } sub set_policy { @@ -307,16 +317,13 @@ sub _build_members { return [] if !@phids; - my $users = get_phab_bmo_ids( { phids => \@phids } ); - - my @members; - foreach my $user (@$users) { - my $member = Bugzilla::User->new( { id => $user->{id}, cache => 1 } ); - $member->{phab_phid} = $user->{phid}; - push( @members, $member ); - } + my $users = Bugzilla::Extension::PhabBugz::User->match( + { + phids => \@phids + } + ); - return \@members; + return [ map { $_->bugzilla_user } @$users ]; } 1; diff --git a/extensions/PhabBugz/lib/Revision.pm b/extensions/PhabBugz/lib/Revision.pm index 98c3196c2..900454220 100644 --- a/extensions/PhabBugz/lib/Revision.pm +++ b/extensions/PhabBugz/lib/Revision.pm @@ -17,10 +17,9 @@ use Type::Utils; use Bugzilla::Bug; use Bugzilla::Error; use Bugzilla::Util qw(trim); -use Bugzilla::Extension::PhabBugz::Util qw( - get_phab_bmo_ids - request -); +use Bugzilla::Extension::PhabBugz::Project; +use Bugzilla::Extension::PhabBugz::User; +use Bugzilla::Extension::PhabBugz::Util qw(request); ######################### # Initialization # @@ -37,12 +36,12 @@ 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 projects_raw => ( is => 'ro', isa => ArrayRef [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 [Object] ); has subscribers => ( is => 'lazy', isa => ArrayRef [Object] ); +has projects => ( is => 'lazy', isa => ArrayRef [Object] ); has reviewers_raw => ( is => 'ro', isa => ArrayRef [ @@ -62,6 +61,12 @@ has subscribers_raw => ( viewerIsSubscribed => Bool, ] ); +has projects_raw => ( + is => 'ro', + isa => Dict [ + projectPHIDs => ArrayRef [Str] + ] +); sub new_from_query { my ( $class, $params ) = @_; @@ -93,18 +98,18 @@ sub new_from_query { 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} = $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->{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->{subscriber_count} = $params->{attachments}->{subscribers}->{subscriberCount}; @@ -284,12 +289,13 @@ sub _build_bug { sub _build_author { my ($self) = @_; return $self->{author} if $self->{author}; - my $users = get_phab_bmo_ids( { phids => [ $self->author_phid ] } ); - if (@$users) { - $self->{author} = - new Bugzilla::User( { id => $users->[0]->{id}, cache => 1 } ); - $self->{author}->{phab_phid} = $self->author_phid; - return $self->{author}; + my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query( + { + phids => [ $self->author_phid ] + } + ); + if ($phab_user) { + return $self->{author} = $phab_user; } } @@ -306,22 +312,22 @@ sub _build_reviewers { return [] unless @phids; - my $users = get_phab_bmo_ids( { phids => \@phids } ); + my $users = Bugzilla::Extension::PhabBugz::User->match( + { + phids => \@phids + } + ); - my @reviewers; foreach my $user (@$users) { - my $reviewer = Bugzilla::User->new( { id => $user->{id}, cache => 1 } ); - $reviewer->{phab_phid} = $user->{phid}; foreach my $reviewer_data ( @{ $self->reviewers_raw } ) { - if ( $reviewer_data->{reviewerPHID} eq $user->{phid} ) { - $reviewer->{phab_review_status} = $reviewer_data->{status}; + if ( $reviewer_data->{reviewerPHID} eq $user->phid ) { + $user->{phab_review_status} = $reviewer_data->{status}; last; } } - push @reviewers, $reviewer; } - return \@reviewers; + return $self->{reviewers} = $users; } sub _build_subscribers { @@ -335,19 +341,31 @@ sub _build_subscribers { push @phids, $phid; } - my $users = get_phab_bmo_ids( { phids => \@phids } ); + my $users = Bugzilla::Extension::PhabBugz::User->match( + { + phids => \@phids + } + ); - return [] unless @phids; + return $self->{subscribers} = $users; +} - my @subscribers; - foreach my $user (@$users) { - my $subscriber = - Bugzilla::User->new( { id => $user->{id}, cache => 1 } ); - $subscriber->{phab_phid} = $user->{phid}; - push @subscribers, $subscriber; +sub _build_projects { + my ($self) = @_; + + return $self->{projects} if $self->{projects}; + return [] unless $self->projects_raw->{projectPHIDs}; + + my @projects; + foreach my $phid ( @{ $self->projects_raw->{projectPHIDs} } ) { + push @projects, Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + phids => [ $phid ] + } + ); } - return \@subscribers; + return $self->{projects} = \@projects; } ######################### @@ -364,27 +382,27 @@ sub add_comment { sub add_reviewer { my ( $self, $reviewer ) = @_; $self->{add_reviewers} ||= []; - my $reviewer_phid = blessed $reviewer ? $reviewer->phab_phid : $reviewer; + my $reviewer_phid = blessed $reviewer ? $reviewer->phid : $reviewer; push @{ $self->{add_reviewers} }, $reviewer_phid; } sub remove_reviewer { my ( $self, $reviewer ) = @_; $self->{remove_reviewers} ||= []; - my $reviewer_phid = blessed $reviewer ? $reviewer->phab_phid : $reviewer; + my $reviewer_phid = blessed $reviewer ? $reviewer->phid : $reviewer; push @{ $self->{remove_reviewers} }, $reviewer_phid; } sub set_reviewers { my ( $self, $reviewers ) = @_; - $self->{set_reviewers} = [ map { $_->phab_phid } @$reviewers ]; + $self->{set_reviewers} = [ map { $_->phid } @$reviewers ]; } sub add_subscriber { my ( $self, $subscriber ) = @_; $self->{add_subscribers} ||= []; my $subscriber_phid = - blessed $subscriber ? $subscriber->phab_phid : $subscriber; + blessed $subscriber ? $subscriber->phid : $subscriber; push @{ $self->{add_subscribers} }, $subscriber_phid; } @@ -392,7 +410,7 @@ sub remove_subscriber { my ( $self, $subscriber ) = @_; $self->{remove_subscribers} ||= []; my $subscriber_phid = - blessed $subscriber ? $subscriber->phab_phid : $subscriber; + blessed $subscriber ? $subscriber->phid : $subscriber; push @{ $self->{remove_subscribers} }, $subscriber_phid; } @@ -423,4 +441,50 @@ sub remove_project { push @{ $self->{remove_projects} }, $project_phid; } +sub make_private { + my ( $self, $project_names ) = @_; + + my $secure_revision_project = + Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + name => 'secure-revision' + } + ); + + my @set_projects; + foreach my $name (@$project_names) { + my $set_project = + Bugzilla::Extension::PhabBugz::Project->new_from_query( + { + name => $name + } + ); + push @set_projects, $set_project; + } + + my $new_policy = Bugzilla::Extension::PhabBugz::Policy->create(\@set_projects); + $self->set_policy('view', $new_policy->phid); + $self->set_policy('edit', $new_policy->phid); + + foreach my $project ($secure_revision_project, @set_projects) { + $self->add_project($project->phid); + } + + return $self; +} + +sub make_public { + my ( $self ) = @_; + + $self->set_policy('view', 'public'); + $self->set_policy('edit', 'users'); + + my @current_group_projects = grep { $_->name =~ /^(bmo-.*|secure-revision)$/ } @{ $self->projects }; + foreach my $project (@current_group_projects) { + $self->remove_project($project->phid); + } + + return $self; +} + 1; diff --git a/extensions/PhabBugz/lib/User.pm b/extensions/PhabBugz/lib/User.pm index 3b2d87e60..9d4e9eef4 100644 --- a/extensions/PhabBugz/lib/User.pm +++ b/extensions/PhabBugz/lib/User.pm @@ -116,14 +116,14 @@ sub match { my ( $class, $params ) = @_; # BMO id search takes precedence if bugzilla_ids is used. - my $bugzilla_ids = delete $params->{bugzilla_ids}; + my $bugzilla_ids = delete $params->{ids}; if ($bugzilla_ids) { my $bugzilla_data = $class->get_phab_bugzilla_ids( { ids => $bugzilla_ids } ); $params->{phids} = [ map { $_->{phid} } @$bugzilla_data ]; } - return [] if !$params->{phids}; + return [] if !@{ $params->{phids} }; # Look for BMO external user id in external-accounts attachment my $data = { @@ -177,6 +177,7 @@ sub get_phab_bugzilla_ids { # Store new values in memcache for later retrieval foreach my $user ( @{ $result->{result} } ) { + next if !$user->{phid}; $memcache->set( { key => "phab_user_bugzilla_id_" . $user->{id}, diff --git a/extensions/PhabBugz/lib/Util.pm b/extensions/PhabBugz/lib/Util.pm index 844d8c0b5..d25f62f68 100644 --- a/extensions/PhabBugz/lib/Util.pm +++ b/extensions/PhabBugz/lib/Util.pm @@ -21,63 +21,26 @@ 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 Try::Tiny; use base qw(Exporter); our @EXPORT = qw( - add_comment_to_revision add_security_sync_comments create_revision_attachment - create_private_revision_policy - create_project - edit_revision_policy get_attachment_revisions get_bug_role_phids - get_members_by_bmo_id - get_members_by_phid - get_phab_bmo_ids - get_project_phid - get_revisions_by_ids - get_revisions_by_phids + get_needs_review get_security_sync_groups intersect is_attachment_phab_revision - make_revision_private - make_revision_public request set_phab_user - set_project_members - set_revision_subscribers ); -sub get_revisions_by_ids { - my ($ids) = @_; - return _get_revisions({ ids => $ids }); -} - -sub get_revisions_by_phids { - my ($phids) = @_; - return _get_revisions({ phids => $phids }); -} - -sub _get_revisions { - my ($constraints) = @_; - - my $data = { - queryKey => 'all', - constraints => $constraints - }; - - my $result = request('differential.revision.search', $data); - - ThrowUserError('invalid_phabricator_revision_id') - unless (exists $result->{result}{data} && @{ $result->{result}{data} }); - - return $result->{result}{data}; -} - sub create_revision_attachment { - my ( $bug, $revision, $timestamp ) = @_; + my ( $bug, $revision, $timestamp, $submitter ) = @_; my $phab_base_uri = Bugzilla->params->{phabricator_base_uri}; ThrowUserError('invalid_phabricator_uri') unless $phab_base_uri; @@ -97,22 +60,38 @@ sub create_revision_attachment { ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()"); } - my $attachment = Bugzilla::Attachment->create( - { - bug => $bug, - creation_ts => $timestamp, - data => $revision_uri, - description => $revision->title, - filename => 'phabricator-D' . $revision->id . '-url.txt', - ispatch => 0, - isprivate => 0, - mimetype => PHAB_CONTENT_TYPE, + # If submitter, then switch to that user when creating attachment + my ($old_user, $attachment); + try { + if ($submitter) { + $old_user = Bugzilla->user; + $submitter->{groups} = [ Bugzilla::Group->get_all ]; # We need to always be able to add attachment + Bugzilla->set_user($submitter); } - ); - # Insert a comment about the new attachment into the database. - $bug->add_comment($revision->summary, { type => CMT_ATTACHMENT_CREATED, - extra_data => $attachment->id }); + $attachment = Bugzilla::Attachment->create( + { + bug => $bug, + creation_ts => $timestamp, + data => $revision_uri, + description => $revision->title, + filename => 'phabricator-D' . $revision->id . '-url.txt', + ispatch => 0, + isprivate => 0, + mimetype => PHAB_CONTENT_TYPE, + } + ); + + # Insert a comment about the new attachment into the database. + $bug->add_comment($revision->summary, { type => CMT_ATTACHMENT_CREATED, + extra_data => $attachment->id }); + } + catch { + die $_; + } + finally { + Bugzilla->set_user($old_user) if $old_user; + }; return $attachment; } @@ -132,321 +111,47 @@ sub get_bug_role_phids { push(@bug_users, $bug->qa_contact) if $bug->qa_contact; push(@bug_users, @{ $bug->cc_users }) if @{ $bug->cc_users }; - return get_members_by_bmo_id(\@bug_users); -} - -sub create_private_revision_policy { - my ($bug, $groups) = @_; - - my $data = { - objectType => 'DREV', - default => 'deny', - policy => [ - { - action => 'allow', - rule => 'PhabricatorSubscriptionsSubscribersPolicyRule', - } - ] - }; - - if(scalar @$groups gt 0) { - my $project_phids = []; - foreach my $group (@$groups) { - my $phid = get_project_phid('bmo-' . $group); - push(@$project_phids, $phid) if $phid; - } - - ThrowUserError('invalid_phabricator_sync_groups') unless @$project_phids; - - push(@{ $data->{policy} }, - { - action => 'allow', - rule => 'PhabricatorProjectsPolicyRule', - value => $project_phids, - } - ); - } - else { - my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({ - name => 'secure-revision' - }); - push(@{ $data->{policy} }, - { - action => 'allow', - value => $secure_revision->phid, - } - ); - } - - my $result = request('policy.create', $data); - return $result->{result}{phid}; -} - -sub make_revision_public { - my ($revision_phid) = @_; - return request('differential.revision.edit', { - transactions => [ - { - type => 'view', - value => 'public' - }, - { - type => 'edit', - value => 'users' - } - ], - objectIdentifier => $revision_phid - }); -} - -sub make_revision_private { - my ($revision_phid) = @_; - - my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({ - name => 'secure-revision' - }); - - return request('differential.revision.edit', { - transactions => [ - { - type => "view", - value => $secure_revision->phid - }, - { - type => "edit", - value => $secure_revision->phid - } - ], - objectIdentifier => $revision_phid - }); -} - -sub edit_revision_policy { - my ($revision_phid, $policy_phid, $subscribers) = @_; - - my $data = { - transactions => [ - { - type => 'view', - value => $policy_phid - }, - { - type => 'edit', - value => $policy_phid - } - ], - objectIdentifier => $revision_phid - }; - - if (@$subscribers) { - push(@{ $data->{transactions} }, { - type => 'subscribers.set', - value => $subscribers - }); - } - - return request('differential.revision.edit', $data); -} - -sub set_revision_subscribers { - my ($revision_phid, $subscribers) = @_; - - my $data = { - transactions => [ - { - type => 'subscribers.set', - value => $subscribers - } - ], - objectIdentifier => $revision_phid - }; - - return request('differential.revision.edit', $data); -} - -sub add_comment_to_revision { - my ($revision_phid, $comment) = @_; - - my $data = { - transactions => [ - { - type => 'comment', - value => $comment - } - ], - objectIdentifier => $revision_phid - }; - return request('differential.revision.edit', $data); -} - -sub get_project_phid { - my $project = shift; - my $memcache = Bugzilla->memcached; - - # Check memcache - my $project_phid = $memcache->get_config({ key => "phab_project_phid_" . $project }); - if (!$project_phid) { - my $data = { - queryKey => 'all', - constraints => { - name => $project - } - }; - - my $result = request('project.search', $data); - return undef - unless (exists $result->{result}{data} && @{ $result->{result}{data} }); - - $project_phid = $result->{result}{data}[0]{phid}; - $memcache->set_config({ key => "phab_project_phid_" . $project, data => $project_phid }); - } - return $project_phid; -} - -sub create_project { - my ($project, $description, $members) = @_; - - my $secure_revision = Bugzilla::Extension::PhabBugz::Project->new_from_query({ - name => 'secure-revision' - }); - - my $data = { - transactions => [ - { type => 'name', value => $project }, - { type => 'description', value => $description }, - { type => 'edit', value => $secure_revision->phid }. - { type => 'join', value => $secure_revision->phid }, - { type => 'view', value => $secure_revision->phid }, - { type => 'icon', value => 'group' }, - { type => 'color', value => 'red' } - ] - }; - - my $result = request('project.edit', $data); - return $result->{result}{object}{phid}; -} - -sub set_project_members { - my ($project_id, $phab_user_ids) = @_; - - my $data = { - objectIdentifier => $project_id, - transactions => [ - { type => 'members.set', value => $phab_user_ids } - ] - }; - - my $result = request('project.edit', $data); - return $result->{result}{object}{phid}; -} - -sub get_members_by_bmo_id { - my $users = shift; - - my $result = get_phab_bmo_ids({ ids => [ map { $_->id } @$users ] }); - - my @phab_ids; - foreach my $user (@$result) { - push(@phab_ids, $user->{phid}) - if ($user->{phid} && $user->{phid} =~ /^PHID-USER/); - } - - return \@phab_ids; -} - -sub get_members_by_phid { - my $phids = shift; - - my $result = get_phab_bmo_ids({ phids => $phids }); - - my @bmo_ids; - foreach my $user (@$result) { - push(@bmo_ids, $user->{id}) - if ($user->{phid} && $user->{phid} =~ /^PHID-USER/); - } - - return \@bmo_ids; -} - -sub get_phab_bmo_ids { - my ($params) = @_; - my $memcache = Bugzilla->memcached; - - # Try to find the values in memcache first - my @results; - if ($params->{ids}) { - my @bmo_ids = @{ $params->{ids} }; - for (my $i = 0; $i < @bmo_ids; $i++) { - my $phid = $memcache->get({ key => "phab_user_bmo_id_" . $bmo_ids[$i] }); - if ($phid) { - push(@results, { - id => $bmo_ids[$i], - phid => $phid - }); - splice(@bmo_ids, $i, 1); - } - } - $params->{ids} = \@bmo_ids; - } - - if ($params->{phids}) { - my @phids = @{ $params->{phids} }; - for (my $i = 0; $i < @phids; $i++) { - my $bmo_id = $memcache->get({ key => "phab_user_phid_" . $phids[$i] }); - if ($bmo_id) { - push(@results, { - id => $bmo_id, - phid => $phids[$i] - }); - splice(@phids, $i, 1); - } + my $phab_users = + Bugzilla::Extension::PhabBugz::User->match( + { + ids => [ map { $_->id } @bug_users ] } - $params->{phids} = \@phids; - } - - my $result = request('bugzilla.account.search', $params); - - # Store new values in memcache for later retrieval - foreach my $user (@{ $result->{result} }) { - $memcache->set({ key => "phab_user_bmo_id_" . $user->{id}, - value => $user->{phid} }); - $memcache->set({ key => "phab_user_phid_" . $user->{phid}, - value => $user->{id} }); - push(@results, $user); - } + ); - return \@results; + return [ map { $_->phid } @{ $phab_users } ]; } sub is_attachment_phab_revision { my ($attachment) = @_; - return ($attachment->contenttype eq PHAB_CONTENT_TYPE - && $attachment->attacher->login eq PHAB_AUTOMATION_USER) ? 1 : 0; + return $attachment->contenttype eq PHAB_CONTENT_TYPE; } sub get_attachment_revisions { my $bug = shift; - my $revisions; - my @attachments = grep { is_attachment_phab_revision($_) } @{ $bug->attachments() }; - if (@attachments) { - my @revision_ids; - foreach my $attachment (@attachments) { - my ($revision_id) = - ( $attachment->filename =~ PHAB_ATTACHMENT_PATTERN ); - next if !$revision_id; - push( @revision_ids, int($revision_id) ); - } + return unless @attachments; - if (@revision_ids) { - $revisions = get_revisions_by_ids( \@revision_ids ); - } + my @revision_ids; + foreach my $attachment (@attachments) { + my ($revision_id) = + ( $attachment->filename =~ PHAB_ATTACHMENT_PATTERN ); + next if !$revision_id; + push( @revision_ids, int($revision_id) ); } - return @$revisions; + return unless @revision_ids; + + my @revisions; + foreach my $revision_id (@revision_ids) { + push @revisions, Bugzilla::Extension::PhabBugz::Revision->new_from_query({ + ids => [ $revision_id ] + }); + } + + return \@revisions; } sub request { @@ -478,7 +183,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; @@ -493,9 +203,8 @@ sub request { sub get_security_sync_groups { my $bug = shift; - my $phab_sync_groups = Bugzilla->params->{phabricator_sync_groups} - || ThrowUserError('invalid_phabricator_sync_groups'); - my $sync_group_names = [ split('[,\s]+', $phab_sync_groups) ]; + my $sync_groups = Bugzilla::Group->match( { isactive => 1, isbuggroup => 1 } ); + my $sync_group_names = [ map { $_->name } @$sync_groups ]; my $bug_groups = $bug->groups_in; my $bug_group_names = [ map { $_->name } @$bug_groups ]; @@ -519,7 +228,7 @@ sub add_security_sync_comments { my $phab_error_message = 'Revision is being made private due to unknown Bugzilla groups.'; foreach my $revision (@$revisions) { - add_comment_to_revision( $revision->{phid}, $phab_error_message ); + $revision->add_comment($phab_error_message); } my $num_revisions = scalar @$revisions; @@ -536,4 +245,36 @@ sub add_security_sync_comments { Bugzilla->set_user($old_user); } +sub get_needs_review { + my ($user) = @_; + $user //= Bugzilla->user; + return unless $user->id; + + my $phab_user = Bugzilla::Extension::PhabBugz::User->new_from_query( + { + ids => [ $user->id ] + } + ); + + return [] unless $phab_user; + + my $diffs = request( + 'differential.revision.search', + { + attachments => { + reviewers => 1, + }, + constraints => { + reviewerPHIDs => [$phab_user->phid], + statuses => [qw( needs-review )], + }, + order => 'newest', + } + ); + ThrowCodeError('phabricator_api_error', { reason => 'Malformed Response' }) + unless exists $diffs->{result}{data}; + + return $diffs->{result}{data}; +} + 1; diff --git a/extensions/PhabBugz/lib/WebService.pm b/extensions/PhabBugz/lib/WebService.pm index 5b6310de6..0239ccf74 100644 --- a/extensions/PhabBugz/lib/WebService.pm +++ b/extensions/PhabBugz/lib/WebService.pm @@ -13,115 +13,44 @@ use warnings; use base qw(Bugzilla::WebService); -use Bugzilla::Attachment; -use Bugzilla::Bug; -use Bugzilla::BugMail; 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 trick_taint); 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 - edit_revision_policy - get_bug_role_phids - get_project_phid - get_revisions_by_ids - intersect - is_attachment_phab_revision - make_revision_public - request - get_security_sync_groups + get_needs_review ); -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 PUBLIC_METHODS => qw( +use constant READ_ONLY => qw( check_user_permission_for_bug - obsolete_attachments - revision - update_reviewer_statuses + needs_review ); -sub revision { - my ($self, $params) = @_; - - # Phabricator only supports sending credentials via HTTP Basic Auth - # so we exploit that function to pass in an API key as the password - # of basic auth. BMO does not support basic auth but does support - # use of API keys. - my $http_auth = Bugzilla->cgi->http('Authorization'); - $http_auth =~ s/^Basic\s+//; - $http_auth = decode_base64($http_auth); - my ($login, $api_key) = split(':', $http_auth); - $params->{'Bugzilla_login'} = $login; - $params->{'Bugzilla_api_key'} = $api_key; - - my $user = Bugzilla->login(LOGIN_REQUIRED); - - # Prechecks - _phabricator_precheck($user); - - unless (defined $params->{revision} && detaint_natural($params->{revision})) { - ThrowCodeError('param_required', { param => 'revision' }) - } - - # Obtain more information about the revision from Phabricator - my $revision_id = $params->{revision}; - my $revisions = get_revisions_by_ids([$revision_id]); - my $revision = $revisions->[0]; - - my $revision_phid = $revision->{phid}; - my $revision_title = $revision->{fields}{title} || 'Unknown Description'; - my $bug_id = $revision->{fields}{'bugzilla.bug-id'}; - - my $bug = Bugzilla::Bug->new($bug_id); - - # If bug is public then remove privacy policy - my $result; - if (is_public($bug)) { - $result = make_revision_public($revision_id); - } - # else bug is private - else { - my @set_groups = get_security_sync_groups($bug); - - # If bug privacy groups do not have any matching synchronized groups, - # then leave revision private and it will have be dealt with manually. - if (!@set_groups) { - add_security_sync_comments($revisions, $bug); - } - - my $policy_phid = create_private_revision_policy($bug, \@set_groups); - my $subscribers = get_bug_role_phids($bug); - $result = edit_revision_policy($revision_phid, $policy_phid, $subscribers); - } - - my $attachment = create_revision_attachment($bug, $revision_id, $revision_title); - - Bugzilla::BugMail::Send($bug_id, { changer => $user }); - - return { - result => $result, - attachment_id => $attachment->id, - attachment_link => Bugzilla->localconfig->{urlbase} . "attachment.cgi?id=" . $attachment->id - }; -} +use constant PUBLIC_METHODS => qw( + check_user_permission_for_bug + needs_review + set_build_target +); sub check_user_permission_for_bug { my ($self, $params) = @_; my $user = Bugzilla->login(LOGIN_REQUIRED); - # Prechecks - _phabricator_precheck($user); + # Ensure PhabBugz is on + ThrowUserError('phabricator_not_enabled') + unless Bugzilla->params->{phabricator_enabled}; + + # Validate that the requesting user's email matches phab-bot + ThrowUserError('phabricator_unauthorized_user') + unless $user->login eq PHAB_AUTOMATION_USER; # Validate that a bug id and user id are provided ThrowUserError('phabricator_invalid_request_params') @@ -136,184 +65,152 @@ sub check_user_permission_for_bug { }; } -sub update_reviewer_statuses { +sub needs_review { my ($self, $params) = @_; - + ThrowUserError('phabricator_not_enabled') + unless Bugzilla->params->{phabricator_enabled}; my $user = Bugzilla->login(LOGIN_REQUIRED); - - # Prechecks - _phabricator_precheck($user); - - my $revision_id = $params->{revision_id}; - unless (defined $revision_id && detaint_natural($revision_id)) { - ThrowCodeError('param_required', { param => 'revision_id' }) - } - - my $bug_id = $params->{bug_id}; - unless (defined $bug_id && detaint_natural($bug_id)) { - ThrowCodeError('param_required', { param => 'bug_id' }) + my $dbh = Bugzilla->dbh; + + my $reviews = get_needs_review(); + + my $authors = Bugzilla::Extension::PhabBugz::User->match({ + phids => [ + uniq + grep { defined } + map { $_->{fields}{authorPHID} } + @$reviews + ] + }); + + my %author_phab_to_id = map { $_->phid => $_->bugzilla_user->id } @$authors; + my %author_id_to_user = map { $_->bugzilla_user->id => $_->bugzilla_user } @$authors; + + # 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; } - my $accepted_user_ids = $params->{accepted_users}; - defined $accepted_user_ids - || ThrowCodeError('param_required', { param => 'accepted_users' }); - $accepted_user_ids = [ split(':', $accepted_user_ids) ]; - - my $denied_user_ids = $params->{denied_users}; - defined $denied_user_ids - || ThrowCodeError('param_required', { param => 'denied_users' }); - $denied_user_ids = [ split(':', $denied_user_ids) ]; - - my $bug = Bugzilla::Bug->check($bug_id); - - my @attachments = - grep { is_attachment_phab_revision($_) } @{ $bug->attachments() }; - - return { result => [] } if !@attachments; - - my $dbh = Bugzilla->dbh; - my ($timestamp) = $dbh->selectrow_array("SELECT NOW()"); - - my @updated_attach_ids; - foreach my $attachment (@attachments) { - my ($curr_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN); - next if $revision_id != $curr_revision_id; - - # Clear old flags if no longer accepted - my (@denied_flags, @new_flags, @removed_flags, %accepted_done, $flag_type); - foreach my $flag (@{ $attachment->flags }) { - next if $flag->type->name ne 'review'; - $flag_type = $flag->type if $flag->type->is_active; - if (any { $flag->setter->id == $_ } @$denied_user_ids) { - push(@denied_flags, { id => $flag->id, setter => $flag->setter, status => 'X' }); - } - if (any { $flag->setter->id == $_ } @$accepted_user_ids) { - $accepted_done{$flag->setter->id}++; - } - if ($flag->status eq '+' - && !any { $flag->setter->id == $_ } (@$accepted_user_ids, @$denied_user_ids)) { - push(@removed_flags, { id => $flag->id, setter => $flag->setter, status => 'X' }); - } - } - - $flag_type ||= first { $_->name eq 'review' && $_->is_active } @{ $attachment->flag_types }; - - # Create new flags - foreach my $user_id (@$accepted_user_ids) { - next if $accepted_done{$user_id}; - my $user = Bugzilla::User->check({ id => $user_id, cache => 1 }); - push(@new_flags, { type_id => $flag_type->id, setter => $user, status => '+' }); + # build result + my $datetime_now = DateTime->now(time_zone => $user->timezone); + my @result; + foreach my $review (@$reviews) { + my $review_flat = { + id => $review->{id}, + 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; } - - # Also add comment to for attachment update showing the user's name - # that changed the revision. - my $comment; - foreach my $flag_data (@new_flags) { - $comment .= $flag_data->{setter}->name . " has approved the revision.\n"; - } - foreach my $flag_data (@denied_flags) { - $comment .= $flag_data->{setter}->name . " has requested changes to the revision.\n"; - } - foreach my $flag_data (@removed_flags) { - $comment .= $flag_data->{setter}->name . " has been removed from the revision.\n"; + else { + $review_flat->{author_name} = 'anonymous'; + $review_flat->{author_email} = 'anonymous'; } - if ($comment) { - $comment .= "\n" . Bugzilla->params->{phabricator_base_uri} . "D" . $revision_id; - # Add transaction_id as anchor if one present - $comment .= "#" . $params->{transaction_id} if $params->{transaction_id}; - $bug->add_comment($comment, { - isprivate => $attachment->isprivate, - type => CMT_ATTACHMENT_UPDATED, - extra_data => $attachment->id - }); + # 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}; } - $attachment->set_flags([ @denied_flags, @removed_flags ], \@new_flags); - $attachment->update($timestamp); - $bug->update($timestamp) if $comment; - - push(@updated_attach_ids, $attachment->id); + push @result, $review_flat; } - Bugzilla::BugMail::Send($bug_id, { changer => $user }) if @updated_attach_ids; - - return { result => \@updated_attach_ids }; + return { result => \@result }; } -sub obsolete_attachments { - my ($self, $params) = @_; +sub set_build_target { + my ( $self, $params ) = @_; - my $user = Bugzilla->login(LOGIN_REQUIRED); - - # Prechecks - _phabricator_precheck($user); - - my $revision_id = $params->{revision_id}; - unless (defined $revision_id && detaint_natural($revision_id)) { - ThrowCodeError('param_required', { param => 'revision' }) - } - - my $bug_id= $params->{bug_id}; - unless (defined $bug_id && detaint_natural($bug_id)) { - ThrowCodeError('param_required', { param => 'bug_id' }) - } - - my $make_obsolete = $params->{make_obsolete}; - unless (defined $make_obsolete) { - ThrowCodeError('param_required', { param => 'make_obsolete' }) - } - $make_obsolete = $make_obsolete ? 1 : 0; - - my $bug = Bugzilla::Bug->check($bug_id); - - my @attachments = - grep { is_attachment_phab_revision($_) } @{ $bug->attachments() }; - - return { result => [] } if !@attachments; - - my $dbh = Bugzilla->dbh; - my ($timestamp) = $dbh->selectrow_array("SELECT NOW()"); - - my @updated_attach_ids; - foreach my $attachment (@attachments) { - my ($curr_revision_id) = ($attachment->filename =~ PHAB_ATTACHMENT_PATTERN); - next if $revision_id != $curr_revision_id; + # Phabricator only supports sending credentials via HTTP Basic Auth + # so we exploit that function to pass in an API key as the password + # of basic auth. BMO does not support basic auth but does support + # use of API keys. + my $http_auth = Bugzilla->cgi->http('Authorization'); + $http_auth =~ s/^Basic\s+//; + $http_auth = decode_base64($http_auth); + my ($login, $api_key) = split(':', $http_auth); + $params->{'Bugzilla_login'} = $login; + $params->{'Bugzilla_api_key'} = $api_key; - $attachment->set_is_obsolete($make_obsolete); - $attachment->update($timestamp); + my $user = Bugzilla->login(LOGIN_REQUIRED); - push(@updated_attach_ids, $attachment->id); - } + # Ensure PhabBugz is on + ThrowUserError('phabricator_not_enabled') + unless Bugzilla->params->{phabricator_enabled}; + + # Validate that the requesting user's email matches phab-bot + ThrowUserError('phabricator_unauthorized_user') + unless $user->login eq PHAB_AUTOMATION_USER; - Bugzilla::BugMail::Send($bug_id, { changer => $user }) if @updated_attach_ids; + my $revision_id = $params->{revision_id}; + my $build_target = $params->{build_target}; - return { result => \@updated_attach_ids }; -} + ThrowUserError('invalid_phabricator_revision_id') + unless detaint_natural($revision_id); -sub _phabricator_precheck { - my ($user) = @_; + ThrowUserError('invalid_phabricator_build_target') + unless $build_target =~ /^PHID-HMBT-[a-zA-Z0-9]+$/; + trick_taint($build_target); - # Ensure PhabBugz is on - ThrowUserError('phabricator_not_enabled') - unless Bugzilla->params->{phabricator_enabled}; + Bugzilla->dbh->do( + "INSERT INTO phabbugz (name, value) VALUES (?, ?)", + undef, + 'build_target_' . $revision_id, + $build_target + ); - # Validate that the requesting user's email matches phab-bot - ThrowUserError('phabricator_unauthorized_user') - unless $user->login eq PHAB_AUTOMATION_USER; + return { result => 1 }; } sub rest_resources { return [ - # Revision creation - qr{^/phabbugz/revision/([^/]+)$}, { + # Set build target in Phabricator + qr{^/phabbugz/build_target/(\d+)/(PHID-HMBT-.*)$}, { POST => { - method => 'revision', + method => 'set_build_target', params => sub { - return { revision => $_[0] }; + return { + revision_id => $_[0], + build_target => $_[1] + }; } } - }, + }, # Bug permission checks qr{^/phabbugz/check_bug/(\d+)/(\d+)$}, { GET => { @@ -323,17 +220,11 @@ sub rest_resources { } } }, - # Update reviewer statuses - qr{^/phabbugz/update_reviewer_statuses$}, { - PUT => { - method => 'update_reviewer_statuses', - } - }, - # Obsolete attachments - qr{^/phabbugz/obsolete$}, { - PUT => { - method => 'obsolete_attachments', - } + # Review requests + qw{^/phabbugz/needs_review$}, { + GET => { + method => 'needs_review', + }, } ]; } diff --git a/extensions/PhabBugz/template/en/default/admin/email/squatter-alert.txt.tmpl b/extensions/PhabBugz/template/en/default/admin/email/squatter-alert.txt.tmpl new file mode 100644 index 000000000..98e92a379 --- /dev/null +++ b/extensions/PhabBugz/template/en/default/admin/email/squatter-alert.txt.tmpl @@ -0,0 +1,34 @@ +[%# This Source Code Form is subject to the terms of the Mozilla Public + # License, v. 2.0. If a copy of the MPL was not distributed with this + # file, You can obtain one at http://mozilla.org/MPL/2.0/. + # + # This Source Code Form is "Incompatible With Secondary Licenses", as + # defined by the Mozilla Public License, v. 2.0. + #%] + +[% PROCESS global/variables.none.tmpl %] + +From: [% Param('mailfrom') %] +To: phabricator-admin@mozilla.com +Subject: Possible Phabricator Username Squatter Alert +Date: [% date %] +X-Bugzilla-Type: squatter-alert + +Possible username squatter: + +Phabricator Account + +login: [% phab_user_login %] +realname: [% phab_user_realname %] + +Bugzilla Account Matching Phabricator Account + +user id: [% bugzilla_userid %] +login: [% bugzilla_login %] +realname: [% bugzilla_realname %] + +Possible Bugzilla Account Squatting On + +user id: [% squat_userid %] +login: [% squat_login %] +realname: [% squat_realname %] diff --git a/extensions/PhabBugz/template/en/default/admin/params/phabbugz.html.tmpl b/extensions/PhabBugz/template/en/default/admin/params/phabbugz.html.tmpl index d67839cc8..1b5bdda4b 100644 --- a/extensions/PhabBugz/template/en/default/admin/params/phabbugz.html.tmpl +++ b/extensions/PhabBugz/template/en/default/admin/params/phabbugz.html.tmpl @@ -16,7 +16,6 @@ phabricator_enabled => 'Enable Phabricator Integration', phabricator_base_uri => 'Phabricator Base URI', phabricator_api_key => 'Phabricator User API Key', - phabricator_sync_groups => 'Comma delimited list of Bugzilla groups to sync to Phabricator projects', phabricator_auth_callback_url => 'Phabricator Auth Delegation URL', phabricator_app_id => 'app_id for API Keys delegated to Phabricator', } diff --git a/extensions/PhabBugz/template/en/default/hook/global/user-error-errors.html.tmpl b/extensions/PhabBugz/template/en/default/hook/global/user-error-errors.html.tmpl index 1457e3525..0274f72ce 100644 --- a/extensions/PhabBugz/template/en/default/hook/global/user-error-errors.html.tmpl +++ b/extensions/PhabBugz/template/en/default/hook/global/user-error-errors.html.tmpl @@ -14,15 +14,18 @@ [% title = "Invalid Phabricator API Key" %] You must provide a valid Phabricator API Key. -[% ELSIF error == "invalid_phabricator_sync_groups" %] - [% title = "Invalid Phabricator Sync Groups" %] - You must provide a comma delimited list of security groups - to sync with Phabricator. +[% ELSIF error == "invalid_phabricator_projects" %] + [% title = "Invalid Phabricator Projects" %] + One or more Phabricator projects selected are invalid. [% ELSIF error == "invalid_phabricator_revision_id" %] [% title = "Invalid Phabricator Revision ID" %] You must provide a valid Phabricator revision ID. +[% ELSIF error == "invalid_phabricator_build_target" %] + [% title = "Invalid Phabricator Build Target" %] + You must provide a valid Phabricator Build Target PHID. + [% ELSIF error == "phabricator_not_enabled" %] [% title = "Phabricator Support Not Enabled" %] The Phabricator to Bugzilla library, PhabBugz, diff --git a/extensions/ProdCompSearch/web/js/prod_comp_search.js b/extensions/ProdCompSearch/web/js/prod_comp_search.js index 1b7e396a8..30351a3db 100644 --- a/extensions/ProdCompSearch/web/js/prod_comp_search.js +++ b/extensions/ProdCompSearch/web/js/prod_comp_search.js @@ -66,6 +66,8 @@ $(function() { params.Bugzilla_api_token = BUGZILLA.api_token; } that.devbridgeAutocomplete({ + appendTo: $('#main-inner'), + forceFixPosition: true, serviceUrl: function(query) { return 'rest/prod_comp_search/' + encodeURIComponent(query); }, diff --git a/extensions/Push/lib/Backoff.pm b/extensions/Push/lib/Backoff.pm index f0116a2a7..0436cdf14 100644 --- a/extensions/Push/lib/Backoff.pm +++ b/extensions/Push/lib/Backoff.pm @@ -19,6 +19,7 @@ use constant AUDIT_REMOVES => 0; use constant USE_MEMCACHED => 0; use Bugzilla; +use Bugzilla::Logging; use Bugzilla::Util; # @@ -67,9 +68,7 @@ sub reset { my ($self) = @_; $self->{next_attempt_ts} = Bugzilla->dbh->selectrow_array('SELECT NOW()'); $self->{attempts} = 0; - Bugzilla->push_ext->logger->debug( - sprintf("resetting backoff for %s", $self->connector) - ); + INFO( sprintf 'resetting backoff for %s', $self->connector ); } sub inc { @@ -82,8 +81,8 @@ sub inc { $self->{next_attempt_ts} = $date; $self->{attempts} = $attempts; - Bugzilla->push_ext->logger->debug( - sprintf("setting next attempt for %s to %s (attempt %s)", $self->connector, $date, $attempts) + INFO( + sprintf 'setting next attempt for %s to %s (attempt %s)', $self->connector, $date, $attempts ); } diff --git a/extensions/Push/lib/Config.pm b/extensions/Push/lib/Config.pm index 337c2696d..2db95b972 100644 --- a/extensions/Push/lib/Config.pm +++ b/extensions/Push/lib/Config.pm @@ -11,7 +11,7 @@ use 5.10.1; use strict; use warnings; -use Bugzilla; +use Bugzilla::Logging; use Bugzilla::Constants; use Bugzilla::Extension::Push::Option; use Crypt::CBC; @@ -52,7 +52,6 @@ sub option { sub load { my ($self) = @_; my $config = {}; - my $logger = Bugzilla->push_ext->logger; # prime $config with defaults foreach my $rh ($self->options) { @@ -81,7 +80,7 @@ sub load { # done, update self foreach my $name (keys %$config) { my $value = $self->option($name)->{type} eq 'password' ? '********' : $config->{$name}; - $logger->debug(sprintf("%s: set %s=%s\n", $self->{_name}, $name, $value || '')); + TRACE( sprintf "%s: set %s=%s\n", $self->{_name}, $name, $value || '' ); $self->{$name} = $config->{$name}; } } diff --git a/extensions/Push/lib/Connector/Phabricator.pm b/extensions/Push/lib/Connector/Phabricator.pm index 5da64901a..e59ba6c0d 100644 --- a/extensions/Push/lib/Connector/Phabricator.pm +++ b/extensions/Push/lib/Connector/Phabricator.pm @@ -15,24 +15,18 @@ use base 'Bugzilla::Extension::Push::Connector::Base'; use Bugzilla::Bug; use Bugzilla::Constants; -use Bugzilla::Error; -use Bugzilla::User; use Bugzilla::Extension::PhabBugz::Constants; +use Bugzilla::Extension::PhabBugz::Policy; +use Bugzilla::Extension::PhabBugz::Project; +use Bugzilla::Extension::PhabBugz::Revision; use Bugzilla::Extension::PhabBugz::Util qw( - add_comment_to_revision add_security_sync_comments - create_private_revision_policy - edit_revision_policy get_attachment_revisions get_bug_role_phids - get_project_phid get_security_sync_groups - intersect - make_revision_public - make_revision_private - set_revision_subscribers ); + use Bugzilla::Extension::Push::Constants; use Bugzilla::Extension::Push::Util qw(is_public); @@ -76,72 +70,55 @@ sub send { my @set_groups = get_security_sync_groups($bug); - my @revisions = get_attachment_revisions($bug); - - if (!$is_public && !@set_groups) { - foreach my $revision (@revisions) { - Bugzilla->audit(sprintf( - 'Making revision %s for bug %s private due to unkown Bugzilla groups: %s', - $revision->{id}, - $bug->id, - join(', ', @set_groups) - )); - make_revision_private( $revision->{phid} ); - } - - add_security_sync_comments(\@revisions, $bug); - - return PUSH_RESULT_OK; - } + my $revisions = get_attachment_revisions($bug); my $group_change = ($message->routing_key =~ /^(?:attachment|bug)\.modify:.*\bbug_group\b/) ? 1 : 0; - my $subscribers; - if ( !$is_public ) { - $subscribers = get_bug_role_phids($bug); - } - - my $secure_project_phid = get_project_phid('secure-revision'); - - foreach my $revision (@revisions) { - my $revision_phid = $revision->{phid}; - - my $rev_obj = Bugzilla::Extension::PhabBugz::Revision->new_from_query({ phids => [ $revision_phid ] }); - my $revision_updated; - + foreach my $revision (@$revisions) { if ( $is_public && $group_change ) { Bugzilla->audit(sprintf( 'Making revision %s public for bug %s', - $revision->{id}, + $revision->id, $bug->id )); - make_revision_public($revision_phid); - $rev_obj->remove_project($secure_project_phid); - $revision_updated = 1; + $revision->make_public(); + } + elsif ( !$is_public && !@set_groups ) { + Bugzilla->audit(sprintf( + 'Making revision %s for bug %s private due to unkown Bugzilla groups: %s', + $revision->id, + $bug->id, + join(', ', @set_groups) + )); + $revision->make_private(['secure-revision']); + add_security_sync_comments([$revision], $bug); } elsif ( !$is_public && $group_change ) { Bugzilla->audit(sprintf( 'Giving revision %s a custom policy for bug %s', - $revision->{id}, + $revision->id, $bug->id )); - my $policy_phid = create_private_revision_policy( $bug, \@set_groups ); - edit_revision_policy( $revision_phid, $policy_phid, $subscribers ); - $rev_obj->add_project($secure_project_phid); - $revision_updated = 1; + my @set_project_names = map { "bmo-" . $_ } @set_groups; + $revision->make_private(\@set_project_names); } - elsif ( !$is_public && !$group_change ) { + + # Subscriber list of the private revision should always match + # the bug roles such as assignee, qa contact, and cc members. + if (!$is_public) { Bugzilla->audit(sprintf( 'Updating subscribers for %s for bug %s', - $revision->{id}, + $revision->id, $bug->id )); - set_revision_subscribers( $revision_phid, $subscribers ); + my $subscribers = get_bug_role_phids($bug); + $revision->set_subscribers($subscribers) if $subscribers; } - $rev_obj->update() if $revision_updated; + + $revision->update(); } return PUSH_RESULT_OK; diff --git a/extensions/Push/lib/Connectors.pm b/extensions/Push/lib/Connectors.pm index 75a5083ff..d3c55d3ca 100644 --- a/extensions/Push/lib/Connectors.pm +++ b/extensions/Push/lib/Connectors.pm @@ -11,10 +11,12 @@ use 5.10.1; use strict; use warnings; +use Bugzilla::Logging; use Bugzilla::Extension::Push::Util; use Bugzilla::Constants; use Bugzilla::Util qw(trick_taint); use File::Basename; +use Try::Tiny; sub new { my ($class) = @_; @@ -25,16 +27,15 @@ sub new { $self->{objects} = {}; $self->{path} = bz_locations->{'extensionsdir'} . '/Push/lib/Connector'; - my $logger = Bugzilla->push_ext->logger; foreach my $file (glob($self->{path} . '/*.pm')) { my $name = basename($file); $name =~ s/\.pm$//; next if $name eq 'Base'; if (length($name) > 32) { - $logger->info("Ignoring connector '$name': Name longer than 32 characters"); + WARN("Ignoring connector '$name': Name longer than 32 characters"); } push @{$self->{names}}, $name; - $logger->debug("Found connector '$name'"); + TRACE("Found connector '$name'"); } return $self; @@ -44,7 +45,6 @@ sub _load { my ($self) = @_; return if scalar keys %{$self->{objects}}; - my $logger = Bugzilla->push_ext->logger; foreach my $name (@{$self->{names}}) { next if exists $self->{objects}->{$name}; my $file = $self->{path} . "/$name.pm"; @@ -52,34 +52,30 @@ sub _load { require $file; my $package = "Bugzilla::Extension::Push::Connector::$name"; - $logger->debug("Loading connector '$name'"); + TRACE("Loading connector '$name'"); my $old_error_mode = Bugzilla->error_mode; Bugzilla->error_mode(ERROR_MODE_DIE); - eval { + try { my $connector = $package->new(); $connector->load_config(); $self->{objects}->{$name} = $connector; + } catch { + ERROR("Connector '$name' failed to load: " . clean_error($_)); }; - if ($@) { - $logger->error("Connector '$name' failed to load: " . clean_error($@)); - } Bugzilla->error_mode($old_error_mode); } } sub stop { my ($self) = @_; - my $logger = Bugzilla->push_ext->logger; foreach my $connector ($self->list) { next unless $connector->enabled; - $logger->debug("Stopping '" . $connector->name . "'"); - eval { + TRACE("Stopping '" . $connector->name . "'"); + try { $connector->stop(); + } catch { + ERROR("Connector '" . $connector->name . "' failed to stop: " . clean_error($_)); }; - if ($@) { - $logger->error("Connector '" . $connector->name . "' failed to stop: " . clean_error($@)); - $logger->debug("Connector '" . $connector->name . "' failed to stop: $@"); - } } } diff --git a/extensions/Push/lib/Push.pm b/extensions/Push/lib/Push.pm index 45b9b05dd..670b2aa56 100644 --- a/extensions/Push/lib/Push.pm +++ b/extensions/Push/lib/Push.pm @@ -11,6 +11,7 @@ use 5.10.1; use strict; use warnings; +use Bugzilla::Logging; use Bugzilla::Extension::Push::BacklogMessage; use Bugzilla::Extension::Push::Config; use Bugzilla::Extension::Push::Connectors; @@ -107,7 +108,7 @@ sub push { # if the connector is backlogged, push to the backlog queue if ($is_backlogged) { - $logger->debug("backlogged"); + INFO('connector is backlogged'); my $backlog = Bugzilla::Extension::Push::BacklogMessage->create_from_message($message, $connector); } } diff --git a/extensions/Review/Extension.pm b/extensions/Review/Extension.pm index 576c31c7e..72f16e3b6 100644 --- a/extensions/Review/Extension.pm +++ b/extensions/Review/Extension.pm @@ -94,7 +94,7 @@ sub _user_is_active { my ($self) = @_; # never consider .bugs or .tld addresses as inactive. - return 1 if $self->login =~ /bugs$/ || $self->login =~ /\.tld$/; + return 1 if $self->login =~ /\.(?:bugs|tld)$/; return 1 unless Bugzilla->params->{max_reviewer_last_seen}; return 0 if !defined($self->last_seen_date); diff --git a/extensions/SecureMail/Extension.pm b/extensions/SecureMail/Extension.pm index 1595fe98c..2b5e1bdd6 100644 --- a/extensions/SecureMail/Extension.pm +++ b/extensions/SecureMail/Extension.pm @@ -27,6 +27,7 @@ use warnings; use base qw(Bugzilla::Extension); +use Bugzilla::Logging; use Bugzilla::Attachment; use Bugzilla::Comment; use Bugzilla::Group; @@ -35,6 +36,7 @@ use Bugzilla::User; use Bugzilla::Util qw(trim trick_taint is_7bit_clean); use Bugzilla::Error; use Bugzilla::Mailer; +use Bugzilla::Extension::SecureMail::TCT; use Crypt::OpenPGP::Armour; use Crypt::OpenPGP::KeyRing; @@ -128,9 +130,12 @@ sub object_validators { if ($value =~ /PUBLIC KEY/) { # PGP keys must be ASCII-armoured. - if (!Crypt::OpenPGP::Armour->unarmour($value)) { - ThrowUserError('securemail_invalid_key', - { errstr => Crypt::OpenPGP::Armour->errstr }); + my $tct = Bugzilla::Extension::SecureMail::TCT->new( + public_key => $value, + command => Bugzilla->localconfig->{tct_bin}, + ); + unless ($tct->is_valid->get) { + ThrowUserError( 'securemail_invalid_key', { errstr => 'key is invalid or expired' } ); } } elsif ($value =~ /BEGIN CERTIFICATE/) { @@ -471,8 +476,10 @@ sub _make_secure { # PGP Encryption # ################## - my $pubring = new Crypt::OpenPGP::KeyRing(Data => $key); - my $pgp = new Crypt::OpenPGP(PubRing => $pubring); + my $tct = Bugzilla::Extension::SecureMail::TCT->new( + public_key => $key, + command => Bugzilla->localconfig->{tct_bin}, + ); if (scalar $email->parts > 1) { my $old_boundary = $email->{ct}{attributes}{boundary}; @@ -511,7 +518,7 @@ sub _make_secure { disposition => 'inline', encoding => '7bit', }, - body => _pgp_encrypt($pgp, $to_encrypt, $bug_id) + body => _tct_encrypt($tct, $to_encrypt, $bug_id) ), ); $email->parts_set(\@new_parts); @@ -528,7 +535,7 @@ sub _make_secure { if ($sanitise_subject) { _insert_subject($email, $subject); } - $email->body_set(_pgp_encrypt($pgp, $email->body, $bug_id)); + $email->body_set(_tct_encrypt($tct, $email->body, $bug_id)); } } @@ -604,33 +611,21 @@ sub _make_secure { } } -sub _pgp_encrypt { - my ($pgp, $text, $bug_id) = @_; - # "@" matches every key in the public key ring, which is fine, - # because there's only one key in our keyring. - # - # We use the CAST5 cipher because the Rijndael (AES) module doesn't - # like us for some reason I don't have time to debug fully. - # ("key must be an untainted string scalar") - my $encrypted = $pgp->encrypt( - Data => $text, - Recipients => "@", - Cipher => 'CAST5', - Armour => 0 - ); - if (!defined $encrypted) { - return 'Error during Encryption: ' . $pgp->errstr; +sub _tct_encrypt { + my ($tct, $text, $bug_id) = @_; + + my $comment = Bugzilla->localconfig->{urlbase} . ( $bug_id ? 'show_bug.cgi?id=' . $bug_id : '' ); + my $encrypted; + my $ok = eval { $encrypted = $tct->encrypt( $text, $comment )->get; 1 }; + if (!$ok) { + WARN("Error: $@"); + $encrypted = "$comment\nOpenPGP Encryption failed. Check if your key is expired."; } - $encrypted = Crypt::OpenPGP::Armour->armour( - Data => $encrypted, - Object => 'MESSAGE', - Headers => { - Comment => Bugzilla->localconfig->{urlbase} . ($bug_id ? 'show_bug.cgi?id=' . $bug_id : ''), - }, - ); - # until Crypt::OpenPGP makes the Version header optional we have to strip - # it out manually (bug 1181406). - $encrypted =~ s/\nVersion:[^\n]+//; + elsif (!$encrypted) { + WARN('message empty!'); + $encrypted = "$comment\nOpenPGP Encryption failed for unknown reason."; + } + return $encrypted; } diff --git a/extensions/SecureMail/lib/TCT.pm b/extensions/SecureMail/lib/TCT.pm new file mode 100644 index 000000000..3a16309c2 --- /dev/null +++ b/extensions/SecureMail/lib/TCT.pm @@ -0,0 +1,112 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. +# +# This Source Code Form is "Incompatible With Secondary Licenses", as +# defined by the Mozilla Public License, v. 2.0. + +package Bugzilla::Extension::SecureMail::TCT; +use 5.10.1; +use Moo; + +use Bugzilla::DaemonControl qw( on_finish on_exception ); +use File::Temp; +use Future::Utils qw(call); +use Future; +use IO::Async::Process; + +has 'public_key' => ( is => 'ro', required => 1 ); +has 'public_key_file' => ( is => 'lazy' ); +has 'is_valid' => ( is => 'lazy' ); +has 'command' => ( is => 'ro', default => 'tct' ); + +sub _build_public_key_file { + my ($self) = @_; + my $fh = File::Temp->new(SUFFIX => '.pubkey'); + $fh->print($self->public_key); + $fh->close; + return $fh; +} + +sub _build_is_valid { + my ($self) = @_; + + my $loop = IO::Async::Loop->new; + my $exit_f = $loop->new_future; + my ($stderr, $stdout); + my $process = IO::Async::Process->new( + command => [$self->command, 'check', '-k', $self->public_key_file ], + stderr => { + into => \$stderr, + }, + stdout => { + into => \$stdout, + }, + on_finish => on_finish($exit_f), + on_exception => on_exception($self->command, $exit_f), + ); + $loop->add($process); + + return $exit_f->then( + sub { + my ($rv) = @_; + Future->wrap($rv == 0); + } + ); +} + +sub encrypt { + my ($self, $input, $comment) = @_; + $self->is_valid->then( + sub { + my ($is_valid) = @_; + call { + die 'invalid public key!' unless $is_valid; + + my $output; + my $loop = IO::Async::Loop->new; + my $exit_f = $loop->new_future; + my @command = ( $self->command, 'encrypt', '-k', $self->public_key_file ); + push @command, '--comment', $comment if $comment; + my $process = IO::Async::Process->new( + command => \@command, + stdin => { + from => $input, + }, + stdout => { + into => \$output, + }, + on_finish => on_finish($exit_f), + on_exception => on_exception($self->command, $exit_f), + ); + $loop->add($process); + + return $exit_f->then(sub { Future->wrap($output) }); + } + } + ); +} + +1; + +__END__ + +=head1 NAME + +Bugzilla::Extension::SecureMail::TCT - An interface to the tct program + +=head1 SYNOPSIS + + my $key = <<'PUBLIC_KEY'; + -----BEGIN PGP PUBLIC KEY BLOCK----- + + mQINBFakJSsBEACbDwHztgZaVhIb6f4PN0KbXv5BEciqKNbdVLgWQJyqgEMIwTF7 + ... + o858gRM= + =t9lA + -----END PGP PUBLIC KEY BLOCK----- + PUBLIC_KEY + + my $tct = Bugzilla::Extension::SecureMail::TCT->new(public_key => $key); + my $encrypted = $tct->encrypt("message", "comment goes here")->get; + diff --git a/extensions/SecureMail/template/en/default/hook/admin/users/userdata-end.html.tmpl b/extensions/SecureMail/template/en/default/hook/admin/users/userdata-end.html.tmpl index a90266dae..e5e299ef9 100644 --- a/extensions/SecureMail/template/en/default/hook/admin/users/userdata-end.html.tmpl +++ b/extensions/SecureMail/template/en/default/hook/admin/users/userdata-end.html.tmpl @@ -6,7 +6,7 @@ # defined by the Mozilla Public License, v. 2.0. #%] -[% RETURN UNLESS otheruser.id %] +[% RETURN UNLESS otheruser.id && user.in_group('editusers') %] <tr> <th>Has Secure Mail Key/Cert:</th> @@ -14,7 +14,7 @@ [% otheruser.public_key ? "Yes" : "No" %] </td> </tr> - + <tr> <th>Member of Secure Mail Group:</th> <td> |