mozreview: allow publishing from any commit (
bug 1205306) r?smacleod
This modifies the commit approval logic to allow triggering a publish of
all child reviews from any child review.
There's still cleanup to do here, I wanted to get feedback on the approach
before I spend more time on this.
--- a/hgext/reviewboard/tests/test-operation-prevention.t
+++ b/hgext/reviewboard/tests/test-operation-prevention.t
@@ -17,25 +17,19 @@ Create a review request from a regular u
$ exportbzauth author@example.com password
$ bugzilla create-bug TestProduct TestComponent 'First Bug'
$ echo initial > foo
$ hg commit -m 'Bug 1 - Initial commit to review'
$ hg --config bugzilla.username=author@example.com --config bugzilla.apikey=${authorkey} push > /dev/null
-Attempting to publish a commit review request should fail.
+Attempting to publish a child review request should succeed.
$ rbmanage publish 2
- API Error: 500: 225: Error publishing: Publishing commit review requests is prohibited, please publish parent.
- [1]
-
-Publishing the parent should succeed.
-
- $ rbmanage publish 1
$ rbmanage dumpreview 1
id: 1
status: pending
public: true
bugs:
- '1'
commit: bz://1/mynick
@@ -77,10 +71,12 @@ Publishing the parent should succeed.
p2rb.is_squashed: false
approved: false
approval_failure: A suitable reviewer has not given a "Ship It!"
$ cd ..
Cleanup
+ $ mozreview exec rbweb cat /reviewboard/logs/reviewboard.log
+
$ mozreview stop
stopped 10 containers
--- a/pylib/mozreview/mozreview/static/mozreview/css/review.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/review.less
@@ -35,40 +35,22 @@
#discard-banner {
#btn-review-request-reopen,
#field_changedescription,
p {
display: none;
}
}
-#draft-banner {
- #btn-draft-publish,
- #btn-review-request-discard,
- #btn-draft-discard {
- display: none;
- }
-}
-
#review-banner {
#review-banner-publish {
display: none;
}
}
-// We allow publishing on parents so override hiding
-// the buttons.
-.parent-request #draft-banner {
- #btn-draft-publish,
- #btn-review-request-discard,
- #btn-draft-discard {
- display: inline-block;
- }
-}
-
.actions {
#close-review-request-link,
#shipit-link,
#upload-diff-link {
display: none;
}
}
--- a/pylib/rbbz/rbbz/extension.py
+++ b/pylib/rbbz/rbbz/extension.py
@@ -27,16 +27,17 @@ from reviewboard.site.urlresolvers impor
from mozreview.bugzilla.client import Bugzilla
from mozreview.bugzilla.errors import BugzillaError
from mozreview.errors import (CommitPublishProhibited,
ParentShipItError)
from mozreview.extra_data import (UNPUBLISHED_RRIDS_KEY,
gen_child_rrs,
gen_rrs_by_rids,
gen_rrs_by_extra_data_key,
+ get_parent_rr,
is_parent)
from mozreview.models import (BugzillaUserMap,
get_bugzilla_api_key,
get_or_create_bugzilla_users,
get_profile)
from mozreview.signals import commit_request_publishing
from rbbz.auth import BugzillaBackend
from rbbz.diffs import build_plaintext_review
@@ -302,16 +303,18 @@ def on_review_request_publishing(user, r
# locally, we handle it here, and log.
if not review_request_draft:
logging.error('Strangely, there was no review request draft on the '
'review request we were attempting to publish.')
return
review_request = review_request_draft.get_review_request()
+ logging.info('publishing: %s' % review_request.id)
+
# skip review requests that were not pushed
if not is_review_request_pushed(review_request):
return
if not is_parent(review_request):
# Send a signal asking for approval to publish this review request.
# We only want to publish this commit request if we are in the middle
# of publishing the parent. If the parent is publishing it will be
@@ -320,17 +323,20 @@ def on_review_request_publishing(user, r
sender=review_request,
user=user,
review_request_draft=review_request_draft)
for receiver, approved in approvals:
if approved:
break
else:
- # This publish is not approved by the parent review request.
+ # Trigger a publish on the parent. The parent will publish this
+ # child later on.
+ parent_rr = get_parent_rr(review_request)
+ parent_rr.publish(user)
raise CommitPublishProhibited()
# The reviewid passed through p2rb is, for Mozilla's instance anyway,
# bz://<bug id>/<irc nick>.
reviewid = review_request_draft.extra_data.get('p2rb.identifier', None)
m = REVIEWID_RE.match(reviewid)
if not m: