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, in which
# case throw an error. Ship-its are allowed only on child commits.
if review.ship_it:
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])
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)
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)
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)
def on_review_request_closed_discarded(user, review_request, type, **kwargs):
if type != ReviewRequest.DISCARDED:
return
commit_data = fetch_commit_data(review_request)
if is_parent(review_request, commit_data):
# close_child_review_requests will call save on this review request, so
# we don't have to worry about it.
review_request.commit = None
_close_child_review_requests(user, review_request,
ReviewRequest.DISCARDED,
AUTO_CLOSE_DESCRIPTION,
commit_data=commit_data)
else:
# TODO: Remove this once we properly prevent users from closing
# commit review requests.
b = Bugzilla(get_bugzilla_api_key(user))
bug = int(review_request.get_bug_list()[0])
diff_url = '%sdiff/#index_header' % get_obj_url(review_request)
b.obsolete_review_attachments(bug, diff_url)
def update_bugzilla_attachments(bugzilla, bug_id, children_to_post,
children_to_obsolete):
attachment_updates = BugzillaAttachmentUpdates(bugzilla, bug_id)
for child in children_to_obsolete:
attachment_updates.obsolete_review_attachments(get_obj_url(child))
# We publish attachments for each commit/child to Bugzilla so that
# reviewers can easily track their requests.
# The review request exposes a list of usernames for reviewers. We need
# to convert these to Bugzilla emails in order to make the request into
# Bugzilla.
#
# It may seem like there is a data syncing problem here where usernames
# may get out of sync with the reality from Bugzilla. Fortunately,
# Review Board is smarter than that. Internally, the target_people list
# is stored with foreign keys into the numeric primary key of the user
# table. If the RB username changes, this won't impact target_people
# nor the stored mapping to the numeric Bugzilla ID, which is
# immutable.
#
# But we do have a potential data syncing problem with the stored email
# address. Review Board's stored email address could be stale. So
# instead of using it directly, we query Bugzilla and map the stored,
# immutable numeric Bugzilla userid into an email address. This lookup
# could be avoided if Bugzilla accepted a numeric userid in the
# requestee parameter when modifying an attachment.
user_email_cache = {}
for review_request_draft, review_request in children_to_post:
carry_forward = {}
has_code_changes = review_request_draft.diffset is not None
for u in review_request_draft.target_people.all():
bum = BugzillaUserMap.objects.get(user=u)
email = user_email_cache.get(bum.bugzilla_user_id)
if email is None:
user_data = bugzilla.get_user_from_userid(bum.bugzilla_user_id)
# Since we're making the API call, we might as well ensure the
# local database is up to date.
users = get_or_create_bugzilla_users(user_data)
email = users[0].email
user_email_cache[bum.bugzilla_user_id] = email
carry_forward[email] = False
for review in gen_latest_reviews(review_request):
if has_code_changes:
# The code has changed, we need to determine what needs to
# happen to the flags.
# Don't set carry_forward values for reviewers that are not in
# the target_people list (eg. impromptu reviews). This will
# result in the attachment flag being cleared in Bugzilla.
if review.user.email not in carry_forward:
continue
# Carry forward just r+'s. All other flags should be reset
# to r?.
review_flag = review.extra_data.get(REVIEW_FLAG_KEY)
carry_forward[review.user.email] = review_flag == 'r+' or (
# Older reviews didn't set review_flag.
review_flag is None and review.ship_it)
else:
# This is a meta data only change, don't touch any existing
# flags.
carry_forward[review.user.email] = True
flags = []
attachment = attachment_updates.get_attachment(review_request)
if attachment:
# Update flags on an existing attachment.
for f in attachment.get('flags', []):
if f['name'] not in ['review', 'feedback']:
# We only care about review and feedback flags.
continue
# When a new patch is pushed, we need to mimic what
# happens to flags when a new attachment is created
# in Bugzilla:
# - carry forward r+'s
# - clear r-'s
# - clear feedback flags
if f['name'] == 'feedback':
if has_code_changes:
# We always clear feedback flags when the patch
# is updated.
flags.append({'id': f['id'], 'status': 'X'})
elif f['status'] == '+' or f['status'] == '-':
# A reviewer has left a review, either in Review Board or
# in Bugzilla.
if f['setter'] not in carry_forward:
# This flag was set manually in Bugzilla rather
# then through a review on Review Board. Always
# clear these flags.
flags.append({'id': f['id'], 'status': 'X'})
else:
# This flag was set through Review Board; see if
# we should carry it forward.
if not carry_forward[f['setter']]:
# We should not carry this r+/r- forward so
# re-request review.
flags.append({
'id': f['id'],
'name': 'review',
'status': '?',
'requestee': f['setter']
})
# else we leave the flag alone, carrying it forward.
# In either case, we've dealt with this reviewer, so
# remove it from the carry_forward dict.
carry_forward.pop(f['setter'])
elif ('requestee' not in f or
f['requestee'] not in carry_forward):
# We clear review flags where the requestee is not
# a reviewer, or if there is some (possibly future) flag
# other than + or - that does not have a 'requestee' field.
flags.append({'id': f['id'], 'status': 'X'})
elif f['requestee'] in carry_forward:
# We're already waiting for a review from this user
# so don't touch the flag.
carry_forward.pop(f['requestee'])
# Add flags for new reviewers.
# We can't set a missing r+ (if it was manually removed) except in the
# trivial (and useless) case that the setter and the requestee are the
# same person. We could set r? again, but in the event that the
# reviewer is not accepting review requests, this will block
# publishing, with no way for the author to fix it. So we'll just
# ignore manually removed r+s.
# This is sorted so behavior is deterministic (this mucks with test
# output otherwise).
for reviewer, keep in sorted(carry_forward.iteritems()):
if not keep:
flags.append({
'name': 'review',
'status': '?',
'requestee': reviewer,
'new': True
})
attachment_updates.create_or_update_attachment(
review_request, review_request_draft, flags)
attachment_updates.do_updates()
def update_bugzilla_attachments(bugzilla, bug_id, children_to_post,
children_to_obsolete):
attachment_updates = BugzillaAttachmentUpdates(bugzilla, bug_id)
for child in children_to_obsolete:
attachment_updates.obsolete_review_attachments(get_obj_url(child))
# We publish attachments for each commit/child to Bugzilla so that
# reviewers can easily track their requests.
# The review request exposes a list of usernames for reviewers. We need
# to convert these to Bugzilla emails in order to make the request into
# Bugzilla.
#
# It may seem like there is a data syncing problem here where usernames
# may get out of sync with the reality from Bugzilla. Fortunately,
# Review Board is smarter than that. Internally, the target_people list
# is stored with foreign keys into the numeric primary key of the user
# table. If the RB username changes, this won't impact target_people
# nor the stored mapping to the numeric Bugzilla ID, which is
# immutable.
#
# But we do have a potential data syncing problem with the stored email
# address. Review Board's stored email address could be stale. So
# instead of using it directly, we query Bugzilla and map the stored,
# immutable numeric Bugzilla userid into an email address. This lookup
# could be avoided if Bugzilla accepted a numeric userid in the
# requestee parameter when modifying an attachment.
user_email_cache = {}
for review_request_draft, review_request in children_to_post:
reviewers = {}
for u in review_request_draft.target_people.all():
bum = BugzillaUserMap.objects.get(user=u)
email = user_email_cache.get(bum.bugzilla_user_id)
if email is None:
user_data = bugzilla.get_user_from_userid(bum.bugzilla_user_id)
# Since we're making the API call, we might as well ensure the
# local database is up to date.
users = get_or_create_bugzilla_users(user_data)
email = users[0].email
user_email_cache[bum.bugzilla_user_id] = email
reviewers[email] = False
for review in gen_latest_reviews(review_request):
# The last review given by this reviewer had a ship-it, so we
# will carry their r+ forward. If someone had manually changed
# their flag on bugzilla, we may be setting it back to r+, but
# we will consider the manual flag change on bugzilla user
# error for now.
if review.ship_it:
reviewers[review.user.email] = True
rr_url = get_obj_url(review_request)
diff_url = '%sdiff/#index_header' % rr_url
# Only post a comment if the diffset has actually changed
comment = ''
if review_request_draft.get_latest_diffset():
diffset_count = review_request.diffset_history.diffsets.count()
if diffset_count < 1:
# We don't need the first line, since it is also the attachment
# summary, which is displayed in the comment.
full_commit_msg = review_request_draft.description.partition(
'\n')[2].strip()
full_commit_msg = strip_commit_metadata(full_commit_msg)
if full_commit_msg:
full_commit_msg += '\n\n'
comment = '%sReview commit: %s\nSee other reviews: %s' % (
full_commit_msg,
diff_url,
rr_url
)
else:
comment = ('Review request updated; see interdiff: '
'%sdiff/%d-%d/\n' % (rr_url,
diffset_count,
diffset_count + 1))
attachment_updates.create_or_update_attachment(
review_request.id,
review_request_draft.summary,
comment,
diff_url,
reviewers)
attachment_updates.do_updates()
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):
# We only support raw comments on parent review requests to prevent
# confusion. If the ship-it flag or the review flag was set, throw
# an error.
# Otherwise, mirror the comment over, associating it with the first
# commit.
if review.ship_it or review.extra_data.get(REVIEW_FLAG_KEY):
raise ParentShipItError
# TODO: If we ever allow multiple bugs in a single series, and we want
# to continue to allow comments on parents, we'll have to pick one
# child for each unique bug.
first_child = list(gen_child_rrs(review_request))[0]
b.post_comment(int(first_child.get_bug_list()[0]), comment,
get_diff_url(first_child), False)
else:
diff_url = get_diff_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, diff_url, False)
def on_review_request_publishing(user, review_request_draft, **kwargs):
# There have been strange cases (all local, and during development), where
# when attempting to publish a review request, this handler will fail
# because the draft does not exist. This is a really strange case, and not
# one we expect to happen in production. However, since we've seen it
# locally, we handle it here, and log.
if not review_request_draft:
logger.error('Strangely, there was no review request draft on the '
'review request we were attempting to publish.')
return
# If the review request draft has a new DiffSet we will only allow
# publishing if that DiffSet has been verified. It is important to
# do this for every review request, not just pushed ones, because
# we can't trust the storage mechanism which indicates it was pushed.
# TODO: This will be fixed when we transition away from extra_data.
if review_request_draft.diffset:
try:
DiffSetVerification.objects.get(
diffset=review_request_draft.diffset)
except DiffSetVerification.DoesNotExist:
logger.error(
'An attempt was made by User %s to publish an unverified '
'DiffSet with id %s',
user.id,
review_request_draft.diffset.id)
raise PublishError(
'This review request draft contained a manually uploaded '
'diff, which is prohibited. Please push to the review server '
'to create review requests. If you believe you received this '
'message in error, please file a bug.')
review_request = review_request_draft.get_review_request()
commit_data = fetch_commit_data(review_request)
# skip review requests that were not pushed
if not is_pushed(review_request, commit_data=commit_data):
return
if not is_parent(review_request, commit_data):
# 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
# listening for this signal to approve it.
approvals = commit_request_publishing.send_robust(
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.
raise CommitPublishProhibited()
# The reviewid passed through p2rb is, for Mozilla's instance anyway,
# bz://<bug id>/<irc nick>.
reviewid = commit_data.draft_extra_data.get(IDENTIFIER_KEY, None)
m = REVIEWID_RE.match(reviewid)
if not m:
raise InvalidBugIdError('<unknown>')
bug_id = m.group(1)
try:
bug_id = int(bug_id)
except (TypeError, ValueError):
raise InvalidBugIdError(bug_id)
siteconfig = SiteConfiguration.objects.get_current()
using_bugzilla = (
siteconfig.settings.get("auth_backend", "builtin") == "bugzilla")
if using_bugzilla:
b = Bugzilla(get_bugzilla_api_key(user))
try:
if b.is_bug_confidential(bug_id):
raise ConfidentialBugError
except BugzillaError as e:
# Special cases:
# 100: Invalid Bug Alias
# 101: Bug does not exist
if e.fault_code and (e.fault_code == 100 or e.fault_code == 101):
raise InvalidBugIdError(bug_id)
raise
# Note that the bug ID has already been set when the review was created.
# If this is a squashed/parent review request, automatically publish all
# relevant children.
if is_parent(review_request, commit_data):
unpublished_rids = map(int, json.loads(
commit_data.extra_data[UNPUBLISHED_KEY]))
discard_on_publish_rids = map(int, json.loads(
commit_data.extra_data[DISCARD_ON_PUBLISH_KEY]))
child_rrs = list(gen_child_rrs(review_request_draft))
# Create or update Bugzilla attachments for each draft commit. This
# is done before the children are published to ensure that MozReview
# doesn't get into a strange state if communication with Bugzilla is
# broken or attachment creation otherwise fails. The Bugzilla
# attachments will then, of course, be in a weird state, but that
# should be fixed by the next successful publish.
if using_bugzilla:
for child in child_rrs:
child_draft = child.get_draft(user=user)
if child_draft:
if child.id in discard_on_publish_rids:
b.obsolete_review_attachments(
bug_id, get_obj_url(child))
post_bugzilla_attachment(b, bug_id, child_draft, child)
# Publish draft commits. This will already include items that are in
# unpublished_rids, so we'll remove anything we publish out of
# unpublished_rids.
for child in child_rrs:
if child.get_draft(user=user) or not child.public:
def approve_publish(sender, user, review_request_draft,
**kwargs):
return child is sender
# Setup the parent signal handler to approve the publish
# and then publish the child.
commit_request_publishing.connect(approve_publish,
sender=child,
weak=False)
try:
child.publish(user=user)
finally:
commit_request_publishing.disconnect(
receiver=approve_publish,
sender=child,
weak=False)
if child.id in unpublished_rids:
unpublished_rids.remove(child.id)
# The remaining unpubished_rids need to be closed as discarded because
# they have never been published, and they will appear in the user's
# dashboard unless closed.
for child in gen_rrs_by_rids(unpublished_rids):
child.close(ReviewRequest.DISCARDED,
user=user,
description=NEVER_USED_DESCRIPTION)
# We also close the discard_on_publish review requests because, well,
# we don't need them anymore. We use a slightly different message
# though.
for child in gen_rrs_by_rids(discard_on_publish_rids):
child.close(ReviewRequest.DISCARDED,
user=user,
description=OBSOLETE_DESCRIPTION)
commit_data.extra_data[UNPUBLISHED_KEY] = '[]'
commit_data.extra_data[DISCARD_ON_PUBLISH_KEY] = '[]'
# Copy any drafted CommitData from draft_extra_data to extra_data.
for key in DRAFTED_COMMIT_DATA_KEYS:
if key in commit_data.draft_extra_data:
commit_data.extra_data[key] = commit_data.draft_extra_data[key]
commit_data.save(update_fields=['extra_data'])
review_request.save()
def create_or_update_attachment(self, review_request, review_request_draft,
flags):
"""Create or update the MozReview attachment using the provided flags.
The `flags` parameter is an array of flags to set/update/clear. This
array matches the Bugzilla flag API:
Setting:
{
'id': flag.id
'name': 'review',
'status': '?',
'requestee': reviewer.email
}
Clearing:
{
'id': flag.id,
'status': 'X'
}
"""
logger.info('Posting review request %s to bug %d.' %
(review_request.id, self.bug_id))
rr_url = get_obj_url(review_request)
diff_url = get_diff_url(review_request)
# Build the comment. Only post a comment if the diffset has
# actually changed.
comment = ''
if review_request_draft.get_latest_diffset():
diffset_count = review_request.diffset_history.diffsets.count()
if diffset_count < 1:
# We don't need the first line, since it is also the attachment
# summary, which is displayed in the comment.
full_commit_msg = review_request_draft.description.partition(
'\n')[2].strip()
full_commit_msg = strip_commit_metadata(full_commit_msg)
if full_commit_msg:
full_commit_msg += '\n\n'
comment = '%sReview commit: %s\nSee other reviews: %s' % (
full_commit_msg, diff_url, rr_url)
else:
comment = ('Review request updated; see interdiff: '
'%sdiff/%d-%d/\n' %
(rr_url, diffset_count, diffset_count + 1))
# Set up attachment metadata.
attachment = self.get_attachment(review_request)
params = {}
if attachment:
params['attachment_id'] = attachment['id']
if attachment['is_obsolete']:
params['is_obsolete'] = False
else:
params['data'] = diff_url
params['content_type'] = 'text/x-review-board-request'
params['file_name'] = 'reviewboard-%d-url.txt' % review_request.id
params['summary'] = replace_reviewers(review_request_draft.summary,
None)
params['comment'] = comment
if flags:
params['flags'] = flags
if attachment:
self.updates.append(params)
else:
self.creates.append(params)
