Take a bunch of factors into account when sorting reviewers for
assignment to a review request - Legacy-Id: 11626
This commit is contained in:
Ole Laursen
parent
5c8be91b08
commit
db4ea01e20
|
|
@ -12,10 +12,11 @@ from pyquery import PyQuery
|
|||
|
||||
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 @@ class ReviewTests(TestCase):
|
|||
|
||||
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 @@ class ReviewTests(TestCase):
|
|||
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()
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ from ietf.review.utils import (active_review_teams, assign_review_request_to_rev
|
|||
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
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ from ietf.review.utils import (can_manage_review_requests_for_team, close_review
|
|||
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 @@ class ManageReviewRequestForm(forms.Form):
|
|||
|
||||
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:
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
||||
label = "{}: {}".format(e.person, ready_for)
|
||||
explanations.append(ready_for)
|
||||
|
||||
rank = (-100000 if days_past is None else -days_past,)
|
||||
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")))
|
||||
|
||||
rankings.append({
|
||||
scores.append(100000 if days_past is None else days_past)
|
||||
|
||||
label = "{}: {}".format(e.person, "; ".join(explanations))
|
||||
|
||||
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]
|
||||
|
|
|
|||
Loading…
Reference in a new issue