Fix a couple of bugs, add test for reviewer overview page

 - Legacy-Id: 12081

Author: OleLaursen

ietf/doc/resources.py

@@ -11,7 +11,7 @@
     DocumentAuthor, DocEvent, StateDocEvent, DocHistory, ConsensusDocEvent, DocAlias,
     TelechatDocEvent, DocReminder, LastCallDocEvent, NewRevisionDocEvent, WriteupDocEvent,
     InitialReviewDocEvent, DocHistoryAuthor, BallotDocEvent, RelatedDocument,
-    RelatedDocHistory, BallotPositionDocEvent)
+    RelatedDocHistory, BallotPositionDocEvent, ReviewRequestDocEvent)
 
 
 from ietf.name.resources import BallotPositionNameResource, DocTypeNameResource
@@ -513,3 +513,32 @@ class Meta:
         }
 api.doc.register(BallotPositionDocEventResource())
 
+
+
+from ietf.person.resources import PersonResource
+from ietf.review.resources import ReviewRequestResource
+from ietf.name.resources import ReviewRequestStateNameResource
+class ReviewRequestDocEventResource(ModelResource):
+    by               = ToOneField(PersonResource, 'by')
+    doc              = ToOneField(DocumentResource, 'doc')
+    docevent_ptr     = ToOneField(DocEventResource, 'docevent_ptr')
+    review_request   = ToOneField(ReviewRequestResource, 'review_request')
+    state            = ToOneField(ReviewRequestStateNameResource, 'state', null=True)
+    class Meta:
+        queryset = ReviewRequestDocEvent.objects.all()
+        serializer = api.Serializer()
+        cache = SimpleCache()
+        #resource_name = 'reviewrequestdocevent'
+        filtering = { 
+            "id": ALL,
+            "time": ALL,
+            "type": ALL,
+            "desc": ALL,
+            "by": ALL_WITH_RELATIONS,
+            "doc": ALL_WITH_RELATIONS,
+            "docevent_ptr": ALL_WITH_RELATIONS,
+            "review_request": ALL_WITH_RELATIONS,
+            "state": ALL_WITH_RELATIONS,
+        }
+api.doc.register(ReviewRequestDocEventResource())
+

ietf/group/tests_review.py

@@ -9,8 +9,9 @@
 from ietf.doc.models import TelechatDocEvent
 from ietf.iesg.models import TelechatDate
 from ietf.person.models import Email, Person
-from ietf.review.models import ReviewRequest, ReviewRequestStateName, ReviewerSettings, UnavailablePeriod
+from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod
 from ietf.review.utils import suggested_review_requests_for_team
+from ietf.name.models import ReviewTypeName, ReviewResultName, ReviewRequestStateName
 import ietf.group.views_review
 from ietf.utils.mail import outbox, empty_outbox
 
@@ -32,8 +33,8 @@ def test_review_requests(self):
         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.state = ReviewRequestStateName.objects.get(slug="completed")
+        review_req.result = ReviewResultName.objects.get(slug="ready")
         review_req.save()
 
         r = self.client.get(url)
@@ -97,8 +98,50 @@ def test_suggested_review_requests(self):
         review_req.save()
 
         self.assertEqual(len(suggested_review_requests_for_team(team)), 1)
-        
 
+    def test_reviewer_overview(self):
+        doc = make_test_data()
+        review_req1 = make_review_data(doc)
+        review_req1.state = ReviewRequestStateName.objects.get(slug="completed")
+        review_req1.save()
+
+        reviewer = review_req1.reviewer.person
+
+        ReviewRequest.objects.create(
+            doc=review_req1.doc,
+            team=review_req1.team,
+            type_id="early",
+            deadline=datetime.date.today() + datetime.timedelta(days=30),
+            state_id="accepted",
+            reviewer=review_req1.reviewer,
+            requested_by=Person.objects.get(user__username="plain"),
+        )
+
+        UnavailablePeriod.objects.create(
+            team=review_req1.team,
+            person=reviewer,
+            start_date=datetime.date.today() - datetime.timedelta(days=10),
+            availability="unavailable",
+        )
+
+        settings = ReviewerSettings.objects.get(person=reviewer)
+        settings.skip_next = 1
+        settings.save()
+
+        group = review_req1.team
+
+        url = urlreverse(ietf.group.views_review.reviewer_overview, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })
+
+        # get
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(unicode(reviewer) in unicontent(r))
+        self.assertTrue(review_req1.doc.name in unicontent(r))
+
+        self.client.login(username="secretary", password="secretary+password")
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        
     def test_manage_review_requests(self):
         doc = make_test_data()
         review_req1 = make_review_data(doc)
@@ -128,6 +171,33 @@ def test_manage_review_requests(self):
             requested_by=Person.objects.get(user__username="plain"),
         )
 
+        # previous reviews
+        ReviewRequest.objects.create(
+            time=datetime.datetime.now() - datetime.timedelta(days=100),
+            requested_by=Person.objects.get(name="(System)"),
+            doc=doc,
+            type=ReviewTypeName.objects.get(slug="early"),
+            team=review_req1.team,
+            state=ReviewRequestStateName.objects.get(slug="completed"),
+            result=ReviewResultName.objects.get(slug="ready-nits"),
+            reviewed_rev="01",
+            deadline=datetime.date.today() - datetime.timedelta(days=80),
+            reviewer=review_req1.reviewer,
+        )
+
+        ReviewRequest.objects.create(
+            time=datetime.datetime.now() - datetime.timedelta(days=100),
+            requested_by=Person.objects.get(name="(System)"),
+            doc=doc,
+            type=ReviewTypeName.objects.get(slug="early"),
+            team=review_req1.team,
+            state=ReviewRequestStateName.objects.get(slug="completed"),
+            result=ReviewResultName.objects.get(slug="ready"),
+            reviewed_rev="01",
+            deadline=datetime.date.today() - datetime.timedelta(days=80),
+            reviewer=review_req1.reviewer,
+        )
+        
         # get
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)

ietf/group/views_review.py

@@ -1,4 +1,4 @@
-import datetime
+import datetime, math
 from collections import defaultdict
 
 from django.shortcuts import render, redirect, get_object_or_404
@@ -126,7 +126,8 @@ def reviewer_overview(request, acronym, group_type=None):
         for req_pk, doc, req_time, state, deadline, result, late_days, request_to_assignment_days, assignment_to_closure_days, request_to_closure_days in req_data:
             # any open requests pushes the others out
             if ((state in ("requested", "accepted") and len(latest_reqs) < MAX_REQS) or (len(latest_reqs) + open_reqs < MAX_REQS)):
-                print review_state_by_slug.get(state), assignment_to_closure_days
+                if assignment_to_closure_days is not None:
+                    assignment_to_closure_days = int(math.ceil(assignment_to_closure_days))
                 latest_reqs.append((req_pk, doc, deadline, review_state_by_slug.get(state), assignment_to_closure_days))
         person.latest_reqs = latest_reqs
 
@@ -205,14 +206,13 @@ def manage_review_requests(request, acronym, group_type=None):
         set(r.doc_id for r in review_requests),
     )
 
-
     # we need a mutable query dict for resetting upon saving with
     # conflicts
     query_dict = request.POST.copy() if request.method == "POST" else None
     for req in review_requests:
         l = []
         # take all on the latest reviewed rev
-        for r in document_requests[req.doc_id]:
+        for r in document_requests.get(req.doc_id, []):
             if l and l[0].reviewed_rev:
                 if r.doc_id == l[0].doc_id and r.reviewed_rev:
                     if int(r.reviewed_rev) > int(l[0].reviewed_rev):

ietf/ietfauth/tests.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 from __future__ import unicode_literals
 
-import os, shutil, time
+import os, shutil, time, datetime
 from urlparse import urlsplit
 from pyquery import PyQuery
 from unittest import skipIf
@@ -18,7 +18,7 @@
 from ietf.group.models import Group, Role, RoleName
 from ietf.ietfauth.htpasswd import update_htpasswd_file
 from ietf.mailinglists.models import Subscribed
-from ietf.review.models import ReviewWish
+from ietf.review.models import ReviewWish, UnavailablePeriod
 from ietf.utils.decorators import skip_coverage
 
 import ietf.ietfauth.views
@@ -348,6 +348,13 @@ def test_review_overview(self):
         review_req.reviewer = reviewer.email_set.first()
         review_req.save()
 
+        UnavailablePeriod.objects.create(
+            team=review_req.team,
+            person=reviewer,
+            start_date=datetime.date.today() - datetime.timedelta(days=10),
+            availability="unavailable",
+        )
+
         url = urlreverse(ietf.ietfauth.views.review_overview)
 
         login_testing_unauthorized(self, reviewer.user.username, url)

ietf/review/import_from_review_tool.py

@@ -466,33 +466,37 @@ def fix_docname(docname):
                 if "assigned" in event_collection:
                     continue # skip requested unless there's no assigned event
 
-                e = ReviewRequestDocEvent.objects.filter(type="requested_review", doc=review_req.doc).first() or ReviewRequestDocEvent(type="requested_review", doc=review_req.doc)
+                e = ReviewRequestDocEvent.objects.filter(type="requested_review", doc=review_req.doc, review_request=review_req).first()
+                if not e:
+                    e = ReviewRequestDocEvent(type="requested_review", doc=review_req.doc, review_request=review_req)
                 e.time = time
                 e.by = by
                 e.desc = "Requested {} review by {}".format(review_req.type.name, review_req.team.acronym.upper())
-                e.review_request = review_req
                 e.state = None
                 e.skip_community_list_notification = True
                 e.save()
                 print "imported event requested_review", e.desc, e.doc_id
 
             elif key == "assigned":
-                e = ReviewRequestDocEvent.objects.filter(type="assigned_review_request", doc=review_req.doc).first() or ReviewRequestDocEvent(type="assigned_review_request", doc=review_req.doc)
+                e = ReviewRequestDocEvent.objects.filter(type="assigned_review_request", doc=review_req.doc, review_request=review_req).first()
+                if not e:
+                    e = ReviewRequestDocEvent(type="assigned_review_request", doc=review_req.doc, review_request=review_req)
                 e.time = parse_timestamp(timestamp)
                 e.by = by
                 e.desc = "Request for {} review by {} is assigned to {}".format(
                     review_req.type.name,
                     review_req.team.acronym.upper(),
                     review_req.reviewer.person if review_req.reviewer else "(None)",
                 )
-                e.review_request = review_req
                 e.state = None
                 e.skip_community_list_notification = True
                 e.save()
                 print "imported event assigned_review_request", e.pk, e.desc, e.doc_id
 
             elif key == "closed" and review_req.state_id not in ("requested", "accepted"):
-                e = ReviewRequestDocEvent.objects.filter(type="closed_review_request", doc=review_req.doc).first() or ReviewRequestDocEvent(type="closed_review_request", doc=review_req.doc)
+                e = ReviewRequestDocEvent.objects.filter(type="closed_review_request", doc=review_req.doc, review_request=review_req).first()
+                if not e:
+                    e = ReviewRequestDocEvent(type="closed_review_request", doc=review_req.doc, review_request=review_req)
                 e.time = parse_timestamp(timestamp)
                 e.by = by
                 e.state = states.get(state) if state else None
@@ -512,7 +516,6 @@ def fix_docname(docname):
                     )
                 else:
                     e.desc = "Closed request for {} review by {} with state '{}'".format(review_req.type.name, review_req.team.acronym.upper(), e.state.name)
-                e.review_request = review_req
                 e.skip_community_list_notification = True
                 e.save()
                 completion_event = e
@@ -580,4 +583,18 @@ def fix_docname(docname):
             review_req.state = states["overtaken"]
             review_req.save()
 
+            if "closed" not in event_collection and "assigned" in event_collection:
+                e = ReviewRequestDocEvent.objects.filter(type="closed_review_request", doc=review_req.doc, review_request=review_req).first()
+                if not e:
+                    e = ReviewRequestDocEvent(type="closed_review_request", doc=review_req.doc, review_request=review_req)
+                e.time = datetime.datetime.now()
+                e.by = by
+                e.state = review_req.state
+                e.desc = "Closed request for {} review by {} with state '{}'".format(review_req.type.name, review_req.team.acronym.upper(), e.state.name)
+                e.skip_community_list_notification = True
+                e.save()
+                completion_event = e
+                print "imported event closed_review_request (generated upon closing)", e.desc, e.doc_id
+            
+
         print "imported review request", row.reviewid, "as", review_req.pk, review_req.time, review_req.deadline, review_req.type, review_req.doc_id, review_req.state, review_req.doc.get_state_slug("draft-iesg")

ietf/review/utils.py

@@ -161,10 +161,10 @@ def extract_review_request_data(teams=None, reviewers=None, time_from=None, time
     res = defaultdict(list)
 
     # we may be dealing with a big bunch of data, so treat it carefully
-    event_qs = ReviewRequest.objects.filter(filters).order_by("-time", "-id")
+    event_qs = ReviewRequest.objects.filter(filters)
 
     # left outer join with RequestRequestDocEvent for request/assign/close time
-    event_qs = event_qs.values_list("pk", "doc", "time", "state", "deadline", "result", "team", "reviewer__person", "reviewrequestdocevent__time", "reviewrequestdocevent__type")
+    event_qs = event_qs.values_list("pk", "doc", "time", "state", "deadline", "result", "team", "reviewer__person", "reviewrequestdocevent__time", "reviewrequestdocevent__type").order_by("-time", "-pk", "-reviewrequestdocevent__time")
 
     def positive_days(time_from, time_to):
         if time_from is None or time_to is None:

ietf/static/ietf/css/ietf.css

@@ -515,11 +515,11 @@ form.email-open-review-assignments [name=body] {
   font-family: monospace;
 }
 
-table.unavailable-periods td {
+table.simple-table td {
   padding-right: 0.5em;
 }
 
-table.unavailable-periods td:last-child {
+table.simple-table td:last-child {
   padding-right: 0;
 }
 

ietf/templates/group/reviewer_overview.html

@@ -16,7 +16,7 @@ <h2>Reviewers</h2>
       <thead>
         <tr>
           <th>Reviewer</th>
-          <th>Latest assignments (deadline, state)</th>
+          <th>Deadline/state/time between assignment and closure for latest assignments</th>
           <th>Settings</th>
         </tr>
       </thead>
@@ -25,9 +25,20 @@ <h2>Reviewers</h2>
           <tr {% if person.completely_unavailable %}class="completely-unavailable"{% endif %}>
             <td><a {% if person.settings_url %}href="{{ person.settings_url }}"{% endif %}>{{ person }}</a></td>
             <td>
-              {% for req_pk, doc_name, deadline, state, assignment_to_closure_days in person.latest_reqs %}
-                <div><a href="{% url "ietf.doc.views_review.review_request" name=doc_name request_id=req_pk %}">{{ deadline|date }} {{ doc_name }}: {{ state.name }} {% if assignment_to_closure_days != None %}{% if state.slug == "completed" or state.slug == "part-completed" %}(in {{ assignment_to_closure_days|floatformat }} day{{ assignment_to_closure_days|pluralize }}){% endif %}{% endif %}</a></div>
+              <table class="simple-table">
+                {% for req_pk, doc_name, deadline, state, assignment_to_closure_days in person.latest_reqs %}
+                  <tr>
+                    <td><a href="{% url "ietf.doc.views_review.review_request" name=doc_name request_id=req_pk %}">{{ deadline|date }}</a></td>
+                    <td>
+                      <span class="label label-{% if state.slug == "completed" or state.slug == "part-completed" %}success{% elif state.slug == "no-response" %}danger{% elif state.slug == "overtaken" %}warning{% elif state.slug == "requested" or state.slug == "accepted" %}primary{% else %}default{% endif %}">{{ state.name }}</span>
+                    </td>
+                    <td>
+                      {% if assignment_to_closure_days != None %}{{ assignment_to_closure_days }}&nbsp;day{{ assignment_to_closure_days|pluralize }}{% endif %}
+                    </td>
+                    <td>{{ doc_name }}</td>
+                </div>
               {% endfor %}
+              </table>
             </td>
             <td>
               {{ person.settings.get_min_interval_display }} {% if person.settings.skip_next %}(skip: {{ person.settings.skip_next }}){% endif %}<br>

ietf/templates/review/unavailable_table.html

@@ -1,4 +1,4 @@
-<table class="unavailable-periods">
+<table class="simple-table">
   {% for p in unavailable_periods %}
     <tr class="unavailable-period-{{ p.state }}">
       <td>{{ p.start_date }} - {{ p.end_date|default:"" }}</td>