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

Fix #2336 - Add "select me next for an assignment".

Browse source 
Reviewers can set this flag in their reviewer settings, which triggers
a mail to be sent to the secretary. They are then kept on top of the
recommended assignment order. This flag is automatically reset when any
assignment is made to the reviewer.
 - Legacy-Id: 17048
This commit is contained in:
Sasha Romijn 2019-11-18 12:30:11 +00:00
parent e188da5214
commit 554a839864
7 changed files with 63 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
ietf/group/forms.py
View file

@ -276,7 +276,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', [])

2
ietf/group/tests_review.py
View file

@ -332,6 +332,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)
@ -347,6 +348,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, {

5
ietf/group/views.py
View file

@ -1732,7 +1732,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)

26
ietf/review/migrations/0020_add_request_assignment_next.py Normal file
View file

@ -0,0 +1,26 @@
# Copyright The IETF Trust 2016-2019, All Rights Reserved
# -*- coding: utf-8 -*-
# Generated by Django 1.11.23 on 2019-11-18 04:26
from __future__ import unicode_literals
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('review', '0019_auto_20191023_0829'),
]
operations = [
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'),
),
]

1
ietf/review/models.py
View file

@ -38,6 +38,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):

25
ietf/review/policies.py
View file

@ -39,11 +39,13 @@ class AbstractReviewerQueuePolicy:
"""
raise NotImplementedError # pragma: no cover
def update_policy_state_for_assignment(self, assignee_person_id, add_skip=False):
def update_policy_state_for_assignment(self, assignee_person, add_skip=False):
"""
Update the internal state of a policy to reflect an assignment.
"""
raise NotImplementedError # pragma: no cover
settings = self._reviewer_settings_for(assignee_person)
settings.request_assignment_next = False
settings.save()
# TODO : Change this field to deal with multiple already assigned reviewers???
def setup_reviewer_field(self, field, review_req):
@ -89,8 +91,12 @@ class AbstractReviewerQueuePolicy:
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",
@ -178,7 +184,7 @@ class AssignmentOrderResolver:
if periods:
explanations.append(", ".join(format_period(p) for p in periods))
# misc
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_reviewed_previous, "reviewed document before")
add_boolean_score(+1, email.person_id in self.wish_to_review, "wishes to review document")
add_boolean_score(-1, email.person_id in self.connections,
@ -294,6 +300,7 @@ 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])
@ -303,14 +310,10 @@ class RotateAlphabeticallyReviewerQueuePolicy(AbstractReviewerQueuePolicy):
return None
return rotation_list[i % len(rotation_list)]
def reviewer_settings_for(person):
return (ReviewerSettings.objects.filter(team=self.team, person=person).first()
or ReviewerSettings(team=self.team, person=person))
if not rotation_list:
return
rotation_list_without_skip = [r for r in rotation_list if not reviewer_settings_for(r).skip_next]
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
@ -322,7 +325,7 @@ class RotateAlphabeticallyReviewerQueuePolicy(AbstractReviewerQueuePolicy):
if in_order_assignment:
while True:
current_idx_person = reviewer_at_index(current_idx)
settings = reviewer_settings_for(current_idx_person)
settings = self._reviewer_settings_for(current_idx_person)
if settings.skip_next > 0:
settings.skip_next -= 1
settings.save()
@ -336,7 +339,7 @@ class RotateAlphabeticallyReviewerQueuePolicy(AbstractReviewerQueuePolicy):
current_idx += 1
if add_skip:
settings = reviewer_settings_for(assignee_person)
settings = self._reviewer_settings_for(assignee_person)
settings.skip_next += 1
settings.save()

19
ietf/review/test_policies.py
View file

@ -113,6 +113,9 @@ class RotateAlphabeticallyReviewerQueuePolicyTest(TestCase):
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)
@ -120,6 +123,8 @@ class RotateAlphabeticallyReviewerQueuePolicyTest(TestCase):
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)
@ -214,7 +219,7 @@ class AssignmentOrderResolverTests(TestCase):
# 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 and filter_re
# Trigger skip_next, max frequency, filter_re
ReviewerSettings.objects.create(
team=team,
person=reviewer_low,
@ -222,13 +227,19 @@ class AssignmentOrderResolverTests(TestCase):
skip_next=2,
min_interval=91,
)
# Trigger "assign me next"
ReviewerSettings.objects.create(
team=team,
person=reviewer_high,
request_assignment_next=True,
)
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())
self.assertEqual(ranking[0]['scores'], [ 1, 1, 1, 1, 0, 0, -1])
self.assertEqual(ranking[1]['scores'], [-1, -1, -1, -1, -91, -2, 0])
self.assertEqual(ranking[0]['label'], 'Test Reviewer-high: unavailable indefinitely (Can do follow-ups); reviewed document before; wishes to review document; #2; 1 no response, 1 partially complete, 1 fully completed')
self.assertEqual(ranking[0]['scores'], [ 1, 1, 1, 1, 1, 0, 0, -1])
self.assertEqual(ranking[1]['scores'], [-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: is author of document; filter regexp matches; max frequency exceeded, ready in 91 days; skip next 2; #1; currently 1 open, 10 pages')