--- a/pylib/rbbz/rbbz/extension.py
+++ b/pylib/rbbz/rbbz/extension.py
@@ -201,18 +201,21 @@ def bugzilla_to_publish_errors(func):
def _transform_errors(*args, **kwargs):
try:
return func(*args, **kwargs)
except BugzillaError as e:
raise PublishError('Bugzilla error: %s' % e.msg)
return _transform_errors
-def post_bugzilla_attachment(bugzilla, bug_id, review_request_draft,
- review_request):
+def update_bugzilla_attachments(bugzilla, bug_id, children_to_post,
+ children_to_obsolete):
+ for child in children_to_obsolete:
+ bugzilla.obsolete_review_attachments(bug_id, 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
@@ -224,81 +227,89 @@ def post_bugzilla_attachment(bugzilla, b
# 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.
- reviewers = {}
+ user_email_cache = {}
- for u in review_request_draft.target_people.all():
- bum = BugzillaUserMap.objects.get(user=u)
-
- user_data = bugzilla.get_user_from_userid(bum.bugzilla_user_id)
+ for review_request_draft, review_request in children_to_post:
+ reviewers = {}
- # 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)
- reviewers[users[0].email] = False
+ for u in review_request_draft.target_people.all():
+ bum = BugzillaUserMap.objects.get(user=u)
+ email = user_email_cache.get(bum.bugzilla_user_id)
- last_user = None
- relevant_reviews = review_request.get_public_reviews().order_by(
- 'user', '-timestamp')
+ if email is None:
+ user_data = bugzilla.get_user_from_userid(bum.bugzilla_user_id)
- for review in relevant_reviews:
- if review.user == last_user:
- # We only care about the most recent review for each
- # particular user.
- continue
+ # 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
- last_user = review.user
+ reviewers[email] = False
- # 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[last_user.email] = True
+ last_user = None
+ relevant_reviews = review_request.get_public_reviews().order_by(
+ 'user', '-timestamp')
+
+ for review in relevant_reviews:
+ if review.user == last_user:
+ # We only care about the most recent review for each
+ # particular user.
+ continue
+
+ last_user = review.user
- rr_url = get_obj_url(review_request)
- diff_url = '%sdiff/#index_header' % rr_url
+ # 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[last_user.email] = True
- # 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.
- extended_commit_msg = review_request_draft.description.partition(
- '\n')[2].lstrip('\n')
+ rr_url = get_obj_url(review_request)
+ diff_url = '%sdiff/#index_header' % rr_url
- if extended_commit_msg:
- extended_commit_msg += '\n\n'
+ # 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.
+ ext_commit_msg = review_request_draft.description.partition(
+ '\n')[2].lstrip('\n')
- comment = '%sReview commit: %s\nSee other reviews: %s' % (
- extended_commit_msg,
- diff_url,
- rr_url
- )
- else:
- comment = ('Review request updated; see interdiff: '
- '%sdiff/%d-%d/\n' % (rr_url,
- diffset_count,
- diffset_count + 1))
+ if ext_commit_msg:
+ ext_commit_msg += '\n\n'
- bugzilla.post_rb_url(bug_id,
- review_request.id,
- review_request_draft.summary,
- comment,
- diff_url,
- reviewers)
+ comment = '%sReview commit: %s\nSee other reviews: %s' % (
+ ext_commit_msg,
+ diff_url,
+ rr_url
+ )
+ else:
+ comment = ('Review request updated; see interdiff: '
+ '%sdiff/%d-%d/\n' % (rr_url,
+ diffset_count,
+ diffset_count + 1))
+
+ bugzilla.post_rb_url(bug_id,
+ review_request.id,
+ review_request_draft.summary,
+ comment,
+ diff_url,
+ reviewers)
@bugzilla_to_publish_errors
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
@@ -373,24 +384,31 @@ def on_review_request_publishing(user, r
# 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:
+ children_to_post = []
+ children_to_obsolete = []
+
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)
+ children_to_obsolete.append(child)
+
+ children_to_post.append((child_draft, child))
+
+ if children_to_post or children_to_obsolete:
+ update_bugzilla_attachments(b, bug_id, children_to_post,
+ children_to_obsolete)
# 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):