Skip to content

Commit 3c2b01b

Browse files
author
Sasha Romijn
committed
Fix ietf-tools#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. Commit ready for merge. - Legacy-Id: 16883
1 parent f651320 commit 3c2b01b

3 files changed

Lines changed: 45 additions & 13 deletions

File tree

ietf/doc/tests_review.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,8 @@ def test_reject_reviewer_assignment(self):
473473
r = self.client.get(reject_url)
474474
self.assertEqual(r.status_code, 200)
475475
self.assertContains(r, str(assignment.reviewer.person))
476+
self.assertNotContains(r, 'can not be rejected')
477+
self.assertContains(r, '<button type="submit"')
476478

477479
# reject
478480
empty_outbox()
@@ -489,6 +491,28 @@ def test_reject_reviewer_assignment(self):
489491
self.assertFalse("<reviewsecretary@example.com>" in outbox[0]["To"])
490492
self.assertTrue("Test message" in outbox[0].get_payload(decode=True).decode("utf-8"))
491493

494+
# try again, but now with an expired review request, which should not be allowed (#2277)
495+
assignment.state_id = 'assigned'
496+
assignment.save()
497+
review_req.deadline = datetime.date(2019, 1, 1)
498+
review_req.save()
499+
500+
r = self.client.get(reject_url)
501+
self.assertEqual(r.status_code, 200)
502+
self.assertContains(r, str(assignment.reviewer.person))
503+
self.assertContains(r, 'can not be rejected')
504+
self.assertNotContains(r, '<button type="submit"')
505+
506+
# even though the form is not visible, try posting to the URL, which should not work
507+
empty_outbox()
508+
r = self.client.post(reject_url, { "action": "reject", "message_to_secretary": "Test message" })
509+
self.assertEqual(r.status_code, 200)
510+
self.assertContains(r, 'can not be rejected')
511+
512+
assignment = reload_db_objects(assignment)
513+
self.assertEqual(assignment.state_id, "assigned")
514+
self.assertEqual(len(outbox), 0)
515+
492516
def make_test_mbox_tarball(self, review_req):
493517
mbox_path = os.path.join(self.review_dir, "testmbox.tar.gz")
494518
with tarfile.open(mbox_path, "w:gz") as tar:

ietf/doc/views_review.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ class RejectReviewerAssignmentForm(forms.Form):
320320
def reject_reviewer_assignment(request, name, assignment_id):
321321
doc = get_object_or_404(Document, name=name)
322322
review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"])
323+
review_request_past_deadline = review_assignment.review_request.deadline < datetime.date.today()
323324

324325
if not review_assignment.reviewer:
325326
return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk)
@@ -330,7 +331,7 @@ def reject_reviewer_assignment(request, name, assignment_id):
330331
if not (is_reviewer or can_manage_request):
331332
return HttpResponseForbidden("You do not have permission to perform this action")
332333

333-
if request.method == "POST" and request.POST.get("action") == "reject":
334+
if request.method == "POST" and request.POST.get("action") == "reject" and not review_request_past_deadline:
334335
form = RejectReviewerAssignmentForm(request.POST)
335336
if form.is_valid():
336337
# reject the assignment
@@ -367,6 +368,7 @@ def reject_reviewer_assignment(request, name, assignment_id):
367368
'review_req': review_assignment.review_request,
368369
'assignments': review_assignment.review_request.reviewassignment_set.all(),
369370
'form': form,
371+
'review_request_past_deadline': review_request_past_deadline,
370372
})
371373

372374
@login_required

ietf/templates/doc/review/reject_reviewer_assignment.html

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,23 @@ <h1>Reject review assignment<br><small>{{ review_req.doc.name }}</small></h1>
1010

1111
{% include "doc/review/request_info.html" %}
1212

13-
<p>Do you want to reject this assignment?</p>
14-
15-
<form method="post">
16-
{% csrf_token %}
17-
18-
{% bootstrap_form form %}
19-
20-
{% buttons %}
21-
<a class="btn btn-default" href="{% url "ietf.doc.views_review.review_request" name=doc.canonical_name request_id=review_req.pk %}">Cancel</a>
22-
<button type="submit" class="btn btn-primary" name="action" value="reject">Reject assignment</button>
23-
{% endbuttons %}
24-
</form>
13+
{% if not review_request_past_deadline %}
14+
<p>Do you want to reject this assignment?</p>
15+
16+
<form method="post">
17+
{% csrf_token %}
18+
19+
{% bootstrap_form form %}
20+
21+
{% buttons %}
22+
<a class="btn btn-default" href="{% url "ietf.doc.views_review.review_request" name=doc.canonical_name request_id=review_req.pk %}">Cancel</a>
23+
<button type="submit" class="btn btn-primary" name="action" value="reject">Reject assignment</button>
24+
{% endbuttons %}
25+
</form>
26+
{% else %}
27+
<p class="alert alert-info">
28+
This review assignment can not be rejected, as the deadline of the review request has already passed.
29+
</p>
30+
{% endif %}
2531

2632
{% endblock %}

0 commit comments

Comments
 (0)