From e518824a69470a4cbb4e2c1b4865fdfe5b700d1a Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Tue, 5 Nov 2019 16:39:31 +0000 Subject: [PATCH] Initial refactoring of the current reviewer assignment code. - Legacy-Id: 16961 --- ietf/doc/tests_review.py | 2 +- ietf/group/tests_review.py | 4 +- ietf/group/views.py | 8 +- ietf/review/policies.py | 406 +++++++++++++++++++---------------- ietf/review/test_policies.py | 18 +- ietf/review/utils.py | 2 +- 6 files changed, 236 insertions(+), 204 deletions(-) diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index 37af398ae..e6d49f9e7 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -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) diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index 443f0de19..56a05fa5e 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -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) diff --git a/ietf/group/views.py b/ietf/group/views.py index 03e5b1cab..83d86a446 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -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, }) diff --git a/ietf/review/policies.py b/ietf/review/policies.py index 86168e688..6c698d600 100644 --- a/ietf/review/policies.py +++ b/ietf/review/policies.py @@ -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 diff --git a/ietf/review/test_policies.py b/ietf/review/test_policies.py index 8f6450957..8624e6769 100644 --- a/ietf/review/test_policies.py +++ b/ietf/review/test_policies.py @@ -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) diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 6d7d919e0..7f7aef568 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -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",