From ec56a03ec6454d81f26a1db98adc9999d85640f2 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Thu, 5 Sep 2019 15:02:56 +0000 Subject: [PATCH] Fix #2337 - Send periodic reminders of open reviews every X days (opt-in) The interleaved_migrations_test currently fails due to the various migrations that have been added for individual tickets/commits (unless --permit-mixed-migrations is set). I think this is better fixed in a later cleanup, as doing it now could cause confusion when merging individual commits, and more migrations are likely to be added soon. Commit ready for merge. - Legacy-Id: 16705 --- ietf/bin/send-review-reminders | 3 +- ietf/group/forms.py | 3 +- ietf/group/tests_review.py | 27 ++++++++++++- .../0015_add_remind_days_open_reviews.py | 26 +++++++++++++ ietf/review/models.py | 3 +- ietf/review/utils.py | 38 +++++++++++++++++++ ietf/templates/ietfauth/review_overview.html | 4 ++ .../reviewer_reminder_all_open_reviews.txt | 10 +++++ 8 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 ietf/review/migrations/0015_add_remind_days_open_reviews.py create mode 100644 ietf/templates/review/reviewer_reminder_all_open_reviews.txt diff --git a/ietf/bin/send-review-reminders b/ietf/bin/send-review-reminders index e7e369ab0..58213140b 100755 --- a/ietf/bin/send-review-reminders +++ b/ietf/bin/send-review-reminders @@ -24,7 +24,7 @@ import datetime from ietf.review.utils import ( review_assignments_needing_reviewer_reminder, email_reviewer_reminder, review_assignments_needing_secretary_reminder, email_secretary_reminder, - email_unavaibility_period_ending_reminder) + email_unavaibility_period_ending_reminder, email_reminder_all_open_reviews) today = datetime.date.today() @@ -39,3 +39,4 @@ for assignment, secretary_role in review_assignments_needing_secretary_reminder( print("Emailed reminder to {} for review of {} in {} (req. id {})".format(secretary_role.email.address, review_req.doc_id, review_req.team.acronym, review_req.pk)) print('\n'.join(email_unavaibility_period_ending_reminder(today))) +print('\n'.join(email_reminder_all_open_reviews(today))) diff --git a/ietf/group/forms.py b/ietf/group/forms.py index 198842854..728d0655c 100644 --- a/ietf/group/forms.py +++ b/ietf/group/forms.py @@ -278,7 +278,8 @@ class EmailOpenAssignmentsForm(forms.Form): class ReviewerSettingsForm(forms.ModelForm): class Meta: model = ReviewerSettings - fields = ['min_interval', 'filter_re', 'skip_next', 'remind_days_before_deadline','expertise'] + fields = ['min_interval', 'filter_re', 'skip_next', 'remind_days_before_deadline', + 'remind_days_open_reviews', 'expertise'] def __init__(self, *args, **kwargs): exclude_fields = kwargs.pop('exclude_fields', []) diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index ba4dd9be5..1bf0751d9 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -23,7 +23,7 @@ from ietf.review.utils import ( review_assignments_needing_reviewer_reminder, email_reviewer_reminder, review_assignments_needing_secretary_reminder, email_secretary_reminder, reviewer_rotation_list, - email_unavaibility_period_ending_reminder) + email_unavaibility_period_ending_reminder, email_reminder_all_open_reviews) from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName import ietf.group.views from ietf.utils.mail import outbox, empty_outbox @@ -316,6 +316,7 @@ class ReviewTests(TestCase): "min_interval": "7", "filter_re": "test-[regexp]", "remind_days_before_deadline": "6", + "remind_days_open_reviews": "8", "expertise": "Some expertise", }) self.assertEqual(r.status_code, 302) @@ -324,6 +325,7 @@ class ReviewTests(TestCase): self.assertEqual(settings.filter_re, "test-[regexp]") self.assertEqual(settings.skip_next, 0) self.assertEqual(settings.remind_days_before_deadline, 6) + self.assertEqual(settings.remind_days_open_reviews, 8) self.assertEqual(settings.expertise, "Some expertise") self.assertEqual(len(outbox), 1) self.assertTrue("reviewer availability" in outbox[0]["subject"].lower()) @@ -546,6 +548,29 @@ class ReviewTests(TestCase): self.assertTrue(reviewer.person.name in log[0]) self.assertTrue(review_team.acronym in log[0]) + def test_email_reminder_all_open_reviews(self): + review_req = ReviewRequestFactory(state_id='assigned') + reviewer = RoleFactory(name_id='reviewer', group=review_req.team,person__user__username='reviewer').person + ReviewAssignmentFactory(review_request=review_req, state_id='assigned', assigned_on=review_req.time, reviewer=reviewer.email_set.first()) + RoleFactory(name_id='secr', group=review_req.team, person__user__username='reviewsecretary') + ReviewerSettingsFactory(team=review_req.team, person=reviewer, remind_days_open_reviews=1) + + empty_outbox() + today = datetime.date.today() + log = email_reminder_all_open_reviews(today) + + self.assertEqual(len(outbox), 1) + self.assertTrue(reviewer.email_address() in outbox[0]["To"]) + self.assertEqual(outbox[0]["Subject"], "Reminder: you have 1 open review assignment") + message = outbox[0].get_payload(decode=True).decode("utf-8") + self.assertTrue(review_req.team.acronym in message) + self.assertTrue('you have 1 open review' in message) + self.assertTrue(review_req.doc.name in message) + self.assertTrue(review_req.deadline.strftime('%Y-%m-%d') in message) + self.assertEqual(len(log), 1) + self.assertTrue(reviewer.email_address() in log[0]) + self.assertTrue('1 open review' in log[0]) + class BulkAssignmentTests(TestCase): diff --git a/ietf/review/migrations/0015_add_remind_days_open_reviews.py b/ietf/review/migrations/0015_add_remind_days_open_reviews.py new file mode 100644 index 000000000..e60383724 --- /dev/null +++ b/ietf/review/migrations/0015_add_remind_days_open_reviews.py @@ -0,0 +1,26 @@ +# Copyright The IETF Trust 2019, All Rights Reserved +# -*- coding: utf-8 -*- +# Generated by Django 1.11.23 on 2019-09-05 05:03 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('review', '0014_document_primary_key_cleanup'), + ] + + operations = [ + migrations.AddField( + model_name='historicalreviewersettings', + name='remind_days_open_reviews', + field=models.PositiveIntegerField(blank=True, name="Periodic reminder of open reviews every X days", help_text="To get a periodic email reminder of all your open reviews, enter the number of days between these reminders. Clear the field if you don't want these reminders.", null=True), + ), + migrations.AddField( + model_name='reviewersettings', + name='remind_days_open_reviews', + field=models.PositiveIntegerField(blank=True, verbose_name="Periodic reminder of open reviews every X days", help_text="To get a periodic email reminder of all your open reviews, enter the number of days between these reminders. Clear the field if you don't want these reminders.", null=True), + ), + ] diff --git a/ietf/review/models.py b/ietf/review/models.py index 4cdb34297..7545433a3 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -36,7 +36,8 @@ class ReviewerSettings(models.Model): validators=[validate_regular_expression_string, ], help_text="Draft names matching this 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 review deadline you want to receive it. Clear the field if you don't want a reminder.") + 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 review deadline you want to receive it. Clear the field if you don't want this reminder.") + remind_days_open_reviews = models.PositiveIntegerField(null=True, blank=True, verbose_name="Periodic reminder of open reviews every X days", help_text="To get a periodic email reminder of all your open reviews, enter the number of days between these reminders. Clear the field if you don't want these reminders.") expertise = models.TextField(verbose_name="Reviewer's expertise in this team's area", max_length=2048, blank=True, help_text="Describe the reviewer's expertise in this team's area", default='') def __str__(self): diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 46b2f7b7b..3ee59fae2 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -12,6 +12,7 @@ import six from collections import defaultdict, namedtuple from django.db.models import Q, Max, F +from django.template.defaultfilters import pluralize from django.urls import reverse as urlreverse from django.contrib.sites.models import Site @@ -957,7 +958,44 @@ def email_unavaibility_period_ending_reminder(remind_date): to, period.person, period.team.acronym,period.pk)) return log + +def email_reminder_all_open_reviews(remind_date): + log = [] + # The origin date is arbitrarily chosen, to have a single reference date for "every X days" + origin_date = datetime.date(2019, 1, 1) + days_since_origin = (remind_date - origin_date).days + relevant_reviewer_settings = ReviewerSettings.objects.filter(remind_days_open_reviews__isnull=False) + for reviewer_settings in relevant_reviewer_settings: + if days_since_origin % reviewer_settings.remind_days_open_reviews != 0: + continue + + assignments = ReviewAssignment.objects.filter( + state__in=("assigned", "accepted"), + reviewer__person=reviewer_settings.person, + ) + if not assignments: + continue + + to = reviewer_settings.person.formatted_email() + subject = "Reminder: you have {} open review assignment{}".format(len(assignments), pluralize(len(assignments))) + + domain = Site.objects.get_current().domain + url = urlreverse("ietf.group.views.reviewer_overview", + kwargs={"group_type": reviewer_settings.team.type_id, + "acronym": reviewer_settings.team.acronym}) + + send_mail(None, to, None, subject, "review/reviewer_reminder_all_open_reviews.txt", { + "reviewer_overview_url": "https://{}{}".format(domain, url), + "assignments": assignments, + "team": reviewer_settings.team, + "remind_days": reviewer_settings.remind_days_open_reviews, + }) + log.append("Emailed reminder to {} of their {} open reviews".format(to, len(assignments))) + + return log + + def review_assignments_needing_reviewer_reminder(remind_date): assignment_qs = ReviewAssignment.objects.filter( state__in=("assigned", "accepted"), diff --git a/ietf/templates/ietfauth/review_overview.html b/ietf/templates/ietfauth/review_overview.html index 86f503dae..39c2ab326 100644 --- a/ietf/templates/ietfauth/review_overview.html +++ b/ietf/templates/ietfauth/review_overview.html @@ -146,6 +146,10 @@ Remind days before deadline {{ t.reviewer_settings.remind_days_before_deadline|default:"(Do not remind)" }} + + Periodic reminder of open reviews every X days + {{ t.reviewer_settings.remind_days_open_reviews|default:"(Do not remind)" }} + Unavailable periods diff --git a/ietf/templates/review/reviewer_reminder_all_open_reviews.txt b/ietf/templates/review/reviewer_reminder_all_open_reviews.txt new file mode 100644 index 000000000..4faeb2eee --- /dev/null +++ b/ietf/templates/review/reviewer_reminder_all_open_reviews.txt @@ -0,0 +1,10 @@ +{% load ietf_filters %}{% autoescape off %}{% filter wordwrap:78 %}This is just a friendly reminder that you have {{ assignments|length }} open review{{ assignments|length|pluralize }} in team {{ team.acronym }}. + +The following reviews are open:{% for assignment in assignments %} +- {{ assignment.review_request.doc.name }} (deadline {{ assignment.review_request.deadline }}) +{% endfor %} + +You are receiving this reminder because you have configured the Datatracker to remind you of all your open reviews every {{ remind_days }} day{{ remind_days|pluralize }}. You can see your reviews and change your settings here: + +{{ reviewer_overview_url }} +{% endfilter %}{% endautoescape %}