From 59180240afcff35eaffb65a0667e86b4ca4398ae Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Fri, 7 Oct 2016 16:43:29 +0000 Subject: [PATCH] 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 --- ietf/bin/send-reviewer-reminders | 25 +++++++++++ ietf/group/tests_review.py | 35 ++++++++++++++++ ietf/group/views_review.py | 4 +- ietf/review/migrations/0001_initial.py | 31 +++++++------- ietf/review/models.py | 1 + ietf/review/utils.py | 42 ++++++++++++++++++- .../templates/doc/review_request_summary.html | 1 + ietf/templates/ietfauth/review_overview.html | 6 ++- ietf/templates/review/reviewer_reminder.txt | 8 ++++ 9 files changed, 134 insertions(+), 19 deletions(-) create mode 100755 ietf/bin/send-reviewer-reminders create mode 100644 ietf/templates/review/reviewer_reminder.txt diff --git a/ietf/bin/send-reviewer-reminders b/ietf/bin/send-reviewer-reminders new file mode 100755 index 000000000..07d7f3e46 --- /dev/null +++ b/ietf/bin/send-reviewer-reminders @@ -0,0 +1,25 @@ +#!/usr/bin/env python + +import os, sys +import syslog + +# boilerplate +basedir = os.path.abspath(os.path.join(os.path.dirname(__file__), "../..")) +sys.path = [ basedir ] + sys.path +os.environ["DJANGO_SETTINGS_MODULE"] = "ietf.settings" + +virtualenv_activation = os.path.join(basedir, "bin", "activate_this.py") +if os.path.exists(virtualenv_activation): + execfile(virtualenv_activation, dict(__file__=virtualenv_activation)) + +syslog.openlog(os.path.basename(__file__), syslog.LOG_PID, syslog.LOG_USER) + +import django +django.setup() + +import datetime +from ietf.review.utils import review_requests_needing_reviewer_reminder, email_reviewer_reminder + +for review_req in review_requests_needing_reviewer_reminder(datetime.date.today()): + email_reviewer_reminder(review_req) + 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)) diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index b93e60f11..58457865e 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -11,6 +11,7 @@ from ietf.iesg.models import TelechatDate from ietf.person.models import Email, Person from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod from ietf.review.utils import suggested_review_requests_for_team +from ietf.review.utils import review_requests_needing_reviewer_reminder, email_reviewer_reminder from ietf.name.models import ReviewTypeName, ReviewResultName, ReviewRequestStateName import ietf.group.views_review from ietf.utils.mail import outbox, empty_outbox @@ -316,12 +317,14 @@ class ReviewTests(TestCase): "min_interval": "7", "filter_re": "test-[regexp]", "skip_next": "2", + "remind_days_before_deadline": "6" }) self.assertEqual(r.status_code, 302) settings = ReviewerSettings.objects.get(person=reviewer, team=review_req.team) self.assertEqual(settings.min_interval, 7) self.assertEqual(settings.filter_re, "test-[regexp]") self.assertEqual(settings.skip_next, 2) + self.assertEqual(settings.remind_days_before_deadline, 6) self.assertEqual(len(outbox), 1) self.assertTrue("reviewer availability" in outbox[0]["subject"].lower()) self.assertTrue("frequency changed", unicode(outbox[0]).lower()) @@ -370,3 +373,35 @@ class ReviewTests(TestCase): self.assertEqual(len(outbox), 1) self.assertTrue(start_date.isoformat(), unicode(outbox[0]).lower()) self.assertTrue(end_date.isoformat(), unicode(outbox[0]).lower()) + + def test_reviewer_reminders(self): + doc = make_test_data() + + reviewer = Person.objects.get(name="Plain Man") + + review_req = make_review_data(doc) + + settings = ReviewerSettings.objects.get(team=review_req.team, person=reviewer) + settings.remind_days_before_deadline = 6 + settings.save() + + today = datetime.date.today() + + review_req.reviewer = reviewer.email_set.first() + review_req.deadline = today + datetime.timedelta(days=settings.remind_days_before_deadline) + review_req.save() + + needing_reminders = review_requests_needing_reviewer_reminder(today - datetime.timedelta(days=1)) + self.assertEqual(list(needing_reminders), []) + + needing_reminders = review_requests_needing_reviewer_reminder(today) + self.assertEqual(list(needing_reminders), [review_req]) + + needing_reminders = review_requests_needing_reviewer_reminder(today + datetime.timedelta(days=1)) + self.assertEqual(list(needing_reminders), []) + + empty_outbox() + email_reviewer_reminder(review_req) + self.assertEqual(len(outbox), 1) + print outbox[0] + self.assertTrue(review_req.doc_id in unicode(outbox[0])) diff --git a/ietf/group/views_review.py b/ietf/group/views_review.py index 0d1f74c57..d5cd6074c 100644 --- a/ietf/group/views_review.py +++ b/ietf/group/views_review.py @@ -350,7 +350,7 @@ def email_open_review_assignments(request, acronym, group_type=None): class ReviewerSettingsForm(forms.ModelForm): class Meta: model = ReviewerSettings - fields = ['min_interval', 'filter_re', 'skip_next'] + fields = ['min_interval', 'filter_re', 'skip_next', 'remind_days_before_deadline'] class AddUnavailablePeriodForm(forms.ModelForm): class Meta: @@ -406,7 +406,7 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None): back_url = request.GET.get("next") if not back_url: import ietf.group.views_review - back_url = urlreverse(ietf.group.views_review.review_requests, kwargs={ "group_type": group.type_id, "acronym": group.acronym}) + back_url = urlreverse(ietf.group.views_review.reviewer_overview, kwargs={ "group_type": group.type_id, "acronym": group.acronym}) # settings if request.method == "POST" and request.POST.get("action") == "change_settings": diff --git a/ietf/review/migrations/0001_initial.py b/ietf/review/migrations/0001_initial.py index 94e72bb4d..7f618abe1 100644 --- a/ietf/review/migrations/0001_initial.py +++ b/ietf/review/migrations/0001_initial.py @@ -26,6 +26,17 @@ class Migration(migrations.Migration): }, bases=(models.Model,), ), + migrations.CreateModel( + name='ResultUsedInReviewTeam', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('result', models.ForeignKey(to='name.ReviewResultName')), + ('team', models.ForeignKey(to='group.Group')), + ], + options={ + }, + bases=(models.Model,), + ), migrations.CreateModel( name='ReviewerSettings', fields=[ @@ -33,6 +44,7 @@ class Migration(migrations.Migration): ('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')])), ('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)), ('skip_next', models.IntegerField(default=0, verbose_name=b'Skip next assignments')), + ('remind_days_before_deadline', models.IntegerField(null=True, blank=True)), ('person', models.ForeignKey(to='person.Person')), ('team', models.ForeignKey(to='group.Group')), ], @@ -63,10 +75,12 @@ class Migration(migrations.Migration): bases=(models.Model,), ), migrations.CreateModel( - name='ResultUsedInReviewTeam', + name='ReviewWish', fields=[ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), - ('result', models.ForeignKey(to='name.ReviewResultName')), + ('time', models.DateTimeField(default=datetime.datetime.now)), + ('doc', models.ForeignKey(to='doc.Document')), + ('person', models.ForeignKey(to='person.Person')), ('team', models.ForeignKey(to='group.Group')), ], options={ @@ -84,19 +98,6 @@ class Migration(migrations.Migration): }, bases=(models.Model,), ), - migrations.CreateModel( - name='ReviewWish', - fields=[ - ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), - ('time', models.DateTimeField(default=datetime.datetime.now)), - ('doc', models.ForeignKey(to='doc.Document')), - ('person', models.ForeignKey(to='person.Person')), - ('team', models.ForeignKey(to='group.Group')), - ], - options={ - }, - bases=(models.Model,), - ), migrations.CreateModel( name='UnavailablePeriod', fields=[ diff --git a/ietf/review/models.py b/ietf/review/models.py index c2d7ac2b0..8233a681c 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -23,6 +23,7 @@ class ReviewerSettings(models.Model): min_interval = models.IntegerField(default=30, verbose_name="Can review at most", choices=INTERVALS) filter_re = models.CharField(max_length=255, verbose_name="Filter regexp", blank=True, help_text="Draft names matching regular expression should not be assigned") skip_next = models.IntegerField(default=0, verbose_name="Skip next assignments") + 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.") def __unicode__(self): return u"{} in {}".format(self.person, self.team) diff --git a/ietf/review/utils.py b/ietf/review/utils.py index e578b9410..bdc3e9380 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -1,8 +1,9 @@ import datetime, re, itertools from collections import defaultdict -from django.db.models import Q, Max +from django.db.models import Q, Max, F from django.core.urlresolvers import reverse as urlreverse +from django.contrib.sites.models import Site from ietf.group.models import Group, Role from ietf.doc.models import (Document, ReviewRequestDocEvent, State, @@ -674,3 +675,42 @@ def make_assignment_choices(email_queryset, review_req): ranking.sort(key=lambda r: r["scores"], reverse=True) return [(r["email"].pk, r["label"]) for r in ranking] + +def review_requests_needing_reviewer_reminder(remind_date): + reqs_qs = ReviewRequest.objects.filter( + state__in=("requested", "accepted"), + reviewer__person__reviewersettings__remind_days_before_deadline__isnull=False, + reviewer__person__reviewersettings__team=F("team"), + ).exclude( + reviewer=None + ).values_list("pk", "deadline", "reviewer__person__reviewersettings__remind_days_before_deadline").distinct() + + req_pks = [] + for r_pk, deadline, remind_days in reqs_qs: + if (deadline - remind_date).days == remind_days: + req_pks.append(r_pk) + + return ReviewRequest.objects.filter(pk__in=req_pks).select_related("reviewer", "reviewer__person", "state", "team") + +def email_reviewer_reminder(review_request): + team = review_request.team + + deadline_days = (review_request.deadline - datetime.date.today()).days + + subject = "Reminder: deadline for review of {} in {} is {}".format(review_request.doc_id, team.acronym, review_request.deadline.isoformat()) + + overview_url = urlreverse("ietf.ietfauth.views.review_overview") + request_url = urlreverse("ietf.doc.views_review.review_request", kwargs={ "name": review_request.doc_id, "request_id": review_request.pk }) + + domain = Site.objects.get_current().domain + + settings = ReviewerSettings.objects.filter(person=review_request.reviewer.person, team=team).first() + remind_days = settings.remind_days_before_deadline if settings else 0 + + send_mail(None, [review_request.reviewer.formatted_email()], None, subject, "review/reviewer_reminder.txt", { + "reviewer_overview_url": "https://{}{}".format(domain, overview_url), + "review_request_url": "https://{}{}".format(domain, request_url), + "review_request": review_request, + "deadline_days": deadline_days, + "remind_days": remind_days, + }) diff --git a/ietf/templates/doc/review_request_summary.html b/ietf/templates/doc/review_request_summary.html index 36aedc8c7..7f1faa9c6 100644 --- a/ietf/templates/doc/review_request_summary.html +++ b/ietf/templates/doc/review_request_summary.html @@ -3,6 +3,7 @@ {{ 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 %}: {{ review_request.result.name }}{% endif %} {% if review_request.state_id == "part-completed" %}(partially completed){% endif %} + {% else %} {{ review_request.team.acronym|upper }} {{ review_request.type.name }} Review diff --git a/ietf/templates/ietfauth/review_overview.html b/ietf/templates/ietfauth/review_overview.html index d91d8e6d4..edf39f11f 100644 --- a/ietf/templates/ietfauth/review_overview.html +++ b/ietf/templates/ietfauth/review_overview.html @@ -134,7 +134,11 @@ Filter regexp - {{ t.reviewer_settings.filter_re|default:"(None)" }} + {% if t.reviewer_settings.filter_re %}{{ t.reviewer_settings.filter_re }}{% else %}(None){% endif %} + + + Remind days before deadline + {{ t.reviewer_settings.remind_days_before_deadline|default:"(Do not remind)" }} Unavailable periods diff --git a/ietf/templates/review/reviewer_reminder.txt b/ietf/templates/review/reviewer_reminder.txt new file mode 100644 index 000000000..4a0dcc540 --- /dev/null +++ b/ietf/templates/review/reviewer_reminder.txt @@ -0,0 +1,8 @@ +{% 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 }}: + +{{ review_request_url }} + +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: + +{{ reviewer_overview_url }} +{% endfilter %}{% endautoescape %}