MozReview: Move 'on_review_publishing' signal to mozreview (
Bug 1262548). r?smacleod
MozReview-Commit-ID: 51HqyHhUSlb
--- a/pylib/mozreview/mozreview/signal_handlers.py
+++ b/pylib/mozreview/mozreview/signal_handlers.py
@@ -1,14 +1,17 @@
from __future__ import unicode_literals
import copy
import json
import logging
+from django.contrib.sites.models import (
+ Site
+)
from django.db.models.signals import (
post_save,
pre_delete,
pre_save,
)
from djblets.siteconfig.models import (
SiteConfiguration,
@@ -20,36 +23,41 @@ from reviewboard.extensions.hooks import
SignalHook,
)
from reviewboard.reviews.models import (
Review,
ReviewRequest,
ReviewRequestDraft,
)
from reviewboard.reviews.signals import (
+ review_publishing,
review_request_closed,
review_request_publishing,
review_request_reopened,
)
from mozreview.bugzilla.attachments import (
update_bugzilla_attachments,
)
from mozreview.bugzilla.client import (
Bugzilla,
BugzillaAttachmentUpdates,
)
from mozreview.bugzilla.errors import (
BugzillaError,
bugzilla_to_publish_errors,
)
+from mozreview.diffs import (
+ build_plaintext_review,
+)
from mozreview.errors import (
CommitPublishProhibited,
ConfidentialBugError,
InvalidBugIdError,
+ ParentShipItError,
)
from mozreview.extra_data import (
DISCARD_ON_PUBLISH_KEY,
DRAFTED_COMMIT_DATA_KEYS,
fetch_commit_data,
gen_child_rrs,
gen_rrs_by_extra_data_key,
gen_rrs_by_rids,
@@ -131,16 +139,22 @@ def initialize_signal_handlers(extension
sandbox_errors=False)
SignalHook(
extension,
pre_save,
pre_save_review,
sender=Review)
+ SignalHook(
+ extension,
+ review_publishing,
+ on_review_publishing,
+ sandbox_errors=False)
+
def post_save_review_request_draft(sender, **kwargs):
"""Handle post_save for a ReviewRequestDraft."""
draft = kwargs["instance"]
if kwargs["created"] and not kwargs["raw"]:
copy_commit_data(draft)
@@ -579,8 +593,71 @@ def pre_save_review(sender, *args, **kwa
# all the reviews, which is what get_reviewers_status does.
reviewers_status = get_reviewers_status(review.review_request,
reviewers=[review.user])
user = review.user.username
flag = reviewers_status.get(user, {}).get('review_flag', ' ')
review.extra_data[REVIEW_FLAG_KEY] = flag
review.ship_it = (review.extra_data[REVIEW_FLAG_KEY] == 'r+')
+
+
+@bugzilla_to_publish_errors
+def on_review_publishing(user, review, **kwargs):
+ """Comment in the bug and potentially r+ or clear a review flag.
+
+ Note that a reviewer *must* have editbugs to set an attachment flag on
+ someone else's attachment (i.e. the standard BMO review process).
+
+ TODO: Report lack-of-editbugs properly; see bug 1119065.
+ """
+ review_request = review.review_request
+ logger.info('Publishing review for user: %s review id: %s '
+ 'review request id: %s' % (user, review.id,
+ review_request.id))
+
+ # skip review requests that were not pushed
+ if not is_pushed(review_request):
+ logger.info('Did not publish review: %s: for user: %d: review not '
+ 'pushed.' % (user, review.id))
+ return
+
+ site = Site.objects.get_current()
+ siteconfig = SiteConfiguration.objects.get_current()
+ comment = build_plaintext_review(review,
+ get_obj_url(review, site,
+ siteconfig),
+ {"user": user})
+ b = Bugzilla(get_bugzilla_api_key(user))
+
+ if is_parent(review_request):
+ # Mirror the comment to the bug, unless it's a ship-it or the review
+ # flag was set, in which case throw an error.
+ # Ship-its and review flags are allowed only on child commits.
+ if review.ship_it or review.extra_data.get(REVIEW_FLAG_KEY):
+ raise ParentShipItError
+
+ [b.post_comment(int(bug_id), comment) for bug_id in
+ review_request.get_bug_list()]
+ else:
+ diff_url = '%sdiff/#index_header' % get_obj_url(review_request)
+ bug_id = int(review_request.get_bug_list()[0])
+
+ commented = False
+ flag = review.extra_data.get(REVIEW_FLAG_KEY)
+
+ if flag is not None:
+ commented = b.set_review_flag(bug_id, flag, review.user.email,
+ diff_url, comment)
+ else:
+ # If for some reasons we don't have the flag set in extra_data,
+ # fall back to ship_it
+ logger.warning('Review flag not set on review %s, '
+ 'updating attachment based on ship_it' % review.id)
+ if review.ship_it:
+ commented = b.r_plus_attachment(bug_id, review.user.email,
+ diff_url, comment)
+ else:
+ commented = b.cancel_review_request(bug_id, review.user.email,
+ diff_url, comment)
+
+ if comment and not commented:
+ b.post_comment(bug_id, comment)
--- a/pylib/rbbz/rbbz/extension.py
+++ b/pylib/rbbz/rbbz/extension.py
@@ -1,37 +1,27 @@
# 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/.
import logging
-from django.contrib.sites.models import Site
-
-from djblets.siteconfig.models import SiteConfiguration
-
from reviewboard.extensions.base import Extension
from reviewboard.extensions.hooks import SignalHook
from reviewboard.reviews.signals import (
reply_publishing,
- review_publishing,
)
from mozreview.bugzilla.client import Bugzilla
from mozreview.bugzilla.errors import (
bugzilla_to_publish_errors,
)
from mozreview.diffs import build_plaintext_review
-from mozreview.errors import (
- ParentShipItError,
-)
from mozreview.extra_data import (
- is_parent,
is_pushed,
- REVIEW_FLAG_KEY
)
from mozreview.models import (
get_bugzilla_api_key,
)
from mozreview.rb_utils import (
get_obj_url,
)
@@ -39,98 +29,30 @@ logger = logging.getLogger(__name__)
class BugzillaExtension(Extension):
def initialize(self):
# Any abortable signal hooks that talk to Bugzilla should have
# sandbox_errors=False, since we don't want to complete the action if
# updating Bugzilla failed for any reason.
- SignalHook(self, review_publishing, on_review_publishing,
- sandbox_errors=False)
SignalHook(self, reply_publishing, on_reply_publishing,
sandbox_errors=False)
def get_reply_url(reply, site=None, siteconfig=None):
""" Get the URL for a reply to a review.
Since replies can have multiple comments, we can't link to a specific
comment, so we link to the parent review which the reply is targeted at.
"""
return get_obj_url(reply.base_reply_to, site=site, siteconfig=siteconfig)
@bugzilla_to_publish_errors
-def on_review_publishing(user, review, **kwargs):
- """Comment in the bug and potentially r+ or clear a review flag.
-
- Note that a reviewer *must* have editbugs to set an attachment flag on
- someone else's attachment (i.e. the standard BMO review process).
-
- TODO: Report lack-of-editbugs properly; see bug 1119065.
- """
- review_request = review.review_request
- logger.info('Publishing review for user: %s review id: %s '
- 'review request id: %s' % (user, review.id,
- review_request.id))
-
- # skip review requests that were not pushed
- if not is_pushed(review_request):
- logger.info('Did not publish review: %s: for user: %d: review not '
- 'pushed.' % (user, review.id))
- return
-
- site = Site.objects.get_current()
- siteconfig = SiteConfiguration.objects.get_current()
- comment = build_plaintext_review(review,
- get_obj_url(review, site,
- siteconfig),
- {"user": user})
- b = Bugzilla(get_bugzilla_api_key(user))
-
- # TODO: Update all attachments in one call. This is not possible right
- # now because we have to potentially mix changing and creating flags.
-
- if is_parent(review_request):
- # Mirror the comment to the bug, unless it's a ship-it or the review
- # flag was set, in which case throw an error.
- # Ship-its and review flags are allowed only on child commits.
- if review.ship_it or review.extra_data.get(REVIEW_FLAG_KEY):
- raise ParentShipItError
-
- [b.post_comment(int(bug_id), comment) for bug_id in
- review_request.get_bug_list()]
- else:
- diff_url = '%sdiff/#index_header' % get_obj_url(review_request)
- bug_id = int(review_request.get_bug_list()[0])
-
- commented = False
- flag = review.extra_data.get(REVIEW_FLAG_KEY)
-
- if flag is not None:
- commented = b.set_review_flag(bug_id, flag, review.user.email,
- diff_url, comment)
- else:
- # If for some reasons we don't have the flag set in extra_data,
- # fall back to ship_it
- logger.warning('Review flag not set on review %s, '
- 'updating attachment based on ship_it' % review.id)
- if review.ship_it:
- commented = b.r_plus_attachment(bug_id, review.user.email,
- diff_url, comment)
- else:
- commented = b.cancel_review_request(bug_id, review.user.email,
- diff_url, comment)
-
- if comment and not commented:
- b.post_comment(bug_id, comment)
-
-
-@bugzilla_to_publish_errors
def on_reply_publishing(user, reply, **kwargs):
review_request = reply.review_request
logger.info('Posting bugzilla reply for review request %s' % (
review_request.id))
# skip review requests that were not pushed
if not is_pushed(review_request):
return