Add a page for displaying a review request, add support for

withdrawing requests, add tests for these two pages
 - Legacy-Id: 11217

Author: OleLaursen

ietf/doc/models.py

@@ -707,6 +707,7 @@ class DocReminder(models.Model):
 
     # review
     ("requested_review", "Requested review"),
+    ("withdrew_review_request", "Withdrew review"),
     ]
 
 class DocEvent(models.Model):

ietf/doc/tests_review.py

@@ -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 @@ def test_request_review(self):
         })
         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 @@ def test_request_review_by_reviewer(self):
         })
         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")

ietf/doc/urls.py

@@ -36,7 +36,6 @@
 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 @@
     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'),
 

ietf/doc/urls_review.py

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

ietf/doc/views_review.py

@@ -2,7 +2,6 @@
 
 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.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)
+
+    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
 
-    print doc, review_request
+        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,
+    })

ietf/name/models.py

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

ietf/review/migrations/0001_initial.py

@@ -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={
             },

ietf/review/models.py

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

ietf/templates/doc/document_draft.html

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

ietf/templates/doc/review/request_review.html

@@ -23,7 +23,7 @@ <h1>Request review<br><small>{{ doc.name }}</small></h1>
     {% 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>