Add statistics section with review statistics. Fix a couple of missing tests.

 - Legacy-Id: 12124

Author: OleLaursen

ietf/externals/static/flot/jquery.flot.min.js

@@ -23,9 +23,8 @@ def test_review_requests(self):
 
         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}),
-                   ]:
+        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))
@@ -131,13 +130,13 @@ def test_reviewer_overview(self):
 
         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))
+        for url in [urlreverse(ietf.group.views_review.reviewer_overview, kwargs={ 'acronym': group.acronym }),
+                    urlreverse(ietf.group.views_review.reviewer_overview, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })]:
+            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)
@@ -149,10 +148,12 @@ def test_manage_review_requests(self):
 
         group = review_req1.team
 
-        url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })
+        url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym })
 
         login_testing_unauthorized(self, "secretary", url)
 
+        url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })
+
         review_req2 = ReviewRequest.objects.create(
             doc=review_req1.doc,
             team=review_req1.team,
@@ -265,10 +266,12 @@ def test_email_open_review_assignments(self):
 
         group = review_req1.team
 
-        url = urlreverse(ietf.group.views_review.email_open_review_assignments, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })
-        
+        url = urlreverse(ietf.group.views_review.email_open_review_assignments, kwargs={ 'acronym': group.acronym })
+
         login_testing_unauthorized(self, "secretary", url)
 
+        url = urlreverse(ietf.group.views_review.email_open_review_assignments, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })
+
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
         q = PyQuery(r.content)
@@ -299,13 +302,18 @@ def test_change_reviewer_settings(self):
         review_req.save()
 
         url = urlreverse(ietf.group.views_review.change_reviewer_settings, kwargs={
-            "group_type": review_req.team.type_id,
             "acronym": review_req.team.acronym,
             "reviewer_email": review_req.reviewer_id,
         })
 
         login_testing_unauthorized(self, reviewer.user.username, url)
 
+        url = urlreverse(ietf.group.views_review.change_reviewer_settings, kwargs={
+            "group_type": review_req.team.type_id,
+            "acronym": review_req.team.acronym,
+            "reviewer_email": review_req.reviewer_id,
+        })
+
         # get
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)

ietf/externals/static/flot/jquery.flot.time.min.js

@@ -1,4 +1,4 @@
-import datetime, math
+import datetime, math, itertools
 from collections import defaultdict
 
 from django.shortcuts import render, redirect, get_object_or_404
@@ -105,7 +105,12 @@ def reviewer_overview(request, acronym, group_type=None):
 
     today = datetime.date.today()
 
-    all_req_data = extract_review_request_data(teams=[group], time_from=today - datetime.timedelta(days=365))
+    extracted_data = extract_review_request_data(teams=[group], time_from=today - datetime.timedelta(days=365), ordering=["reviewer"])
+    req_data_for_reviewer = {}
+    for reviewer, req_data_items in itertools.groupby(extracted_data, key=lambda data: data.reviewer):
+        l = list(req_data_items)
+        l.reverse()
+        req_data_for_reviewer[reviewer] = l
     review_state_by_slug = { n.slug: n for n in ReviewRequestStateName.objects.all() }
 
     for person in reviewers:
@@ -120,15 +125,15 @@ def reviewer_overview(request, acronym, group_type=None):
                                        for p in person.unavailable_periods)
 
         MAX_REQS = 5
-        req_data = all_req_data.get((group.pk, person.pk), [])
-        open_reqs = sum(1 for _, _, _, _, state, _, _, _, _, _ in req_data if state in ("requested", "accepted"))
+        req_data = req_data_for_reviewer.get(person.pk, [])
+        open_reqs = sum(1 for d in req_data if d.state in ("requested", "accepted"))
         latest_reqs = []
-        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:
+        for d 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)):
-                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))
+            if ((d.state in ("requested", "accepted") and len(latest_reqs) < MAX_REQS) or (len(latest_reqs) + open_reqs < MAX_REQS)):
+                latest_reqs.append((d.req_pk, d.doc, d.deadline,
+                                    review_state_by_slug.get(d.state),
+                                    int(math.ceil(d.assignment_to_closure_days)) if d.assignment_to_closure_days is not None else None))
         person.latest_reqs = latest_reqs
 
     return render(request, 'group/reviewer_overview.html',

ietf/group/tests_review.py

@@ -10,7 +10,7 @@ def insert_initial_review_data(apps, schema_editor):
     ReviewRequestStateName.objects.get_or_create(slug="accepted", name="Accepted", order=2)
     ReviewRequestStateName.objects.get_or_create(slug="rejected", name="Rejected", order=3)
     ReviewRequestStateName.objects.get_or_create(slug="withdrawn", name="Withdrawn", order=4)
-    ReviewRequestStateName.objects.get_or_create(slug="overtaken", name="Overtaken By Events", order=5)
+    ReviewRequestStateName.objects.get_or_create(slug="overtaken", name="Overtaken by Events", order=5)
     ReviewRequestStateName.objects.get_or_create(slug="no-response", name="No Response", order=6)
     ReviewRequestStateName.objects.get_or_create(slug="no-review-version", name="Team Will not Review Version", order=7)
     ReviewRequestStateName.objects.get_or_create(slug="no-review-document", name="Team Will not Review Document", order=8)

ietf/group/views_review.py

@@ -1,5 +1,5 @@
 import datetime, re, itertools
-from collections import defaultdict
+from collections import defaultdict, namedtuple
 
 from django.db.models import Q, Max, F
 from django.core.urlresolvers import reverse as urlreverse
@@ -142,8 +142,15 @@ def days_needed_to_fulfill_min_interval_for_reviewers(team):
 
     return res
 
-def extract_review_request_data(teams=None, reviewers=None, time_from=None, time_to=None):
-    """Returns a dict keyed on (team.pk, reviewer_person.pk) which lists data on each review request."""
+ReviewRequestData = namedtuple("ReviewRequestData", [
+    "req_pk", "doc", "doc_pages", "req_time", "state", "deadline", "result", "team", "reviewer",
+    "late_days",
+    "request_to_assignment_days", "assignment_to_closure_days", "request_to_closure_days"])
+
+
+def extract_review_request_data(teams=None, reviewers=None, time_from=None, time_to=None, ordering=[]):
+    """Yield data on each review request, sorted by (*ordering, time)
+    for easy use with itertools.groupby. Valid entries in *ordering are "team" and "reviewer"."""
 
     filters = Q()
 
@@ -159,13 +166,16 @@ def extract_review_request_data(teams=None, reviewers=None, time_from=None, time
     if time_to:
         filters &= Q(time__lte=time_to)
 
-    res = defaultdict(list)
-
     # we may be dealing with a big bunch of data, so treat it carefully
     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").order_by("-time", "-pk", "-reviewrequestdocevent__time")
+    event_qs = event_qs.values_list(
+        "pk", "doc", "doc__pages", "time", "state", "deadline", "result", "team",
+        "reviewer__person", "reviewrequestdocevent__time", "reviewrequestdocevent__type"
+    )
+
+    event_qs = event_qs.order_by(*[o.replace("reviewer", "reviewer__person") for o in ordering] + ["time", "pk", "-reviewrequestdocevent__time"])
 
     def positive_days(time_from, time_to):
         if time_from is None or time_to is None:
@@ -182,7 +192,7 @@ def positive_days(time_from, time_to):
         requested_time = assigned_time = closed_time = None
 
         for e in events:
-            req_pk, doc, req_time, state, deadline, result, team, reviewer, event_time, event_type = e
+            req_pk, doc, doc_pages, req_time, state, deadline, result, team, reviewer, event_time, event_type = e
 
             if event_type == "requested_review" and requested_time is None:
                 requested_time = event_time
@@ -196,12 +206,58 @@ def positive_days(time_from, time_to):
         assignment_to_closure_days = positive_days(assigned_time, closed_time)
         request_to_closure_days = positive_days(requested_time, closed_time)
 
-        res[(team, reviewer)].append((req_pk, doc, req_time, state, deadline, result,
-                                      late_days, request_to_assignment_days, assignment_to_closure_days,
-                                      request_to_closure_days))
+        d = ReviewRequestData(req_pk, doc, doc_pages, req_time, state, deadline, result, team, reviewer,
+                              late_days, request_to_assignment_days, assignment_to_closure_days,
+                              request_to_closure_days)
+
+        yield d
+
+def aggregate_review_request_stats(review_request_data, count=None):
+    """Take a sequence of review request data from
+    extract_review_request_data and compute aggregated statistics."""
+
+    state_dict = defaultdict(int)
+    late_state_dict = defaultdict(int)
+    result_dict = defaultdict(int)
+    assignment_to_closure_days_list = []
+    assignment_to_closure_days_count = 0
+
+    for (req_pk, doc, doc_pages, req_time, state, deadline, result, team, reviewer,
+         late_days, request_to_assignment_days, assignment_to_closure_days, request_to_closure_days) in review_request_data:
+        if count == "pages":
+            c = doc_pages
+        else:
+            c = 1
+
+        state_dict[state] += c
+
+        if late_days is not None and late_days > 0:
+            late_state_dict[state] += c
+
+        if state in ("completed", "part-completed"):
+            result_dict[result] += c
+            if assignment_to_closure_days is not None:
+                assignment_to_closure_days_list.append(assignment_to_closure_days)
+                assignment_to_closure_days_count += c
+
+    res = {}
+    res["state"] = state_dict
+    res["result"] = result_dict
+
+    res["open"] = sum(state_dict.get(s, 0) for s in ("requested", "accepted"))
+    res["completed"] = sum(state_dict.get(s, 0) for s in ("completed", "part-completed"))
+    res["not_completed"] = sum(state_dict.get(s, 0) for s in state_dict if s in ("rejected", "withdrawn", "overtaken", "no-response"))
+
+    res["open_late"] = sum(late_state_dict.get(s, 0) for s in ("requested", "accepted"))
+    res["open_in_time"] = res["open"] - res["open_late"]
+    res["completed_late"] = sum(late_state_dict.get(s, 0) for s in ("completed", "part-completed"))
+    res["completed_in_time"] = res["completed"] - res["completed_late"]
+
+    res["average_assignment_to_closure_days"] = float(sum(assignment_to_closure_days_list)) / (assignment_to_closure_days_count or 1) if assignment_to_closure_days_list else None
 
     return res
 
+
 def make_new_review_request_from_existing(review_req):
     obj = ReviewRequest()
     obj.time = review_req.time
@@ -215,6 +271,7 @@ def make_new_review_request_from_existing(review_req):
     return obj
 
 def email_review_request_change(request, review_req, subject, msg, by, notify_secretary, notify_reviewer, notify_requested_by):
+
     """Notify stakeholders about change, skipping a party if the change
     was done by that party."""