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

@@ -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)
 

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, {

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={

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

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=[

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

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())
+

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,

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 %}

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)