altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 3 Packages Projects Releases Wiki Activity

Initial refactoring of the current reviewer assignment code.

Browse source 
- Legacy-Id: 16961
This commit is contained in:
Sasha Romijn 2019-11-05 16:39:31 +00:00
parent eab14ea1c5
commit e518824a69
6 changed files with 236 additions and 204 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
ietf/doc/tests_review.py
View file

@ -325,7 +325,7 @@ class ReviewTests(TestCase):
# assign
empty_outbox()
rotation_list = policy_for_team(review_req.team).reviewer_rotation_list(review_req.team)
rotation_list = policy_for_team(review_req.team).default_reviewer_rotation_list()
reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team, person=rotation_list[0]).first()
r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk })
self.assertEqual(r.status_code, 302)

4
ietf/group/tests_review.py
View file

@ -656,7 +656,7 @@ class BulkAssignmentTests(TestCase):
docs = [DocumentFactory.create(type_id='draft',group=None) for i in range(4)]
requests = [ReviewRequestFactory(team=group,doc=docs[i]) for i in range(4)]
policy = policy_for_team(group)
rot_list = policy.reviewer_rotation_list(group)
rot_list = policy.default_reviewer_rotation_list()
expected_ending_head_of_rotation = rot_list[3]
@ -677,6 +677,6 @@ class BulkAssignmentTests(TestCase):
self.client.login(username=secretary.person.user.username,password=secretary.person.user.username+'+password')
r = self.client.post(unassigned_url, postdict)
self.assertEqual(r.status_code,302)
self.assertEqual(expected_ending_head_of_rotation, policy.reviewer_rotation_list(group)[0])
self.assertEqual(expected_ending_head_of_rotation, policy.default_reviewer_rotation_list()[0])
self.assertMailboxContains(outbox, subject='Last Call assignment', text='Requested by', count=4)

8
ietf/group/views.py
View file

@ -1391,7 +1391,7 @@ def reviewer_overview(request, acronym, group_type=None):
can_manage = can_manage_review_requests_for_team(request.user, group)
reviewers = policy_for_team(group).reviewer_rotation_list(group)
reviewers = policy_for_team(group).default_reviewer_rotation_list()
reviewer_settings = { s.person_id: s for s in ReviewerSettings.objects.filter(team=group) }
unavailable_periods = defaultdict(list)
@ -1542,13 +1542,13 @@ def manage_review_requests(request, acronym, group_type=None, assignment_status=
# of the rotation queue are processed first so that the queue
# rotates before any more assignments are processed
reviewer_policy = policy_for_team(group)
head_of_rotation = reviewer_policy.reviewer_rotation_list(group)[0]
head_of_rotation = reviewer_policy.default_reviewer_rotation_list()[0]
while head_of_rotation in assignments_by_person:
for review_req in assignments_by_person[head_of_rotation]:
assign_review_request_to_reviewer(request, review_req, review_req.form.cleaned_data["reviewer"],review_req.form.cleaned_data["add_skip"])
reqs_to_assign.remove(review_req)
del assignments_by_person[head_of_rotation]
head_of_rotation = reviewer_policy.reviewer_rotation_list(group)[0]
head_of_rotation = reviewer_policy.default_reviewer_rotation_list()[0]
for review_req in reqs_to_assign:
assign_review_request_to_reviewer(request, review_req, review_req.form.cleaned_data["reviewer"],review_req.form.cleaned_data["add_skip"])
@ -1661,7 +1661,7 @@ def email_open_review_assignments(request, acronym, group_type=None):
partial_msg = render_to_string(template.path, {
"review_assignments": review_assignments,
"rotation_list": policy_for_team(group).reviewer_rotation_list(group)[:10],
"rotation_list": policy_for_team(group).default_reviewer_rotation_list()[:10],
"group" : group,
})

406
ietf/review/policies.py
View file

@ -18,207 +18,240 @@ from ietf.review.utils import (current_unavailable_periods_for_reviewers,
def policy_for_team(team):
return RotateWithSkipReviewerPolicy()
return RotateWithSkipReviewerPolicy(team)
class AbstractReviewerPolicy:
def __init__(self, team):
self.team = team
def default_reviewer_rotation_list(self, skip_unavailable=False, dont_skip=[]):
"""
Return a list of reviewers in the default reviewer rotation for a policy.
TODO: fix return types
"""
raise NotImplementedError
# TODO : Change this field to deal with multiple already assigned reviewers???
def setup_reviewer_field(self, field, review_req):
"""
Fill a choice field with the recommended assignment order of reviewers for a review request.
The field should be an instance similar to
PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)")
"""
field.queryset = field.queryset.filter(role__name="reviewer", role__group=review_req.team)
one_assignment = review_req.reviewassignment_set.first()
if one_assignment:
field.initial = one_assignment.reviewer_id
def _unavailable_reviewers(self, team, dont_skip):
choices = self._recommended_assignment_order(field.queryset, review_req)
if not field.required:
choices = [("", field.empty_label)] + choices
field.choices = choices
def _recommended_assignment_order(self, email_queryset, review_req):
"""
Determine the recommended assignment order for a review request,
choosing from the reviewers in email_queryset, which should be a queryset
returning Email objects.
"""
if review_req.team != self.team:
raise ValueError('Reviewer policy was passed review request belonging to different team.')
resolver = AssignmentOrderResolver(email_queryset, review_req, self.default_reviewer_rotation_list())
return resolver.determine_ranking()
def _unavailable_reviewers(self, dont_skip):
# prune reviewers not in the rotation (but not the assigned
# reviewer who must have been available for assignment anyway)
reviewers_to_skip = set()
unavailable_periods = current_unavailable_periods_for_reviewers(team)
unavailable_periods = current_unavailable_periods_for_reviewers(self.team)
for person_id, periods in unavailable_periods.items():
if periods and person_id not in dont_skip:
reviewers_to_skip.add(person_id)
days_needed_for_reviewers = days_needed_to_fulfill_min_interval_for_reviewers(team)
days_needed_for_reviewers = days_needed_to_fulfill_min_interval_for_reviewers(self.team)
for person_id, days_needed in days_needed_for_reviewers.items():
if person_id not in dont_skip:
reviewers_to_skip.add(person_id)
return reviewers_to_skip
class RotateWithSkipReviewerPolicy(AbstractReviewerPolicy):
# TODO : Change this field to deal with multiple already assigned reviewers???
def setup_reviewer_field(self, field, review_req):
field.queryset = field.queryset.filter(role__name="reviewer", role__group=review_req.team)
one_assignment = review_req.reviewassignment_set.first()
if one_assignment:
field.initial = one_assignment.reviewer_id
class AssignmentOrderResolver:
def __init__(self, email_queryset, review_req, rotation_list):
self.review_req = review_req
self.doc = review_req.doc
self.team = review_req.team
self.rotation_list = rotation_list
choices = self._make_assignment_choices(field.queryset, review_req)
if not field.required:
choices = [("", field.empty_label)] + choices
self.possible_emails = list(email_queryset)
self.possible_person_ids = [e.person_id for e in self.possible_emails]
self._collect_context()
field.choices = choices
def _collect_context(self):
# Collect all relevant data about this team, document and review request.
def _make_assignment_choices(self, email_queryset, review_req):
doc = review_req.doc
team = review_req.team
self.doc_aliases = DocAlias.objects.filter(docs=self.doc).values_list("name", flat=True)
possible_emails = list(email_queryset)
possible_person_ids = [e.person_id for e in possible_emails]
# This data is collected as a dict, keys being person IDs, values being numbers/objects.
self.reviewer_settings = self._reviewer_settings_for_person_ids(self.possible_person_ids)
self.days_needed_for_reviewers = days_needed_to_fulfill_min_interval_for_reviewers(self.team)
self.rotation_index = {p.pk: i for i, p in enumerate(self.rotation_list)}
aliases = DocAlias.objects.filter(docs=doc).values_list("name", flat=True)
# This data is collected as a set of person IDs.
self.has_reviewed_previous = self._persons_with_previous_review(self.review_req, self.possible_person_ids)
self.wish_to_review = set(ReviewWish.objects.filter(team=self.team, person__in=self.possible_person_ids,
doc=self.doc).values_list("person", flat=True))
# settings
reviewer_settings = {
r.person_id: r
for r in ReviewerSettings.objects.filter(team=team, person__in=possible_person_ids)
}
for p in possible_person_ids:
if p not in reviewer_settings:
reviewer_settings[p] = ReviewerSettings(team=team,
filter_re=get_default_filter_re(p))
# frequency
days_needed_for_reviewers = days_needed_to_fulfill_min_interval_for_reviewers(team)
# rotation
rotation_index = {p.pk: i for i, p in enumerate(self.reviewer_rotation_list(team))}
# previous review of document
has_reviewed_previous = ReviewRequest.objects.filter(
doc=doc,
reviewassignment__reviewer__person__in=possible_person_ids,
reviewassignment__state="completed",
team=team,
).distinct()
if review_req.pk is not None:
has_reviewed_previous = has_reviewed_previous.exclude(pk=review_req.pk)
has_reviewed_previous = set(
has_reviewed_previous.values_list("reviewassignment__reviewer__person", flat=True))
# review wishes
wish_to_review = set(ReviewWish.objects.filter(team=team, person__in=possible_person_ids,
doc=doc).values_list("person", flat=True))
# connections
connections = {}
# examine the closest connections last to let them override
connections[doc.ad_id] = "is associated Area Director"
for r in Role.objects.filter(group=doc.group_id,
person__in=possible_person_ids).select_related("name"):
connections[r.person_id] = "is group {}".format(r.name)
if doc.shepherd:
connections[doc.shepherd.person_id] = "is shepherd of document"
for author in DocumentAuthor.objects.filter(document=doc,
person__in=possible_person_ids).values_list(
"person", flat=True):
connections[author] = "is author of document"
# unavailable periods
unavailable_periods = current_unavailable_periods_for_reviewers(team)
# reviewers statistics
assignment_data_for_reviewers = latest_review_assignments_for_reviewers(team)
self.connections = self._connections_with_doc(self.doc, self.possible_person_ids)
self.unavailable_periods = current_unavailable_periods_for_reviewers(self.team)
self.assignment_data_for_reviewers = latest_review_assignments_for_reviewers(self.team)
def determine_ranking(self):
ranking = []
for e in possible_emails:
settings = reviewer_settings.get(e.person_id)
# we sort the reviewers by separate axes, listing the most
# important things first
scores = []
explanations = []
def add_boolean_score(direction, expr, explanation=None):
scores.append(direction if expr else -direction)
if expr and explanation:
explanations.append(explanation)
# unavailable for review periods
periods = unavailable_periods.get(e.person_id, [])
unavailable_at_the_moment = periods and not (
e.person_id in has_reviewed_previous and all(
p.availability == "canfinish" for p in periods))
add_boolean_score(-1, unavailable_at_the_moment)
def format_period(p):
if p.end_date:
res = "unavailable until {}".format(p.end_date.isoformat())
else:
res = "unavailable indefinitely"
return "{} ({})".format(res, p.get_availability_display())
if periods:
explanations.append(", ".join(format_period(p) for p in periods))
# misc
add_boolean_score(+1, e.person_id in has_reviewed_previous, "reviewed document before")
add_boolean_score(+1, e.person_id in wish_to_review, "wishes to review document")
add_boolean_score(-1, e.person_id in connections,
connections.get(e.person_id)) # reviewer is somehow connected: bad
add_boolean_score(-1, settings.filter_re and any(
re.search(settings.filter_re, n) for n in aliases), "filter regexp matches")
# minimum interval between reviews
days_needed = days_needed_for_reviewers.get(e.person_id, 0)
scores.append(-days_needed)
if days_needed > 0:
explanations.append("max frequency exceeded, ready in {} {}".format(days_needed,
"day" if days_needed == 1 else "days"))
# skip next
scores.append(-settings.skip_next)
if settings.skip_next > 0:
explanations.append("skip next {}".format(settings.skip_next))
# index
index = rotation_index.get(e.person_id, 0)
scores.append(-index)
explanations.append("#{}".format(index + 1))
# stats
stats = []
assignment_data = assignment_data_for_reviewers.get(e.person_id, [])
currently_open = sum(1 for d in assignment_data if d.state in ["assigned", "accepted"])
pages = sum(
rd.doc_pages for rd in assignment_data if rd.state in ["assigned", "accepted"])
if currently_open > 0:
stats.append("currently {count} open, {pages} pages".format(count=currently_open,
pages=pages))
could_have_completed = [d for d in assignment_data if
d.state in ["part-completed", "completed", "no-response"]]
if could_have_completed:
no_response = len([d for d in assignment_data if d.state == 'no-response'])
if no_response:
stats.append("%s no response" % no_response)
part_completed = len([d for d in assignment_data if d.state == 'part-completed'])
if part_completed:
stats.append("%s partially complete" % part_completed)
completed = len([d for d in assignment_data if d.state == 'completed'])
if completed:
stats.append("%s fully completed" % completed)
if stats:
explanations.append(", ".join(stats))
label = six.text_type(e.person)
if explanations:
label = "{}: {}".format(label, "; ".join(explanations))
ranking.append({
"email": e,
"scores": scores,
"label": label,
})
for e in self.possible_emails:
ranking.append(self._ranking_for_email(e))
ranking.sort(key=lambda r: r["scores"], reverse=True)
return [(r["email"].pk, r["label"]) for r in ranking]
def possibly_advance_next_reviewer_for_team(self, team, assigned_review_to_person_id, add_skip=False):
assert assigned_review_to_person_id is not None
def _ranking_for_email(self, email):
settings = self.reviewer_settings.get(email.person_id)
scores = []
explanations = []
rotation_list = self.reviewer_rotation_list(team, skip_unavailable=True,
dont_skip=[assigned_review_to_person_id])
def add_boolean_score(direction, expr, explanation=None):
scores.append(direction if expr else -direction)
if expr and explanation:
explanations.append(explanation)
# unavailable for review periods
periods = self.unavailable_periods.get(email.person_id, [])
unavailable_at_the_moment = periods and not (
email.person_id in self.has_reviewed_previous and all(
p.availability == "canfinish" for p in periods))
add_boolean_score(-1, unavailable_at_the_moment)
def format_period(p):
if p.end_date:
res = "unavailable until {}".format(p.end_date.isoformat())
else:
res = "unavailable indefinitely"
return "{} ({})".format(res, p.get_availability_display())
if periods:
explanations.append(", ".join(format_period(p) for p in periods))
# misc
add_boolean_score(+1, email.person_id in self.has_reviewed_previous, "reviewed document before")
add_boolean_score(+1, email.person_id in self.wish_to_review, "wishes to review document")
add_boolean_score(-1, email.person_id in self.connections,
self.connections.get(email.person_id)) # reviewer is somehow connected: bad
add_boolean_score(-1, settings.filter_re and any(
re.search(settings.filter_re, n) for n in self.doc_aliases), "filter regexp matches")
# minimum interval between reviews
days_needed = self.days_needed_for_reviewers.get(email.person_id, 0)
scores.append(-days_needed)
if days_needed > 0:
explanations.append("max frequency exceeded, ready in {} {}".format(days_needed,
"day" if days_needed == 1 else "days"))
# skip next
scores.append(-settings.skip_next)
if settings.skip_next > 0:
explanations.append("skip next {}".format(settings.skip_next))
# index
index = self.rotation_index.get(email.person_id, 0)
scores.append(-index)
explanations.append("#{}".format(index + 1))
# stats (for information, do not affect score)
stats = self._collect_reviewer_stats(email)
if stats:
explanations.append(", ".join(stats))
label = six.text_type(email.person)
if explanations:
label = "{}: {}".format(label, "; ".join(explanations))
return {
"email": email,
"scores": scores,
"label": label,
}
def _collect_reviewer_stats(self, email):
stats = []
assignment_data = self.assignment_data_for_reviewers.get(email.person_id, [])
currently_open = sum(1 for d in assignment_data if d.state in ["assigned", "accepted"])
pages = sum(
rd.doc_pages for rd in assignment_data if rd.state in ["assigned", "accepted"])
if currently_open > 0:
stats.append("currently {count} open, {pages} pages".format(count=currently_open,
pages=pages))
could_have_completed = [d for d in assignment_data if
d.state in ["part-completed", "completed", "no-response"]]
if could_have_completed:
no_response = len([d for d in assignment_data if d.state == 'no-response'])
if no_response:
stats.append("%s no response" % no_response)
part_completed = len([d for d in assignment_data if d.state == 'part-completed'])
if part_completed:
stats.append("%s partially complete" % part_completed)
completed = len([d for d in assignment_data if d.state == 'completed'])
if completed:
stats.append("%s fully completed" % completed)
return stats
def _connections_with_doc(self, doc, person_ids):
connections = {}
# examine the closest connections last to let them override
connections[doc.ad_id] = "is associated Area Director"
for r in Role.objects.filter(group=doc.group_id,
person__in=person_ids).select_related("name"):
connections[r.person_id] = "is group {}".format(r.name)
if doc.shepherd:
connections[doc.shepherd.person_id] = "is shepherd of document"
for author in DocumentAuthor.objects.filter(document=doc,
person__in=person_ids).values_list(
"person", flat=True):
connections[author] = "is author of document"
return connections
def _persons_with_previous_review(self, review_req, possible_person_ids):
has_reviewed_previous = ReviewRequest.objects.filter(
doc=review_req.doc,
reviewassignment__reviewer__person__in=possible_person_ids,
reviewassignment__state="completed",
team=self.team,
).distinct()
if review_req.pk is not None:
has_reviewed_previous = has_reviewed_previous.exclude(pk=review_req.pk)
has_reviewed_previous = set(
has_reviewed_previous.values_list("reviewassignment__reviewer__person", flat=True))
return has_reviewed_previous
def _reviewer_settings_for_person_ids(self, person_ids):
reviewer_settings = {
r.person_id: r
for r in ReviewerSettings.objects.filter(team=self.team, person__in=person_ids)
}
for p in person_ids:
if p not in reviewer_settings:
reviewer_settings[p] = ReviewerSettings(team=self.team,
filter_re=get_default_filter_re(p))
return reviewer_settings
class RotateWithSkipReviewerPolicy(AbstractReviewerPolicy):
def update_policy_state_for_assignment(self, assignee_person_id, add_skip=False):
assert assignee_person_id is not None
rotation_list = self.default_reviewer_rotation_list(skip_unavailable=True,
dont_skip=[assignee_person_id])
def reviewer_at_index(i):
if not rotation_list:
@ -226,46 +259,45 @@ class RotateWithSkipReviewerPolicy(AbstractReviewerPolicy):
return rotation_list[i % len(rotation_list)]
def reviewer_settings_for(person_id):
return (ReviewerSettings.objects.filter(team=team, person=person_id).first()
or ReviewerSettings(team=team, person_id=person_id))
current_i = 0
if assigned_review_to_person_id == reviewer_at_index(current_i):
# move 1 ahead
current_i += 1
return (ReviewerSettings.objects.filter(team=self.team, person=person_id).first()
or ReviewerSettings(team=self.team, person_id=person_id))
if add_skip:
settings = reviewer_settings_for(assigned_review_to_person_id)
settings = reviewer_settings_for(assignee_person_id)
settings.skip_next += 1
settings.save()
if not rotation_list:
return
current_idx = 0
if assignee_person_id == reviewer_at_index(current_idx):
# Skip the first reviewer in considering who is next.
current_idx += 1
while True:
# as a clean-up step go through any with a skip next > 0
current_reviewer_person_id = reviewer_at_index(current_i)
current_reviewer_person_id = reviewer_at_index(current_idx)
settings = reviewer_settings_for(current_reviewer_person_id)
if settings.skip_next > 0:
settings.skip_next -= 1
settings.save()
current_i += 1
current_idx += 1
else:
nr = NextReviewerInTeam.objects.filter(team=team).first() or NextReviewerInTeam(
team=team)
nr = NextReviewerInTeam.objects.filter(team=self.team).first() or NextReviewerInTeam(
team=self.team)
nr.next_reviewer_id = current_reviewer_person_id
nr.save()
break
def reviewer_rotation_list(self, team, skip_unavailable=False, dont_skip=[]):
reviewers = list(Person.objects.filter(role__name="reviewer", role__group=team))
def default_reviewer_rotation_list(self, skip_unavailable=False, dont_skip=[]):
reviewers = list(Person.objects.filter(role__name="reviewer", role__group=self.team))
reviewers.sort(key=lambda p: p.last_name())
next_reviewer_index = 0
# now to figure out where the rotation is currently at
saved_reviewer = NextReviewerInTeam.objects.filter(team=team).select_related("next_reviewer").first()
saved_reviewer = NextReviewerInTeam.objects.filter(team=self.team).select_related("next_reviewer").first()
if saved_reviewer:
n = saved_reviewer.next_reviewer
@ -284,7 +316,7 @@ class RotateWithSkipReviewerPolicy(AbstractReviewerPolicy):
reviewers_to_skip = []
if skip_unavailable:
reviewers_to_skip = self._unavailable_reviewers(team, dont_skip)
reviewers_to_skip = self._unavailable_reviewers(dont_skip)
rotation_list = [p.pk for p in rotation_list if p.pk not in reviewers_to_skip]
return rotation_list

18
ietf/review/test_policies.py
View file

@ -26,25 +26,25 @@ class RotateWithSkipReviewerPolicyTests(TestCase):
for i in range(5)
]
self.assertEqual(reviewers, policy.reviewer_rotation_list(team))
self.assertEqual(reviewers, policy.default_reviewer_rotation_list())
def get_skip_next(person):
settings = (ReviewerSettings.objects.filter(team=team, person=person).first()
or ReviewerSettings(team=team))
return settings.skip_next
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[0].pk, add_skip=False)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[0].pk, add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[1])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[1].pk, add_skip=True)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[1].pk, add_skip=True)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[1]), 1)
self.assertEqual(get_skip_next(reviewers[2]), 0)
# skip reviewer 2
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[3].pk, add_skip=True)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[3].pk, add_skip=True)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1)
@ -52,7 +52,7 @@ class RotateWithSkipReviewerPolicyTests(TestCase):
self.assertEqual(get_skip_next(reviewers[3]), 1)
# pick reviewer 2, use up reviewer 3's skip_next
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[2].pk, add_skip=False)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[2].pk, add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1)
@ -61,7 +61,7 @@ class RotateWithSkipReviewerPolicyTests(TestCase):
self.assertEqual(get_skip_next(reviewers[4]), 0)
# check wrap-around
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[4].pk)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[4].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[0])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1)
@ -72,7 +72,7 @@ class RotateWithSkipReviewerPolicyTests(TestCase):
# unavailable
today = datetime.date.today()
UnavailablePeriod.objects.create(team=team, person=reviewers[1], start_date=today, end_date=today, availability="unavailable")
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[0].pk)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[0].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1) # don't consume that skip while the reviewer is unavailable
@ -81,7 +81,7 @@ class RotateWithSkipReviewerPolicyTests(TestCase):
self.assertEqual(get_skip_next(reviewers[4]), 0)
# pick unavailable anyway
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[1].pk, add_skip=False)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[1].pk, add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1)
@ -95,7 +95,7 @@ class RotateWithSkipReviewerPolicyTests(TestCase):
settings.save()
req = ReviewRequest.objects.create(team=team, doc=doc, type_id="early", state_id="assigned", deadline=today, requested_by=reviewers[0])
ReviewAssignmentFactory(review_request=req, state_id="accepted", reviewer = reviewers[2].email_set.first(),assigned_on = req.time)
policy.possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[3].pk)
policy.update_policy_state_for_assignment(assignee_person_id=reviewers[3].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1)

2
ietf/review/utils.py
View file

@ -381,7 +381,7 @@ def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=Fa
assignment = review_req.reviewassignment_set.create(state_id='assigned', reviewer = reviewer, assigned_on = datetime.datetime.now())
from ietf.review.policies import policy_for_team
policy_for_team(review_req.team).possibly_advance_next_reviewer_for_team(review_req.team, reviewer.person_id, add_skip)
policy_for_team(review_req.team).update_policy_state_for_assignment(reviewer.person_id, add_skip)
ReviewRequestDocEvent.objects.create(
type="assigned_review_request",