Skip to content

Commit 3e9bad1

Browse files
committed
Merged in [16883] from sasha@dashcare.nl:
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. - Legacy-Id: 16909 Note: SVN reference [16883] has been migrated to Git commit 3c2b01b
2 parents 2b211f1 + 3c2b01b commit 3e9bad1

4 files changed

Lines changed: 46 additions & 14 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()
@@ -490,6 +492,28 @@ def test_reject_reviewer_assignment(self):
490492
self.assertNotIn("<reviewsecretary@example.com>", outbox[0]["To"])
491493
self.assertTrue("Test message" in outbox[0].get_payload(decode=True).decode("utf-8"))
492494

495+
# try again, but now with an expired review request, which should not be allowed (#2277)
496+
assignment.state_id = 'assigned'
497+
assignment.save()
498+
review_req.deadline = datetime.date(2019, 1, 1)
499+
review_req.save()
500+
501+
r = self.client.get(reject_url)
502+
self.assertEqual(r.status_code, 200)
503+
self.assertContains(r, str(assignment.reviewer.person))
504+
self.assertContains(r, 'can not be rejected')
505+
self.assertNotContains(r, '<button type="submit"')
506+
507+
# even though the form is not visible, try posting to the URL, which should not work
508+
empty_outbox()
509+
r = self.client.post(reject_url, { "action": "reject", "message_to_secretary": "Test message" })
510+
self.assertEqual(r.status_code, 200)
511+
self.assertContains(r, 'can not be rejected')
512+
513+
assignment = reload_db_objects(assignment)
514+
self.assertEqual(assignment.state_id, "assigned")
515+
self.assertEqual(len(outbox), 0)
516+
493517
def make_test_mbox_tarball(self, review_req):
494518
mbox_path = os.path.join(self.review_dir, "testmbox.tar.gz")
495519
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
@@ -322,6 +322,7 @@ class RejectReviewerAssignmentForm(forms.Form):
322322
def reject_reviewer_assignment(request, name, assignment_id):
323323
doc = get_object_or_404(Document, name=name)
324324
review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"])
325+
review_request_past_deadline = review_assignment.review_request.deadline < datetime.date.today()
325326

326327
if not review_assignment.reviewer:
327328
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):
332333
if not (is_reviewer or can_manage_request):
333334
return HttpResponseForbidden("You do not have permission to perform this action")
334335

335-
if request.method == "POST" and request.POST.get("action") == "reject":
336+
if request.method == "POST" and request.POST.get("action") == "reject" and not review_request_past_deadline:
336337
form = RejectReviewerAssignmentForm(request.POST)
337338
if form.is_valid():
338339
# reject the assignment
@@ -370,6 +371,7 @@ def reject_reviewer_assignment(request, name, assignment_id):
370371
'review_req': review_assignment.review_request,
371372
'assignments': review_assignment.review_request.reviewassignment_set.all(),
372373
'form': form,
374+
'review_request_past_deadline': review_request_past_deadline,
373375
})
374376

375377
@login_required

ietf/idindex/tests.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ def get_fields(content):
108108
self.assertEqual(t[12], ".pdf,.txt")
109109
self.assertEqual(t[13], draft.title)
110110
author = draft.documentauthor_set.order_by("order").get()
111-
self.assertEqual(t[14], "%s <%s>" % (author.person.name, author.email.address))
111+
self.assertEqual(t[14], "%s <%s>" % (author.person.plain_name(), author.email.address))
112112
self.assertEqual(t[15], "%s <%s>" % (draft.shepherd.person.plain_ascii(), draft.shepherd.address))
113113
self.assertEqual(t[16], "%s <%s>" % (draft.ad.plain_ascii(), draft.ad.email_address()))
114114

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)