From 99beb58291714ab5a357eb64bdc516f2211e52a5 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Wed, 21 Sep 2016 16:52:50 +0000 Subject: [PATCH] Implement a round-robin rotation scheme for reviewers instead of relying on when they last reviewed. In-order assignments automatically move the rotation forwards while out-of-order assignments increment the skip next of the assigned reviewer. Include rotation in open review assignments email. Fix a couple of issues in the importer. - Legacy-Id: 12015 --- ietf/doc/tests_review.py | 115 ++++++++++++- ietf/group/tests_review.py | 7 +- ietf/group/views_review.py | 6 +- ietf/review/import_from_review_tool.py | 24 ++- ietf/review/migrations/0001_initial.py | 11 ++ ietf/review/models.py | 7 + ietf/review/resources.py | 21 ++- ietf/review/utils.py | 162 ++++++++++++++---- .../group/email_open_review_assignments.txt | 6 +- ietf/utils/test_data.py | 3 +- 10 files changed, 313 insertions(+), 49 deletions(-) diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index 4c8709180..54eabbe6f 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -12,13 +12,16 @@ from pyquery import PyQuery import debug # pyflakes:ignore -from ietf.review.models import ReviewRequest, ReviewTeamResult, ReviewerSettings, ReviewWish, UnavailablePeriod +from ietf.review.models import (ReviewRequest, ReviewTeamResult, ReviewerSettings, + ReviewWish, UnavailablePeriod, NextReviewerInTeam) +from ietf.review.utils import reviewer_rotation_list, possibly_advance_next_reviewer_for_team import ietf.review.mailarch from ietf.person.models import Email, Person from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewTypeName, DocRelationshipName +from ietf.group.models import Group from ietf.doc.models import DocumentAuthor, Document, DocAlias, RelatedDocument, DocEvent from ietf.utils.test_utils import TestCase -from ietf.utils.test_data import make_test_data, make_review_data +from ietf.utils.test_data import make_test_data, make_review_data, create_person from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects from ietf.utils.mail import outbox, empty_outbox @@ -133,6 +136,94 @@ class ReviewTests(TestCase): self.assertEqual(len(outbox), 1) self.assertTrue("closed" in unicode(outbox[0]).lower()) + def make_data_for_rotation_tests(self, doc): + team = Group.objects.create(state_id="active", acronym="rotationteam", name="Review Team", type_id="dir", + list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut")) + + # make a bunch of reviewers + reviewers = [ + create_person(team, "reviewer", name="Test Reviewer{}".format(i), username="testreviewer{}".format(i)) + for i in range(5) + ] + + self.assertEqual(reviewers, reviewer_rotation_list(team)) + + return team, reviewers + + def test_possibly_advance_next_reviewer_for_team(self): + doc = make_test_data() + + team, reviewers = self.make_data_for_rotation_tests(doc) + + def get_skip_next(person): + settings = (ReviewerSettings.objects.filter(team=team, person=person).first() + or ReviewerSettings(team=team)) + return settings.skip_next + + possibly_advance_next_reviewer_for_team(team, reviewers[0].pk) + 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) + + possibly_advance_next_reviewer_for_team(team, reviewers[1].pk) + self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2]) + + # skip reviewer 2 + possibly_advance_next_reviewer_for_team(team, reviewers[3].pk) + self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2]) + 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]), 1) + + # pick reviewer 2, use up reviewer 3's skip_next + possibly_advance_next_reviewer_for_team(team, reviewers[2].pk) + self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4]) + 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) + + # check wrap-around + possibly_advance_next_reviewer_for_team(team, reviewers[4].pk) + self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[0]) + 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) + + # unavailable + today = datetime.date.today() + UnavailablePeriod.objects.create(team=team, person=reviewers[1], start_date=today, end_date=today, availability="unavailable") + possibly_advance_next_reviewer_for_team(team, reviewers[0].pk) + self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2]) + 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) + + # pick unavailable anyway + possibly_advance_next_reviewer_for_team(team, reviewers[1].pk) + self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2]) + self.assertEqual(get_skip_next(reviewers[0]), 0) + self.assertEqual(get_skip_next(reviewers[1]), 1) + self.assertEqual(get_skip_next(reviewers[2]), 0) + self.assertEqual(get_skip_next(reviewers[3]), 0) + self.assertEqual(get_skip_next(reviewers[4]), 0) + + # not through min_interval + ReviewRequest.objects.create(team=team, doc=doc, type_id="early", state_id="accepted", deadline=today, requested_by=reviewers[0], reviewer=reviewers[2].email_set.first()) + possibly_advance_next_reviewer_for_team(team, reviewers[3].pk) + self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4]) + self.assertEqual(get_skip_next(reviewers[0]), 0) + self.assertEqual(get_skip_next(reviewers[1]), 1) + self.assertEqual(get_skip_next(reviewers[2]), 0) + self.assertEqual(get_skip_next(reviewers[3]), 0) + self.assertEqual(get_skip_next(reviewers[4]), 0) + def test_assign_reviewer(self): doc = make_test_data() @@ -166,6 +257,7 @@ class ReviewTests(TestCase): reviewer_settings = ReviewerSettings.objects.get(person__email=plain_email) reviewer_settings.filter_re = doc.name + reviewer_settings.skip_next = 1 reviewer_settings.save() UnavailablePeriod.objects.create( @@ -177,6 +269,14 @@ class ReviewTests(TestCase): ReviewWish.objects.create(person=plain_email.person, team=review_req.team, doc=doc) + # pick a non-existing reviewer as next to see that we can + # handle reviewers who have left + NextReviewerInTeam.objects.filter(team=review_req.team).delete() + NextReviewerInTeam.objects.create( + team=review_req.team, + next_reviewer=Person.objects.exclude(pk=plain_email.person_id).first(), + ) + assign_url = urlreverse('ietf.doc.views_review.assign_reviewer', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -194,16 +294,18 @@ class ReviewTests(TestCase): 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("wishes to review", plain_label) self.assertIn("is author", plain_label) self.assertIn("regexp matches", plain_label) - self.assertIn("unavailable", plain_label) + self.assertIn("unavailable indefinitely", plain_label) + self.assertIn("skip next 1", plain_label) + self.assertIn("#1", plain_label) # assign empty_outbox() - reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team).first() + rotation_list = reviewer_rotation_list(review_req.team) + 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) @@ -212,12 +314,13 @@ class ReviewTests(TestCase): self.assertEqual(review_req.reviewer, reviewer) self.assertEqual(len(outbox), 1) self.assertTrue("assigned" in unicode(outbox[0])) + self.assertEqual(NextReviewerInTeam.objects.get(team=review_req.team).next_reviewer, rotation_list[1]) # re-assign empty_outbox() review_req.state = ReviewRequestStateName.objects.get(slug="accepted") review_req.save() - reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team).exclude(pk=reviewer.pk).first() + reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team, person=rotation_list[1]).first() r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk }) self.assertEqual(r.status_code, 302) diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index 72da1c92c..85260a67b 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -1,6 +1,6 @@ import datetime -#from pyquery import PyQuery +from pyquery import PyQuery from django.core.urlresolvers import reverse as urlreverse @@ -175,7 +175,10 @@ class ReviewTests(TestCase): r = self.client.get(url) self.assertEqual(r.status_code, 200) - self.assertTrue(review_req1.doc.name in unicontent(r)) + q = PyQuery(r.content) + generated_text = q("[name=body]").text() + self.assertTrue(review_req1.doc.name in generated_text) + self.assertTrue(unicode(Person.objects.get(user__username="marschairman")) in generated_text) empty_outbox() r = self.client.post(url, { diff --git a/ietf/group/views_review.py b/ietf/group/views_review.py index 40cda50eb..108a4d189 100644 --- a/ietf/group/views_review.py +++ b/ietf/group/views_review.py @@ -16,7 +16,8 @@ from ietf.review.utils import (can_manage_review_requests_for_team, close_review suggested_review_requests_for_team, unavailability_periods_to_list, current_unavailable_periods_for_reviewers, - email_reviewer_availability_change) + email_reviewer_availability_change, + reviewer_rotation_list) from ietf.group.models import Role from ietf.group.utils import get_group_or_404 from ietf.person.fields import PersonEmailChoiceField @@ -221,9 +222,10 @@ def email_open_review_assignments(request, acronym, group_type=None): else: to = group.list_email subject = "Open review assignments in {}".format(group.acronym) - # FIXME: add rotation info + body = render_to_string("group/email_open_review_assignments.txt", { "review_requests": review_requests, + "rotation_list": reviewer_rotation_list(group)[:10], }) form = EmailOpenAssignmentsForm(initial={ diff --git a/ietf/review/import_from_review_tool.py b/ietf/review/import_from_review_tool.py index 8e5706997..5da78119d 100755 --- a/ietf/review/import_from_review_tool.py +++ b/ietf/review/import_from_review_tool.py @@ -18,7 +18,7 @@ from collections import namedtuple from django.db import connections from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewResultName, ReviewRequestStateName, ReviewTypeName, ReviewTeamResult, - UnavailablePeriod) + UnavailablePeriod, NextReviewerInTeam) from ietf.group.models import Group, Role, RoleName from ietf.person.models import Person, Email, Alias import argparse @@ -138,9 +138,6 @@ with db_con.cursor() as c: availability="unavailable", ) - -# review requests - # check that we got the needed names results = { n.name.lower(): n for n in ReviewResultName.objects.all() } @@ -150,8 +147,23 @@ with db_con.cursor() as c: missing_result_names = set(summaries) - set(results.keys()) assert not missing_result_names, "missing result names: {} {}".format(missing_result_names, results.keys()) - for s in summaries: - ReviewTeamResult.objects.get_or_create(team=team, result=results[s]) + +# configuration options +with db_con.cursor() as c: + c.execute("select * from config;") + + for row in namedtuplefetchall(c): + if row.name == "next": # next reviewer + NextReviewerInTeam.objects.filter(team=team).delete() + NextReviewerInTeam.objects.create(team=team, next_reviewer=known_personnel[row.value].person) + + if row.name == "summary-list": # review results used in team + summaries = [v.strip().lower() for v in row.value.split(";") if v.strip()] + + for s in summaries: + ReviewTeamResult.objects.get_or_create(team=team, result=results[s]) + +# review requests states = { n.slug: n for n in ReviewRequestStateName.objects.all() } # map some names diff --git a/ietf/review/migrations/0001_initial.py b/ietf/review/migrations/0001_initial.py index 431600ce6..ec090266c 100644 --- a/ietf/review/migrations/0001_initial.py +++ b/ietf/review/migrations/0001_initial.py @@ -15,6 +15,17 @@ class Migration(migrations.Migration): ] operations = [ + migrations.CreateModel( + name='NextReviewerInTeam', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('next_reviewer', models.ForeignKey(to='person.Person')), + ('team', models.ForeignKey(to='group.Group')), + ], + options={ + }, + bases=(models.Model,), + ), migrations.CreateModel( name='ReviewerSettings', fields=[ diff --git a/ietf/review/models.py b/ietf/review/models.py index 52ce2d42f..c60825576 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -77,6 +77,13 @@ class ReviewTeamResult(models.Model): def __unicode__(self): return u"{} in {}".format(self.result.name, self.group.acronym) +class NextReviewerInTeam(models.Model): + team = models.ForeignKey(Group) + next_reviewer = models.ForeignKey(Person) + + def __unicode__(self): + return u"{} next in {}".format(self.next_reviewer, self.team) + class ReviewRequest(models.Model): """Represents a request for a review and the process it goes through. There should be one ReviewRequest entered for each combination of diff --git a/ietf/review/resources.py b/ietf/review/resources.py index be7364c7c..638f385b6 100644 --- a/ietf/review/resources.py +++ b/ietf/review/resources.py @@ -8,7 +8,7 @@ from ietf import api from ietf.api import ToOneField # pyflakes:ignore from ietf.review.models import (ReviewerSettings, ReviewRequest, ReviewTeamResult, - UnavailablePeriod, ReviewWish) + UnavailablePeriod, ReviewWish, NextReviewerInTeam) from ietf.person.resources import PersonResource @@ -125,3 +125,22 @@ class ReviewWishResource(ModelResource): } api.review.register(ReviewWishResource()) + + +from ietf.person.resources import PersonResource +from ietf.group.resources import GroupResource +class NextReviewerInTeamResource(ModelResource): + team = ToOneField(GroupResource, 'team') + next_reviewer = ToOneField(PersonResource, 'next_reviewer') + class Meta: + queryset = NextReviewerInTeam.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'nextreviewerinteam' + filtering = { + "id": ALL, + "team": ALL_WITH_RELATIONS, + "next_reviewer": ALL_WITH_RELATIONS, + } +api.review.register(NextReviewerInTeamResource()) + diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 021586c3a..abfaf56af 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -1,8 +1,7 @@ import datetime, re from collections import defaultdict -from django.db import models -from django.db.models import Q +from django.db.models import Q, Max from django.core.urlresolvers import reverse as urlreverse from ietf.group.models import Group, Role @@ -11,7 +10,7 @@ from ietf.iesg.models import TelechatDate 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, - ReviewerSettings, UnavailablePeriod, ReviewWish) + ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam) from ietf.utils.mail import send_mail from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs @@ -72,6 +71,56 @@ def current_unavailable_periods_for_reviewers(team): return res +def reviewer_rotation_list(team): + """Returns person id -> index in rotation (next reviewer has index 0).""" + reviewers = list(Person.objects.filter(role__name="reviewer", role__group=team)) + reviewers.sort(key=lambda p: p.last_name()) + + next_reviewer_index = 0 + + # now to figure out where the rotation is currently at + saved_reviewer = NextReviewerInTeam.objects.filter(team=team).select_related("next_reviewer").first() + if saved_reviewer: + n = saved_reviewer.next_reviewer + + if n not in reviewers: + # saved reviewer might not still be here, if not just + # insert and use that position (Python will wrap around, + # so no harm done by using the index on the original list + # afterwards) + reviewers_with_next = reviewers[:] + [n] + reviewers_with_next.sort(key=lambda p: p.last_name()) + next_reviewer_index = reviewers_with_next.index(n) + else: + next_reviewer_index = reviewers.index(n) + + return reviewers[next_reviewer_index:] + reviewers[:next_reviewer_index] + +def days_needed_to_fulfill_min_interval_for_reviewers(team): + """Returns person_id -> days needed until min_interval is fulfilled for + reviewer.""" + latest_assignments = dict(ReviewRequest.objects.filter( + team=team, + ).values_list("reviewer__person").annotate(Max("time"))) + + min_intervals = dict(ReviewerSettings.objects.filter(team=team).values_list("person_id", "min_interval")) + + default_min_interval = ReviewerSettings(team=team).min_interval + + now = datetime.datetime.now() + + res = {} + for person_id, latest_assignment_time in latest_assignments.iteritems(): + if latest_assignment_time is not None: + min_interval = min_intervals.get(person_id, default_min_interval) + + days_needed = max(0, min_interval - (now - latest_assignment_time).days) + + if days_needed > 0: + res[person_id] = days_needed + + return res + def make_new_review_request_from_existing(review_req): obj = ReviewRequest() obj.time = review_req.time @@ -184,6 +233,8 @@ def assign_review_request_to_reviewer(request, review_req, reviewer): review_req.reviewer = reviewer review_req.save() + possibly_advance_next_reviewer_for_team(review_req.team, review_req.reviewer.person_id) + DocEvent.objects.create( type="changed_review_request", doc=review_req.doc, @@ -201,6 +252,59 @@ def assign_review_request_to_reviewer(request, review_req, reviewer): "%s has assigned you to review the document." % request.user.person, by=request.user.person, notify_secretary=False, notify_reviewer=True, notify_requested_by=False) +def possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id): + # prune reviewers not in the rotation (but not the assigned + # reviewer who must have been available for assignment anyway) + reviewers_to_skip = set() + + unavailable_periods = current_unavailable_periods_for_reviewers(team) + for person_id, periods in unavailable_periods.iteritems(): + if periods and person_id != assigned_review_to_person_id: + reviewers_to_skip.add(person_id) + + days_needed_for_reviewers = days_needed_to_fulfill_min_interval_for_reviewers(team) + for person_id, days_needed in days_needed_for_reviewers.iteritems(): + if days_needed > 0 and person_id != assigned_review_to_person_id: + reviewers_to_skip.add(person_id) + + rotation_list = [p.pk for p in reviewer_rotation_list(team) if p.pk not in reviewers_to_skip] + + if not rotation_list: + return + + def reviewer_at_index(i): + return rotation_list[i % len(rotation_list)] + + def reviewer_settings_for(person_id): + return (ReviewerSettings.objects.filter(team=team, person=person_id).first() + or ReviewerSettings(team=team, person_id=person_id)) + + current_i = 0 + + if reviewer_at_index(current_i) == assigned_review_to_person_id: + # move 1 ahead + current_i += 1 + else: + settings = reviewer_settings_for(assigned_review_to_person_id) + settings.skip_next += 1 + settings.save() + + while True: + # as a clean-up step go through any with a skip next > 0 + current_reviewer_person_id = reviewer_at_index(current_i) + settings = reviewer_settings_for(current_reviewer_person_id) + if settings.skip_next > 0: + settings.skip_next -= 1 + settings.save() + + current_i += 1 + else: + nr = NextReviewerInTeam.objects.filter(team=team).first() or NextReviewerInTeam(team=team) + nr.next_reviewer_id = current_reviewer_person_id + nr.save() + + break + def close_review_request(request, review_req, close_state): suggested_req = review_req.pk is None @@ -384,10 +488,11 @@ def make_assignment_choices(email_queryset, review_req): if p not in reviewer_settings: reviewer_settings[p] = ReviewerSettings() - # time since past assignment - latest_assignment_for_reviewer = dict(ReviewRequest.objects.filter( - reviewer__in=possible_emails, - ).values_list("reviewer").annotate(models.Max("time"))) + # frequency + days_needed_for_reviewers = days_needed_to_fulfill_min_interval_for_reviewers(team) + + # rotation + rotation_index = { p.pk: i for i, p in enumerate(reviewer_rotation_list(team)) } # previous review of document has_reviewed_previous = ReviewRequest.objects.filter( @@ -419,28 +524,10 @@ def make_assignment_choices(email_queryset, review_req): # unavailable periods unavailable_periods = current_unavailable_periods_for_reviewers(team) - now = datetime.datetime.now() - ranking = [] for e in possible_emails: settings = reviewer_settings.get(e.person_id) - days_past = None - latest = latest_assignment_for_reviewer.get(e.pk) - if latest is not None: - days_past = (now - latest).days - settings.min_interval - - if days_past is None: - ready_for = "first time" - else: - d = int(round(days_past)) - if d > 0: - ready_for = "ready for {} {}".format(d, "day" if d == 1 else "days") - else: - d = -d - ready_for = "max frequency exceeded, ready in {} {}".format(d, "day" if d == 1 else "days") - - # we sort the reviewers by separate axes, listing the most # important things first scores = [] @@ -451,8 +538,7 @@ def make_assignment_choices(email_queryset, review_req): if expr and explanation: explanations.append(explanation) - explanations.append(ready_for) # show ready for explanation first, but sort it after the other issues - + # unavailable for review periods periods = unavailable_periods.get(e.person_id, []) unavailable_at_the_moment = periods and not (e.pk in has_reviewed_previous and all(p.availability == "canfinish" for p in periods)) add_boolean_score(-1, unavailable_at_the_moment) @@ -466,15 +552,31 @@ def make_assignment_choices(email_queryset, review_req): if periods: explanations.append(", ".join(format_period(p) for p in periods)) - + + # minimum interval between reviews + days_needed = days_needed_for_reviewers.get(e.person_id, 0) + scores.append(-days_needed) + if days_needed > 0: + explanations.append("max frequency exceeded, ready in {} {}".format(days_needed, "day" if days_needed == 1 else "days")) + + # misc add_boolean_score(+1, e.pk in has_reviewed_previous, "reviewed document before") add_boolean_score(+1, e.person_id in wish_to_review, "wishes to review document") add_boolean_score(-1, e.pk in connections, connections.get(e.pk)) # reviewer is somehow connected: bad add_boolean_score(-1, settings.filter_re and any(re.search(settings.filter_re, n) for n in aliases), "filter regexp matches") - scores.append(100000 if days_past is None else days_past) + # skip next + scores.append(-settings.skip_next) + if settings.skip_next > 0: + explanations.append("skip next {}".format(settings.skip_next)) - label = "{}: {}".format(e.person, "; ".join(explanations)) + index = rotation_index.get(e.person_id, 0) + scores.append(-index) + explanations.append("#{}".format(index + 1)) + + label = unicode(e.person) + if explanations: + label = u"{}: {}".format(label, u"; ".join(explanations)) ranking.append({ "email": e, diff --git a/ietf/templates/group/email_open_review_assignments.txt b/ietf/templates/group/email_open_review_assignments.txt index c9bb458e4..ef6b3f692 100644 --- a/ietf/templates/group/email_open_review_assignments.txt +++ b/ietf/templates/group/email_open_review_assignments.txt @@ -1,4 +1,8 @@ {% autoescape off %} Reviewer Deadline Draft {% for r in review_requests %}{{ r.reviewer.person.plain_name|ljust:"22" }} {{ r.deadline|date:"Y-m-d" }} {{ r.doc_id }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %} -{% endfor %}{% endautoescape %} +{% endfor %} +{% if rotation_list %}Next in the reviewer rotation: + +{% for p in rotation_list %} {{ p }} +{% endfor %}{% endif %}{% endautoescape %} diff --git a/ietf/utils/test_data.py b/ietf/utils/test_data.py index cd0f9235d..36f033548 100644 --- a/ietf/utils/test_data.py +++ b/ietf/utils/test_data.py @@ -33,6 +33,7 @@ def create_person(group, role_name, name=None, username=None, email_address=None person = Person.objects.create(name=name, ascii=name, user=user) email = Email.objects.create(address=email_address, person=person) Role.objects.create(group=group, name_id=role_name, person=person, email=email) + return person def create_group(**kwargs): return Group.objects.create(state_id="active", **kwargs) @@ -393,7 +394,7 @@ def make_test_data(): return draft def make_review_data(doc): - team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="dir", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) + team = create_group(acronym="reviewteam", name="Review Team", type_id="dir", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut")) for r in ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"]): ReviewTeamResult.objects.create(team=team, result=r)