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

Merged in work from sasha@dashcare.nl on Review Queue Managemnt:

Browse source 
This abstracts queue management, making it possible to implement different
policies for each team. It provides two concrete policies:
RotateAlphabeticallyReviewerQueuePolicy, which rotates an alphabetically
ordered reviewer list with consideration for skip indications, and is the
default policy; and LeastRecentlyUsedReviewerQueuePolicy, a simple
least-recently-used policy.  Also see issues #2721 and #2656.
 - Legacy-Id: 17121
This commit is contained in:
Henrik Levkowetz 2019-12-04 23:02:52 +00:00
parent ac6b664fa5 9e2dfbfe7f
commit fcb6806d17
18 changed files with 996 additions and 372 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

107
ietf/doc/tests_review.py
View file

@ -33,11 +33,10 @@ from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAss
from ietf.person.models import Email, Person
from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory
from ietf.review.models import (ReviewRequest, ReviewerSettings,
ReviewWish, UnavailablePeriod, NextReviewerInTeam)
from ietf.review.utils import reviewer_rotation_list, possibly_advance_next_reviewer_for_team
ReviewWish, NextReviewerInTeam)
from ietf.review.policies import get_reviewer_queue_policy
from ietf.utils.test_utils import TestCase
from ietf.utils.test_data import create_person
from ietf.utils.test_utils import login_testing_unauthorized, reload_db_objects
from ietf.utils.mail import outbox, empty_outbox, parseaddr, on_behalf_of
from ietf.person.factories import PersonFactory
@ -235,94 +234,6 @@ class ReviewTests(TestCase):
self.assertIn("closed", mail_content)
self.assertIn("review_request_close_comment", mail_content)
def test_possibly_advance_next_reviewer_for_team(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
doc = WgDraftFactory()
# make a bunch of reviewers
reviewers = [
create_person(team, "reviewer", name="Test Reviewer{}".format(i), username="testreviewer{}".format(i))
for i in range(5)
]
self.assertEqual(reviewers, reviewer_rotation_list(team))
def get_skip_next(person):
settings = (ReviewerSettings.objects.filter(team=team, person=person).first()
or ReviewerSettings(team=team))
return settings.skip_next
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1)
# pick reviewer 2, use up reviewer 3's skip_next
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# check wrap-around
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# unavailable
today = datetime.date.today()
UnavailablePeriod.objects.create(team=team, person=reviewers[1], start_date=today, end_date=today, availability="unavailable")
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# pick unavailable anyway
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# not through min_interval so advance past reviewer[2]
settings, _ = ReviewerSettings.objects.get_or_create(team=team, person=reviewers[2])
settings.min_interval = 30
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)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_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)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
def test_assign_reviewer(self):
doc = WgDraftFactory(pages=2)
review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
@ -330,6 +241,7 @@ class ReviewTests(TestCase):
RoleFactory(group=review_team,person__user__username='marschairman',person__name='WG Cháir Man',name_id='reviewer')
secretary = RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com',name_id='secr')
ReviewerSettings.objects.create(team=review_team, person=rev_role.person, min_interval=14, skip_next=0)
queue_policy = get_reviewer_queue_policy(review_team)
# review to assign to
review_req = ReviewRequestFactory(team=review_team,doc=doc,state_id='requested')
@ -371,17 +283,8 @@ class ReviewTests(TestCase):
another_reviewer = PersonFactory.create(name = "Extra TestReviewer") # needs to be lexically greater than the existing one
another_reviewer.role_set.create(name_id='reviewer', email=another_reviewer.email(), group=review_req.team)
UnavailablePeriod.objects.create(
team=review_req.team,
person=reviewer_email.person,
start_date=datetime.date.today() - datetime.timedelta(days=10),
availability="unavailable",
)
ReviewWish.objects.create(person=reviewer_email.person, team=review_req.team, doc=doc)
# pick a non-existing reviewer as next to see that we can
# handle reviewers who have left
NextReviewerInTeam.objects.filter(team=review_req.team).delete()
NextReviewerInTeam.objects.create(
team=review_req.team,
@ -409,14 +312,13 @@ class ReviewTests(TestCase):
self.assertIn("wishes to review", reviewer_label)
self.assertIn("is author", reviewer_label)
self.assertIn("regexp matches", reviewer_label)
self.assertIn("unavailable indefinitely", reviewer_label)
self.assertIn("skip next 1", reviewer_label)
self.assertIn("#1", reviewer_label)
self.assertIn("1 fully completed", reviewer_label)
# assign
empty_outbox()
rotation_list = reviewer_rotation_list(review_req.team)
rotation_list = queue_policy.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)
@ -427,7 +329,6 @@ class ReviewTests(TestCase):
assignment = review_req.reviewassignment_set.first()
self.assertEqual(assignment.reviewer, reviewer)
self.assertEqual(assignment.state_id, "assigned")
self.assertEqual(NextReviewerInTeam.objects.get(team=review_req.team).next_reviewer, rotation_list[1])
self.assertEqual(len(outbox), 1)
self.assertEqual('"Some Reviewer" <reviewer@example.com>', outbox[0]["To"])

11
ietf/doc/views_review.py
View file

@ -38,11 +38,12 @@ from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has
from ietf.message.models import Message
from ietf.message.utils import infer_message
from ietf.person.fields import PersonEmailChoiceField, SearchablePersonField
from ietf.review.policies import get_reviewer_queue_policy
from ietf.review.utils import (active_review_teams, assign_review_request_to_reviewer,
can_request_review_of_doc, can_manage_review_requests_for_team,
email_review_assignment_change, email_review_request_change,
close_review_request_states,
close_review_request, setup_reviewer_field)
close_review_request)
from ietf.review import mailarch
from ietf.utils.fields import DatepickerDateField
from ietf.utils.text import strip_prefix, xslugify
@ -307,7 +308,7 @@ class AssignReviewerForm(forms.Form):
def __init__(self, review_req, *args, **kwargs):
super(AssignReviewerForm, self).__init__(*args, **kwargs)
setup_reviewer_field(self.fields["reviewer"], review_req)
get_reviewer_queue_policy(review_req.team).setup_reviewer_field(self.fields["reviewer"], review_req)
@login_required
@ -378,6 +379,9 @@ def reject_reviewer_assignment(request, name, assignment_id):
state=review_assignment.state,
)
policy = get_reviewer_queue_policy(review_assignment.review_request.team)
policy.return_reviewer_to_rotation_top(review_assignment.reviewer.person)
msg = render_to_string("review/reviewer_assignment_rejected.txt", {
"by": request.user.person,
"message_to_secretary": form.cleaned_data.get("message_to_secretary")
@ -425,6 +429,9 @@ def withdraw_reviewer_assignment(request, name, assignment_id):
state=review_assignment.state,
)
policy = get_reviewer_queue_policy(review_assignment.review_request.team)
policy.return_reviewer_to_rotation_top(review_assignment.reviewer.person)
msg = "Review assignment withdrawn by %s"%request.user.person
email_review_assignment_change(request, review_assignment, "Reviewer assignment withdrawn", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False)

7
ietf/group/forms.py
View file

@ -20,7 +20,8 @@ from ietf.name.models import ReviewTypeName
from ietf.person.fields import SearchableEmailsField, PersonEmailChoiceField
from ietf.person.models import Person
from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings
from ietf.review.utils import close_review_request_states, setup_reviewer_field
from ietf.review.policies import get_reviewer_queue_policy
from ietf.review.utils import close_review_request_states
from ietf.utils.textupload import get_cleaned_text_file_content
from ietf.utils.text import strip_suffix
#from ietf.utils.ordereddict import insert_after_in_ordered_dict
@ -256,7 +257,7 @@ class ManageReviewRequestForm(forms.Form):
self.fields["close"].widget.attrs["class"] = "form-control input-sm"
setup_reviewer_field(self.fields["reviewer"], review_req)
get_reviewer_queue_policy(review_req.team).setup_reviewer_field(self.fields["reviewer"], review_req)
self.fields["reviewer"].widget.attrs["class"] = "form-control input-sm"
if not getattr(review_req, 'in_lc_and_telechat', False):
@ -280,7 +281,7 @@ class ReviewerSettingsForm(forms.ModelForm):
class Meta:
model = ReviewerSettings
fields = ['min_interval', 'filter_re', 'skip_next', 'remind_days_before_deadline',
'remind_days_open_reviews', 'expertise']
'remind_days_open_reviews', 'request_assignment_next', 'expertise']
def __init__(self, *args, **kwargs):
exclude_fields = kwargs.pop('exclude_fields', [])

9
ietf/group/tests_review.py
View file

@ -12,6 +12,7 @@ from pyquery import PyQuery
from django.urls import reverse as urlreverse
from ietf.review.policies import get_reviewer_queue_policy
from ietf.utils.test_utils import login_testing_unauthorized, TestCase, reload_db_objects
from ietf.doc.models import TelechatDocEvent, LastCallDocEvent, State
from ietf.group.models import Role
@ -23,7 +24,6 @@ from ietf.review.utils import (
suggested_review_requests_for_team,
review_assignments_needing_reviewer_reminder, email_reviewer_reminder,
review_assignments_needing_secretary_reminder, email_secretary_reminder,
reviewer_rotation_list,
send_unavaibility_period_ending_reminder, send_reminder_all_open_reviews,
send_review_reminder_overdue_assignment, send_reminder_unconfirmed_assignments)
from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName, \
@ -576,6 +576,7 @@ class ReviewTests(TestCase):
"filter_re": "test-[regexp]",
"remind_days_before_deadline": "6",
"remind_days_open_reviews": "8",
"request_assignment_next": "1",
"expertise": "Some expertise",
})
self.assertEqual(r.status_code, 302)
@ -591,6 +592,7 @@ class ReviewTests(TestCase):
msg_content = outbox[0].get_payload(decode=True).decode("utf-8").lower()
self.assertTrue("frequency changed", msg_content)
self.assertTrue("skip next", msg_content)
self.assertTrue("requested to be the next person", msg_content)
# Normal reviewer should not be able to change skip_next
r = self.client.post(url, {
@ -903,7 +905,8 @@ class BulkAssignmentTests(TestCase):
secretary = RoleFactory.create(group=group,name_id='secr')
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)]
rot_list = reviewer_rotation_list(group)
policy = get_reviewer_queue_policy(group)
rot_list = policy.default_reviewer_rotation_list()
expected_ending_head_of_rotation = rot_list[3]
@ -924,6 +927,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,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)

16
ietf/group/views.py
View file

@ -93,6 +93,7 @@ from ietf.name.models import GroupTypeName, StreamName
from ietf.person.models import Email
from ietf.review.models import (ReviewRequest, ReviewAssignment, ReviewerSettings,
ReviewSecretarySettings, UnavailablePeriod )
from ietf.review.policies import get_reviewer_queue_policy
from ietf.review.utils import (can_manage_review_requests_for_team,
can_access_review_stats_for_team,
@ -104,7 +105,6 @@ from ietf.review.utils import (can_manage_review_requests_for_team,
unavailable_periods_to_list,
current_unavailable_periods_for_reviewers,
email_reviewer_availability_change,
reviewer_rotation_list,
latest_review_assignments_for_reviewers,
augment_review_requests_with_events,
get_default_filter_re,
@ -1395,7 +1395,7 @@ def reviewer_overview(request, acronym, group_type=None):
can_manage = can_manage_review_requests_for_team(request.user, group)
reviewers = reviewer_rotation_list(group)
reviewers = get_reviewer_queue_policy(group).default_reviewer_rotation_list(include_unavailable=True)
reviewer_settings = { s.person_id: s for s in ReviewerSettings.objects.filter(team=group) }
unavailable_periods = defaultdict(list)
@ -1566,13 +1566,14 @@ def manage_review_requests(request, acronym, group_type=None, assignment_status=
# Make sure the any assignments to the person at the head
# of the rotation queue are processed first so that the queue
# rotates before any more assignments are processed
head_of_rotation = reviewer_rotation_list(group)[0]
reviewer_policy = get_reviewer_queue_policy(group)
head_of_rotation = reviewer_policy.default_reviewer_rotation_list_without_skipped()[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_rotation_list(group)[0]
head_of_rotation = reviewer_policy.default_reviewer_rotation_list_without_skipped()[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"])
@ -1685,7 +1686,7 @@ def email_open_review_assignments(request, acronym, group_type=None):
partial_msg = render_to_string(template.path, {
"review_assignments": review_assignments,
"rotation_list": reviewer_rotation_list(group)[:10],
"rotation_list": get_reviewer_queue_policy(group).default_reviewer_rotation_list()[:10],
"group" : group,
})
@ -1756,7 +1757,10 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None):
changes.append("Frequency changed to \"{}\" from \"{}\".".format(settings.get_min_interval_display() or "Not specified", prev_min_interval or "Not specified"))
if settings.skip_next != prev_skip_next:
changes.append("Skip next assignments changed to {} from {}.".format(settings.skip_next, prev_skip_next))
if settings.request_assignment_next:
changes.append("Reviewer has requested to be the next person selected for an "
"assignment, as soon as possible, and will be on the top of "
"the queue.")
if changes:
email_reviewer_availability_change(request, group, reviewer_role, "\n\n".join(changes), request.user.person)

4
ietf/name/admin.py
View file

@ -1,3 +1,4 @@
# Copyright The IETF Trust 2016-2019, All Rights Reserved
from django.contrib import admin
from ietf.name.models import (
@ -9,7 +10,7 @@ from ietf.name.models import (
LiaisonStatementState, LiaisonStatementTagName, MeetingTypeName, NomineePositionStateName,
ReviewRequestStateName, ReviewResultName, ReviewTypeName, RoleName, RoomResourceName,
SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, TopicAudienceName,
DocUrlTagName, ReviewAssignmentStateName)
DocUrlTagName, ReviewAssignmentStateName, ReviewerQueuePolicyName)
from ietf.stats.models import CountryAlias
@ -70,6 +71,7 @@ admin.site.register(NomineePositionStateName, NameAdmin)
admin.site.register(ReviewRequestStateName, NameAdmin)
admin.site.register(ReviewAssignmentStateName, NameAdmin)
admin.site.register(ReviewResultName, NameAdmin)
admin.site.register(ReviewerQueuePolicyName, NameAdmin)
admin.site.register(ReviewTypeName, NameAdmin)
admin.site.register(RoleName, NameAdmin)
admin.site.register(RoomResourceName, NameAdmin)

20
ietf/name/fixtures/names.json
View file

@ -10765,6 +10765,26 @@
"model": "name.reviewassignmentstatename",
"pk": "withdrawn"
},
{
"fields": {
"desc": "",
"name": "Least recently used",
"order": 2,
"used": true
},
"model": "name.reviewerqueuepolicyname",
"pk": "LeastRecentlyUsed"
},
{
"fields": {
"desc": "",
"name": "Rotate alphabetically",
"order": 1,
"used": true
},
"model": "name.reviewerqueuepolicyname",
"pk": "RotateAlphabetically"
},
{
"fields": {
"desc": "",

38
ietf/name/migrations/0008_reviewerqueuepolicyname.py Normal file
View file

@ -0,0 +1,38 @@
# Copyright The IETF Trust 2019, All Rights Reserved
# -*- coding: utf-8 -*-
# Generated by Django 1.11.23 on 2019-11-18 08:35
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
def forward(apps, schema_editor):
ReviewerQueuePolicyName = apps.get_model('name', 'ReviewerQueuePolicyName')
ReviewerQueuePolicyName.objects.create(slug='RotateAlphabetically', name='Rotate alphabetically')
ReviewerQueuePolicyName.objects.create(slug='LeastRecentlyUsed', name='Least recently used')
def reverse(self, apps):
pass
dependencies = [
('name', '0007_fix_m2m_slug_id_length'),
]
operations = [
migrations.CreateModel(
name='ReviewerQueuePolicyName',
fields=[
('slug', models.CharField(max_length=32, primary_key=True, serialize=False)),
('name', models.CharField(max_length=255)),
('desc', models.TextField(blank=True)),
('used', models.BooleanField(default=True)),
('order', models.IntegerField(default=0)),
],
options={
'ordering': ['order', 'name'],
'abstract': False,
},
),
migrations.RunPython(forward, reverse),
]

2
ietf/name/models.py
View file

@ -110,6 +110,8 @@ class ReviewResultName(NameModel):
"""Almost ready, Has issues, Has nits, Not Ready,
On the right track, Ready, Ready with issues,
Ready with nits, Serious Issues"""
class ReviewerQueuePolicyName(NameModel):
"""RotateAlphabetically, LeastRecentlyUsed"""
class TopicAudienceName(NameModel):
"""General, Nominee, Nomcom Member"""
class ContinentName(NameModel):

16
ietf/name/resources.py
View file

@ -1,3 +1,4 @@
# Copyright The IETF Trust 2016-2019, All Rights Reserved
# Autogenerated by the makeresources management command 2015-08-27 11:01 PDT
from ietf.api import ModelResource
from ietf.api import ToOneField # pyflakes:ignore
@ -16,7 +17,7 @@ from ietf.name.models import ( AgendaTypeName, BallotPositionName, ConstraintNam
LiaisonStatementState, LiaisonStatementTagName, MeetingTypeName, NomineePositionStateName,
ReviewAssignmentStateName, ReviewRequestStateName, ReviewResultName, ReviewTypeName,
RoleName, RoomResourceName, SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName,
TopicAudienceName, )
TopicAudienceName, ReviewerQueuePolicyName)
class TimeSlotTypeNameResource(ModelResource):
class Meta:
@ -471,6 +472,19 @@ class ReviewResultNameResource(ModelResource):
}
api.name.register(ReviewResultNameResource())
class ReviewerQueuePolicyNameResource(ModelResource):
class Meta:
cache = SimpleCache()
queryset = ReviewerQueuePolicyName.objects.all()
filtering = {
"slug": ALL,
"name": ALL,
"desc": ALL,
"used": ALL,
"order": ALL,
}
api.name.register(ReviewerQueuePolicyNameResource())
class TopicAudienceNameResource(ModelResource):
class Meta:
cache = SimpleCache()

3
ietf/review/factories.py
View file

@ -1,14 +1,17 @@
# Copyright The IETF Trust 2016-2019, All Rights Reserved
import factory
import datetime
from ietf.review.models import ReviewTeamSettings, ReviewRequest, ReviewAssignment, ReviewerSettings
from ietf.name.models import ReviewTypeName, ReviewResultName
class ReviewTeamSettingsFactory(factory.DjangoModelFactory):
class Meta:
model = ReviewTeamSettings
group = factory.SubFactory('ietf.group.factories.GroupFactory',type_id='review')
reviewer_queue_policy_id = 'RotateAlphabetically'
@factory.post_generation
def review_types(obj, create, extracted, **kwargs):

37
ietf/review/migrations/0021_reviewer_queue_policy_and_request_assignment_next.py Normal file
View file

@ -0,0 +1,37 @@
# Copyright The IETF Trust 2019, All Rights Reserved
# -*- coding: utf-8 -*-
# Generated by Django 1.11.23 on 2019-11-18 08:50
from __future__ import unicode_literals
from django.db import migrations, models
import django.db.models.deletion
class Migration(migrations.Migration):
dependencies = [
('name', '0008_reviewerqueuepolicyname'),
('review', '0020_auto_20191115_2059'),
]
operations = [
migrations.AddField(
model_name='reviewteamsettings',
name='reviewer_queue_policy',
field=models.ForeignKey(default='RotateAlphabetically', on_delete=django.db.models.deletion.PROTECT, to='name.ReviewerQueuePolicyName'),
),
migrations.AddField(
model_name='historicalreviewersettings',
name='request_assignment_next',
field=models.BooleanField(default=False,
help_text='If you would like to be assigned to a review as soon as possible, select this option. It is automatically reset once you receive any assignment.',
verbose_name='Select me next for an assignment'),
),
migrations.AddField(
model_name='reviewersettings',
name='request_assignment_next',
field=models.BooleanField(default=False,
help_text='If you would like to be assigned to a review as soon as possible, select this option. It is automatically reset once you receive any assignment.',
verbose_name='Select me next for an assignment'),
),
]

5
ietf/review/models.py
View file

@ -14,7 +14,8 @@ from django.utils.encoding import python_2_unicode_compatible
from ietf.doc.models import Document
from ietf.group.models import Group
from ietf.person.models import Person, Email
from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName, ReviewAssignmentStateName
from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName, \
ReviewAssignmentStateName, ReviewerQueuePolicyName
from ietf.utils.validators import validate_regular_expression_string
from ietf.utils.models import ForeignKey, OneToOneField
@ -38,6 +39,7 @@ class ReviewerSettings(models.Model):
skip_next = models.IntegerField(default=0, verbose_name="Skip next assignments")
remind_days_before_deadline = models.IntegerField(null=True, blank=True, help_text="To get an email reminder in case you forget to do an assigned review, enter the number of days before review deadline you want to receive it. Clear the field if you don't want this reminder.")
remind_days_open_reviews = models.PositiveIntegerField(null=True, blank=True, verbose_name="Periodic reminder of open reviews every X days", help_text="To get a periodic email reminder of all your open reviews, enter the number of days between these reminders. Clear the field if you don't want these reminders.")
request_assignment_next = models.BooleanField(default=False, verbose_name="Select me next for an assignment", help_text="If you would like to be assigned to a review as soon as possible, select this option. It is automatically reset once you receive any assignment.")
expertise = models.TextField(verbose_name="Reviewer's expertise in this team's area", max_length=2048, blank=True, help_text="Describe the reviewer's expertise in this team's area", default='')
def __str__(self):
@ -188,6 +190,7 @@ class ReviewTeamSettings(models.Model):
"""Holds configuration specific to groups that are review teams"""
group = OneToOneField(Group)
autosuggest = models.BooleanField(default=True, verbose_name="Automatically suggest possible review requests")
reviewer_queue_policy = models.ForeignKey(ReviewerQueuePolicyName, default='RotateAlphabetically', on_delete=models.PROTECT)
review_types = models.ManyToManyField(ReviewTypeName, default=get_default_review_types)
review_results = models.ManyToManyField(ReviewResultName, default=get_default_review_results, related_name='reviewteamsettings_review_results_set')
notify_ad_when = models.ManyToManyField(ReviewResultName, related_name='reviewteamsettings_notify_ad_set', blank=True)

459
ietf/review/policies.py Normal file
View file

@ -0,0 +1,459 @@
# Copyright The IETF Trust 2019, All Rights Reserved
from __future__ import absolute_import, print_function, unicode_literals
import re
import six
from django.db.models.aggregates import Max
from ietf.doc.models import DocumentAuthor, DocAlias
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, \
ReviewAssignment, ReviewTeamSettings
from ietf.review.utils import (current_unavailable_periods_for_reviewers,
days_needed_to_fulfill_min_interval_for_reviewers,
get_default_filter_re,
latest_review_assignments_for_reviewers)
"""
This file contains policies regarding reviewer queues.
The policies are documented in more detail on:
https://trac.tools.ietf.org/tools/ietfdb/wiki/ReviewerQueuePolicy
Terminology used here should match terminology used in that document.
"""
def get_reviewer_queue_policy(team):
try:
settings = ReviewTeamSettings.objects.get(group=team)
except ReviewTeamSettings.DoesNotExist:
raise ValueError('Request for a reviewer queue policy for team {} '
'which has no ReviewTeamSettings'.format(team))
try:
policy = QUEUE_POLICY_NAME_MAPPING[settings.reviewer_queue_policy.slug]
except KeyError:
raise ValueError('Team {} has unknown reviewer queue policy: '
'{}'.format(team, settings.reviewer_queue_policy.slug))
return policy(team)
class AbstractReviewerQueuePolicy:
def __init__(self, team):
self.team = team
def default_reviewer_rotation_list(self, dont_skip_person_ids=None):
"""
Return a list of reviewers (Person objects) in the default reviewer rotation for a policy.
"""
raise NotImplementedError # pragma: no cover
def return_reviewer_to_rotation_top(self, reviewer_person):
"""
Return a reviewer to the top of the rotation, e.g. because they rejected a review,
and should retroactively not have been rotated over.
"""
raise NotImplementedError # pragma: no cover
def default_reviewer_rotation_list_without_skipped(self):
"""
Return a list of reviewers (Person objects) in the default reviewer rotation for a policy,
while skipping those with a skip_next>0.
"""
return [r for r in self.default_reviewer_rotation_list() if not self._reviewer_settings_for(r).skip_next]
def update_policy_state_for_assignment(self, assignee_person, add_skip=False):
"""
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 = self.default_reviewer_rotation_list_without_skipped()
# 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. There is a cap on the number of iterations, which can be hit e.g. if there
# is only a single reviewer, and the current assignee is excluded from being set as
# NextReviewerInTeam.
current_idx = 0
max_iter = sum([self._reviewer_settings_for(r).skip_next for r in rotation_list]) + len(rotation_list)
if in_order_assignment:
while current_idx <= max_iter:
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):
"""
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
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 queue policy was passed a review request belonging to a different team.')
resolver = AssignmentOrderResolver(email_queryset, review_req, self.default_reviewer_rotation_list())
return [(r['email'].pk, r['label']) for r in resolver.determine_ranking()]
def _entirely_unavailable_reviewers(self, dont_skip_person_ids=None):
"""
Return a set of PKs of Persons that should not be considered
to be in the rotation list at all.
"""
reviewers_to_skip = set()
if not dont_skip_person_ids:
dont_skip_person_ids = []
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_person_ids and not any(p.availability == "canfinish" for p in periods):
reviewers_to_skip.add(person_id)
return reviewers_to_skip
def _reviewer_settings_for(self, person):
return (ReviewerSettings.objects.filter(team=self.team, person=person).first()
or ReviewerSettings(team=self.team, person=person))
class AssignmentOrderResolver:
"""
The AssignmentOrderResolver resolves the "recommended assignment order",
for a set of possible reviewers (email_queryset), a review request, and a
rotation list.
"""
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
self.possible_emails = list(email_queryset)
self.possible_person_ids = [e.person_id for e in self.possible_emails]
self._collect_context()
def _collect_context(self):
"""Collect all relevant data about this team, document and review request."""
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.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))
def determine_ranking(self):
"""
Determine the ranking of reviewers.
Returns a list of tuples, each tuple containing an Email pk and an explanation label.
"""
ranking = []
for e in self.possible_emails:
ranking_for_email = self._ranking_for_email(e)
if ranking_for_email:
ranking.append(ranking_for_email)
ranking.sort(key=lambda r: r["scores"], reverse=True)
return ranking
def _ranking_for_email(self, email):
"""
Determine the ranking for a specific Email.
Returns a dict with an email object, the scores and an explanation label.
The scores are a list of individual scores, i.e. they are prioritised, not
cumulative; so when comparing scores, elements later in the scores list
will only matter if all earlier scores in the list are equal.
"""
settings = self.reviewer_settings.get(email.person_id)
scores = []
explanations = []
def add_boolean_score(direction, expr, explanation=None):
scores.append(direction if expr else -direction)
if expr and explanation:
explanations.append(explanation)
if email.person_id not in self.rotation_index:
return
# 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
all(p.availability == "canfinish" for p in periods)
)
if unavailable_at_the_moment:
return
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))
add_boolean_score(-1, email.person_id in self.has_rejected_review_previous, "rejected review of document before")
add_boolean_score(+1, settings.request_assignment_next, "requested to be selected next for assignment")
add_boolean_score(+1, email.person_id in self.has_completed_review_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 value
scores.append(-settings.skip_next)
if settings.skip_next > 0:
explanations.append("skip next {}".format(settings.skip_next))
# index in the default rotation order
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):
"""Collect statistics on past reviews for a particular 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):
"""
Collect any connections any Person in person_ids has with a document.
Returns a dict containing Person IDs that have a connection as keys,
values being an explanation string,
"""
connections = {}
# examine the closest connections last to let them override the label
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, state_id):
"""
Collect anyone in possible_person_ids that have reviewed the document before,
or an ancestor document.
Returns a set with Person IDs of anyone who has.
"""
doc_names = {review_req.doc.name}.union(*extract_complete_replaces_ancestor_mapping_for_docs([review_req.doc.name]).values())
has_reviewed_previous = ReviewRequest.objects.filter(
doc__name__in=doc_names,
reviewassignment__reviewer__person__in=possible_person_ids,
reviewassignment__state=state_id,
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 RotateAlphabeticallyReviewerQueuePolicy(AbstractReviewerQueuePolicy):
"""
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))
reviewers.sort(key=lambda p: p.last_name())
next_reviewer_index = 0
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 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 deal with too large slice indexes
# so no harm done by using the index on the original list
# afterwards)
reviewers_with_next = reviewers[:] + [next_reviewer]
reviewers_with_next.sort(key=lambda p: p.last_name())
next_reviewer_index = reviewers_with_next.index(next_reviewer)
else:
next_reviewer_index = reviewers.index(next_reviewer)
rotation_list = reviewers[next_reviewer_index:] + reviewers[:next_reviewer_index]
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
def return_reviewer_to_rotation_top(self, reviewer_person):
# As RotateAlphabetically does not keep a full rotation list,
# returning someone to a particular order is complex.
# Instead, the "assign me next" flag is set.
settings = self._reviewer_settings_for(reviewer_person)
settings.request_assignment_next = True
settings.save()
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))
reviewers_dict = {p.pk: p for p in reviewers}
assignments = ReviewAssignment.objects.filter(
review_request__team=self.team,
state__in=['accepted', 'assigned', 'completed'],
reviewer__person__in=reviewers,
).values('reviewer__person').annotate(most_recent=Max('assigned_on')).order_by('most_recent')
reviewers_with_assignment = [
reviewers_dict[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 += 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
def return_reviewer_to_rotation_top(self, reviewer_person):
# Reviewer rotation for this policy ignores rejected/withdrawn
# reviews, so it automatically adjusts the position of someone
# who rejected a review and no further action is needed.
pass
QUEUE_POLICY_NAME_MAPPING = {
'RotateAlphabetically': RotateAlphabeticallyReviewerQueuePolicy,
'LeastRecentlyUsed': LeastRecentlyUsedReviewerQueuePolicy,
}

372
ietf/review/tests_policies.py Normal file
View file

@ -0,0 +1,372 @@
# Copyright The IETF Trust 2016-2019, All Rights Reserved
import debug # pyflakes:ignore
from ietf.doc.factories import WgDraftFactory, IndividualDraftFactory
from ietf.group.factories import ReviewTeamFactory
from ietf.group.models import Group, Role
from ietf.name.models import ReviewerQueuePolicyName
from ietf.person.factories import PersonFactory
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, \
ReviewTeamSettings
from ietf.review.policies import (AssignmentOrderResolver, LeastRecentlyUsedReviewerQueuePolicy,
RotateAlphabeticallyReviewerQueuePolicy,
get_reviewer_queue_policy)
from ietf.utils.test_data import create_person
from ietf.utils.test_utils import TestCase
class GetReviewerQueuePolicyTest(TestCase):
def test_valid_policy(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"), settings__reviewer_queue_policy_id='LeastRecentlyUsed')
policy = get_reviewer_queue_policy(team)
self.assertEqual(policy.__class__, LeastRecentlyUsedReviewerQueuePolicy)
def test_missing_settings(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
ReviewTeamSettings.objects.all().delete()
with self.assertRaises(ValueError):
get_reviewer_queue_policy(team)
def test_invalid_policy_name(self):
ReviewerQueuePolicyName.objects.create(slug='invalid')
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"), settings__reviewer_queue_policy_id='invalid')
with self.assertRaises(ValueError):
get_reviewer_queue_policy(team)
class RotateAlphabeticallyReviewerAndGenericQueuePolicyTest(TestCase):
"""
These tests also cover the common behaviour in AbstractReviewerQueuePolicy,
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 = RotateAlphabeticallyReviewerQueuePolicy(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',
)
# Default policy without a NextReviewerInTeam
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers)
# Policy with a current NextReviewerInTeam
NextReviewerInTeam.objects.create(team=team, next_reviewer=reviewers[3])
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers[3:] + reviewers[:3])
# Policy with a NextReviewerInTeam that has left the team.
Role.objects.get(person=reviewers[1]).delete()
NextReviewerInTeam.objects.filter(team=team).update(next_reviewer=reviewers[1])
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers[2:] + reviewers[:1])
def test_setup_reviewer_field(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
reviewer_0 = create_person(team, "reviewer", name="Test Reviewer-0", username="testreviewer0")
reviewer_1 = create_person(team, "reviewer", name="Test Reviewer-1", username="testreviewer1")
review_req = ReviewRequestFactory(team=team, type_id='early')
ReviewAssignmentFactory(review_request=review_req, reviewer=reviewer_1.email(), state_id='part-completed')
field = PersonEmailChoiceField(label="Assign Reviewer", empty_label="(None)", required=False)
policy.setup_reviewer_field(field, review_req)
self.assertEqual(field.choices[0], ('', '(None)'))
self.assertEqual(field.choices[1][0], str(reviewer_0.email()))
self.assertEqual(field.choices[2][0], str(reviewer_1.email()))
self.assertEqual(field.choices[1][1], 'Test Reviewer-0: #1')
self.assertEqual(field.choices[2][1], 'Test Reviewer-1: #2; 1 partially complete')
self.assertEqual(field.initial, str(reviewer_1.email()))
def test_recommended_assignment_order(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
reviewer_high = create_person(team, "reviewer", name="Test Reviewer-1-high", username="testreviewerhigh")
reviewer_low = create_person(team, "reviewer", name="Test Reviewer-0-low", username="testreviewerlow")
# reviewer_high appears later in the default rotation, but reviewer_low is the author
doc = WgDraftFactory(group__acronym='mars', rev='01', authors=[reviewer_low])
review_req = ReviewRequestFactory(doc=doc, team=team, type_id='early')
order = policy.recommended_assignment_order(Email.objects.all(), review_req)
self.assertEqual(order[0][0], str(reviewer_high.email()))
self.assertEqual(order[1][0], str(reviewer_low.email()))
self.assertEqual(order[0][1], 'Test Reviewer-1-high: #2')
self.assertEqual(order[1][1], 'Test Reviewer-0-low: is author of document; #1')
with self.assertRaises(ValueError):
review_req_other_team = ReviewRequestFactory(doc=doc, type_id='early')
policy.recommended_assignment_order(Email.objects.all(), review_req_other_team)
def test_update_policy_state_for_assignment(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
# make a bunch of reviewers
reviewers = [
create_person(team, "reviewer", name="Test Reviewer{}".format(i), username="testreviewer{}".format(i))
for i in range(5)
]
self.assertEqual(reviewers, policy.default_reviewer_rotation_list())
def reviewer_settings_for(person):
return (ReviewerSettings.objects.filter(team=team, person=person).first()
or ReviewerSettings(team=team, person=person))
def get_skip_next(person):
return reviewer_settings_for(person).skip_next
# Regular in-order assignment without skips
reviewer0_settings = reviewer_settings_for(reviewers[0])
reviewer0_settings.request_assignment_next = True
reviewer0_settings.save()
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], 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)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# request_assignment_next should be reset after any assignment
self.assertFalse(reviewer_settings_for(reviewers[0]).request_assignment_next)
# In-order assignment with add_skip
policy.update_policy_state_for_assignment(assignee_person=reviewers[1], 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) # from current add_skip=True
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# In-order assignment to 2, but 3 has a skip_next, so 4 should be assigned.
# 3 has skip_next decreased as it is skipped over, 1 retains its skip_next
reviewer3_settings = reviewer_settings_for(reviewers[3])
reviewer3_settings.skip_next = 2
reviewer3_settings.save()
policy.update_policy_state_for_assignment(assignee_person=reviewers[2], 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) # from previous add_skip=true
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1) # from manually set skip_next - 1
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Out of order assignments, nothing should change,
# except the add_skip=True should still apply
policy.update_policy_state_for_assignment(assignee_person=reviewers[3], add_skip=False)
policy.update_policy_state_for_assignment(assignee_person=reviewers[2], add_skip=False)
policy.update_policy_state_for_assignment(assignee_person=reviewers[1], add_skip=False)
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=True)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 1) # from current add_skip=True
self.assertEqual(get_skip_next(reviewers[1]), 1)
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Regular assignment, testing wrap-around
policy.update_policy_state_for_assignment(assignee_person=reviewers[4], add_skip=False)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0) # skipped over with this assignment
self.assertEqual(get_skip_next(reviewers[1]), 0) # skipped over with this assignment
self.assertEqual(get_skip_next(reviewers[2]), 0)
self.assertEqual(get_skip_next(reviewers[3]), 1)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# Leave only a single reviewer remaining, which should not trigger an infinite loop.
# The deletion also causes NextReviewerInTeam to be deleted.
[reviewer.delete() for reviewer in reviewers[1:]]
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
policy.update_policy_state_for_assignment(assignee_person=reviewers[0], add_skip=False)
# No NextReviewerInTeam should be created, the only possible next is the excluded assignee.
self.assertFalse(NextReviewerInTeam.objects.filter(team=team))
self.assertEqual([reviewers[0]], policy.default_reviewer_rotation_list())
def test_return_reviewer_to_rotation_top(self):
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
list_email="rotationteam@ietf.org",
parent=Group.objects.get(acronym="farfut"))
reviewer = create_person(team, "reviewer", name="reviewer", username="reviewer")
policy = RotateAlphabeticallyReviewerQueuePolicy(team)
policy.return_reviewer_to_rotation_top(reviewer)
self.assertTrue(ReviewerSettings.objects.get(person=reviewer).request_assignment_next)
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',
)
# This reviewer has an assignment, but is no longer in the team and should not be in rotation.
out_of_team_reviewer = PersonFactory()
ReviewAssignmentFactory(review_request__team=team, reviewer=out_of_team_reviewer.email())
# No known assignments, order in PK order.
rotation = policy.default_reviewer_rotation_list()
self.assertNotIn(unavailable_reviewer, rotation)
self.assertEqual(rotation, reviewers)
# Regular accepted assignments. Note that one is older and one is newer than reviewer 0's,
# the newest one should count for ordering, i.e. reviewer 1 should be later in the list.
ReviewAssignmentFactory(reviewer=reviewers[1].email(), assigned_on='2019-01-01',
state_id='accepted', review_request__team=team)
ReviewAssignmentFactory(reviewer=reviewers[1].email(), assigned_on='2017-01-01',
state_id='accepted', review_request__team=team)
# Rejected assignment, should not affect reviewer 2's position
ReviewAssignmentFactory(reviewer=reviewers[2].email(), assigned_on='2020-01-01',
state_id='rejected', review_request__team=team)
# Completed assignments, assigned before the most recent assignment of 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)
ReviewAssignmentFactory(reviewer=reviewers[0].email(), assigned_on='2018-05-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]])
def test_return_reviewer_to_rotation_top(self):
# Should do nothing, this is implicit in this policy, no state change is needed.
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team",
list_email="rotationteam@ietf.org",
parent=Group.objects.get(acronym="farfut"))
reviewer = create_person(team, "reviewer", name="reviewer", username="reviewer")
policy = LeastRecentlyUsedReviewerQueuePolicy(team)
policy.return_reviewer_to_rotation_top(reviewer)
class AssignmentOrderResolverTests(TestCase):
def test_determine_ranking(self):
# reviewer_high is second in the default rotation, reviewer_low is first
# however, reviewer_high hits every score increase, reviewer_low hits every score decrease
team = ReviewTeamFactory(acronym="rotationteam", name="Review Team", list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
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 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
doc_individual = IndividualDraftFactory()
doc_wg = WgDraftFactory(relations=[('replaces', doc_individual)])
doc_middle_wg = WgDraftFactory(relations=[('replaces', doc_wg)])
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')
# 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
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='no-response')
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_high.email(), state_id='part-completed')
# Trigger review wish check
ReviewWish.objects.create(team=team, doc=doc, person=reviewer_high)
# This period should not have an impact, because it is the canfinish type,
# and this reviewer has reviewed previously.
UnavailablePeriod.objects.create(
team=team,
person=reviewer_high,
start_date='2000-01-01',
availability='canfinish',
)
# This period should exclude this reviewer entirely, as it is 'canfinish',
# but this reviewer has not reviewed previously.
UnavailablePeriod.objects.create(
team=team,
person=reviewer_unavailable,
start_date='2000-01-01',
availability='canfinish',
)
# Trigger "reviewer has rejected before"
ReviewAssignmentFactory(review_request__team=team, review_request__doc=doc, reviewer=reviewer_low.email(), state_id='rejected')
# Trigger max frequency and open review stats
ReviewAssignmentFactory(review_request__team=team, reviewer=reviewer_low.email(), state_id='assigned', review_request__doc__pages=10)
# Trigger skip_next, max frequency, filter_re
ReviewerSettings.objects.create(
team=team,
person=reviewer_low,
filter_re='.*draft.*',
skip_next=2,
min_interval=91,
)
# Trigger "assign me next"
ReviewerSettings.objects.create(
team=team,
person=reviewer_high,
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')
self.assertEqual(ranking[1]['label'], 'Test Reviewer-low: rejected review of document before; is author of document; filter regexp matches; max frequency exceeded, ready in 91 days; skip next 2; #1; currently 1 open, 10 pages')

256
ietf/review/utils.py
View file

@ -6,8 +6,6 @@ from __future__ import absolute_import, print_function, unicode_literals
import datetime
import itertools
import re
import six
from collections import defaultdict, namedtuple
@ -23,15 +21,14 @@ from ietf.dbtemplate.models import DBTemplate
from ietf.group.models import Group, Role
from ietf.doc.models import (Document, ReviewRequestDocEvent, ReviewAssignmentDocEvent, State,
LastCallDocEvent, TelechatDocEvent,
DocumentAuthor, DocAlias)
LastCallDocEvent, TelechatDocEvent)
from ietf.iesg.models import TelechatDate
from ietf.mailtrigger.utils import gather_address_lists
from ietf.person.models import Person
from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream
from ietf.review.models import (ReviewRequest, ReviewAssignment, ReviewRequestStateName, ReviewTypeName,
ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam,
ReviewSecretarySettings, ReviewTeamSettings)
ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings,
ReviewTeamSettings)
from ietf.utils.mail import send_mail
from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs
from ietf.utils import log
@ -113,49 +110,6 @@ def current_unavailable_periods_for_reviewers(team):
return res
def reviewer_rotation_list(team, skip_unavailable=False, dont_skip=[]):
"""Returns person id -> index in rotation (next reviewer has index 0)."""
reviewers = list(Person.objects.filter(role__name="reviewer", role__group=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()
if saved_reviewer:
n = saved_reviewer.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,
# so no harm done by using the index on the original list
# afterwards)
reviewers_with_next = reviewers[:] + [n]
reviewers_with_next.sort(key=lambda p: p.last_name())
next_reviewer_index = reviewers_with_next.index(n)
else:
next_reviewer_index = reviewers.index(n)
rotation_list = reviewers[next_reviewer_index:] + reviewers[:next_reviewer_index]
if skip_unavailable:
# 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)
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)
for person_id, days_needed in days_needed_for_reviewers.items():
if person_id not in dont_skip:
reviewers_to_skip.add(person_id)
rotation_list = [p.pk for p in rotation_list if p.pk not in reviewers_to_skip]
return rotation_list
def days_needed_to_fulfill_min_interval_for_reviewers(team):
"""Returns person_id -> days needed until min_interval is fulfilled
@ -430,7 +384,8 @@ 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())
possibly_advance_next_reviewer_for_team(review_req.team, reviewer.person_id, add_skip)
from ietf.review.policies import get_reviewer_queue_policy
get_reviewer_queue_policy(review_req.team).update_policy_state_for_assignment(reviewer.person, add_skip)
descr = "Request for {} review by {} is assigned to {}".format(
review_req.type.name,
@ -482,49 +437,6 @@ def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=Fa
msg ,
by=request.user.person, notify_secretary=False, notify_reviewer=True, notify_requested_by=False)
def possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id, add_skip=False):
assert assigned_review_to_person_id is not None
rotation_list = reviewer_rotation_list(team, skip_unavailable=True, dont_skip=[assigned_review_to_person_id])
def reviewer_at_index(i):
if not rotation_list:
return None
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
if add_skip:
settings = reviewer_settings_for(assigned_review_to_person_id)
settings.skip_next += 1
settings.save()
if not rotation_list:
return
while True:
# as a clean-up step go through any with a skip next > 0
current_reviewer_person_id = reviewer_at_index(current_i)
settings = reviewer_settings_for(current_reviewer_person_id)
if settings.skip_next > 0:
settings.skip_next -= 1
settings.save()
current_i += 1
else:
nr = NextReviewerInTeam.objects.filter(team=team).first() or NextReviewerInTeam(team=team)
nr.next_reviewer_id = current_reviewer_person_id
nr.save()
break
def close_review_request(request, review_req, close_state, close_comment=''):
suggested_req = review_req.pk is None
@ -783,18 +695,6 @@ def extract_revision_ordered_review_requests_for_documents_and_replaced(review_r
return res
# TODO : Change this field to deal with multiple already assigned reviewers
def setup_reviewer_field(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
choices = make_assignment_choices(field.queryset, review_req)
if not field.required:
choices = [("", field.empty_label)] + choices
field.choices = choices
def get_default_filter_re(person):
if type(person) != Person:
@ -805,152 +705,6 @@ def get_default_filter_re(person):
else:
return '^draft-(%s|%s)-.*$' % ( person.last_name().lower(), '|'.join(['ietf-%s' % g.acronym for g in groups_to_avoid]))
def make_assignment_choices(email_queryset, review_req):
doc = review_req.doc
team = review_req.team
possible_emails = list(email_queryset)
possible_person_ids = [e.person_id for e in possible_emails]
aliases = DocAlias.objects.filter(docs=doc).values_list("name", 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(reviewer_rotation_list(team)) }
# previous review of document
has_reviewed_previous = ReviewRequest.objects.filter(
doc__name__in=set([doc.name]).union(*extract_complete_replaces_ancestor_mapping_for_docs([doc.name]).values()),
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)
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,
})
ranking.sort(key=lambda r: r["scores"], reverse=True)
return [(r["email"].pk, r["label"]) for r in ranking]
def send_unavaibility_period_ending_reminder(remind_date):
reminder_days = 3

4
ietf/templates/ietfauth/review_overview.html
View file

@ -150,6 +150,10 @@
<th>Periodic reminder of open reviews every X days</th>
<td>{{ t.reviewer_settings.remind_days_open_reviews|default:"(Do not remind)" }}</td>
</tr>
<tr>
<th>Select me next for an assignment</th>
<td>{{ t.reviewer_settings.request_assignment_next|yesno }}</td>
</tr>
<tr>
<th>Unavailable periods</th>
<td>

2
ietf/utils/text.py
View file

@ -195,4 +195,4 @@ def unwrap(s):
return s.replace('\n', ' ')
def normalize_text(s):
return re.sub(r'[\s\u2028\u2029\n\r]+', ' ', s).strip()
return re.sub(r'[\s\n\r\u2028\u2029]+', ' ', s, flags=re.U).strip()