Take a bunch of factors into account when sorting reviewers for

assignment to a review request
 - Legacy-Id: 11626

Author: OleLaursen

ietf/doc/tests_review.py

@@ -12,10 +12,11 @@
 
 import debug                            # pyflakes:ignore
 
-from ietf.review.models import ReviewRequest, ReviewTeamResult
+from ietf.review.models import ReviewRequest, ReviewTeamResult, Reviewer
 import ietf.review.mailarch
 from ietf.person.models import Email, Person
-from ietf.name.models import ReviewResultName, ReviewRequestStateName
+from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewTypeName
+from ietf.doc.models import DocumentAuthor
 from ietf.utils.test_utils import TestCase
 from ietf.utils.test_data import make_test_data, make_review_data
 from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects
@@ -120,11 +121,40 @@ def test_close_request(self):
 
     def test_assign_reviewer(self):
         doc = make_test_data()
+
+        # set up some reviewer-suitability factors
+        plain_email = Email.objects.filter(person__user__username="plain").first()
+        DocumentAuthor.objects.create(
+            author=plain_email,
+            document=doc,
+        )
+        doc.rev = "10"
+        doc.save()
+
+        # review to assign to
         review_req = make_review_data(doc)
         review_req.state = ReviewRequestStateName.objects.get(slug="requested")
         review_req.reviewer = None
         review_req.save()
 
+        # previous review
+        ReviewRequest.objects.create(
+            time=datetime.datetime.now() - datetime.timedelta(days=100),
+            requested_by=Person.objects.get(name="(System)"),
+            doc=doc,
+            type=ReviewTypeName.objects.get(slug="early"),
+            team=review_req.team,
+            state=ReviewRequestStateName.objects.get(slug="completed"),
+            reviewed_rev="01",
+            deadline=datetime.datetime.now() - datetime.timedelta(days=80),
+            reviewer=plain_email,
+        )
+
+        reviewer_obj = Reviewer.objects.get(person__email=plain_email)
+        reviewer_obj.filter_re = doc.name
+        reviewer_obj.unavailable_until = datetime.datetime.now() + datetime.timedelta(days=10)
+        reviewer_obj.save()
+
         assign_url = urlreverse('ietf.doc.views_review.assign_reviewer', kwargs={ "name": doc.name, "request_id": review_req.pk })
 
 
@@ -140,6 +170,13 @@ def test_assign_reviewer(self):
         login_testing_unauthorized(self, "secretary", assign_url)
         r = self.client.get(assign_url)
         self.assertEqual(r.status_code, 200)
+        q = PyQuery(r.content)
+        plain_label = q("option[value=\"{}\"]".format(plain_email.address)).text().lower()
+        self.assertIn("ready for", plain_label)
+        self.assertIn("reviewed document before", plain_label)
+        self.assertIn("is author", plain_label)
+        self.assertIn("regexp matches", plain_label)
+        self.assertIn("unavailable until", plain_label)
 
         # assign
         empty_outbox()

ietf/doc/views_review.py

@@ -18,7 +18,7 @@
                                can_request_review_of_doc, can_manage_review_requests_for_team,
                                email_review_request_change, make_new_review_request_from_existing,
                                close_review_request_states, close_review_request,
-                               construct_review_request_assignment_choices)
+                               setup_reviewer_field)
 from ietf.review import mailarch
 from ietf.utils.fields import DatepickerDateField
 from ietf.utils.text import skip_prefix
@@ -205,15 +205,11 @@ def close_request(request, name, request_id):
 
 
 class AssignReviewerForm(forms.Form):
-    reviewer = PersonEmailChoiceField(widget=forms.RadioSelect, empty_label="(None)", required=False)
+    reviewer = PersonEmailChoiceField(empty_label="(None)", required=False)
 
     def __init__(self, review_req, *args, **kwargs):
         super(AssignReviewerForm, self).__init__(*args, **kwargs)
-        f = self.fields["reviewer"]
-        f.queryset = f.queryset.filter(role__name="reviewer", role__group=review_req.team)
-        if review_req.reviewer:
-            f.initial = review_req.reviewer_id
-        f.choices = construct_review_request_assignment_choices(f.queryset, review_req.team, review_req)
+        setup_reviewer_field(self.fields["reviewer"], review_req)
 
 
 @login_required

ietf/group/views_review.py

@@ -8,7 +8,7 @@
                                extract_revision_ordered_review_requests_for_documents,
                                assign_review_request_to_reviewer,
                                close_review_request,
-                               construct_review_request_assignment_choices,
+                               setup_reviewer_field,
 #                               make_new_review_request_from_existing,
                                suggested_review_requests_for_team)
 from ietf.group.utils import get_group_or_404
@@ -55,11 +55,7 @@ def __init__(self, review_req, *args, **kwargs):
 
         self.fields["close"].widget.attrs["class"] = "form-control input-sm"
 
-        self.fields["reviewer"].queryset = self.fields["reviewer"].queryset.filter(
-            role__name="reviewer",
-            role__group=review_req.team,
-        )
-        self.fields["reviewer"].choices = construct_review_request_assignment_choices(self.fields["reviewer"].queryset, review_req.team, review_req)
+        setup_reviewer_field(self.fields["reviewer"], review_req)
         self.fields["reviewer"].widget.attrs["class"] = "form-control input-sm"
 
         if self.is_bound:

ietf/review/models.py

@@ -1,3 +1,5 @@
+import datetime
+
 from django.db import models
 
 from ietf.doc.models import Document
@@ -44,7 +46,7 @@ class ReviewRequest(models.Model):
 
     # Fields filled in on the initial record creation - these
     # constitute the request part.
-    time          = models.DateTimeField(auto_now_add=True)
+    time          = models.DateTimeField(default=datetime.datetime.now)
     type          = models.ForeignKey(ReviewTypeName)
     doc           = models.ForeignKey(Document, related_name='review_request_set')
     team          = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamresult=None))

ietf/review/utils.py

@@ -1,13 +1,13 @@
-import datetime
+import datetime, re
 from collections import defaultdict
 
 from django.contrib.sites.models import Site
 from django.db import models
 
 from ietf.group.models import Group, Role
-from ietf.doc.models import Document, DocEvent, State, LastCallDocEvent
+from ietf.doc.models import Document, DocEvent, State, LastCallDocEvent, DocumentAuthor, DocAlias
 from ietf.iesg.models import TelechatDate
-from ietf.person.models import Person
+from ietf.person.models import Person, Email
 from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream
 from ietf.review.models import ReviewRequest, ReviewRequestStateName, ReviewTypeName, Reviewer
 from ietf.utils.mail import send_mail
@@ -275,38 +275,80 @@ def extract_revision_ordered_review_requests_for_documents(queryset, names):
 
     return res
 
-def construct_review_request_assignment_choices(possible_emails, team, review_req=None):
-    possible_emails = list(possible_emails)
+def setup_reviewer_field(field, review_req):
+    field.queryset = field.queryset.filter(role__name="reviewer", role__group=review_req.team)
+    if review_req.reviewer:
+        field.initial = review_req.reviewer_id
+
+    choices = make_assignment_choices(field.queryset, review_req)
+    if not field.required:
+        choices = [("", field.empty_label)] + choices
+
+    field.choices = choices
+
+def make_assignment_choices(email_queryset, review_req):
+    doc = review_req.doc
+    team = review_req.team
+
+    possible_emails = list(email_queryset)
+
+    aliases = DocAlias.objects.filter(document=doc).values_list("name", flat=True)
 
     reviewers = { r.person_id: r for r in Reviewer.objects.filter(team=team, person__in=[e.person_id for e in possible_emails]) }
 
+    # time since past assignment
     latest_assignment_for_reviewer = dict(ReviewRequest.objects.filter(
         reviewer__in=possible_emails,
     ).values_list("reviewer").annotate(models.Max("time")))
 
+    # previous review of document
+    has_reviewed_previous = ReviewRequest.objects.filter(
+        doc=doc,
+        reviewer__in=possible_emails,
+        state="completed",
+    )
+
+    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("reviewer", flat=True))
+
+    # review indications
+    would_like_to_review = set() # FIXME: fill in
+
+    # connections
+    connections = {}
+    # examine the closest connections last to let them override
+    for e in Email.objects.filter(pk__in=possible_emails, person=doc.ad_id):
+        connections[e] = "is associated Area Director"
+    for r in Role.objects.filter(group=doc.group_id, email__in=possible_emails).select_related("name"):
+        connections[r.email_id] = "is group {}".format(r.name)
+    if doc.shepherd_id:
+        connections[doc.shepherd_id] = "is shepherd of document"
+    for e in DocumentAuthor.objects.filter(document=doc, author__in=possible_emails).values_list("author", flat=True):
+        connections[e] = "is author of document"
+
     now = datetime.datetime.now()
 
-    rankings = []
+    def add_boolean_score(scores, direction, expr, expl):
+        scores.append(int(bool(expr)) * direction)
+        if expr:
+            explanations.append(expl)
+
+    ranking = []
     for e in possible_emails:
         reviewer = reviewers.get(e.person_id)
         if not reviewer:
             reviewer = Reviewer()
 
+        explanations = []
+        scores = [] # build up score in separate independent components
+
         days_past = None
         latest = latest_assignment_for_reviewer.get(e.pk)
         if latest is not None:
             days_past = (now - latest).days - reviewer.frequency
 
-        # FIXME:
-        # positive:  (Perhaps do these separately? As initial values?)
-        # has done review of previous rev
-        # would like to review
-
-        # blocks:
-        # connections to doc + filter_re
-        # has rejected same request/completed partial review
-        # is unavailable_until
-
         if days_past is None:
             ready_for = "first time"
         else:
@@ -315,19 +357,26 @@ def construct_review_request_assignment_choices(possible_emails, team, review_re
                 ready_for = "ready for {} {}".format(d, "day" if d == 1 else "days")
             else:
                 d = -d
-                ready_for = "frequency exceeded - ready in {} {}".format(d, "day" if d == 1 else "days")
+                ready_for = "frequency exceeded, ready in {} {}".format(d, "day" if d == 1 else "days")
+
+        explanations.append(ready_for)
+
+        add_boolean_score(scores, +1, e.pk in has_reviewed_previous, "reviewed document before")
+        add_boolean_score(scores, +1, e.pk in would_like_to_review, "wants to review document")
+        add_boolean_score(scores, -1, e.pk in connections, connections.get(e.pk))
+        add_boolean_score(scores, -1, reviewer.filter_re and any(re.search(reviewer.filter_re, n) for n in aliases), "filter regexp matches")
+        add_boolean_score(scores, -1, reviewer.unavailable_until and reviewer.unavailable_until > now, "unavailable until {}".format((reviewer.unavailable_until or now).strftime("%Y-%m-%d %H:%M:%S")))
 
-        label = "{}: {}".format(e.person, ready_for)
+        scores.append(100000 if days_past is None else days_past)
 
-        rank = (-100000 if days_past is None else -days_past,)
+        label = "{}: {}".format(e.person, "; ".join(explanations))
 
-        rankings.append({
+        ranking.append({
             "email": e,
-            "rank": rank,
+            "scores": scores,
             "label": label,
         })
 
-    rankings.sort(key=lambda r: r["rank"])
+    ranking.sort(key=lambda r: r["scores"], reverse=True)
 
-    # FIXME: empty choices
-    return [(r["email"].pk, r["label"]) for r in rankings]
+    return [(r["email"].pk, r["label"]) for r in ranking]