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

Update some terminology and docstrings.

Browse source 
- Legacy-Id: 16983
This commit is contained in:
Sasha Romijn 2019-11-11 12:31:16 +00:00
parent e518824a69
commit b5a31c3c6a
8 changed files with 58 additions and 25 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
ietf/doc/tests_review.py
View file

@ -33,7 +33,7 @@ from ietf.person.models import Email, Person
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 @@ class ReviewTests(TestCase):
# 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)

4
ietf/doc/views_review.py
View file

@ -33,7 +33,7 @@ from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has
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

4
ietf/group/forms.py
View file

@ -19,7 +19,7 @@ from ietf.group.models import Group, GroupHistory, GroupStateName
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 @@ class ManageReviewRequestForm(forms.Form):
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:

4
ietf/group/tests_review.py
View file

@ -12,7 +12,7 @@ from pyquery import PyQuery
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 @@ class BulkAssignmentTests(TestCase):
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]

8
ietf/group/views.py
View file

@ -92,7 +92,7 @@ from ietf.meeting.utils import group_sessions
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 email_open_review_assignments(request, acronym, group_type=None):
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,
})

51
ietf/review/policies.py
View file

@ -16,12 +16,19 @@ from ietf.review.utils import (current_unavailable_periods_for_reviewers,
get_default_filter_re,
latest_review_assignments_for_reviewers)
def policy_for_team(team):
return RotateWithSkipReviewerPolicy(team)
"""
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.
"""
class AbstractReviewerPolicy:
def get_reviewer_queue_policy(team):
return RotateWithSkipReviewerQueuePolicy(team)
class AbstractReviewerQueuePolicy:
def __init__(self, team):
self.team = team
@ -32,6 +39,12 @@ class AbstractReviewerPolicy:
"""
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 @@ class AbstractReviewerPolicy:
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 @@ class AssignmentOrderResolver:
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 @@ class AssignmentOrderResolver:
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 @@ class AssignmentOrderResolver:
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 @@ class AssignmentOrderResolver:
}
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 @@ class AssignmentOrderResolver:
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 @@ class AssignmentOrderResolver:
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 @@ class AssignmentOrderResolver:
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 @@ class AssignmentOrderResolver:
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

4
ietf/review/test_policies.py
View file

@ -8,7 +8,7 @@ from ietf.group.models import Group
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

4
ietf/review/utils.py
View file

@ -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",