def previous_reviewers(rr):
"""Return the result of the most recent review given by each reviewer"""
# TODO: Ideally this would get approval state for each reviewer as
# calculated in the MozReviewApprovalHook to be absolutely sure we're
# consistent.
pr = {}
for review in gen_latest_reviews(rr):
pr[review.user.username] = review.ship_it
return pr
def get(self, request, *args, **kwargs):
try:
parent_request = get_parent_rr(ReviewRequest.objects.get(
id=kwargs[self.uri_object_key]))
except ReviewRequest.DoesNotExist:
return DOES_NOT_EXIST
if parent_request is None:
return DOES_NOT_EXIST
if not is_parent(parent_request):
return NOT_PARENT
if not parent_request.is_accessible_by(request.user):
return PERMISSION_DENIED
if COMMITS_KEY not in parent_request.extra_data:
logging.error('Parent review request %s missing COMMIT_KEY'
% parent_request.id)
return NOT_PARENT
result = []
children = json.loads(parent_request.extra_data[COMMITS_KEY])
for child in children:
try:
child_request = ReviewRequest.objects.get(id=child[1])
except ReviewRequest.DoesNotExist:
return DOES_NOT_EXIST
if not child_request.approved:
return AUTOLAND_REVIEW_NOT_APPROVED
reviewers = [
r.user.username for r in gen_latest_reviews(child_request) if
r.ship_it and
r.user != child_request.submitter
]
if not reviewers and child_request.approved:
# This review request is approved (the repeated check is
# to ensure this is guaranteed if other parts of the code
# change) but we have an empty list of reviewers. We'll
# assume the author has just approved this themself and
# set r=me
reviewers.append('me')
result.append({
'commit': child[0],
'id': child[1],
'reviewers': reviewers,
'summary': replace_reviewers(child_request.description,
reviewers)
})
return 200, {
'commits': result,
'total_results': len(result),
'links': self.get_links(request=request),
}
def previous_reviewers(rr):
"""Return the result of the most recent review given by each reviewer"""
# TODO: Ideally this would get approval state for each reviewer as
# calculated in the MozReviewApprovalHook to be absolutely sure we're
# consistent.
pr = {}
for review in gen_latest_reviews(rr):
pr[review.user.username] = review.ship_it
return pr
def get(self, request, *args, **kwargs):
try:
parent_request = get_parent_rr(ReviewRequest.objects.get(
id=kwargs[self.uri_object_key]))
except ReviewRequest.DoesNotExist:
return DOES_NOT_EXIST
if parent_request is None:
return DOES_NOT_EXIST
if not is_parent(parent_request):
return NOT_PARENT
if not parent_request.is_accessible_by(request.user):
return PERMISSION_DENIED
if COMMITS_KEY not in parent_request.extra_data:
logging.error('Parent review request %s missing COMMIT_KEY'
% parent_request.id)
return NOT_PARENT
result = []
children = json.loads(parent_request.extra_data[COMMITS_KEY])
for child in children:
try:
child_request = ReviewRequest.objects.get(id=child[1])
except ReviewRequest.DoesNotExist:
return DOES_NOT_EXIST
if not child_request.approved:
return AUTOLAND_REVIEW_NOT_APPROVED
reviewers = map(lambda review: review.user.username,
gen_latest_reviews(child_request))
result.append({
'commit': child[0],
'id': child[1],
'reviewers': reviewers,
'summary': replace_reviewers(child_request.description,
reviewers)
})
return 200, {
'commits': result,
'total_results': len(result),
'links': self.get_links(request=request),
}
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 get(self, request, *args, **kwargs):
try:
parent_request = get_parent_rr(
ReviewRequest.objects.get(id=kwargs[self.uri_object_key]))
except ReviewRequest.DoesNotExist:
return DOES_NOT_EXIST
if parent_request is None:
return DOES_NOT_EXIST
commit_data = fetch_commit_data(parent_request)
if not is_parent(parent_request, commit_data):
return NOT_PARENT
if not parent_request.is_accessible_by(request.user):
return PERMISSION_DENIED
if COMMITS_KEY not in commit_data.extra_data:
logging.error('Parent review request %s missing COMMIT_KEY' %
parent_request.id)
return NOT_PARENT
result = []
children = json.loads(commit_data.extra_data[COMMITS_KEY])
for child in children:
try:
child_request = ReviewRequest.objects.get(id=child[1])
except ReviewRequest.DoesNotExist:
return DOES_NOT_EXIST
if not child_request.approved:
return AUTOLAND_REVIEW_NOT_APPROVED
reviewers = [
r.user.username for r in gen_latest_reviews(child_request)
if r.ship_it and r.user != child_request.submitter
]
if not reviewers and child_request.approved:
# This review request is approved (the repeated check is
# to ensure this is guaranteed if other parts of the code
# change) but we have an empty list of reviewers. We'll
# assume the author has just approved this themself.
reviewers.append(child_request.submitter.username)
# Detect if the commit has been changed since the last review.
shipit_carryforward = has_shipit_carryforward(child_request)
result.append({
'commit':
child[0],
'id':
child[1],
'reviewers':
reviewers,
'shipit_carryforward':
shipit_carryforward,
'summary':
replace_reviewers(child_request.description, reviewers)
})
return 200, {
'commits': result,
'total_results': len(result),
'links': self.get_links(request=request),
}
