Update some terminology and docstrings.

 - Legacy-Id: 16983

Author: Sasha Romijn

ietf/doc/tests_review.py

@@ -33,7 +33,7 @@
 from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory
 from ietf.review.models import (ReviewRequest, ReviewerSettings,
                                 ReviewWish, UnavailablePeriod, NextReviewerInTeam)
-from ietf.review.policies import policy_for_team
+from ietf.review.policies import get_reviewer_queue_policy
 
 from ietf.utils.test_utils import TestCase
 from ietf.utils.test_utils import login_testing_unauthorized, reload_db_objects
@@ -325,7 +325,7 @@ def test_assign_reviewer(self):
 
         # assign
         empty_outbox()
-        rotation_list = policy_for_team(review_req.team).default_reviewer_rotation_list()
+        rotation_list = get_reviewer_queue_policy(review_req.team).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)

ietf/doc/views_review.py

@@ -33,7 +33,7 @@
 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 policy_for_team
+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,
@@ -294,7 +294,7 @@ class AssignReviewerForm(forms.Form):
 
     def __init__(self, review_req, *args, **kwargs):
         super(AssignReviewerForm, self).__init__(*args, **kwargs)
-        policy_for_team(review_req.team).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

ietf/group/forms.py

@@ -19,7 +19,7 @@
 from ietf.person.fields import SearchableEmailsField, PersonEmailChoiceField
 from ietf.person.models import Person
 from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings
-from ietf.review.policies import policy_for_team
+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
@@ -255,7 +255,7 @@ def __init__(self, review_req, *args, **kwargs):
 
         self.fields["close"].widget.attrs["class"] = "form-control input-sm"
 
-        policy_for_team(review_req.team).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 self.is_bound:

ietf/group/tests_review.py

@@ -12,7 +12,7 @@
 
 from django.urls import reverse as urlreverse
 
-from ietf.review.policies import policy_for_team
+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
 from ietf.group.models import Role
@@ -655,7 +655,7 @@ def test_rotation_queue_update(self):
         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)]
-        policy = policy_for_team(group)
+        policy = get_reviewer_queue_policy(group)
         rot_list = policy.default_reviewer_rotation_list()
 
         expected_ending_head_of_rotation = rot_list[3]

ietf/group/views.py

@@ -92,7 +92,7 @@
 from ietf.name.models import GroupTypeName, StreamName
 from ietf.person.models import Email
 from ietf.review.models import ReviewRequest, ReviewAssignment, ReviewerSettings, ReviewSecretarySettings
-from ietf.review.policies import policy_for_team
+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,
 
@@ -1391,7 +1391,7 @@ def reviewer_overview(request, acronym, group_type=None):
 
     can_manage = can_manage_review_requests_for_team(request.user, group)
 
-    reviewers = policy_for_team(group).default_reviewer_rotation_list()
+    reviewers = get_reviewer_queue_policy(group).default_reviewer_rotation_list()
 
     reviewer_settings = { s.person_id: s for s in ReviewerSettings.objects.filter(team=group) }
     unavailable_periods = defaultdict(list)
@@ -1541,7 +1541,7 @@ 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
-            reviewer_policy = policy_for_team(group)
+            reviewer_policy = get_reviewer_queue_policy(group)
             head_of_rotation = reviewer_policy.default_reviewer_rotation_list()[0]
             while head_of_rotation in assignments_by_person:
                 for review_req in assignments_by_person[head_of_rotation]:
@@ -1661,7 +1661,7 @@ def should_be_replicated_in_last_call_section(r):
 
         partial_msg = render_to_string(template.path, {
             "review_assignments": review_assignments,
-            "rotation_list": policy_for_team(group).default_reviewer_rotation_list()[:10],
+            "rotation_list": get_reviewer_queue_policy(group).default_reviewer_rotation_list()[:10],
             "group" : group,
         })
 

ietf/review/policies.py

@@ -16,12 +16,19 @@
                                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 policy_for_team(team):
-    return RotateWithSkipReviewerPolicy(team)
 
+def get_reviewer_queue_policy(team):
+    return RotateWithSkipReviewerQueuePolicy(team)
 
-class AbstractReviewerPolicy:
+
+class AbstractReviewerQueuePolicy:
     def __init__(self, team):
         self.team = team
 
@@ -32,6 +39,12 @@ def default_reviewer_rotation_list(self, skip_unavailable=False, dont_skip=[]):
         """
         raise NotImplementedError
 
+    def update_policy_state_for_assignment(self, assignee_person_id, add_skip=False):
+        """
+        Update the internal state of a policy to reflect an assignment. 
+        """
+        raise NotImplementedError
+
     # TODO : Change this field to deal with multiple already assigned reviewers???
     def setup_reviewer_field(self, field, review_req):
         """
@@ -57,7 +70,7 @@ def _recommended_assignment_order(self, email_queryset, review_req):
         returning Email objects.
         """
         if review_req.team != self.team:
-            raise ValueError('Reviewer policy was passed review request belonging to different team.')            
+            raise ValueError('Reviewer queue policy was passed review request belonging to different team.')            
         resolver = AssignmentOrderResolver(email_queryset, review_req, self.default_reviewer_rotation_list())
         return resolver.determine_ranking()
 
@@ -90,7 +103,7 @@ def __init__(self, email_queryset, review_req, rotation_list):
         self._collect_context()
 
     def _collect_context(self):
-        # Collect all relevant data about this team, document and review request.
+        """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)
 
@@ -109,6 +122,10 @@ def _collect_context(self):
         self.assignment_data_for_reviewers = latest_review_assignments_for_reviewers(self.team)
 
     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.append(self._ranking_for_email(e))
@@ -118,6 +135,12 @@ def determine_ranking(self):
         return [(r["email"].pk, r["label"]) for r in 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. 
+        """
         settings = self.reviewer_settings.get(email.person_id)
         scores = []
         explanations = []
@@ -183,6 +206,7 @@ def format_period(p):
         }
 
     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"])
@@ -206,8 +230,13 @@ def _collect_reviewer_stats(self, email):
         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
+        # 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"):
@@ -221,6 +250,10 @@ def _connections_with_doc(self, doc, person_ids):
         return connections
 
     def _persons_with_previous_review(self, review_req, possible_person_ids):
+        """
+        Collect anyone in possible_person_ids that have reviewed the request before.
+        Returns a set with Person IDs of anyone who has.
+        """
         has_reviewed_previous = ReviewRequest.objects.filter(
             doc=review_req.doc,
             reviewassignment__reviewer__person__in=possible_person_ids,
@@ -232,7 +265,7 @@ def _persons_with_previous_review(self, review_req, possible_person_ids):
         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
@@ -245,7 +278,7 @@ def _reviewer_settings_for_person_ids(self, person_ids):
         return reviewer_settings
 
 
-class RotateWithSkipReviewerPolicy(AbstractReviewerPolicy):
+class RotateWithSkipReviewerQueuePolicy(AbstractReviewerQueuePolicy):
 
     def update_policy_state_for_assignment(self, assignee_person_id, add_skip=False):
         assert assignee_person_id is not None

ietf/review/test_policies.py

@@ -8,7 +8,7 @@
 from ietf.review.factories import ReviewAssignmentFactory
 from ietf.review.models import ReviewerSettings, NextReviewerInTeam, UnavailablePeriod, \
     ReviewRequest
-from ietf.review.policies import policy_for_team
+from ietf.review.policies import get_reviewer_queue_policy
 from ietf.utils.test_data import create_person
 from ietf.utils.test_utils import TestCase
 
@@ -17,7 +17,7 @@ class RotateWithSkipReviewerPolicyTests(TestCase):
     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"))
-        policy = policy_for_team(team)
+        policy = get_reviewer_queue_policy(team)
         doc = WgDraftFactory()
 
         # make a bunch of reviewers

ietf/review/utils.py

@@ -380,8 +380,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())
 
-    from ietf.review.policies import policy_for_team
-    policy_for_team(review_req.team).update_policy_state_for_assignment(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_id, add_skip)
 
     ReviewRequestDocEvent.objects.create(
         type="assigned_review_request",