Skip to content

Commit 5918024

Browse files
committed
Add a simple reminder system for reviewers so that they can opt in to
being reminded X days before deadline of an assignment - Legacy-Id: 12101
1 parent 5e030ed commit 5918024

9 files changed

Lines changed: 134 additions & 19 deletions

File tree

ietf/bin/send-reviewer-reminders

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#!/usr/bin/env python
2+
3+
import os, sys
4+
import syslog
5+
6+
# boilerplate
7+
basedir = os.path.abspath(os.path.join(os.path.dirname(__file__), "../.."))
8+
sys.path = [ basedir ] + sys.path
9+
os.environ["DJANGO_SETTINGS_MODULE"] = "ietf.settings"
10+
11+
virtualenv_activation = os.path.join(basedir, "bin", "activate_this.py")
12+
if os.path.exists(virtualenv_activation):
13+
execfile(virtualenv_activation, dict(__file__=virtualenv_activation))
14+
15+
syslog.openlog(os.path.basename(__file__), syslog.LOG_PID, syslog.LOG_USER)
16+
17+
import django
18+
django.setup()
19+
20+
import datetime
21+
from ietf.review.utils import review_requests_needing_reviewer_reminder, email_reviewer_reminder
22+
23+
for review_req in review_requests_needing_reviewer_reminder(datetime.date.today()):
24+
email_reviewer_reminder(review_req)
25+
print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_req.reviewer.address, review_req.doc_id, review_req.team.acronym, review_req.pk))

ietf/group/tests_review.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from ietf.person.models import Email, Person
1212
from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod
1313
from ietf.review.utils import suggested_review_requests_for_team
14+
from ietf.review.utils import review_requests_needing_reviewer_reminder, email_reviewer_reminder
1415
from ietf.name.models import ReviewTypeName, ReviewResultName, ReviewRequestStateName
1516
import ietf.group.views_review
1617
from ietf.utils.mail import outbox, empty_outbox
@@ -316,12 +317,14 @@ def test_change_reviewer_settings(self):
316317
"min_interval": "7",
317318
"filter_re": "test-[regexp]",
318319
"skip_next": "2",
320+
"remind_days_before_deadline": "6"
319321
})
320322
self.assertEqual(r.status_code, 302)
321323
settings = ReviewerSettings.objects.get(person=reviewer, team=review_req.team)
322324
self.assertEqual(settings.min_interval, 7)
323325
self.assertEqual(settings.filter_re, "test-[regexp]")
324326
self.assertEqual(settings.skip_next, 2)
327+
self.assertEqual(settings.remind_days_before_deadline, 6)
325328
self.assertEqual(len(outbox), 1)
326329
self.assertTrue("reviewer availability" in outbox[0]["subject"].lower())
327330
self.assertTrue("frequency changed", unicode(outbox[0]).lower())
@@ -370,3 +373,35 @@ def test_change_reviewer_settings(self):
370373
self.assertEqual(len(outbox), 1)
371374
self.assertTrue(start_date.isoformat(), unicode(outbox[0]).lower())
372375
self.assertTrue(end_date.isoformat(), unicode(outbox[0]).lower())
376+
377+
def test_reviewer_reminders(self):
378+
doc = make_test_data()
379+
380+
reviewer = Person.objects.get(name="Plain Man")
381+
382+
review_req = make_review_data(doc)
383+
384+
settings = ReviewerSettings.objects.get(team=review_req.team, person=reviewer)
385+
settings.remind_days_before_deadline = 6
386+
settings.save()
387+
388+
today = datetime.date.today()
389+
390+
review_req.reviewer = reviewer.email_set.first()
391+
review_req.deadline = today + datetime.timedelta(days=settings.remind_days_before_deadline)
392+
review_req.save()
393+
394+
needing_reminders = review_requests_needing_reviewer_reminder(today - datetime.timedelta(days=1))
395+
self.assertEqual(list(needing_reminders), [])
396+
397+
needing_reminders = review_requests_needing_reviewer_reminder(today)
398+
self.assertEqual(list(needing_reminders), [review_req])
399+
400+
needing_reminders = review_requests_needing_reviewer_reminder(today + datetime.timedelta(days=1))
401+
self.assertEqual(list(needing_reminders), [])
402+
403+
empty_outbox()
404+
email_reviewer_reminder(review_req)
405+
self.assertEqual(len(outbox), 1)
406+
print outbox[0]
407+
self.assertTrue(review_req.doc_id in unicode(outbox[0]))

ietf/group/views_review.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ def email_open_review_assignments(request, acronym, group_type=None):
350350
class ReviewerSettingsForm(forms.ModelForm):
351351
class Meta:
352352
model = ReviewerSettings
353-
fields = ['min_interval', 'filter_re', 'skip_next']
353+
fields = ['min_interval', 'filter_re', 'skip_next', 'remind_days_before_deadline']
354354

355355
class AddUnavailablePeriodForm(forms.ModelForm):
356356
class Meta:
@@ -406,7 +406,7 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None):
406406
back_url = request.GET.get("next")
407407
if not back_url:
408408
import ietf.group.views_review
409-
back_url = urlreverse(ietf.group.views_review.review_requests, kwargs={ "group_type": group.type_id, "acronym": group.acronym})
409+
back_url = urlreverse(ietf.group.views_review.reviewer_overview, kwargs={ "group_type": group.type_id, "acronym": group.acronym})
410410

411411
# settings
412412
if request.method == "POST" and request.POST.get("action") == "change_settings":

ietf/review/migrations/0001_initial.py

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,25 @@ class Migration(migrations.Migration):
2626
},
2727
bases=(models.Model,),
2828
),
29+
migrations.CreateModel(
30+
name='ResultUsedInReviewTeam',
31+
fields=[
32+
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
33+
('result', models.ForeignKey(to='name.ReviewResultName')),
34+
('team', models.ForeignKey(to='group.Group')),
35+
],
36+
options={
37+
},
38+
bases=(models.Model,),
39+
),
2940
migrations.CreateModel(
3041
name='ReviewerSettings',
3142
fields=[
3243
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
3344
('min_interval', models.IntegerField(default=30, verbose_name=b'Can review at most', choices=[(7, b'Once per week'), (14, b'Once per fortnight'), (30, b'Once per month'), (61, b'Once per two months'), (91, b'Once per quarter')])),
3445
('filter_re', models.CharField(help_text=b'Draft names matching regular expression should not be assigned', max_length=255, verbose_name=b'Filter regexp', blank=True)),
3546
('skip_next', models.IntegerField(default=0, verbose_name=b'Skip next assignments')),
47+
('remind_days_before_deadline', models.IntegerField(null=True, blank=True)),
3648
('person', models.ForeignKey(to='person.Person')),
3749
('team', models.ForeignKey(to='group.Group')),
3850
],
@@ -63,10 +75,12 @@ class Migration(migrations.Migration):
6375
bases=(models.Model,),
6476
),
6577
migrations.CreateModel(
66-
name='ResultUsedInReviewTeam',
78+
name='ReviewWish',
6779
fields=[
6880
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
69-
('result', models.ForeignKey(to='name.ReviewResultName')),
81+
('time', models.DateTimeField(default=datetime.datetime.now)),
82+
('doc', models.ForeignKey(to='doc.Document')),
83+
('person', models.ForeignKey(to='person.Person')),
7084
('team', models.ForeignKey(to='group.Group')),
7185
],
7286
options={
@@ -84,19 +98,6 @@ class Migration(migrations.Migration):
8498
},
8599
bases=(models.Model,),
86100
),
87-
migrations.CreateModel(
88-
name='ReviewWish',
89-
fields=[
90-
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
91-
('time', models.DateTimeField(default=datetime.datetime.now)),
92-
('doc', models.ForeignKey(to='doc.Document')),
93-
('person', models.ForeignKey(to='person.Person')),
94-
('team', models.ForeignKey(to='group.Group')),
95-
],
96-
options={
97-
},
98-
bases=(models.Model,),
99-
),
100101
migrations.CreateModel(
101102
name='UnavailablePeriod',
102103
fields=[

ietf/review/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class ReviewerSettings(models.Model):
2323
min_interval = models.IntegerField(default=30, verbose_name="Can review at most", choices=INTERVALS)
2424
filter_re = models.CharField(max_length=255, verbose_name="Filter regexp", blank=True, help_text="Draft names matching regular expression should not be assigned")
2525
skip_next = models.IntegerField(default=0, verbose_name="Skip next assignments")
26+
remind_days_before_deadline = models.IntegerField(null=True, blank=True, help_text="To get an email reminder in case you forget to do an assigned review, enter the number of days before a review deadline you want to receive it. Clear the field if you don't want a reminder.")
2627

2728
def __unicode__(self):
2829
return u"{} in {}".format(self.person, self.team)

ietf/review/utils.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import datetime, re, itertools
22
from collections import defaultdict
33

4-
from django.db.models import Q, Max
4+
from django.db.models import Q, Max, F
55
from django.core.urlresolvers import reverse as urlreverse
6+
from django.contrib.sites.models import Site
67

78
from ietf.group.models import Group, Role
89
from ietf.doc.models import (Document, ReviewRequestDocEvent, State,
@@ -674,3 +675,42 @@ def format_period(p):
674675
ranking.sort(key=lambda r: r["scores"], reverse=True)
675676

676677
return [(r["email"].pk, r["label"]) for r in ranking]
678+
679+
def review_requests_needing_reviewer_reminder(remind_date):
680+
reqs_qs = ReviewRequest.objects.filter(
681+
state__in=("requested", "accepted"),
682+
reviewer__person__reviewersettings__remind_days_before_deadline__isnull=False,
683+
reviewer__person__reviewersettings__team=F("team"),
684+
).exclude(
685+
reviewer=None
686+
).values_list("pk", "deadline", "reviewer__person__reviewersettings__remind_days_before_deadline").distinct()
687+
688+
req_pks = []
689+
for r_pk, deadline, remind_days in reqs_qs:
690+
if (deadline - remind_date).days == remind_days:
691+
req_pks.append(r_pk)
692+
693+
return ReviewRequest.objects.filter(pk__in=req_pks).select_related("reviewer", "reviewer__person", "state", "team")
694+
695+
def email_reviewer_reminder(review_request):
696+
team = review_request.team
697+
698+
deadline_days = (review_request.deadline - datetime.date.today()).days
699+
700+
subject = "Reminder: deadline for review of {} in {} is {}".format(review_request.doc_id, team.acronym, review_request.deadline.isoformat())
701+
702+
overview_url = urlreverse("ietf.ietfauth.views.review_overview")
703+
request_url = urlreverse("ietf.doc.views_review.review_request", kwargs={ "name": review_request.doc_id, "request_id": review_request.pk })
704+
705+
domain = Site.objects.get_current().domain
706+
707+
settings = ReviewerSettings.objects.filter(person=review_request.reviewer.person, team=team).first()
708+
remind_days = settings.remind_days_before_deadline if settings else 0
709+
710+
send_mail(None, [review_request.reviewer.formatted_email()], None, subject, "review/reviewer_reminder.txt", {
711+
"reviewer_overview_url": "https://{}{}".format(domain, overview_url),
712+
"review_request_url": "https://{}{}".format(domain, request_url),
713+
"review_request": review_request,
714+
"deadline_days": deadline_days,
715+
"remind_days": remind_days,
716+
})

ietf/templates/doc/review_request_summary.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
<a href="{% if review_request.review %}{% url "doc_view" review_request.review.name %}{% else %}{% url "ietf.doc.views_review.review_request" review_request.doc_id review_request.pk %}{% endif %}">
44
{{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review{% if review_request.reviewed_rev and review_request.reviewed_rev != current_rev or review_request.doc_id != current_doc_name %} (of {% if review_request.doc_id != current_doc_name %}{{ review_request.doc_id }}{% endif %}-{{ review_request.reviewed_rev }}){% endif %}{% if review_request.result %}:
55
{{ review_request.result.name }}{% endif %} {% if review_request.state_id == "part-completed" %}(partially completed){% endif %}
6+
</a>
67
{% else %}
78
<i>
89
<a href="{% url "ietf.doc.views_review.review_request" review_request.doc_id review_request.pk %}">{{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review

ietf/templates/ietfauth/review_overview.html

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,11 @@ <h2>Settings for {{ t }}</h2>
134134
</tr>
135135
<tr>
136136
<th>Filter regexp</th>
137-
<td><code>{{ t.reviewer_settings.filter_re|default:"(None)" }}</code></td>
137+
<td>{% if t.reviewer_settings.filter_re %}<code>{{ t.reviewer_settings.filter_re }}</code>{% else %}(None){% endif %}</td>
138+
</tr>
139+
<tr>
140+
<th>Remind days before deadline</th>
141+
<td>{{ t.reviewer_settings.remind_days_before_deadline|default:"(Do not remind)" }}</td>
138142
</tr>
139143
<tr>
140144
<th>Unavailable periods</th>
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{% autoescape off %}{% filter wordwrap:70 %}This is just a friendly reminder that the deadline for the review of {{ review_request.doc_id }} is in {{ deadline_days }} day{{ deadline_days|pluralize }}:
2+
3+
{{ review_request_url }}
4+
5+
You are receiving this reminder because you have configured the Datatracker to remind you {{ remind_days }} day{{ remind_days|pluralize }} before deadlines in {{ review_request.team.name }}. You can see your reviews and change your settings here:
6+
7+
{{ reviewer_overview_url }}
8+
{% endfilter %}{% endautoescape %}

0 commit comments

Comments
 (0)