Add a page for displaying a review request, add support for
withdrawing requests, add tests for these two pages - Legacy-Id: 11217
This commit is contained in:
Ole Laursen
parent
64a65340a2
commit
44e135345c
|
|
@ -707,6 +707,7 @@ EVENT_TYPES = [
|
|||
|
||||
# review
|
||||
("requested_review", "Requested review"),
|
||||
("withdrew_review_request", "Withdrew review"),
|
||||
]
|
||||
|
||||
class DocEvent(models.Model):
|
||||
|
|
|
|||
|
|
@ -1,33 +1,44 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
|
||||
import datetime
|
||||
from pyquery import PyQuery
|
||||
|
||||
from django.core.urlresolvers import reverse as urlreverse
|
||||
|
||||
import debug # pyflakes:ignore
|
||||
|
||||
from ietf.review.models import ReviewRequest
|
||||
from ietf.review.models import ReviewRequest, Reviewer
|
||||
from ietf.person.models import Person
|
||||
from ietf.group.models import Group, Role
|
||||
from ietf.name.models import ReviewResultName
|
||||
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_utils import login_testing_unauthorized
|
||||
from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects
|
||||
|
||||
def make_review_data():
|
||||
review_team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team")
|
||||
review_team.reviewresultname_set.add(ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"]))
|
||||
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.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=review_team)
|
||||
role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team)
|
||||
reviewer = Reviewer.objects.create(role=role, frequency=14, skip_next=0)
|
||||
|
||||
return review_team
|
||||
review_req = ReviewRequest.objects.create(
|
||||
doc=doc,
|
||||
team=team,
|
||||
type_id="early",
|
||||
deadline=datetime.datetime.now() + datetime.timedelta(days=20),
|
||||
state_id="ready",
|
||||
reviewer=reviewer,
|
||||
reviewed_rev="01",
|
||||
)
|
||||
|
||||
return review_req
|
||||
|
||||
class ReviewTests(TestCase):
|
||||
def test_request_review(self):
|
||||
doc = make_test_data()
|
||||
review_team = make_review_data()
|
||||
review_req = make_review_data(doc)
|
||||
review_team = review_req.team
|
||||
|
||||
url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name })
|
||||
login_testing_unauthorized(self, "secretary", url)
|
||||
|
|
@ -47,17 +58,17 @@ class ReviewTests(TestCase):
|
|||
})
|
||||
self.assertEqual(r.status_code, 302)
|
||||
|
||||
req = ReviewRequest.objects.get(doc=doc)
|
||||
req = ReviewRequest.objects.get(doc=doc, state="requested")
|
||||
self.assertEqual(req.deadline.date(), deadline_date)
|
||||
self.assertEqual(req.deadline.time(), datetime.time(23, 59, 59))
|
||||
self.assertEqual(req.state_id, "requested")
|
||||
self.assertEqual(req.team, review_team)
|
||||
self.assertEqual(req.requested_rev, "01")
|
||||
self.assertEqual(doc.latest_event().type, "requested_review")
|
||||
|
||||
def test_request_review_by_reviewer(self):
|
||||
doc = make_test_data()
|
||||
review_team = make_review_data()
|
||||
review_req = make_review_data(doc)
|
||||
review_team = review_req.team
|
||||
|
||||
url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name })
|
||||
login_testing_unauthorized(self, "plain", url)
|
||||
|
|
@ -73,10 +84,40 @@ class ReviewTests(TestCase):
|
|||
})
|
||||
self.assertEqual(r.status_code, 302)
|
||||
|
||||
req = ReviewRequest.objects.get(doc=doc)
|
||||
req = ReviewRequest.objects.get(doc=doc, state="requested")
|
||||
self.assertEqual(req.state_id, "requested")
|
||||
self.assertEqual(req.team, review_team)
|
||||
|
||||
def test_doc_page(self):
|
||||
# FIXME: fill in
|
||||
pass
|
||||
|
||||
def test_review_request(self):
|
||||
doc = make_test_data()
|
||||
review_req = make_review_data(doc)
|
||||
|
||||
url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
|
||||
|
||||
r = self.client.get(url)
|
||||
self.assertEqual(r.status_code, 200)
|
||||
self.assertTrue(review_req.team.acronym.upper() in unicontent(r))
|
||||
|
||||
def test_withdraw_request(self):
|
||||
doc = make_test_data()
|
||||
review_req = make_review_data(doc)
|
||||
review_req.state = ReviewRequestStateName.objects.get(slug="accepted")
|
||||
review_req.save()
|
||||
|
||||
url = urlreverse('ietf.doc.views_review.withdraw_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
|
||||
login_testing_unauthorized(self, "secretary", url)
|
||||
|
||||
r = self.client.get(url)
|
||||
self.assertEqual(r.status_code, 200)
|
||||
|
||||
# withdraw
|
||||
r = self.client.post(url, { "action": "withdraw" })
|
||||
self.assertEqual(r.status_code, 302)
|
||||
|
||||
review_req = reload_db_objects(review_req)
|
||||
self.assertEqual(review_req.state_id, "withdrawn")
|
||||
self.assertEqual(doc.latest_event().type, "withdrew_review_request")
|
||||
|
|
|
|||
|
|
@ -36,7 +36,6 @@ from django.views.generic import RedirectView
|
|||
from ietf.doc import views_search, views_draft, views_ballot
|
||||
from ietf.doc import views_status_change
|
||||
from ietf.doc import views_doc
|
||||
from ietf.doc import views_review
|
||||
|
||||
session_patterns = [
|
||||
url(r'^add$', views_doc.add_sessionpresentation),
|
||||
|
|
@ -74,8 +73,7 @@ urlpatterns = patterns('',
|
|||
url(r'^(?P<name>[A-Za-z0-9._+-]+)/ballot/$', views_doc.document_ballot, name="doc_ballot"),
|
||||
(r'^(?P<name>[A-Za-z0-9._+-]+)/(?:(?P<rev>[0-9-]+)/)?doc.json$', views_doc.document_json),
|
||||
(r'^(?P<name>[A-Za-z0-9._+-]+)/ballotpopup/(?P<ballot_id>[0-9]+)/$', views_doc.ballot_popup),
|
||||
url(r'^(?P<name>[A-Za-z0-9._+-]+)/requestreview/$', views_review.request_review),
|
||||
url(r'^(?P<name>[A-Za-z0-9._+-]+)/review/(?P<request_id>[0-9]+)/$', views_review.review),
|
||||
url(r'^(?P<name>[A-Za-z0-9._+-]+)/reviewrequest/', include("ietf.doc.urls_review")),
|
||||
|
||||
url(r'^(?P<name>[A-Za-z0-9._+-]+)/email-aliases/$', RedirectView.as_view(pattern_name='doc_email', permanent=False),name='doc_specific_email_aliases'),
|
||||
|
||||
|
|
|
|||
9
ietf/doc/urls_review.py
Normal file
9
ietf/doc/urls_review.py
Normal file
|
|
@ -0,0 +1,9 @@
|
|||
from django.conf.urls import patterns, url
|
||||
from ietf.doc import views_review
|
||||
|
||||
urlpatterns = patterns('',
|
||||
url(r'^$', views_review.request_review),
|
||||
url(r'^(?P<request_id>[0-9]+)/$', views_review.review_request),
|
||||
url(r'^(?P<request_id>[0-9]+)/withdraw/$', views_review.withdraw_request),
|
||||
)
|
||||
|
||||
|
|
@ -2,7 +2,6 @@ import datetime
|
|||
|
||||
from django.http import HttpResponseForbidden
|
||||
from django.shortcuts import render, get_object_or_404, redirect
|
||||
from django.core.urlresolvers import reverse as urlreverse
|
||||
from django import forms
|
||||
from django.contrib.auth.decorators import login_required
|
||||
|
||||
|
|
@ -13,7 +12,6 @@ from ietf.review.models import ReviewRequest, ReviewRequestStateName
|
|||
from ietf.review.utils import active_review_teams
|
||||
from ietf.utils.fields import DatepickerDateField
|
||||
|
||||
|
||||
class RequestReviewForm(forms.ModelForm):
|
||||
deadline_date = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={ "autoclose": "1", "start-date": "+0d" })
|
||||
deadline_time = forms.TimeField(widget=forms.TextInput(attrs={ 'placeholder': "HH:MM" }), help_text="If time is not specified, end of day is assumed", required=False)
|
||||
|
|
@ -87,7 +85,8 @@ def request_review(request, name):
|
|||
type="requested_review",
|
||||
doc=doc,
|
||||
by=request.user.person,
|
||||
desc="{} review by {} requested".format(review_req.type.name, review_req.team.acronym.upper()),
|
||||
desc="Requested {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()),
|
||||
time=review_req.time,
|
||||
)
|
||||
|
||||
# FIXME: if I'm a reviewer, auto-assign to myself?
|
||||
|
|
@ -101,8 +100,41 @@ def request_review(request, name):
|
|||
'form': form,
|
||||
})
|
||||
|
||||
def review(request, name, request_id):
|
||||
def review_request(request, name, request_id):
|
||||
doc = get_object_or_404(Document, name=name)
|
||||
review_request = get_object_or_404(ReviewRequest, pk=request_id)
|
||||
review_req = get_object_or_404(ReviewRequest, pk=request_id)
|
||||
|
||||
print doc, review_request
|
||||
return render(request, 'doc/review/review_request.html', {
|
||||
'doc': doc,
|
||||
'review_req': review_req,
|
||||
'can_withdraw_request': review_req.state_id in ["requested", "accepted"] and is_authorized_in_doc_stream(request.user, doc),
|
||||
})
|
||||
|
||||
def withdraw_request(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"])
|
||||
|
||||
if not is_authorized_in_doc_stream(request.user, doc):
|
||||
return HttpResponseForbidden("You do not have permission to perform this action")
|
||||
|
||||
if request.method == "POST" and request.POST.get("action") == "withdraw":
|
||||
review_req.state = ReviewRequestStateName.objects.get(slug="withdrawn")
|
||||
review_req.save()
|
||||
|
||||
DocEvent.objects.create(
|
||||
type="withdrew_review_request",
|
||||
doc=doc,
|
||||
by=request.user.person,
|
||||
desc="Withdrew request for {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()),
|
||||
)
|
||||
|
||||
if review_req.state_id != "requested":
|
||||
# FIXME: handle this case - by emailing?
|
||||
pass
|
||||
|
||||
return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk)
|
||||
|
||||
return render(request, 'doc/review/withdraw_request.html', {
|
||||
'doc': doc,
|
||||
'review_req': review_req,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -89,12 +89,12 @@ class LiaisonStatementTagName(NameModel):
|
|||
"Action Required, Action Taken"
|
||||
class ReviewRequestStateName(NameModel):
|
||||
"""Requested, Accepted, Rejected, Withdrawn, Overtaken By Events,
|
||||
No Response , Completed"""
|
||||
No Response, Completed"""
|
||||
class ReviewTypeName(NameModel):
|
||||
"""Early Review, Last Call, Telechat"""
|
||||
class ReviewResultName(NameModel):
|
||||
"""Almost ready, Has issues, Has nits, Not Ready,
|
||||
On the right track, Ready, Ready with issues,
|
||||
Ready with nits, Serious Issues"""
|
||||
teams = models.ManyToManyField("group.Group", help_text="Which teams this result can be set for. This also implicitly defines which teams are review teams - if there are no possible review results defined for a given team, it can't be a review team.", blank=True)
|
||||
teams = models.ManyToManyField("group.Group", help_text="Which teams this result can be set for. This also implicitly defines which teams are review teams - if there are no possible review results defined for a given team, it can't be a review team.", blank=True)
|
||||
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ class Migration(migrations.Migration):
|
|||
('available', models.DateTimeField(help_text=b'When will this reviewer be available again', null=True, blank=True)),
|
||||
('filter_re', models.CharField(max_length=255, blank=True)),
|
||||
('skip_next', models.IntegerField(help_text=b'Skip the next N review assignments')),
|
||||
('role', models.ForeignKey(to='group.Role')),
|
||||
('role', models.OneToOneField(to='group.Role')),
|
||||
],
|
||||
options={
|
||||
},
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ class Reviewer(models.Model):
|
|||
of admin data associated with the reviewer in the particular team.
|
||||
There will be one record for each combination of reviewer and team.
|
||||
"""
|
||||
role = models.ForeignKey(Role)
|
||||
role = models.OneToOneField(Role)
|
||||
frequency = models.IntegerField(help_text="Can review every N days")
|
||||
available = models.DateTimeField(blank=True, null=True, help_text="When will this reviewer be available again")
|
||||
filter_re = models.CharField(max_length=255, blank=True)
|
||||
|
|
|
|||
|
|
@ -200,7 +200,7 @@
|
|||
<td>
|
||||
{% for r in review_requests %}
|
||||
<div>
|
||||
<a href="{% url "ietf.doc.views_review.review" doc.name r.pk %}">{{ r.team.acronym|upper }} {{ r.type.name }} Review ({{ r.state.name }})</a>
|
||||
<a href="{% url "ietf.doc.views_review.review_request" doc.name r.pk %}">{{ r.team.acronym|upper }} {{ r.type.name }} Review ({{ r.state.name }})</a>
|
||||
</div>
|
||||
{% endfor %}
|
||||
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@
|
|||
{% bootstrap_field form.requested_rev layout="horizontal" %}
|
||||
|
||||
{% buttons %}
|
||||
<button type="submit" class="btn btn-primary" name="save_addresses" value="Save">Request review</button>
|
||||
<button type="submit" class="btn btn-primary">Request review</button>
|
||||
<a class="btn btn-default pull-right" href="{% url "doc_view" name=doc.canonical_name %}">Back</a>
|
||||
{% endbuttons %}
|
||||
</form>
|
||||
|
|
|
|||
103
ietf/templates/doc/review/review_request.html
Normal file
103
ietf/templates/doc/review/review_request.html
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
{% extends "base.html" %}
|
||||
{# Copyright The IETF Trust 2016, All Rights Reserved #}
|
||||
{% load origin bootstrap3 static %}
|
||||
|
||||
{% block title %}Review request for {{ review_req.doc.name }}{% endblock %}
|
||||
|
||||
{% block content %}
|
||||
{% origin %}
|
||||
<h1>Review request<br><small>{{ review_req.doc.name }}</small></h1>
|
||||
|
||||
<table class="table table-condensed">
|
||||
<tbody class="meta">
|
||||
<tr>
|
||||
<th>Request</th>
|
||||
<th>Review of</th>
|
||||
<td>
|
||||
{% if review_req.requested_rev %}
|
||||
<a href="{% url "doc_view" name=review_req.doc.name rev=review_req.requested_rev %}">{{ review_req.doc.name }}-<b>{{ review_req.requested_rev }}</b></a>
|
||||
{% else %}
|
||||
<a href="{% url "doc_view" name=review_req.doc.name %}">{{ review_req.doc.name }}</a>
|
||||
{% endif %}
|
||||
</td>
|
||||
</tr>
|
||||
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Type</th>
|
||||
<td>{{ review_req.type.name }} Review</td>
|
||||
</tr>
|
||||
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Team</th>
|
||||
<td>{{ review_req.team.acronym|upper }}</td>
|
||||
</tr>
|
||||
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Deadline</th>
|
||||
<td>
|
||||
{% if review_req.deadline|date:"H:i" != "23:59" %}
|
||||
{{ review_req.deadline|date:"Y-m-d H:i" }}
|
||||
{% else %}
|
||||
{{ review_req.deadline|date:"Y-m-d" }}
|
||||
{% endif %}
|
||||
</td>
|
||||
</tr>
|
||||
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Requested</th>
|
||||
<td>{{ review_req.time|date:"Y-m-d" }}</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
|
||||
<tbody class="meta">
|
||||
<tr>
|
||||
<th>Review</th>
|
||||
<th>State</th>
|
||||
<td>{{ review_req.state.name }}</td>
|
||||
</tr>
|
||||
|
||||
{% if review_req.reviewer %}
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Reviewer</th>
|
||||
<td>{{ review_req.reviewer.role.person }}</td>
|
||||
</tr>
|
||||
{% endif %}
|
||||
|
||||
{% if review_req.review %}
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Review</th>
|
||||
<td><a href="{{ review_req.review.get_absolute_url }}">{{ review_req.review.name }}</a></td>
|
||||
</tr>
|
||||
{% endif %}
|
||||
|
||||
{% if review_req.reviewed_rev %}
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Reviewed revision</th>
|
||||
<td><a href="">{{ review_req.reviewed_rev }}</a></td>
|
||||
</tr>
|
||||
{% endif %}
|
||||
|
||||
{% if review_req.result %}
|
||||
<tr>
|
||||
<th></th>
|
||||
<th>Result of review</th>
|
||||
<td>{{ review_req.result.name }</td>
|
||||
</tr>
|
||||
{% endif %}
|
||||
</tbody>
|
||||
</table>
|
||||
|
||||
<div class="buttonlist">
|
||||
{% if can_withdraw_request %}
|
||||
<a class="btn btn-default btn-xs" href="{% url "ietf.doc.views_review.withdraw_request" name=doc.name request_id=review_req.pk %}"><span class="fa fa-ban"></span> Withdraw request</a>
|
||||
{% endif %}
|
||||
</div>
|
||||
|
||||
{% endblock %}
|
||||
22
ietf/templates/doc/review/withdraw_request.html
Normal file
22
ietf/templates/doc/review/withdraw_request.html
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
{% extends "base.html" %}
|
||||
{# Copyright The IETF Trust 2016, All Rights Reserved #}
|
||||
{% load origin bootstrap3 static %}
|
||||
|
||||
{% block title %}Withdraw review request for {{ review_req.doc.name }}{% endblock %}
|
||||
|
||||
{% block content %}
|
||||
{% origin %}
|
||||
<h1>Withdraw review request<br><small>{{ review_req.doc.name }}</small></h1>
|
||||
|
||||
<p>Do you want to withdraw the review request?</p>
|
||||
|
||||
<form method="post">
|
||||
{% csrf_token %}
|
||||
|
||||
{% buttons %}
|
||||
<a class="btn btn-default" href="{% url "ietf.doc.views_review.review_request" name=doc.canonical_name request_id=review_req.pk %}">Cancel</a>
|
||||
<button type="submit" class="btn btn-primary" name="action" value="withdraw">Withdraw request</button>
|
||||
{% endbuttons %}
|
||||
</form>
|
||||
|
||||
{% endblock %}
|
||||
|
|
@ -255,7 +255,7 @@ def canonicalize_sitemap(s):
|
|||
|
||||
def login_testing_unauthorized(test_case, username, url, password=None):
|
||||
r = test_case.client.get(url)
|
||||
test_case.assertTrue(r.status_code in (302, 403))
|
||||
test_case.assertIn(r.status_code, (302, 403))
|
||||
if r.status_code == 302:
|
||||
test_case.assertTrue("/accounts/login" in r['Location'])
|
||||
if not password:
|
||||
|
|
@ -272,6 +272,17 @@ def unicontent(r):
|
|||
encoding = 'utf-8'
|
||||
return r.content.decode(encoding)
|
||||
|
||||
def reload_db_objects(*objects):
|
||||
"""Rerequest the given arguments from the database so they're refreshed, to be used like
|
||||
|
||||
foo, bar = reload_objects(foo, bar)"""
|
||||
|
||||
t = tuple(o.__class__.objects.get(pk=o.pk) for o in objects)
|
||||
if len(objects) == 1:
|
||||
return t[0]
|
||||
else:
|
||||
return t
|
||||
|
||||
class ReverseLazyTest(django.test.TestCase):
|
||||
def test_redirect_with_lazy_reverse(self):
|
||||
response = self.client.get('/ipr/update/')
|
||||
|
|
|
|||
Loading…
Reference in a new issue