Add review request page for review teams and first draft of manage

review requests page.

Add importer for importing review data from the existing Perl tool
(WIP, gets most but not all of the interesting information out).

Fix various bugs.
 - Legacy-Id: 11508

Author: OleLaursen

ietf/doc/tests_review.py

@@ -12,42 +12,15 @@
 
 import debug                            # pyflakes:ignore
 
-from ietf.review.models import ReviewRequest, Reviewer
+from ietf.review.models import ReviewRequest, ReviewTeamResult
 import ietf.review.mailarch
-from ietf.person.models import Person
-from ietf.group.models import Group, Role
+from ietf.person.models import Email
 from ietf.name.models import ReviewResultName, ReviewRequestStateName
 from ietf.utils.test_utils import TestCase
-from ietf.utils.test_data import make_test_data
+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
 from ietf.utils.mail import outbox, empty_outbox
 
-def make_review_data(doc):
-    team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team")
-    team.reviewresultname_set.add(ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"]))
-
-    p = Person.objects.get(user__username="plain")
-    role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team)
-    Reviewer.objects.create(team=team, person=p, frequency=14, skip_next=0)
-
-    review_req = ReviewRequest.objects.create(
-        doc=doc,
-        team=team,
-        type_id="early",
-        deadline=datetime.datetime.now() + datetime.timedelta(days=20),
-        state_id="ready",
-        reviewer=role,
-        reviewed_rev="01",
-    )
-
-    p = Person.objects.get(user__username="marschairman")
-    role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team)
-
-    p = Person.objects.get(user__username="secretary")
-    role = Role.objects.create(name_id="secretary", person=p, email=p.email_set.first(), group=team)
-    
-    return review_req
-
 class ReviewTests(TestCase):
     def setUp(self):
         self.review_dir = os.path.abspath("tmp-review-dir")
@@ -169,7 +142,7 @@ def test_assign_reviewer(self):
 
         # assign
         empty_outbox()
-        reviewer = Role.objects.filter(name="reviewer", group=review_req.team).first()
+        reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team).first()
         r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk })
         self.assertEqual(r.status_code, 302)
 
@@ -183,7 +156,7 @@ def test_assign_reviewer(self):
         empty_outbox()
         review_req.state = ReviewRequestStateName.objects.get(slug="accepted")
         review_req.save()
-        reviewer = Role.objects.filter(name="reviewer", group=review_req.team).exclude(pk=reviewer.pk).first()
+        reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team).exclude(pk=reviewer.pk).first()
         r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk })
         self.assertEqual(r.status_code, 302)
 
@@ -335,7 +308,7 @@ def setup_complete_review_test(self):
         review_req.save()
         review_req.team.list_email = "{}@ietf.org".format(review_req.team.acronym)
         for r in ReviewResultName.objects.filter(slug__in=("issues", "ready")):
-            review_req.team.reviewresultname_set.add(r)
+            ReviewTeamResult.objects.get_or_create(team=review_req.team, result=r)
         review_req.team.save()
 
         url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "request_id": review_req.pk })
@@ -373,7 +346,7 @@ def test_complete_review_upload_content(self):
         test_file.name = "unnamed"
 
         r = self.client.post(url, data={
-            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk,
             "state": ReviewRequestStateName.objects.get(slug="completed").pk,
             "reviewed_rev": review_req.doc.rev,
             "review_submission": "upload",
@@ -408,7 +381,7 @@ def test_complete_review_enter_content(self):
         empty_outbox()
 
         r = self.client.post(url, data={
-            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk,
             "state": ReviewRequestStateName.objects.get(slug="completed").pk,
             "reviewed_rev": review_req.doc.rev,
             "review_submission": "enter",
@@ -439,7 +412,7 @@ def test_complete_review_link_to_mailing_list(self):
         empty_outbox()
 
         r = self.client.post(url, data={
-            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk,
             "state": ReviewRequestStateName.objects.get(slug="completed").pk,
             "reviewed_rev": review_req.doc.rev,
             "review_submission": "link",
@@ -467,7 +440,7 @@ def test_partially_complete_review(self):
         empty_outbox()
 
         r = self.client.post(url, data={
-            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk,
             "state": ReviewRequestStateName.objects.get(slug="part-completed").pk,
             "reviewed_rev": review_req.doc.rev,
             "review_submission": "enter",
@@ -501,7 +474,7 @@ def test_partially_complete_review(self):
         url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": review_req.doc.name, "request_id": review_req.pk })
 
         r = self.client.post(url, data={
-            "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk,
+            "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk,
             "state": ReviewRequestStateName.objects.get(slug="completed").pk,
             "reviewed_rev": review_req.doc.rev,
             "review_submission": "enter",

ietf/doc/utils.py

@@ -3,6 +3,7 @@
 import urllib
 import math
 import datetime
+from collections import defaultdict
 
 from django.conf import settings
 from django.db.models.query import EmptyQuerySet
@@ -556,6 +557,39 @@ def uppercase_std_abbreviated_name(name):
     else:
         return name
 
+def extract_complete_replaces_ancestor_mapping_for_docs(names):
+    """Return dict mapping all replaced by relationships of the
+    replacement ancestors to docs. So if x is directly replaced by y
+    and y is in names or replaced by something in names, x in
+    replaces[y]."""
+
+    replaces = defaultdict(set)
+
+    checked = set()
+    front = names
+    while True:
+        if not front:
+            break
+
+        relations = RelatedDocument.objects.filter(
+            source__in=front, relationship="replaces"
+        ).select_related("target").values_list("source", "target__document")
+
+        if not relations:
+            break
+
+        checked.update(front)
+
+        front = []
+        for source_doc, target_doc in relations:
+            replaces[source_doc].add(target_doc)
+
+            if target_doc not in checked:
+                front.append(target_doc)
+
+    return replaces
+
+
 def crawl_history(doc):
     # return document history data for inclusion in doc.json (used by timeline)
     def get_ancestors(doc):

ietf/doc/views_doc.py

@@ -357,7 +357,7 @@ def document_main(request, name, rev=None):
         published = doc.latest_event(type="published_rfc")
         started_iesg_process = doc.latest_event(type="started_iesg_process")
 
-        review_requests = ReviewRequest.objects.filter(doc=doc).exclude(state__in=["withdrawn", "rejected"])
+        review_requests = ReviewRequest.objects.filter(doc=doc).exclude(state__in=["withdrawn", "rejected", "overtaken", "no-response"]).order_by("-time", "-id")
 
         return render_to_response("doc/document_draft.html",
                                   dict(doc=doc,

ietf/doc/views_review.py

@@ -12,8 +12,8 @@
 from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent, State, DocAlias
 from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person
 from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName
-from ietf.group.models import Role
 from ietf.review.models import ReviewRequest
+from ietf.person.fields import PersonEmailChoiceField
 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_about_review_request, make_new_review_request_from_existing)
@@ -188,22 +188,13 @@ def withdraw_request(request, name, request_id):
         'review_req': review_req,
     })
 
-class PersonEmailLabeledRoleModelChoiceField(forms.ModelChoiceField):
-    def __init__(self, *args, **kwargs):
-        if not "queryset" in kwargs:
-            kwargs["queryset"] = Role.objects.select_related("person", "email")
-        super(PersonEmailLabeledRoleModelChoiceField, self).__init__(*args, **kwargs)
-
-    def label_from_instance(self, role):
-        return u"{} <{}>".format(role.person.name, role.email.address)
-
 class AssignReviewerForm(forms.Form):
-    reviewer = PersonEmailLabeledRoleModelChoiceField(widget=forms.RadioSelect, empty_label="(None)", required=False)
+    reviewer = PersonEmailChoiceField(widget=forms.RadioSelect, 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(name="reviewer", group=review_req.team)
+        f.queryset = f.queryset.filter(role__name="reviewer", role__group=review_req.team)
         if review_req.reviewer:
             f.initial = review_req.reviewer_id
 
@@ -212,9 +203,7 @@ def assign_reviewer(request, name, request_id):
     doc = get_object_or_404(Document, name=name)
     review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"])
 
-    can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team)
-
-    if not can_manage_request:
+    if not can_manage_review_requests_for_team(request.user, review_req.team):
         return HttpResponseForbidden("You do not have permission to perform this action")
 
     if request.method == "POST" and request.POST.get("action") == "assign":
@@ -322,7 +311,7 @@ def __init__(self, review_req, *args, **kwargs):
             " ".join("<a class=\"rev label label-default\">{}</a>".format(r)
                      for r in known_revisions))
 
-        self.fields["result"].queryset = self.fields["result"].queryset.filter(teams=review_req.team)
+        self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamresult__team=review_req.team)
         self.fields["review_submission"].choices = [
             (k, label.format(mailing_list=review_req.team.list_email or "[error: team has no mailing list set]"))
             for k, label in self.fields["review_submission"].choices
@@ -428,10 +417,12 @@ def complete_review(request, name, request_id):
                 type="changed_review_request",
                 doc=review_req.doc,
                 by=request.user.person,
-                desc="Request for {} review by {} {}".format(
+                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,
                 ),
             )
 

ietf/group/features.py

@@ -6,6 +6,7 @@ class GroupFeatures(object):
     has_chartering_process = False
     has_documents = False # i.e. drafts/RFCs
     has_materials = False
+    has_reviews = False
     customize_workflow = False
     about_page = "group_about"
     default_tab = about_page
@@ -24,3 +25,9 @@ def __init__(self, group):
 
         if self.has_chartering_process:
             self.about_page = "group_charter"
+
+        from ietf.review.utils import active_review_teams
+        if group in active_review_teams():
+            self.has_reviews = True
+            import ietf.group.views
+            self.default_tab = ietf.group.views.review_requests

ietf/group/tests_info.py

@@ -25,10 +25,11 @@
 from ietf.person.models import Person, Email
 from ietf.utils.test_utils import TestCase, unicontent
 from ietf.utils.mail import outbox, empty_outbox
-from ietf.utils.test_data import make_test_data, create_person
+from ietf.utils.test_data import make_test_data, create_person, make_review_data
 from ietf.utils.test_utils import login_testing_unauthorized
 from ietf.group.factories import GroupFactory, RoleFactory, GroupEventFactory
 from ietf.meeting.factories import SessionFactory
+import ietf.group.views
 
 class GroupPagesTests(TestCase):
     def setUp(self):
@@ -313,6 +314,31 @@ 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

@@ -0,0 +1,72 @@
+import datetime
+
+#from pyquery import PyQuery
+
+from django.core.urlresolvers import reverse as urlreverse
+
+from ietf.utils.test_data import make_test_data, make_review_data
+from ietf.utils.test_utils import login_testing_unauthorized, TestCase, unicontent, reload_db_objects
+from ietf.review.models import ReviewRequest
+from ietf.person.models import Email
+import ietf.group.views_review
+
+class ReviewTests(TestCase):
+    def test_manage_review_requests(self):
+        doc = make_test_data()
+        review_req1 = make_review_data(doc)
+
+        group = review_req1.team
+
+        url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym })
+
+        login_testing_unauthorized(self, "secretary", url)
+
+        review_req2 = ReviewRequest.objects.create(
+            doc=review_req1.doc,
+            team=review_req1.team,
+            type_id="early",
+            deadline=datetime.datetime.combine(datetime.date.today() + datetime.timedelta(days=30), datetime.time(23, 59, 59)),
+            state_id="accepted",
+            reviewer=review_req1.reviewer,
+        )
+
+        review_req3 = ReviewRequest.objects.create(
+            doc=review_req1.doc,
+            team=review_req1.team,
+            type_id="early",
+            deadline=datetime.datetime.combine(datetime.date.today() + datetime.timedelta(days=30), datetime.time(23, 59, 59)),
+            state_id="requested",
+        )
+
+        # get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(review_req1.doc.name in unicontent(r))
+
+        # close and assign
+        new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman")
+        r = self.client.post(url, {
+            # close
+            "r{}-action".format(review_req1.pk): "close",
+            "r{}-close".format(review_req1.pk): "no-response",
+
+            # assign
+            "r{}-action".format(review_req2.pk): "assign",
+            "r{}-reviewer".format(review_req2.pk): new_reviewer.pk,
+
+            # no change
+            "r{}-action".format(review_req3.pk): "",
+            "r{}-close".format(review_req3.pk): "no-response",
+            "r{}-reviewer".format(review_req3.pk): "",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        review_req1, review_req2, review_req3 = reload_db_objects(review_req1, review_req2, review_req3)
+        self.assertEqual(review_req1.state_id, "no-response")
+        self.assertEqual(review_req2.state_id, "requested")
+        self.assertEqual(review_req2.reviewer, new_reviewer)
+        self.assertEqual(review_req3.state_id, "requested")
+
+        # FIXME: test suggested
+
+

ietf/group/urls_info.py

@@ -3,7 +3,7 @@
 from django.conf.urls import patterns, include
 from django.views.generic import RedirectView
 
-from ietf.group import views, views_edit
+from ietf.group import views, views_edit, views_review
 
 urlpatterns = patterns('',
     (r'^$', views.active_groups), 
@@ -20,5 +20,7 @@
     (r'^email-aliases/$', 'ietf.group.views.email_aliases'),
     (r'^bofs/create/$', views_edit.edit, {'action': "create", }, "bof_create"),
     (r'^photos/$', views.chair_photos),
+    (r'^reviews/$', views.review_requests),
+    (r'^reviews/manage/$', views_review.manage_review_requests),
     (r'^(?P<acronym>[a-zA-Z0-9-._]+)/', include('ietf.group.urls_info_details')),
 )