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])
attachment_updates = BugzillaAttachmentUpdates(b, bug)
attachment_updates.obsolete_review_attachments(
get_diff_url(review_request))
attachment_updates.do_updates()
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)
attachment_updates = BugzillaAttachmentUpdates(b, bug)
attachment_updates.obsolete_review_attachments(diff_url)
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:
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()
