altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 3 Packages Projects Releases Wiki Activity

Add simple email notifications for assigning/rejecting review requests

Browse source 
- Legacy-Id: 11236
This commit is contained in:
Ole Laursen 2016-05-24 14:02:59 +00:00
parent 5dd079e2f8
commit b9f4b7005e
5 changed files with 117 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
ietf/doc/tests_review.py
View file

@ -107,6 +107,7 @@ class ReviewTests(TestCase):
self.assertEqual(r.status_code, 200)
# withdraw
empty_outbox()
r = self.client.post(withdraw_url, { "action": "withdraw" })
self.assertEqual(r.status_code, 302)
@ -115,6 +116,8 @@ class ReviewTests(TestCase):
e = doc.latest_event()
self.assertEqual(e.type, "changed_review_request")
self.assertTrue("Withdrew" in e.desc)
self.assertEqual(len(outbox), 1)
self.assertTrue("withdrawn" in unicode(outbox[0]))
def test_assign_reviewer(self):
doc = make_test_data()
@ -140,6 +143,7 @@ class ReviewTests(TestCase):
self.assertEqual(r.status_code, 200)
# assign
empty_outbox()
reviewer = Role.objects.filter(name="reviewer", group=review_req.team).first()
r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk })
self.assertEqual(r.status_code, 302)
@ -147,8 +151,11 @@ class ReviewTests(TestCase):
review_req = reload_db_objects(review_req)
self.assertEqual(review_req.state_id, "requested")
self.assertEqual(review_req.reviewer, reviewer)
self.assertEqual(len(outbox), 1)
self.assertTrue("assigned" in unicode(outbox[0]))
# re-assign
empty_outbox()
review_req.state = ReviewRequestStateName.objects.get(slug="accepted")
review_req.save()
reviewer = Role.objects.filter(name="reviewer", group=review_req.team).exclude(pk=reviewer.pk).first()
@ -156,8 +163,11 @@ class ReviewTests(TestCase):
self.assertEqual(r.status_code, 302)
review_req = reload_db_objects(review_req)
self.assertEqual(review_req.state_id, "requested")
self.assertEqual(review_req.state_id, "requested") # check that state is reset
self.assertEqual(review_req.reviewer, reviewer)
self.assertEqual(len(outbox), 2)
self.assertTrue("cancelled your assignment" in unicode(outbox[0]))
self.assertTrue("assigned" in unicode(outbox[1]))
def test_reject_reviewer_assignment(self):
doc = make_test_data()
@ -194,4 +204,5 @@ class ReviewTests(TestCase):
self.assertTrue("rejected" in e.desc)
self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="requested").count(), 1)
self.assertEqual(len(outbox), 1)
self.assertTrue("Test message" in unicode(outbox[0]))

80
ietf/doc/views_review.py
View file

@ -10,8 +10,9 @@ from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person
from ietf.name.models import ReviewRequestStateName
from ietf.group.models import Role
from ietf.review.models import ReviewRequest
from ietf.review.utils import active_review_teams, assign_review_request_to_reviewer
from ietf.review.utils import can_request_review_of_doc, can_manage_review_requests_for_team
from ietf.review.utils import (active_review_teams, assign_review_request_to_reviewer,
can_request_review_of_doc, can_manage_review_requests_for_team,
email_about_review_request)
from ietf.utils.fields import DatepickerDateField
class RequestReviewForm(forms.ModelForm):
@ -134,6 +135,7 @@ def withdraw_request(request, name, request_id):
return HttpResponseForbidden("You do not have permission to perform this action")
if request.method == "POST" and request.POST.get("action") == "withdraw":
prev_state = review_req.state
review_req.state = ReviewRequestStateName.objects.get(slug="withdrawn")
review_req.save()
@ -144,9 +146,12 @@ def withdraw_request(request, name, request_id):
desc="Withdrew request for {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()),
)
if review_req.state_id != "requested":
# FIXME: handle this case - by emailing?
pass
if prev_state.slug != "requested":
email_about_review_request(
request, review_req,
"Withdrew review request for %s" % review_req.doc.name,
"Review request has been withdrawn by %s." % request.user.person,
by=request.user.person, notify_secretary=False, notify_reviewer=True)
return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk)
@ -187,7 +192,7 @@ def assign_reviewer(request, name, request_id):
form = AssignReviewerForm(review_req, request.POST)
if form.is_valid():
reviewer = form.cleaned_data["reviewer"]
assign_review_request_to_reviewer(review_req, reviewer, request.user.person)
assign_review_request_to_reviewer(request, review_req, reviewer)
return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk)
else:
@ -216,35 +221,48 @@ def reject_reviewer_assignment(request, name, request_id):
return HttpResponseForbidden("You do not have permission to perform this action")
if request.method == "POST" and request.POST.get("action") == "reject":
# reject the request
review_req.state = ReviewRequestStateName.objects.get(slug="rejected")
review_req.save()
form = RejectReviewerAssignmentForm(request.POST)
if form.is_valid():
# reject the request
review_req.state = ReviewRequestStateName.objects.get(slug="rejected")
review_req.save()
DocEvent.objects.create(
type="changed_review_request",
doc=review_req.doc,
by=request.user.person,
desc="Assignment of request for {} review by {} to {} was rejected".format(
review_req.type.name,
review_req.team.acronym.upper(),
review_req.reviewer.person,
),
)
# make a new unassigned review request
new_review_req = 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=ReviewRequestStateName.objects.get(slug="requested"),
)
DocEvent.objects.create(
type="changed_review_request",
doc=review_req.doc,
by=request.user.person,
desc="Assignment of request for {} review by {} to {} was rejected".format(
review_req.type.name,
review_req.team.acronym.upper(),
review_req.reviewer.person,
),
)
return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk)
# make a new unassigned review request
new_review_req = 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=ReviewRequestStateName.objects.get(slug="requested"),
)
msg = u"Reviewer assignment rejected by %s." % request.user.person
m = form.cleaned_data.get("message_to_secretary")
if m:
msg += "\n\n" + "Explanation:" + "\n" + m
email_about_review_request(request, review_req, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True)
return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk)
else:
form = RejectReviewerAssignmentForm()
return render(request, 'doc/review/reject_reviewer_assignment.html', {
'doc': doc,
'review_req': review_req,
'form': form,
})

2
ietf/review/models.py
View file

@ -27,7 +27,7 @@ class ReviewRequest(models.Model):
time = models.DateTimeField(auto_now_add=True)
type = models.ForeignKey(ReviewTypeName)
doc = models.ForeignKey(Document, related_name='review_request_set')
team = models.ForeignKey(Group)
team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewresultname=None))
deadline = models.DateTimeField()
requested_rev = models.CharField(verbose_name="requested revision", max_length=16, blank=True, help_text="Fill in if a specific revision is to be reviewed, e.g. 02")

58
ietf/review/utils.py
View file

@ -1,7 +1,10 @@
from django.contrib.sites.models import Site
from ietf.group.models import Group, Role
from ietf.doc.models import DocEvent
from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream
from ietf.review.models import ReviewRequestStateName
from ietf.utils.mail import send_mail
def active_review_teams():
# if there's a ReviewResultName defined, it's a review team
@ -17,16 +20,47 @@ 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")
return Role.objects.filter(name__in=["secretary", "delegate"], person__user=user, group=team).exists() or has_role(user, "Secretariat")
def assign_review_request_to_reviewer(review_req, reviewer, by):
assert review_req.state_id in ("requested", "accepted")
def email_about_review_request(request, review_req, subject, msg, by, notify_secretary, notify_reviewer):
"""Notify possibly both secretary and reviewer about change, skipping
a party if the change was done by that party."""
if review_req.reviewer == reviewer:
def extract_email_addresses(roles):
if any(r.person == by for r in roles if r):
return []
else:
return [r.formatted_email() for r in roles if r]
to = []
if notify_secretary:
to += extract_email_addresses(Role.objects.filter(name__in=["secretary", "delegate"], group=review_req.team).distinct())
if notify_reviewer:
to += extract_email_addresses([review_req.reviewer])
if not to:
return
prev_state = review_req.state
prev_reviewer = review_req.reviewer
send_mail(request, list(set(to)), None, subject, "doc/mail/review_request_changed.txt", {
"domain": Site.objects.get_current().domain,
"review_req": review_req,
"msg": msg,
})
def assign_review_request_to_reviewer(request, review_req, reviewer):
assert review_req.state_id in ("requested", "accepted")
if reviewer == review_req.reviewer:
return
if review_req.reviewer:
email_about_review_request(
request, review_req,
"Unassigned from review of %s" % review_req.doc.name,
"%s has cancelled your assignment to the review." % request.user.person,
by=request.user.person, notify_secretary=False, notify_reviewer=True)
review_req.state = ReviewRequestStateName.objects.get(slug="requested")
review_req.reviewer = reviewer
@ -35,14 +69,16 @@ def assign_review_request_to_reviewer(review_req, reviewer, by):
DocEvent.objects.create(
type="changed_review_request",
doc=review_req.doc,
by=by,
by=request.user.person,
desc="Request for {} review by {} is assigned to {}".format(
review_req.type.name,
review_req.team.acronym.upper(),
review_req.reviewer.person if review_req.reviewer else "(None)",
),
)
if prev_state.slug != "requested" and prev_reviewer:
# FIXME: email old reviewer?
pass
email_about_review_request(
request, review_req,
"Assigned to review of %s" % review_req.doc.name,
"%s has assigned you to review the document." % request.user.person,
by=request.user.person, notify_secretary=False, notify_reviewer=True)

7
ietf/templates/doc/mail/review_request_changed.txt Normal file
View file

@ -0,0 +1,7 @@
{% autoescape off %}
{{ review_req.type.name }} review of: {{ review_req.doc.name }}{% if review_req.requested_rev %}-{{ review_req.requested_rev }}{% endif %}
https://{{ domain }}{% url "ietf.doc.views_review.review_request" name=review_req.doc.name request_id=review_req.pk %}
{{ msg|wordwrap:72 }}
{% endautoescape %}