diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 26d610c03..ea969cfe2 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -707,7 +707,7 @@ EVENT_TYPES = [ # review ("requested_review", "Requested review"), - ("withdrew_review_request", "Withdrew review"), + ("changed_review_request", "Changed review request"), ] class DocEvent(models.Model): diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index 01c27711f..99e307b90 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -108,16 +108,63 @@ class ReviewTests(TestCase): 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) + diff --git a/ietf/doc/urls_review.py b/ietf/doc/urls_review.py index afc2f6536..8ed0b5094 100644 --- a/ietf/doc/urls_review.py +++ b/ietf/doc/urls_review.py @@ -5,5 +5,6 @@ urlpatterns = patterns('', url(r'^$', views_review.request_review), url(r'^(?P[0-9]+)/$', views_review.review_request), url(r'^(?P[0-9]+)/withdraw/$', views_review.withdraw_request), + url(r'^(?P[0-9]+)/rejectassignment/$', views_review.reject_request_assignment), ) diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 923715895..3aaa1604b 100644 --- a/ietf/doc/utils.py +++ b/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() diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index d25d88fc3..0e7bc7838 100644 --- a/ietf/doc/views_doc.py +++ b/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, diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index d1f7861bb..5c8face33 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -6,8 +6,8 @@ from django import forms 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, + }) diff --git a/ietf/name/migrations/0012_insert_review_name_data.py b/ietf/name/migrations/0012_insert_review_name_data.py index ed1a4bfe6..1ebbba69f 100644 --- a/ietf/name/migrations/0012_insert_review_name_data.py +++ b/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) diff --git a/ietf/name/models.py b/ietf/name/models.py index 733f957b3..b16da81f7 100644 --- a/ietf/name/models.py +++ b/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): diff --git a/ietf/templates/doc/review/reject_request_assignment.html b/ietf/templates/doc/review/reject_request_assignment.html new file mode 100644 index 000000000..9ccd6c56b --- /dev/null +++ b/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 %} +

Reject review assignment
{{ review_req.doc.name }}

+ +

{{ review_req.reviewer.role.person }} is currently assigned to do the review. Do you want to reject this assignment?

+ +
+ {% csrf_token %} + + {% buttons %} + Cancel + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index f1087cd84..954c4adcf 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -64,7 +64,13 @@ Reviewer - {{ review_req.reviewer.role.person }} + + {{ review_req.reviewer.role.person }} + + {% if can_reject_request_assignment %} + Reject request assignment + {% endif %} + {% endif %}