In the MozReview panel, when we move to one attachment per child commit (bug 1055021), we don't want to make one call to MozReview per attachment, since most will be redundant. We could send a list of review-request IDs, but even easier would be to just ask for all review requests belonging to a given bug. The BMO MozReview extension will just check if there are any non-obsolete MozReview attachments, and then make a single call to MozReview.

Note that we can (and should; see bug 1055021's user story) switch to this mode before we fully switch to attachment-per-child.

This blocks complete resolution of bug 1055021, although it cannot be deployed until step 1 in bug 1055021's user story is complete.

The dependent MozReview patch has been deployed to reviewboard-dev for your testing convenience, e.g. curl 'https://reviewboard-dev.allizom.org/api/extensions/mozreview.extension.MozReviewExtension/summary/?bug=1007920'

Comment on attachment 8600204 [details] [diff] [review] Get MozReview summaries by bug Review of attachment 8600204 [details] [diff] [review]: ----------------------------------------------------------------- this will conflict with the changes in bug 1153100. i'll chat with dylan about the timing of these two reviews. - if rb returns 0 results we end up with a weird empty mozreview table. an error message should be shown if this happens. side note: the urls in the responses from rb are invalid. eg "https://reviewboard-dev.allizom.org/api/extensions/mozreview.extension.MozReviewExtension/summary/383/" should probably be "https://reviewboard-dev.allizom.org/r/383/". ::: extensions/MozReview/Extension.pm @@ +36,5 @@ > } > } else { > + my $has_mozreview = 0; > + my $attachments = > + Bugzilla::Attachment->get_attachments_by_bug($bug); nit: no need to wrap this line - we don't have a max line length of 80 chars and it looks weird @@ +51,2 @@ > $vars->{'mozreview'} = 1; > + $vars->{'bug_id'} = $bug->id; while this line should be removed due to adding the bug_id to the BUGZILLA object, the bug id is already visible to the show_bug template as [% bug.id %] and doesn't need to be passed in to templates. ::: extensions/MozReview/template/en/default/hook/bug/edit-after_bug_data.html.tmpl @@ +13,5 @@ > <br> > <table > class="mozreview-table" > + data-mozreview-url="[% Bugzilla.params.mozreview_base_url FILTER html %]" > + data-bug-id="[% bug_id FILTER none %]"> instead of duplicating the bug_id here, copy the BUGZILLA.bug_id setting line from BugModal's header (extensions/BugModal/template/en/default/bug_modal/header.html.tmpl) to template/en/default/bug/show-header.html.tmpl to make it accessible as BUGZILLA.bug_id

Regarding your side note, I filed bug 1161054. I'll get to it eventually, but it doesn't affect BMO usage. In the event that 0 requests are returned (which shouldn't happen, but I agree that an error message is useful in case something goes terribly wrong), the table now just says "Error: no MozReview requests found." I'll wait until bug 1153100 lands before posting an updated patch here.

Updated for review comments and rebased on master.

Comment on attachment 8601222 [details] [diff] [review] Get MozReview summaries by bug, v2 Review of attachment 8601222 [details] [diff] [review]: ----------------------------------------------------------------- r=glob

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git 27db7bb..63b0ecb master -> master