mozreview: allow publishing from any commit (bug 1205306) r?smacleod draft
authorDan Minor <dminor@mozilla.com>
Fri, 11 Dec 2015 10:12:50 -0500
changeset 6625 5131adcbdb80d97446abd787a066752d4eba3140
parent 6576 f1e1b526e19dff79b1bdf7bdbc596c4e2f09350f
push id498
push userdminor@mozilla.com
push dateWed, 06 Jan 2016 19:39:58 +0000
reviewerssmacleod
bugs1205306
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.
hgext/reviewboard/tests/test-operation-prevention.t
pylib/mozreview/mozreview/static/mozreview/css/review.less
pylib/rbbz/rbbz/extension.py
--- 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: