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

- Legacy-Id: 12081
This commit is contained in:
Ole Laursen 2016-10-04 20:22:11 +00:00
parent 4c7b2847ba
commit a177dc616b
9 changed files with 159 additions and 25 deletions

View file

@ -11,7 +11,7 @@ from ietf.doc.models import (BallotType, DeletedEvent, StateType, State, Documen
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 BallotPositionDocEventResource(ModelResource):
}
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())

View file

@ -9,8 +9,9 @@ from ietf.utils.test_utils import login_testing_unauthorized, TestCase, uniconte
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 @@ class ReviewTests(TestCase):
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 @@ class ReviewTests(TestCase):
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 @@ class ReviewTests(TestCase):
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)

View file

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

View file

@ -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.person.models import Person, Email
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 @@ class IetfAuthTests(TestCase):
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)

View file

@ -466,18 +466,21 @@ with db_con.cursor() as c:
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(
@ -485,14 +488,15 @@ with db_con.cursor() as c:
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 @@ with db_con.cursor() as c:
)
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 @@ with db_con.cursor() as c:
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")

View file

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

View file

@ -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;
}

View file

@ -16,7 +16,7 @@
<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 @@
<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>

View file

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