Cleanup. Branch ready for merge. (see email)
- Legacy-Id: 17059
This commit is contained in:
parent
0c0980c641
commit
b1eb2643f0
|
@ -184,20 +184,19 @@ class AssignmentOrderResolver:
|
|||
self.doc_aliases = DocAlias.objects.filter(docs=self.doc).values_list("name", flat=True)
|
||||
|
||||
# This data is collected as a dict, keys being person IDs, values being numbers/objects.
|
||||
self.rotation_index = {p.pk: i for i, p in enumerate(self.rotation_list)}
|
||||
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)}
|
||||
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)
|
||||
self.unavailable_periods = current_unavailable_periods_for_reviewers(self.team)
|
||||
|
||||
# This data is collected as a set of person IDs.
|
||||
self.has_completed_review_previous = self._persons_with_previous_review(self.review_req, self.possible_person_ids, 'completed')
|
||||
self.has_rejected_review_previous = self._persons_with_previous_review(self.review_req, self.possible_person_ids, 'rejected')
|
||||
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))
|
||||
|
||||
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)
|
||||
self.unavailable_periods = current_unavailable_periods_for_reviewers(self.team)
|
||||
|
||||
def determine_ranking(self):
|
||||
"""
|
||||
|
@ -232,7 +231,7 @@ class AssignmentOrderResolver:
|
|||
if email.person_id not in self.rotation_index:
|
||||
return
|
||||
|
||||
# If a reviewer is unavailable, they are ignored.
|
||||
# If a reviewer is unavailable at the moment, they are ignored.
|
||||
periods = self.unavailable_periods.get(email.person_id, [])
|
||||
unavailable_at_the_moment = periods and not (
|
||||
email.person_id in self.has_completed_review_previous and
|
||||
|
@ -265,12 +264,12 @@ class AssignmentOrderResolver:
|
|||
if days_needed > 0:
|
||||
explanations.append("max frequency exceeded, ready in {} {}".format(days_needed,
|
||||
"day" if days_needed == 1 else "days"))
|
||||
# skip next
|
||||
# skip next value
|
||||
scores.append(-settings.skip_next)
|
||||
if settings.skip_next > 0:
|
||||
explanations.append("skip next {}".format(settings.skip_next))
|
||||
|
||||
# index
|
||||
# index in the default rotation order
|
||||
index = self.rotation_index.get(email.person_id, 0)
|
||||
scores.append(-index)
|
||||
explanations.append("#{}".format(index + 1))
|
||||
|
@ -375,21 +374,21 @@ class RotateAlphabeticallyReviewerQueuePolicy(AbstractReviewerQueuePolicy):
|
|||
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=self.team).select_related("next_reviewer").first()
|
||||
if saved_reviewer:
|
||||
n = saved_reviewer.next_reviewer
|
||||
next_reviewer_in_team = NextReviewerInTeam.objects.filter(team=self.team).select_related("next_reviewer").first()
|
||||
if next_reviewer_in_team:
|
||||
next_reviewer = next_reviewer_in_team.next_reviewer
|
||||
|
||||
if n not in reviewers:
|
||||
# saved reviewer might not still be here, if not just
|
||||
# insert and use that position (Python will wrap around,
|
||||
if next_reviewer not in reviewers:
|
||||
# If the next reviewer is no longer on the team,
|
||||
# advance to the person that would be after them in
|
||||
# the rotation. (Python will wrap around,
|
||||
# so no harm done by using the index on the original list
|
||||
# afterwards)
|
||||
reviewers_with_next = reviewers[:] + [n]
|
||||
reviewers_with_next = reviewers[:] + [next_reviewer]
|
||||
reviewers_with_next.sort(key=lambda p: p.last_name())
|
||||
next_reviewer_index = reviewers_with_next.index(n)
|
||||
next_reviewer_index = reviewers_with_next.index(next_reviewer)
|
||||
else:
|
||||
next_reviewer_index = reviewers.index(n)
|
||||
next_reviewer_index = reviewers.index(next_reviewer)
|
||||
|
||||
rotation_list = reviewers[next_reviewer_index:] + reviewers[:next_reviewer_index]
|
||||
|
||||
|
@ -418,7 +417,7 @@ class LeastRecentlyUsedReviewerQueuePolicy(AbstractReviewerQueuePolicy):
|
|||
assignments = ReviewAssignment.objects.filter(
|
||||
review_request__team=self.team,
|
||||
state__in=['accepted', 'assigned', 'completed'],
|
||||
).order_by('assigned_on')
|
||||
).order_by('assigned_on').select_related('reviewer')
|
||||
|
||||
reviewers_with_assignment = [assignment.reviewer.person for assignment in assignments]
|
||||
reviewers_without_assignment = set(reviewers) - set(reviewers_with_assignment)
|
||||
|
@ -433,9 +432,9 @@ class LeastRecentlyUsedReviewerQueuePolicy(AbstractReviewerQueuePolicy):
|
|||
return rotation_list
|
||||
|
||||
def return_reviewer_to_top_rotation(self, reviewer_person):
|
||||
# Reviewer rotation for this policy ignored rejected/withdrawn
|
||||
# Reviewer rotation for this policy ignores rejected/withdrawn
|
||||
# reviews, so it automatically adjusts the position of someone
|
||||
# who rejected a review and no action is needed.
|
||||
# who rejected a review and no further action is needed.
|
||||
pass
|
||||
|
||||
|
||||
|
|
|
@ -281,7 +281,7 @@ class AssignmentOrderResolverTests(TestCase):
|
|||
reviewer_high = create_person(team, "reviewer", name="Test Reviewer-high", username="testreviewerhigh")
|
||||
reviewer_low = create_person(team, "reviewer", name="Test Reviewer-low", username="testreviewerlow")
|
||||
reviewer_unavailable = create_person(team, "reviewer", name="Test Reviewer-unavailable", username="testreviewerunavailable")
|
||||
# This reviewer should be ignored because it is not in the rotation list.
|
||||
# This reviewer should be entirely ignored because it is not in the rotation list.
|
||||
create_person(team, "reviewer", name="Test Reviewer-out-of-rotation", username="testreviewer-out-of-rotation")
|
||||
|
||||
# Create a document with ancestors, that also triggers author check, AD check and group check
|
||||
|
@ -291,9 +291,6 @@ class AssignmentOrderResolverTests(TestCase):
|
|||
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low], ad=reviewer_low, shepherd=reviewer_low.email(), relations=[('replaces', doc_middle_wg)])
|
||||
Role.objects.create(group=doc.group, person=reviewer_low, email=reviewer_low.email(), name_id='advisor')
|
||||
|
||||
review_req = ReviewRequestFactory(doc=doc, team=team, type_id='early')
|
||||
rotation_list = [reviewer_low, reviewer_high, reviewer_unavailable]
|
||||
|
||||
# Trigger previous review check (including finding ancestor documents) and completed review stats.
|
||||
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc_individual, reviewer=reviewer_high.email(), state_id='completed')
|
||||
# Trigger other review stats
|
||||
|
@ -338,11 +335,15 @@ class AssignmentOrderResolverTests(TestCase):
|
|||
request_assignment_next=True,
|
||||
)
|
||||
|
||||
review_req = ReviewRequestFactory(doc=doc, team=team, type_id='early')
|
||||
rotation_list = [reviewer_low, reviewer_high, reviewer_unavailable]
|
||||
|
||||
order = AssignmentOrderResolver(Email.objects.all(), review_req, rotation_list)
|
||||
ranking = order.determine_ranking()
|
||||
self.assertEqual(len(ranking), 2)
|
||||
self.assertEqual(ranking[0]['email'], reviewer_high.email())
|
||||
self.assertEqual(ranking[1]['email'], reviewer_low.email())
|
||||
# These scores follow the ordering of https://trac.tools.ietf.org/tools/ietfdb/wiki/ReviewerQueuePolicy,
|
||||
self.assertEqual(ranking[0]['scores'], [ 1, 1, 1, 1, 1, 1, 0, 0, -1])
|
||||
self.assertEqual(ranking[1]['scores'], [-1, -1, -1, -1, -1, -1, -91, -2, 0])
|
||||
self.assertEqual(ranking[0]['label'], 'Test Reviewer-high: unavailable indefinitely (Can do follow-ups); requested to be selected next for assignment; reviewed document before; wishes to review document; #2; 1 no response, 1 partially complete, 1 fully completed')
|
||||
|
|
Loading…
Reference in a new issue