Add support for rejecting a review assignment

 - Legacy-Id: 11225

Author: OleLaursen

ietf/doc/models.py

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

ietf/doc/tests_review.py

@@ -108,16 +108,63 @@ def test_withdraw_request(self):
         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)
+        withdraw_url = urlreverse('ietf.doc.views_review.withdraw_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
 
-        r = self.client.get(url)
+
+        # follow link
+        req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
+        self.client.login(username="secretary", password="secretary+password")
+        r = self.client.get(req_url)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(withdraw_url in unicontent(r))
+        self.client.logout()
+
+        # get withdraw page
+        login_testing_unauthorized(self, "secretary", withdraw_url)
+        r = self.client.get(withdraw_url)
         self.assertEqual(r.status_code, 200)
 
         # withdraw
-        r = self.client.post(url, { "action": "withdraw" })
+        r = self.client.post(withdraw_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")
+        e = doc.latest_event()
+        self.assertEqual(e.type, "changed_review_request")
+        self.assertTrue("Withdrew" in e.desc)
+
+    def test_reject_request_assignment(self):
+        doc = make_test_data()
+        review_req = make_review_data(doc)
+        review_req.state = ReviewRequestStateName.objects.get(slug="accepted")
+        review_req.save()
+
+        reject_url = urlreverse('ietf.doc.views_review.reject_request_assignment', kwargs={ "name": doc.name, "request_id": review_req.pk })
+
+
+        # follow link
+        req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk })
+        self.client.login(username="secretary", password="secretary+password")
+        r = self.client.get(req_url)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(reject_url in unicontent(r))
+        self.client.logout()
+
+        # get reject page
+        login_testing_unauthorized(self, "secretary", reject_url)
+        r = self.client.get(reject_url)
+        self.assertEqual(r.status_code, 200)
+        self.assertTrue(unicode(review_req.reviewer.role.person) in unicontent(r))
+
+        # reject
+        r = self.client.post(reject_url, { "action": "reject" })
+        self.assertEqual(r.status_code, 302)
+
+        review_req = reload_db_objects(review_req)
+        self.assertEqual(review_req.state_id, "rejected")
+        e = doc.latest_event()
+        self.assertEqual(e.type, "changed_review_request")
+        self.assertTrue("unassigned" in e.desc)
+        self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="accepted").count(), 1)
+        

ietf/doc/urls_review.py

@@ -5,5 +5,6 @@
     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),
+    url(r'^(?P<request_id>[0-9]+)/rejectassignment/$', views_review.reject_request_assignment),
 )
 

ietf/doc/utils.py

@@ -99,6 +99,12 @@ def can_request_review_of_doc(user, doc):
 
     return is_authorized_in_doc_stream(user, doc)
 
+def can_manage_review_requests_for_team(user, team):
+    if not user.is_authenticated():
+        return False
+
+    return Role.objects.filter(name="secretary", person__user=user, group=team).exists() or has_role(user, "Secretariat")
+
 def two_thirds_rule( recused=0 ):
     # For standards-track, need positions from 2/3 of the non-recused current IESG.
     active = Role.objects.filter(name="ad",group__type="area",group__state="active").count()

ietf/doc/views_doc.py

@@ -357,7 +357,7 @@ def document_main(request, name, rev=None):
         published = doc.latest_event(type="published_rfc")
         started_iesg_process = doc.latest_event(type="started_iesg_process")
 
-        review_requests = ReviewRequest.objects.filter(doc=doc)
+        review_requests = ReviewRequest.objects.filter(doc=doc).exclude(state__in=["withdrawn", "rejected"])
 
         return render_to_response("doc/document_draft.html",
                                   dict(doc=doc,

ietf/doc/views_review.py

@@ -6,8 +6,8 @@
 from django.contrib.auth.decorators import login_required
 
 from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent
-from ietf.doc.utils import can_request_review_of_doc
-from ietf.ietfauth.utils import is_authorized_in_doc_stream
+from ietf.doc.utils import can_request_review_of_doc, can_manage_review_requests_for_team
+from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person
 from ietf.review.models import ReviewRequest, ReviewRequestStateName
 from ietf.review.utils import active_review_teams
 from ietf.utils.fields import DatepickerDateField
@@ -104,10 +104,21 @@ def review_request(request, name, request_id):
     doc = get_object_or_404(Document, name=name)
     review_req = get_object_or_404(ReviewRequest, pk=request_id)
 
+    is_reviewer = review_req.reviewer and user_is_person(request.user, review_req.reviewer.role.person)
+    can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team)
+
+    can_withdraw_request = (review_req.state_id in ["requested", "accepted"]
+                            and is_authorized_in_doc_stream(request.user, doc))
+
+    can_reject_request_assignment = (review_req.state_id in ["requested", "accepted"]
+                                     and review_req.reviewer_id is not None
+                                     and (is_reviewer or can_manage_req))
+
     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),
+        'can_withdraw_request': can_withdraw_request,
+        'can_reject_request_assignment': can_reject_request_assignment,
     })
 
 def withdraw_request(request, name, request_id):
@@ -122,7 +133,7 @@ def withdraw_request(request, name, request_id):
         review_req.save()
 
         DocEvent.objects.create(
-            type="withdrew_review_request",
+            type="changed_review_request",
             doc=doc,
             by=request.user.person,
             desc="Withdrew request for {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()),
@@ -138,3 +149,53 @@ def withdraw_request(request, name, request_id):
         'doc': doc,
         'review_req': review_req,
     })
+
+class RejectRequestAssignmentForm(forms.Form):
+    message_to_secretary = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional explanation of rejection, will be emailed to team secretary")
+
+def reject_request_assignment(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 review_req.reviewer:
+        return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk)
+
+    is_reviewer = user_is_person(request.user, review_req.reviewer.role.person)
+    can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team)
+
+    if not (is_reviewer or can_manage_req):
+        return HttpResponseForbidden("You do not have permission to perform this action")
+
+    if request.method == "POST" and request.POST.get("action") == "reject":
+        # reject the old request
+        prev_state = review_req.state
+        review_req.state = ReviewRequestStateName.objects.get(slug="rejected")
+        review_req.save()
+
+        # assignment of reviewer is currently considered minutia, so
+        # not reported in the log
+        if prev_state.slug == "accepted":
+            DocEvent.objects.create(
+                type="changed_review_request",
+                doc=doc,
+                by=request.user.person,
+                desc="Request for {} review by {} is unassigned".format(review_req.type.name, review_req.team.acronym.upper()),
+            )
+
+        # make a new, open review request
+        ReviewRequest.objects.create(
+            time=review_req.time,
+            type=review_req.type,
+            doc=review_req.doc,
+            team=review_req.team,
+            deadline=review_req.deadline,
+            requested_rev=review_req.requested_rev,
+            state=prev_state,
+        )
+
+        return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk)
+
+    return render(request, 'doc/review/reject_request_assignment.html', {
+        'doc': doc,
+        'review_req': review_req,
+    })

ietf/name/migrations/0012_insert_review_name_data.py

@@ -12,7 +12,9 @@ def insert_initial_review_data(apps, schema_editor):
     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="noresponse", name="No Response", order=6)
-    ReviewRequestStateName.objects.get_or_create(slug="completed", name="Completed", order=7)
+    ReviewRequestStateName.objects.get_or_create(slug="part-completed", name="Partially Completed", order=6)
+    ReviewRequestStateName.objects.get_or_create(slug="completed", name="Completed", order=8)
+
 
     ReviewTypeName = apps.get_model("name", "ReviewTypeName")
     ReviewTypeName.objects.get_or_create(slug="early", name="Early", order=1)

ietf/name/models.py

@@ -89,7 +89,7 @@ class LiaisonStatementTagName(NameModel):
     "Action Required, Action Taken"
 class ReviewRequestStateName(NameModel):
     """Requested, Accepted, Rejected, Withdrawn, Overtaken By Events,
-    No Response, Completed"""
+    No Response, Partially Completed, Completed"""
 class ReviewTypeName(NameModel):
     """Early Review, Last Call, Telechat"""
 class ReviewResultName(NameModel):

ietf/templates/doc/review/reject_request_assignment.html

@@ -0,0 +1,22 @@
+{% extends "base.html" %}
+{# Copyright The IETF Trust 2016, All Rights Reserved #}
+{% load origin bootstrap3 static %}
+
+{% block title %}Reject review assignment for {{ review_req.doc.name }}{% endblock %}
+
+{% block content %}
+  {% origin %}
+  <h1>Reject review assignment<br><small>{{ review_req.doc.name }}</small></h1>
+
+  <p>{{ review_req.reviewer.role.person }} is currently assigned to do the review. Do you want to reject this assignment?</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="reject">Reject assignment</button>
+    {% endbuttons %}
+  </form>
+
+{% endblock %}

ietf/templates/doc/review/review_request.html

@@ -64,7 +64,13 @@ <h1>Review request<br><small>{{ review_req.doc.name }}</small></h1>
         <tr>
           <th></th>
           <th>Reviewer</th>
-          <td>{{ review_req.reviewer.role.person }}</td>
+          <td>
+            {{ review_req.reviewer.role.person }}
+
+            {% if can_reject_request_assignment %}
+              <a class="btn btn-default btn-xs" href="{% url "ietf.doc.views_review.reject_request_assignment" name=doc.name request_id=review_req.pk %}"><span class="fa fa-ban"></span> Reject request assignment</a>
+            {% endif %}
+          </td>
         </tr>
       {% endif %}