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

Add LeastRecentlyUsed reviewer queue policy.

Browse source 
- Legacy-Id: 17049
This commit is contained in:
Sasha Romijn 2019-11-18 14:47:45 +00:00
parent 554a839864
commit 57ec2b3ef8
2 changed files with 145 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

126
ietf/review/policies.py
View file

@ -3,6 +3,7 @@
from __future__ import absolute_import, print_function, unicode_literals
import re
from collections import OrderedDict
import six
@ -11,7 +12,8 @@ from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs
from ietf.group.models import Role
from ietf.person.models import Person
import debug # pyflakes:ignore
from ietf.review.models import NextReviewerInTeam, ReviewerSettings, ReviewWish, ReviewRequest
from ietf.review.models import NextReviewerInTeam, ReviewerSettings, ReviewWish, ReviewRequest, \
ReviewAssignment
from ietf.review.utils import (current_unavailable_periods_for_reviewers,
days_needed_to_fulfill_min_interval_for_reviewers,
get_default_filter_re,
@ -41,12 +43,58 @@ class AbstractReviewerQueuePolicy:
def update_policy_state_for_assignment(self, assignee_person, add_skip=False):
"""
Update the internal state of a policy to reflect an assignment.
Update the skip_count if the assignment was in order, and
update NextReviewerInTeam. Note that NextReviewerInTeam is
not used by all policies.
"""
settings = self._reviewer_settings_for(assignee_person)
settings.request_assignment_next = False
settings.save()
rotation_list = self.default_reviewer_rotation_list(
dont_skip_person_ids=[assignee_person.pk])
def reviewer_at_index(i):
if not rotation_list:
return None
return rotation_list[i % len(rotation_list)]
if not rotation_list:
return
rotation_list_without_skip = [r for r in rotation_list if
not self._reviewer_settings_for(r).skip_next]
# In order means: assigned to the first person in the rotation list with skip_next=0
# If the assignment is not in order, skip_next and NextReviewerInTeam are not modified.
in_order_assignment = rotation_list_without_skip[0] == assignee_person
# Loop through the list until finding the first person with skip_next=0,
# who is not the current assignee. Anyone with skip_next>0 encountered before
# has their skip_next decreased.
current_idx = 0
if in_order_assignment:
while True:
current_idx_person = reviewer_at_index(current_idx)
settings = self._reviewer_settings_for(current_idx_person)
if settings.skip_next > 0:
settings.skip_next -= 1
settings.save()
elif current_idx_person != assignee_person:
# NextReviewerInTeam is not used by all policies to determine
# default rotation order, but updated regardless.
nr = NextReviewerInTeam.objects.filter(
team=self.team).first() or NextReviewerInTeam(
team=self.team)
nr.next_reviewer = current_idx_person
nr.save()
break
current_idx += 1
if add_skip:
settings = self._reviewer_settings_for(assignee_person)
settings.skip_next += 1
settings.save()
# TODO : Change this field to deal with multiple already assigned reviewers???
def setup_reviewer_field(self, field, review_req):
"""
@ -298,50 +346,11 @@ class AssignmentOrderResolver:
class RotateAlphabeticallyReviewerQueuePolicy(AbstractReviewerQueuePolicy):
def update_policy_state_for_assignment(self, assignee_person, add_skip=False):
super(RotateAlphabeticallyReviewerQueuePolicy, self).update_policy_state_for_assignment(assignee_person, add_skip)
assert assignee_person is not None
rotation_list = self.default_reviewer_rotation_list(dont_skip_person_ids=[assignee_person.pk])
def reviewer_at_index(i):
if not rotation_list:
return None
return rotation_list[i % len(rotation_list)]
if not rotation_list:
return
rotation_list_without_skip = [r for r in rotation_list if not self._reviewer_settings_for(r).skip_next]
# In order means: assigned to the first person in the rotation list with skip_next=0
# If the assignment is not in order, skip_next and NextReviewerInTeam are not modified.
in_order_assignment = rotation_list_without_skip[0] == assignee_person
# Loop through the list until finding the first person with skip_next=0,
# who is not the current assignee. Anyone with skip_next>0 encountered before
# has their skip_next decreased.
current_idx = 0
if in_order_assignment:
while True:
current_idx_person = reviewer_at_index(current_idx)
settings = self._reviewer_settings_for(current_idx_person)
if settings.skip_next > 0:
settings.skip_next -= 1
settings.save()
elif current_idx_person != assignee_person:
nr = NextReviewerInTeam.objects.filter(team=self.team).first() or NextReviewerInTeam(
team=self.team)
nr.next_reviewer = current_idx_person
nr.save()
break
current_idx += 1
if add_skip:
settings = self._reviewer_settings_for(assignee_person)
settings.skip_next += 1
settings.save()
"""
A policy in which the default rotation list is based on last name, alphabetically.
NextReviewerInTeam is used to store a pointer to where the queue is currently
positioned.
"""
def default_reviewer_rotation_list(self, include_unavailable=False, dont_skip_person_ids=None):
reviewers = list(Person.objects.filter(role__name="reviewer", role__group=self.team))
@ -372,3 +381,28 @@ class RotateAlphabeticallyReviewerQueuePolicy(AbstractReviewerQueuePolicy):
return rotation_list
class LeastRecentlyUsedReviewerQueuePolicy(AbstractReviewerQueuePolicy):
"""
A policy where the default rotation list is based on the most recent
assigned, accepted or completed review assignment.
"""
def default_reviewer_rotation_list(self, include_unavailable=False, dont_skip_person_ids=None):
reviewers = list(Person.objects.filter(role__name="reviewer", role__group=self.team))
assignments = ReviewAssignment.objects.filter(
review_request__team=self.team,
state__in=['accepted', 'assigned', 'completed'],
).order_by('assigned_on')
reviewers_with_assignment = [assignment.reviewer.person for assignment in assignments]
reviewers_without_assignment = set(reviewers) - set(reviewers_with_assignment)
rotation_list = sorted(list(reviewers_without_assignment), key=lambda r: r.pk)
rotation_list += list(OrderedDict.fromkeys(reviewers_with_assignment))
if not include_unavailable:
reviewers_to_skip = self._entirely_unavailable_reviewers(dont_skip_person_ids)
rotation_list = [p for p in rotation_list if p.pk not in reviewers_to_skip]
return rotation_list

67
ietf/review/test_policies.py
View file

@ -7,15 +7,20 @@ from ietf.person.fields import PersonEmailChoiceField
from ietf.person.models import Email
from ietf.review.factories import ReviewAssignmentFactory, ReviewRequestFactory
from ietf.review.models import ReviewerSettings, NextReviewerInTeam, UnavailablePeriod, ReviewWish
from ietf.review.policies import get_reviewer_queue_policy, AssignmentOrderResolver
from ietf.review.policies import get_reviewer_queue_policy, AssignmentOrderResolver, \
LeastRecentlyUsedReviewerQueuePolicy, RotateAlphabeticallyReviewerQueuePolicy
from ietf.utils.test_data import create_person
from ietf.utils.test_utils import TestCase
class RotateAlphabeticallyReviewerQueuePolicyTest(TestCase):
"""
These tests also cover the common behaviour in RotateAlphabeticallyReviewerQueuePolicy,
as that's difficult to test on it's own.
"""
def test_default_reviewer_rotation_list(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = get_reviewer_queue_policy(team)
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
reviewers = [
create_person(team, "reviewer", name="Test Reviewer{}".format(i), username="testreviewer{}".format(i))
@ -171,6 +176,64 @@ class RotateAlphabeticallyReviewerQueuePolicyTest(TestCase):
self.assertEqual(get_skip_next(reviewers[4]), 0)
class LeastRecentlyUsedReviewerQueuePolicyTest(TestCase):
"""
These tests only cover where this policy deviates from
RotateAlphabeticallyReviewerQueuePolicy - the common behaviour
inherited from AbstractReviewerQueuePolicy is covered in
RotateAlphabeticallyReviewerQueuePolicyTest.
"""
def test_default_reviewer_rotation_list(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
list_email="rotationteam@ietf.org",
parent=Group.objects.get(acronym="farfut"))
policy = LeastRecentlyUsedReviewerQueuePolicy(team)
reviewers = [
create_person(team, "reviewer", name="Test Reviewer{}".format(i),
username="testreviewer{}".format(i))
for i in range(5)
]
# This reviewer should never be included.
unavailable_reviewer = create_person(team, "reviewer", name="unavailable reviewer",
username="unavailablereviewer")
UnavailablePeriod.objects.create(
team=team,
person=unavailable_reviewer,
start_date='2000-01-01',
availability='unavailable',
)
# This should not have any impact. Canfinish unavailable reviewers are included in
# the default rotation, and filtered further when making assignment choices.
UnavailablePeriod.objects.create(
team=team,
person=reviewers[1],
start_date='2000-01-01',
availability='canfinish',
)
# No known assignments
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers)
# Regular accepted assignment
ReviewAssignmentFactory(reviewer=reviewers[1].email(), assigned_on='2019-01-01',
state_id='accepted', review_request__team=team)
# Rejected assignment, should not affect reviewer 2's position
ReviewAssignmentFactory(reviewer=reviewers[2].email(), state_id='rejected',
review_request__team=team)
# Completed assignment, assigned before reviewer 1,
# but completed after (assign date should count).
ReviewAssignmentFactory(reviewer=reviewers[0].email(), assigned_on='2018-01-01',
completed_on='2020-01-01', state_id='completed',
review_request__team=team)
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, [reviewers[2], reviewers[3], reviewers[4], reviewers[0], reviewers[1]])
class AssignmentOrderResolverTests(TestCase):
def test_determine_ranking(self):
# reviewer_high is second in the default rotation, reviewer_low is first