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

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

Author: Sasha Romijn

ietf/group/forms.py

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

ietf/group/tests_review.py

@@ -332,6 +332,7 @@ def test_change_reviewer_settings(self):
             "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 @@ def test_change_reviewer_settings(self):
         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, {

ietf/group/views.py

@@ -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)
 

ietf/review/migrations/0020_add_request_assignment_next.py

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

ietf/review/models.py

@@ -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):

ietf/review/policies.py

@@ -39,11 +39,13 @@ def default_reviewer_rotation_list(self, dont_skip_person_ids=None):
         """
         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 @@ def _entirely_unavailable_reviewers(self, dont_skip_person_ids=None):
                 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 @@ def format_period(p):
         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 @@ def _reviewer_settings_for_person_ids(self, person_ids):
 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 @@ def reviewer_at_index(i):
                 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 @@ def reviewer_settings_for(person):
         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 @@ def reviewer_settings_for(person):
                 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()
 

ietf/review/test_policies.py

@@ -113,13 +113,18 @@ 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)
@@ -214,21 +219,27 @@ def test_determine_ranking(self):
 
         # 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,
             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,
+        )
 
         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')