Merged in [16883] from sasha@dashcare.nl:

Fix #2277 - Do not allow reviewers to reject overdue reviews.
If a review request is past the deadline, reviewers will no longer be
able to reject the assignment.
 - Legacy-Id: 16909
Note: SVN reference [16883] has been migrated to Git commit 3c2b01b3ff
This commit is contained in:
Henrik Levkowetz 2019-10-22 20:59:13 +00:00
commit 3e9bad1c8b
4 changed files with 43 additions and 11 deletions

View file

@ -473,6 +473,8 @@ class ReviewTests(TestCase):
r = self.client.get(reject_url)
self.assertEqual(r.status_code, 200)
self.assertContains(r, str(assignment.reviewer.person))
self.assertNotContains(r, 'can not be rejected')
self.assertContains(r, '<button type="submit"')
# reject
empty_outbox()
@ -490,6 +492,28 @@ class ReviewTests(TestCase):
self.assertNotIn("<reviewsecretary@example.com>", outbox[0]["To"])
self.assertTrue("Test message" in outbox[0].get_payload(decode=True).decode("utf-8"))
# try again, but now with an expired review request, which should not be allowed (#2277)
assignment.state_id = 'assigned'
assignment.save()
review_req.deadline = datetime.date(2019, 1, 1)
review_req.save()
r = self.client.get(reject_url)
self.assertEqual(r.status_code, 200)
self.assertContains(r, str(assignment.reviewer.person))
self.assertContains(r, 'can not be rejected')
self.assertNotContains(r, '<button type="submit"')
# even though the form is not visible, try posting to the URL, which should not work
empty_outbox()
r = self.client.post(reject_url, { "action": "reject", "message_to_secretary": "Test message" })
self.assertEqual(r.status_code, 200)
self.assertContains(r, 'can not be rejected')
assignment = reload_db_objects(assignment)
self.assertEqual(assignment.state_id, "assigned")
self.assertEqual(len(outbox), 0)
def make_test_mbox_tarball(self, review_req):
mbox_path = os.path.join(self.review_dir, "testmbox.tar.gz")
with tarfile.open(mbox_path, "w:gz") as tar:

View file

@ -322,6 +322,7 @@ class RejectReviewerAssignmentForm(forms.Form):
def reject_reviewer_assignment(request, name, assignment_id):
doc = get_object_or_404(Document, name=name)
review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"])
review_request_past_deadline = review_assignment.review_request.deadline < datetime.date.today()
if not review_assignment.reviewer:
return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk)
@ -332,7 +333,7 @@ def reject_reviewer_assignment(request, name, assignment_id):
if not (is_reviewer or can_manage_request):
return HttpResponseForbidden("You do not have permission to perform this action")
if request.method == "POST" and request.POST.get("action") == "reject":
if request.method == "POST" and request.POST.get("action") == "reject" and not review_request_past_deadline:
form = RejectReviewerAssignmentForm(request.POST)
if form.is_valid():
# reject the assignment
@ -370,6 +371,7 @@ def reject_reviewer_assignment(request, name, assignment_id):
'review_req': review_assignment.review_request,
'assignments': review_assignment.review_request.reviewassignment_set.all(),
'form': form,
'review_request_past_deadline': review_request_past_deadline,
})
@login_required

View file

@ -108,7 +108,7 @@ class IndexTests(TestCase):
self.assertEqual(t[12], ".pdf,.txt")
self.assertEqual(t[13], draft.title)
author = draft.documentauthor_set.order_by("order").get()
self.assertEqual(t[14], "%s <%s>" % (author.person.name, author.email.address))
self.assertEqual(t[14], "%s <%s>" % (author.person.plain_name(), author.email.address))
self.assertEqual(t[15], "%s <%s>" % (draft.shepherd.person.plain_ascii(), draft.shepherd.address))
self.assertEqual(t[16], "%s <%s>" % (draft.ad.plain_ascii(), draft.ad.email_address()))

View file

@ -10,17 +10,23 @@
{% include "doc/review/request_info.html" %}
<p>Do you want to reject this assignment?</p>
{% if not review_request_past_deadline %}
<p>Do you want to reject this assignment?</p>
<form method="post">
{% csrf_token %}
<form method="post">
{% csrf_token %}
{% bootstrap_form form %}
{% bootstrap_form form %}
{% 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>
{% 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>
{% else %}
<p class="alert alert-info">
This review assignment can not be rejected, as the deadline of the review request has already passed.
</p>
{% endif %}
{% endblock %}