Add a utility function for extracting information about review

requests for a given set of teams/reviewers (making it trivial to
compute statistics), revamp the related doc event code to support this
by referencing the review request directly, add a reviewer overview
page with recent performance for each reviewer as well as
settings/unavailable periods. Fix some bugs and shuffle some of the
review code a bit around.

Finish the importer from the previous Perl-based review tool,
importing log entries, figuring out whether a given review is
early/telechat/last call and fixing corner cases.
 - Legacy-Id: 12080

Author: OleLaursen

ietf/community/models.py

@@ -98,6 +98,9 @@ def notify_events(sender, instance, **kwargs):
     if instance.doc.type_id != 'draft':
         return
 
+    if getattr(instance, "skip_community_list_notification", False):
+        return
+
     from ietf.community.utils import notify_event_to_subscribers
     notify_event_to_subscribers(instance)
 

ietf/doc/migrations/0015_auto_20160927_0713.py

@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import models, migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('name', '0015_insert_review_name_data'),
+        ('review', '0001_initial'),
+        ('doc', '0014_auto_20160824_2218'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='ReviewRequestDocEvent',
+            fields=[
+                ('docevent_ptr', models.OneToOneField(parent_link=True, auto_created=True, primary_key=True, serialize=False, to='doc.DocEvent')),
+                ('review_request', models.ForeignKey(to='review.ReviewRequest')),
+                ('state', models.ForeignKey(blank=True, to='name.ReviewRequestStateName', null=True)),
+            ],
+            options={
+            },
+            bases=('doc.docevent',),
+        ),
+        migrations.AlterField(
+            model_name='docevent',
+            name='type',
+            field=models.CharField(max_length=50, choices=[(b'new_revision', b'Added new revision'), (b'changed_document', b'Changed document metadata'), (b'added_comment', b'Added comment'), (b'deleted', b'Deleted document'), (b'changed_state', b'Changed state'), (b'changed_stream', b'Changed document stream'), (b'expired_document', b'Expired document'), (b'extended_expiry', b'Extended expiry of document'), (b'requested_resurrect', b'Requested resurrect'), (b'completed_resurrect', b'Completed resurrect'), (b'changed_consensus', b'Changed consensus'), (b'published_rfc', b'Published RFC'), (b'added_suggested_replaces', b'Added suggested replacement relationships'), (b'reviewed_suggested_replaces', b'Reviewed suggested replacement relationships'), (b'changed_group', b'Changed group'), (b'changed_protocol_writeup', b'Changed protocol writeup'), (b'changed_charter_milestone', b'Changed charter milestone'), (b'initial_review', b'Set initial review time'), (b'changed_review_announcement', b'Changed WG Review text'), (b'changed_action_announcement', b'Changed WG Action text'), (b'started_iesg_process', b'Started IESG process on document'), (b'created_ballot', b'Created ballot'), (b'closed_ballot', b'Closed ballot'), (b'sent_ballot_announcement', b'Sent ballot announcement'), (b'changed_ballot_position', b'Changed ballot position'), (b'changed_ballot_approval_text', b'Changed ballot approval text'), (b'changed_ballot_writeup_text', b'Changed ballot writeup text'), (b'changed_rfc_editor_note_text', b'Changed RFC Editor Note text'), (b'changed_last_call_text', b'Changed last call text'), (b'requested_last_call', b'Requested last call'), (b'sent_last_call', b'Sent last call'), (b'scheduled_for_telechat', b'Scheduled for telechat'), (b'iesg_approved', b'IESG approved document (no problem)'), (b'iesg_disapproved', b'IESG disapproved document (do not publish)'), (b'approved_in_minute', b'Approved in minute'), (b'iana_review', b'IANA review comment'), (b'rfc_in_iana_registry', b'RFC is in IANA registry'), (b'rfc_editor_received_announcement', b'Announcement was received by RFC Editor'), (b'requested_publication', b'Publication at RFC Editor requested'), (b'sync_from_rfc_editor', b'Received updated information from RFC Editor'), (b'requested_review', b'Requested review'), (b'assigned_review_request', b'Assigned review request'), (b'closed_review_request', b'Closed review request')]),
+            preserve_default=True,
+        ),
+    ]

ietf/doc/models.py

@@ -14,7 +14,7 @@
 
 from ietf.group.models import Group
 from ietf.name.models import ( DocTypeName, DocTagName, StreamName, IntendedStdLevelName, StdLevelName,
-    DocRelationshipName, DocReminderTypeName, BallotPositionName )
+    DocRelationshipName, DocReminderTypeName, BallotPositionName, ReviewRequestStateName )
 from ietf.person.models import Email, Person
 from ietf.utils.admin import admin_link
 
@@ -685,7 +685,8 @@ class DocReminder(models.Model):
 
     # review
     ("requested_review", "Requested review"),
-    ("changed_review_request", "Changed review request"),
+    ("assigned_review_request", "Assigned review request"),
+    ("closed_review_request", "Closed review request"),
     ]
 
 class DocEvent(models.Model):
@@ -816,11 +817,14 @@ class TelechatDocEvent(DocEvent):
     telechat_date = models.DateField(blank=True, null=True)
     returning_item = models.BooleanField(default=False)
 
+class ReviewRequestDocEvent(DocEvent):
+    review_request = models.ForeignKey('review.ReviewRequest')
+    state = models.ForeignKey(ReviewRequestStateName, blank=True, null=True)
+
 # charter events
 class InitialReviewDocEvent(DocEvent):
     expires = models.DateTimeField(blank=True, null=True)
 
-
 # dumping store for removed events
 class DeletedEvent(models.Model):
     content_type = models.ForeignKey(ContentType)

ietf/doc/tests_review.py

@@ -131,7 +131,7 @@ def test_close_request(self):
         review_req = reload_db_objects(review_req)
         self.assertEqual(review_req.state_id, "withdrawn")
         e = doc.latest_event()
-        self.assertEqual(e.type, "changed_review_request")
+        self.assertEqual(e.type, "closed_review_request")
         self.assertTrue("closed" in e.desc.lower())
         self.assertEqual(len(outbox), 1)
         self.assertTrue("closed" in unicode(outbox[0]).lower())
@@ -255,7 +255,7 @@ def test_assign_reviewer(self):
             reviewer=plain_email,
         )
 
-        reviewer_settings = ReviewerSettings.objects.get(person__email=plain_email)
+        reviewer_settings = ReviewerSettings.objects.get(person__email=plain_email, team=review_req.team)
         reviewer_settings.filter_re = doc.name
         reviewer_settings.skip_next = 1
         reviewer_settings.save()
@@ -383,7 +383,7 @@ def test_reject_reviewer_assignment(self):
         review_req = reload_db_objects(review_req)
         self.assertEqual(review_req.state_id, "rejected")
         e = doc.latest_event()
-        self.assertEqual(e.type, "changed_review_request")
+        self.assertEqual(e.type, "closed_review_request")
         self.assertTrue("rejected" in e.desc)
         self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="requested").count(), 1)
         self.assertEqual(len(outbox), 1)

ietf/doc/views_review.py

@@ -9,7 +9,8 @@
 from django.template.loader import render_to_string
 from django.core.urlresolvers import reverse as urlreverse
 
-from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent, State, DocAlias, LastCallDocEvent
+from ietf.doc.models import (Document, NewRevisionDocEvent, State, DocAlias,
+                             LastCallDocEvent, ReviewRequestDocEvent)
 from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName
 from ietf.review.models import ReviewRequest
 from ietf.group.models import Group
@@ -103,12 +104,14 @@ def request_review(request, name):
                 review_req.team = team
                 review_req.save()
 
-                DocEvent.objects.create(
+                ReviewRequestDocEvent.objects.create(
                     type="requested_review",
                     doc=doc,
                     by=request.user.person,
                     desc="Requested {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()),
                     time=review_req.time,
+                    review_request=review_req,
+                    state=None,
                 )
 
             return redirect('doc_view', name=doc.name)
@@ -272,22 +275,24 @@ def reject_reviewer_assignment(request, name, request_id):
             review_req.state = ReviewRequestStateName.objects.get(slug="rejected")
             review_req.save()
 
-            DocEvent.objects.create(
-                type="changed_review_request",
+            ReviewRequestDocEvent.objects.create(
+                type="closed_review_request",
                 doc=review_req.doc,
                 by=request.user.person,
                 desc="Assignment of request for {} review by {} to {} was rejected".format(
                     review_req.type.name,
                     review_req.team.acronym.upper(),
                     review_req.reviewer.person,
                 ),
+                review_request=review_req,
+                state=review_req.state,
             )
 
             # make a new unassigned review request
             new_review_req = make_new_review_request_from_existing(review_req)
             new_review_req.save()
 
-            msg = render_to_string("doc/mail/reviewer_assignment_rejected.txt", {
+            msg = render_to_string("review/reviewer_assignment_rejected.txt", {
                 "by": request.user.person,
                 "message_to_secretary": form.cleaned_data.get("message_to_secretary")
             })
@@ -393,7 +398,7 @@ def complete_review(request, name, request_id):
                     strip_prefix(review_req.doc.name, "draft-"),
                     form.cleaned_data["reviewed_rev"],
                     review_req.team.acronym,
-                    review_req.type.slug if review_req.type.slug != "unknown" else "",
+                    review_req.type.slug,
                     review_req.reviewer.person.ascii_parts()[3],
                     datetime.date.today().isoformat(),
                 ]
@@ -403,8 +408,16 @@ def complete_review(request, name, request_id):
                 name = "-".join(c for c in name_components if c).lower()
                 if not Document.objects.filter(name=name).exists():
                     review = Document.objects.create(name=name)
+                    DocAlias.objects.create(document=review, name=review.name)
                     break
 
+            review.type = DocTypeName.objects.get(slug="review")
+            review.rev = "00"
+            review.title = "{} Review of {}-{}".format(review_req.type.name, review_req.doc.name, form.cleaned_data["reviewed_rev"])
+            review.group = review_req.team
+            if review_submission == "link":
+                review.external_url = form.cleaned_data['review_url']
+
             e = NewRevisionDocEvent.objects.create(
                 type="new_revision",
                 doc=review,
@@ -414,15 +427,9 @@ def complete_review(request, name, request_id):
                 time=review.time,
             )
 
-            review.type = DocTypeName.objects.get(slug="review")
-            review.rev = "00"
-            review.title = "{} Review of {}-{}".format(review_req.type.name, review_req.doc.name, form.cleaned_data["reviewed_rev"])
-            review.group = review_req.team
-            if review_submission == "link":
-                review.external_url = form.cleaned_data['review_url']
-            review.save_with_history([e])
             review.set_state(State.objects.get(type="review", slug="active"))
-            DocAlias.objects.create(document=review, name=review.name)
+
+            review.save_with_history([e])
 
             # save file on disk
             if review_submission == "upload":
@@ -441,17 +448,25 @@ def complete_review(request, name, request_id):
             review_req.review = review
             review_req.save()
 
-            DocEvent.objects.create(
-                type="changed_review_request",
+            need_to_email_review = review_submission != "link" and review_req.team.list_email
+
+            desc = "Request for {} review by {} {}: {}. Reviewer: {}.".format(
+                review_req.type.name,
+                review_req.team.acronym.upper(),
+                review_req.state.name,
+                review_req.result.name,
+                review_req.reviewer.person,
+            )
+            if need_to_email_review:
+                desc += " " + "Sent review to list."
+
+            close_event = ReviewRequestDocEvent.objects.create(
+                type="closed_review_request",
                 doc=review_req.doc,
                 by=request.user.person,
-                desc="Request for {} review by {} {}: {}. Reviewer: {}".format(
-                    review_req.type.name,
-                    review_req.team.acronym.upper(),
-                    review_req.state.name,
-                    review_req.result.name,
-                    review_req.reviewer,
-                ),
+                desc=desc,
+                review_request=review_req,
+                state=review_req.state,
             )
 
             if review_req.state_id == "part-completed":
@@ -463,36 +478,29 @@ def complete_review(request, name, request_id):
                 url = urlreverse("ietf.doc.views_review.review_request", kwargs={ "name": new_review_req.doc.name, "request_id": new_review_req.pk })
                 url = request.build_absolute_uri(url)
 
-                msg = render_to_string("doc/mail/partially_completed_review.txt", {
+                msg = render_to_string("review/partially_completed_review.txt", {
                     'new_review_req_url': url,
                     "by": request.user.person,
                     "new_review_req": new_review_req,
                 })
 
                 email_review_request_change(request, review_req, subject, msg, request.user.person, notify_secretary=True, notify_reviewer=False, notify_requested_by=False)
 
-            if review_submission != "link" and review_req.team.list_email:
+            if need_to_email_review:
                 # email the review
                 subject = "{} of {}-{}".format("Partial review" if review_req.state_id == "part-completed" else "Review", review_req.doc.name, review_req.reviewed_rev)
                 msg = send_mail(request, [(review_req.team.name, review_req.team.list_email)], None,
                                 subject,
-                                "doc/mail/completed_review.txt", {
+                                "review/completed_review.txt", {
                                     "review_req": review_req,
                                     "content": encoded_content.decode("utf-8"),
                                 },
                                 cc=form.cleaned_data["cc"])
 
-                e = DocEvent.objects.create(
-                    type="changed_review_request",
-                    doc=review_req.doc,
-                    by=request.user.person,
-                    desc="Sent review to list.",
-                )
-
                 list_name = mailarch.list_name_from_email(review_req.team.list_email)
                 if list_name:
                     review.external_url = mailarch.construct_message_url(list_name, email.utils.unquote(msg["Message-ID"]))
-                    review.save_with_history([e])
+                    review.save_with_history([close_event])
 
             return redirect("doc_view", name=review_req.review.name)
     else:

ietf/group/features.py

@@ -31,7 +31,7 @@ def __init__(self, group):
         if group in active_review_teams():
             self.has_reviews = True
             import ietf.group.views
-            self.default_tab = ietf.group.views.review_requests
+            self.default_tab = ietf.group.views_review.review_requests
 
         if group.type_id == "dir":
             self.admin_roles = ["chair", "secr"]

ietf/group/tests_info.py

@@ -31,7 +31,6 @@
 from ietf.utils.mail import outbox, empty_outbox
 from ietf.utils.test_data import make_test_data, create_person, make_review_data
 from ietf.utils.test_utils import login_testing_unauthorized, TestCase, unicontent, reload_db_objects
-import ietf.group.views
 
 def group_urlreverse_list(group, viewname):
     return [
@@ -336,31 +335,6 @@ def test_materials(self):
         self.assertEqual(r.status_code, 200)
         self.assertTrue(doc.title not in unicontent(r))
 
-    def test_review_requests(self):
-        doc = make_test_data()
-        review_req = make_review_data(doc)
-
-        group = review_req.team
-
-        for url in [ urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym }),
-                     urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym , 'group_type': group.type_id}),
-                   ]:
-            r = self.client.get(url)
-            self.assertEqual(r.status_code, 200)
-            self.assertTrue(review_req.doc.name in unicontent(r))
-            self.assertTrue(unicode(review_req.reviewer.person) in unicontent(r))
-
-        url = urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym })
-
-        # close request, listed under closed
-        review_req.state_id = "completed"
-        review_req.result_id = "ready"
-        review_req.save()
-
-        r = self.client.get(url)
-        self.assertEqual(r.status_code, 200)
-        self.assertTrue(review_req.doc.name in unicontent(r))
-
     def test_history(self):
         draft = make_test_data()
         group = draft.group

ietf/group/tests_review.py

@@ -15,6 +15,31 @@
 from ietf.utils.mail import outbox, empty_outbox
 
 class ReviewTests(TestCase):
+    def test_review_requests(self):
+        doc = make_test_data()
+        review_req = make_review_data(doc)
+
+        group = review_req.team
+
+        for url in [ urlreverse(ietf.group.views_review.review_requests, kwargs={ 'acronym': group.acronym }),
+                     urlreverse(ietf.group.views_review.review_requests, kwargs={ 'acronym': group.acronym , 'group_type': group.type_id}),
+                   ]:
+            r = self.client.get(url)
+            self.assertEqual(r.status_code, 200)
+            self.assertTrue(review_req.doc.name in unicontent(r))
+            self.assertTrue(unicode(review_req.reviewer.person) in unicontent(r))
+
+        url = urlreverse(ietf.group.views_review.review_requests, kwargs={ 'acronym': group.acronym })
+
+        # close request, listed under closed
+        review_req.state_id = "completed"
+        review_req.result_id = "ready"
+        review_req.save()
+
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(review_req.doc.name in unicontent(r))
+
     def test_suggested_review_requests(self):
         doc = make_test_data()
         review_req = make_review_data(doc)

ietf/group/urls_info_details.py

@@ -30,7 +30,7 @@
     (r'^materials/new/(?P<doc_type>[\w-]+)/$', 'ietf.doc.views_material.edit_material', { 'action': "new" }, "group_new_material"),
     (r'^archives/$', 'ietf.group.views.derived_archives'),
     (r'^photos/$', views.group_photos),
-    (r'^reviews/$', views.review_requests),
+    (r'^reviews/$', views_review.review_requests),
     (r'^reviews/manage/$', views_review.manage_review_requests),
     (r'^reviews/email-assignments/$', views_review.email_open_review_assignments),
     (r'^reviewers/$', views_review.reviewer_overview),