Fix several review reminder problems.
Send secretary's review reminders to secretary instead of assignee. Send unconfirmed assignment reminders based on assignment age and CC secretaries. Correctly separate open review reminders by review team. Fixes #3482. Fixes #3324. Commit ready for merge. - Legacy-Id: 19848
This commit is contained in:
Jennifer Richards
parent
64e904804d
commit
6df0377fa1
|
|
@ -23,7 +23,7 @@ django.setup()
|
|||
from ietf.review.utils import (
|
||||
review_assignments_needing_reviewer_reminder, email_reviewer_reminder,
|
||||
review_assignments_needing_secretary_reminder, email_secretary_reminder,
|
||||
send_unavaibility_period_ending_reminder, send_reminder_all_open_reviews,
|
||||
send_unavailability_period_ending_reminder, send_reminder_all_open_reviews,
|
||||
send_review_reminder_overdue_assignment, send_reminder_unconfirmed_assignments)
|
||||
from ietf.utils.log import log
|
||||
|
||||
|
|
@ -38,7 +38,7 @@ for assignment, secretary_role in review_assignments_needing_secretary_reminder(
|
|||
review_req = assignment.review_request
|
||||
log("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))
|
||||
|
||||
period_end_reminders_sent = send_unavaibility_period_ending_reminder(today)
|
||||
period_end_reminders_sent = send_unavailability_period_ending_reminder(today)
|
||||
for msg in period_end_reminders_sent:
|
||||
log(msg)
|
||||
|
||||
|
|
|
|||
|
|
@ -12,17 +12,10 @@ from django.urls import reverse as urlreverse
|
|||
from ietf.review.policies import get_reviewer_queue_policy
|
||||
from ietf.utils.test_utils import login_testing_unauthorized, TestCase, reload_db_objects
|
||||
from ietf.doc.models import TelechatDocEvent, LastCallDocEvent, State
|
||||
from ietf.group.models import Role
|
||||
from ietf.iesg.models import TelechatDate
|
||||
from ietf.person.models import Person
|
||||
from ietf.review.models import ( ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings,
|
||||
ReviewTeamSettings, NextReviewerInTeam )
|
||||
from ietf.review.utils import (
|
||||
suggested_review_requests_for_team,
|
||||
review_assignments_needing_reviewer_reminder, email_reviewer_reminder,
|
||||
review_assignments_needing_secretary_reminder, email_secretary_reminder,
|
||||
send_unavaibility_period_ending_reminder, send_reminder_all_open_reviews,
|
||||
send_review_reminder_overdue_assignment, send_reminder_unconfirmed_assignments)
|
||||
from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings,NextReviewerInTeam
|
||||
from ietf.review.utils import suggested_review_requests_for_team
|
||||
from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName, \
|
||||
ReviewTypeName
|
||||
import ietf.group.views
|
||||
|
|
@ -703,199 +696,6 @@ class ReviewTests(TestCase):
|
|||
self.assertEqual(settings.max_items_to_show_in_reviewer_list, 10)
|
||||
self.assertEqual(settings.days_to_show_in_reviewer_list, 365)
|
||||
|
||||
def test_review_reminders(self):
|
||||
review_req = ReviewRequestFactory(state_id='assigned')
|
||||
reviewer = RoleFactory(name_id='reviewer',group=review_req.team,person__user__username='reviewer').person
|
||||
assignment = 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 = 6
|
||||
|
||||
reviewer_settings = ReviewerSettings.objects.get(team=review_req.team, person=reviewer)
|
||||
reviewer_settings.remind_days_before_deadline = remind_days
|
||||
reviewer_settings.save()
|
||||
|
||||
secretary = Person.objects.get(user__username="reviewsecretary")
|
||||
secretary_role = Role.objects.get(group=review_req.team, name="secr", person=secretary)
|
||||
|
||||
secretary_settings = ReviewSecretarySettings(team=review_req.team, person=secretary)
|
||||
secretary_settings.remind_days_before_deadline = remind_days
|
||||
secretary_settings.save()
|
||||
|
||||
today = datetime.date.today()
|
||||
|
||||
review_req.reviewer = reviewer.email_set.first()
|
||||
review_req.deadline = today + datetime.timedelta(days=remind_days)
|
||||
review_req.save()
|
||||
|
||||
# reviewer
|
||||
needing_reminders = review_assignments_needing_reviewer_reminder(today - datetime.timedelta(days=1))
|
||||
self.assertEqual(list(needing_reminders), [])
|
||||
|
||||
needing_reminders = review_assignments_needing_reviewer_reminder(today)
|
||||
self.assertEqual(list(needing_reminders), [assignment])
|
||||
|
||||
needing_reminders = review_assignments_needing_reviewer_reminder(today + datetime.timedelta(days=1))
|
||||
self.assertEqual(list(needing_reminders), [])
|
||||
|
||||
# secretary
|
||||
needing_reminders = review_assignments_needing_secretary_reminder(today - datetime.timedelta(days=1))
|
||||
self.assertEqual(list(needing_reminders), [])
|
||||
|
||||
needing_reminders = review_assignments_needing_secretary_reminder(today)
|
||||
self.assertEqual(list(needing_reminders), [(assignment, secretary_role)])
|
||||
|
||||
needing_reminders = review_assignments_needing_secretary_reminder(today + datetime.timedelta(days=1))
|
||||
self.assertEqual(list(needing_reminders), [])
|
||||
|
||||
# email reviewer
|
||||
empty_outbox()
|
||||
email_reviewer_reminder(assignment)
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertTrue(review_req.doc.name in get_payload_text(outbox[0]))
|
||||
|
||||
# email secretary
|
||||
empty_outbox()
|
||||
email_secretary_reminder(assignment, secretary_role)
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertTrue(review_req.doc.name in get_payload_text(outbox[0]))
|
||||
|
||||
def test_send_unavaibility_period_ending_reminder(self):
|
||||
review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review",
|
||||
list_email="reviewteam@ietf.org")
|
||||
reviewer = RoleFactory(group=review_team, person__user__username='reviewer',
|
||||
person__user__email='reviewer@example.com',
|
||||
person__name='Some Reviewer', name_id='reviewer')
|
||||
secretary = RoleFactory(group=review_team, person__user__username='reviewsecretary',
|
||||
person__user__email='reviewsecretary@example.com', name_id='secr')
|
||||
empty_outbox()
|
||||
today = datetime.date.today()
|
||||
UnavailablePeriod.objects.create(
|
||||
team=review_team,
|
||||
person=reviewer.person,
|
||||
start_date=today - datetime.timedelta(days=40),
|
||||
end_date=today + datetime.timedelta(days=3),
|
||||
availability="unavailable",
|
||||
)
|
||||
UnavailablePeriod.objects.create(
|
||||
team=review_team,
|
||||
person=reviewer.person,
|
||||
# This object should be ignored, length is too short
|
||||
start_date=today - datetime.timedelta(days=20),
|
||||
end_date=today + datetime.timedelta(days=3),
|
||||
availability="unavailable",
|
||||
)
|
||||
UnavailablePeriod.objects.create(
|
||||
team=review_team,
|
||||
person=reviewer.person,
|
||||
start_date=today - datetime.timedelta(days=40),
|
||||
# This object should be ignored, end date is too far away
|
||||
end_date=today + datetime.timedelta(days=4),
|
||||
availability="unavailable",
|
||||
)
|
||||
UnavailablePeriod.objects.create(
|
||||
team=review_team,
|
||||
person=reviewer.person,
|
||||
# This object should be ignored, end date is too close
|
||||
start_date=today - datetime.timedelta(days=40),
|
||||
end_date=today + datetime.timedelta(days=2),
|
||||
availability="unavailable",
|
||||
)
|
||||
log = send_unavaibility_period_ending_reminder(today)
|
||||
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertTrue(reviewer.person.email_address() in outbox[0]["To"])
|
||||
self.assertTrue(secretary.person.email_address() in outbox[0]["To"])
|
||||
message = get_payload_text(outbox[0])
|
||||
self.assertTrue(reviewer.person.name in message)
|
||||
self.assertTrue(review_team.acronym in message)
|
||||
self.assertEqual(len(log), 1)
|
||||
self.assertTrue(reviewer.person.name in log[0])
|
||||
self.assertTrue(review_team.acronym in log[0])
|
||||
|
||||
def test_send_review_reminder_overdue_assignment(self):
|
||||
today = datetime.date.today()
|
||||
|
||||
# An assignment that's exactly on the date at which the grace period expires
|
||||
review_req = ReviewRequestFactory(state_id='assigned', deadline=today - datetime.timedelta(5))
|
||||
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())
|
||||
secretary = RoleFactory(name_id='secr', group=review_req.team, person__user__username='reviewsecretary')
|
||||
|
||||
# A assignment that is not yet overdue
|
||||
not_overdue = today + datetime.timedelta(days=1)
|
||||
ReviewAssignmentFactory(review_request__team=review_req.team, review_request__state_id='assigned', review_request__deadline=not_overdue, state_id='assigned', assigned_on=not_overdue, reviewer=reviewer.email_set.first())
|
||||
|
||||
# An assignment that is overdue but is not past the grace period
|
||||
in_grace_period = today - datetime.timedelta(days=1)
|
||||
ReviewAssignmentFactory(review_request__team=review_req.team, review_request__state_id='assigned', review_request__deadline=in_grace_period, state_id='assigned', assigned_on=in_grace_period, reviewer=reviewer.email_set.first())
|
||||
|
||||
empty_outbox()
|
||||
log = send_review_reminder_overdue_assignment(today)
|
||||
self.assertEqual(len(log), 1)
|
||||
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertTrue(secretary.person.email_address() in outbox[0]["To"])
|
||||
self.assertEqual(outbox[0]["Subject"], "1 Overdue review for team {}".format(review_req.team.acronym))
|
||||
message = get_payload_text(outbox[0])
|
||||
self.assertIn(review_req.team.acronym + ' has 1 accepted or assigned review overdue by at least 5 days.', message)
|
||||
self.assertIn('Review of {} by {}'.format(review_req.doc.name, reviewer.plain_name()), message)
|
||||
self.assertEqual(len(log), 1)
|
||||
self.assertIn(secretary.person.email_address(), log[0])
|
||||
self.assertIn('1 overdue review', log[0])
|
||||
|
||||
def test_send_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 = send_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 = get_payload_text(outbox[0])
|
||||
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])
|
||||
|
||||
def test_send_reminder_unconfirmed_assignments(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')
|
||||
today = datetime.date.today()
|
||||
|
||||
# By default, these reminders are disabled for all teams.
|
||||
empty_outbox()
|
||||
log = send_reminder_unconfirmed_assignments(today)
|
||||
self.assertEqual(len(outbox), 0)
|
||||
self.assertFalse(log)
|
||||
|
||||
ReviewTeamSettings.objects.update(remind_days_unconfirmed_assignments=1)
|
||||
|
||||
empty_outbox()
|
||||
log = send_reminder_unconfirmed_assignments(today)
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertIn(reviewer.email_address(), outbox[0]["To"])
|
||||
self.assertEqual(outbox[0]["Subject"], "Reminder: you have not responded to a review assignment")
|
||||
message = get_payload_text(outbox[0])
|
||||
self.assertIn(review_req.team.acronym, message)
|
||||
self.assertIn('accept or reject the assignment on', message)
|
||||
self.assertIn(review_req.doc.name, message)
|
||||
self.assertEqual(len(log), 1)
|
||||
self.assertIn(reviewer.email_address(), log[0])
|
||||
self.assertIn('not accepted/rejected review assignment', log[0])
|
||||
|
||||
|
||||
class BulkAssignmentTests(TestCase):
|
||||
|
||||
|
|
|
|||
|
|
@ -1,10 +1,17 @@
|
|||
# Copyright The IETF Trust 2019-2020, All Rights Reserved
|
||||
# -*- coding: utf-8 -*-
|
||||
import datetime
|
||||
|
||||
|
||||
from ietf.review.factories import ReviewAssignmentFactory, ReviewRequestFactory
|
||||
from ietf.group.factories import RoleFactory
|
||||
from ietf.utils.mail import empty_outbox, get_payload_text, outbox
|
||||
from ietf.utils.test_utils import TestCase, reload_db_objects
|
||||
from .factories import ReviewAssignmentFactory, ReviewRequestFactory, ReviewerSettingsFactory
|
||||
from .mailarch import hash_list_message_id
|
||||
from .models import ReviewerSettings, ReviewSecretarySettings, ReviewTeamSettings, UnavailablePeriod
|
||||
from .utils import (email_secretary_reminder, review_assignments_needing_secretary_reminder,
|
||||
email_reviewer_reminder, review_assignments_needing_reviewer_reminder,
|
||||
send_reminder_unconfirmed_assignments, send_review_reminder_overdue_assignment,
|
||||
send_reminder_all_open_reviews, send_unavailability_period_ending_reminder)
|
||||
|
||||
class HashTest(TestCase):
|
||||
|
||||
|
|
@ -63,3 +70,434 @@ class ReviewAssignmentTest(TestCase):
|
|||
assignment.save()
|
||||
review_req = reload_db_objects(review_req)
|
||||
self.assertEqual(review_req.state_id, 'withdrawn')
|
||||
|
||||
|
||||
class ReviewAssignmentReminderTests(TestCase):
|
||||
today = datetime.date.today()
|
||||
deadline = today + datetime.timedelta(days=6)
|
||||
|
||||
def setUp(self):
|
||||
super().setUp()
|
||||
self.review_req = ReviewRequestFactory(
|
||||
state_id='assigned',
|
||||
deadline=self.deadline,
|
||||
)
|
||||
self.team = self.review_req.team
|
||||
self.reviewer = RoleFactory(
|
||||
name_id='reviewer',
|
||||
group=self.team,
|
||||
person__user__username='reviewer',
|
||||
).person
|
||||
self.assignment = ReviewAssignmentFactory(
|
||||
review_request=self.review_req,
|
||||
state_id='assigned',
|
||||
assigned_on=self.review_req.time,
|
||||
reviewer=self.reviewer.email_set.first(),
|
||||
)
|
||||
|
||||
def make_secretary(self, username, remind_days=None):
|
||||
secretary_role = RoleFactory(
|
||||
name_id='secr',
|
||||
group=self.team,
|
||||
person__user__username=username,
|
||||
)
|
||||
ReviewSecretarySettings.objects.create(
|
||||
team=self.team,
|
||||
person=secretary_role.person,
|
||||
remind_days_before_deadline=remind_days,
|
||||
)
|
||||
return secretary_role
|
||||
|
||||
def make_non_secretary(self, username, remind_days=None):
|
||||
"""Make a non-secretary role that has a ReviewSecretarySettings
|
||||
|
||||
This is a little odd, but might come up if an ex-secretary takes on another role and still
|
||||
has a ReviewSecretarySettings record.
|
||||
"""
|
||||
role = RoleFactory(
|
||||
name_id='reviewer',
|
||||
group=self.team,
|
||||
person__user__username=username,
|
||||
)
|
||||
ReviewSecretarySettings.objects.create(
|
||||
team=self.team,
|
||||
person=role.person,
|
||||
remind_days_before_deadline=remind_days,
|
||||
)
|
||||
return role
|
||||
|
||||
def test_review_assignments_needing_secretary_reminder(self):
|
||||
"""Notification sent to multiple secretaries"""
|
||||
# Set up two secretaries with the same remind_days one with a different, and one with None.
|
||||
secretary_roles = [
|
||||
self.make_secretary(username='reviewsecretary0', remind_days=6),
|
||||
self.make_secretary(username='reviewsecretary1', remind_days=6),
|
||||
self.make_secretary(username='reviewsecretary2', remind_days=5),
|
||||
self.make_secretary(username='reviewsecretary3', remind_days=None), # never notified
|
||||
]
|
||||
self.make_non_secretary(username='nonsecretary', remind_days=6) # never notified
|
||||
|
||||
# Check from more than remind_days before the deadline all the way through the day before.
|
||||
# Should only get reminders on the expected days.
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_secretary_reminder(self.deadline - datetime.timedelta(days=7)),
|
||||
[],
|
||||
'No reminder needed when deadline is more than remind_days away',
|
||||
)
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_secretary_reminder(self.deadline - datetime.timedelta(days=6)),
|
||||
[(self.assignment, secretary_roles[0]), (self.assignment, secretary_roles[1])],
|
||||
'Reminders needed for all secretaries when deadline is exactly remind_days away',
|
||||
)
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_secretary_reminder(self.deadline - datetime.timedelta(days=5)),
|
||||
[(self.assignment, secretary_roles[2])],
|
||||
'Reminder needed when deadline is exactly remind_days away',
|
||||
)
|
||||
for days in range(1, 5):
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_secretary_reminder(self.deadline - datetime.timedelta(days=days)),
|
||||
[],
|
||||
f'No reminder needed when deadline is less than remind_days away (tried {days})',
|
||||
)
|
||||
|
||||
def test_email_secretary_reminder_emails_secretaries(self):
|
||||
"""Secretary review assignment reminders are sent to secretaries"""
|
||||
secretary_role = self.make_secretary(username='reviewsecretary')
|
||||
# create a couple other roles for the team to check that only the requested secretary is reminded
|
||||
self.make_secretary(username='ignoredsecretary')
|
||||
self.make_non_secretary(username='nonsecretary')
|
||||
|
||||
empty_outbox()
|
||||
email_secretary_reminder(self.assignment, secretary_role)
|
||||
self.assertEqual(len(outbox), 1)
|
||||
msg = outbox[0]
|
||||
text = get_payload_text(msg)
|
||||
self.assertIn(secretary_role.email.address, msg['to'])
|
||||
self.assertIn(self.review_req.doc.name, msg['subject'])
|
||||
self.assertIn(self.review_req.doc.name, text)
|
||||
self.assertIn(self.team.acronym, msg['subject'])
|
||||
self.assertIn(self.team.acronym, text)
|
||||
|
||||
def test_review_assignments_needing_reviewer_reminder(self):
|
||||
# method should find lists of assignments
|
||||
reviewer_settings = ReviewerSettings.objects.create(
|
||||
team=self.team,
|
||||
person=self.reviewer,
|
||||
remind_days_before_deadline=6,
|
||||
)
|
||||
|
||||
# Give this reviewer another team with a review to be sure
|
||||
# we don't have cross-talk between teams.
|
||||
second_req = ReviewRequestFactory(state_id='assigned', deadline=self.deadline)
|
||||
second_team = second_req.team
|
||||
second_assignment = ReviewAssignmentFactory(
|
||||
review_request=second_req,
|
||||
state_id='assigned',
|
||||
assigned_on=second_req.time,
|
||||
reviewer=self.reviewer.email(),
|
||||
)
|
||||
ReviewerSettingsFactory(
|
||||
team=second_team,
|
||||
person=self.reviewer,
|
||||
remind_days_before_deadline=5,
|
||||
)
|
||||
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_reviewer_reminder(self.deadline - datetime.timedelta(days=7)),
|
||||
[],
|
||||
'No reminder needed when deadline is more than remind_days away'
|
||||
)
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_reviewer_reminder(self.deadline - datetime.timedelta(days=6)),
|
||||
[self.assignment],
|
||||
'Reminder needed when deadline is exactly remind_days away',
|
||||
)
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_reviewer_reminder(self.deadline - datetime.timedelta(days=5)),
|
||||
[second_assignment],
|
||||
'Reminder needed for other assignment'
|
||||
)
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_reviewer_reminder(self.deadline - datetime.timedelta(days=4)),
|
||||
[],
|
||||
'No reminder needed when deadline is less than remind_days away'
|
||||
)
|
||||
|
||||
# should never send a reminder when disabled
|
||||
reviewer_settings.remind_days_before_deadline = None
|
||||
reviewer_settings.save()
|
||||
second_assignment.delete() # get rid of this one for the second test
|
||||
|
||||
# test over a range that includes when we *did* send a reminder above
|
||||
for days in range(1, 8):
|
||||
self.assertCountEqual(
|
||||
review_assignments_needing_reviewer_reminder(self.deadline - datetime.timedelta(days=days)),
|
||||
[],
|
||||
f'No reminder should be sent when reminders are disabled (sent for days={days})',
|
||||
)
|
||||
|
||||
def test_email_review_reminder_emails_reviewers(self):
|
||||
"""Reviewer assignment reminders are sent to the reviewers"""
|
||||
empty_outbox()
|
||||
email_reviewer_reminder(self.assignment)
|
||||
self.assertEqual(len(outbox), 1)
|
||||
msg = outbox[0]
|
||||
text = get_payload_text(msg)
|
||||
self.assertIn(self.reviewer.email_address(), msg['to'])
|
||||
self.assertIn(self.review_req.doc.name, msg['subject'])
|
||||
self.assertIn(self.review_req.doc.name, text)
|
||||
self.assertIn(self.team.acronym, msg['subject'])
|
||||
|
||||
def test_send_reminder_unconfirmed_assignments(self):
|
||||
"""Unconfirmed assignment reminders are sent to reviewer and team secretary"""
|
||||
assigned_on = self.assignment.assigned_on.date()
|
||||
secretaries = [
|
||||
self.make_secretary(username='reviewsecretary0').person,
|
||||
self.make_secretary(username='reviewsecretary1').person,
|
||||
]
|
||||
|
||||
# assignments that should be ignored (will result in extra emails being sent if not)
|
||||
ReviewAssignmentFactory(
|
||||
review_request=self.review_req,
|
||||
state_id='accepted',
|
||||
assigned_on=self.review_req.time,
|
||||
)
|
||||
ReviewAssignmentFactory(
|
||||
review_request=self.review_req,
|
||||
state_id='completed',
|
||||
assigned_on=self.review_req.time,
|
||||
)
|
||||
ReviewAssignmentFactory(
|
||||
review_request=self.review_req,
|
||||
state_id='rejected',
|
||||
assigned_on=self.review_req.time,
|
||||
)
|
||||
|
||||
# Create a second review for a different team to test for cross-talk between teams.
|
||||
ReviewAssignmentFactory(
|
||||
state_id='completed', # something that does not need a reminder
|
||||
reviewer=self.reviewer.email(),
|
||||
)
|
||||
|
||||
# By default, these reminders are disabled for all teams.
|
||||
ReviewTeamSettings.objects.update(remind_days_unconfirmed_assignments=1)
|
||||
|
||||
empty_outbox()
|
||||
log = send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=1))
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertIn(self.reviewer.email_address(), outbox[0]["To"])
|
||||
for secretary in secretaries:
|
||||
self.assertIn(
|
||||
secretary.email_address(),
|
||||
outbox[0]["Cc"],
|
||||
f'Secretary {secretary.user.username} was not copied on the reminder',
|
||||
)
|
||||
self.assertEqual(outbox[0]["Subject"], "Reminder: you have not responded to a review assignment")
|
||||
message = get_payload_text(outbox[0])
|
||||
self.assertIn(self.team.acronym, message)
|
||||
self.assertIn('accept or reject the assignment on', message)
|
||||
self.assertIn(self.review_req.doc.name, message)
|
||||
self.assertEqual(len(log), 1)
|
||||
self.assertIn(self.reviewer.email_address(), log[0])
|
||||
self.assertIn('not accepted/rejected review assignment', log[0])
|
||||
|
||||
def test_send_reminder_unconfirmed_assignments_respects_remind_days(self):
|
||||
"""Unconfirmed assignment reminders should respect the team settings"""
|
||||
assigned_on = self.assignment.assigned_on.date()
|
||||
|
||||
# By default, these reminders are disabled for all teams.
|
||||
empty_outbox()
|
||||
for days in range(10):
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=days))
|
||||
self.assertEqual(len(outbox), 0)
|
||||
|
||||
# expect a notification every day except the day of assignment
|
||||
ReviewTeamSettings.objects.update(remind_days_unconfirmed_assignments=1)
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=0))
|
||||
self.assertEqual(len(outbox), 0) # no message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=1))
|
||||
self.assertEqual(len(outbox), 1) # one new message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=2))
|
||||
self.assertEqual(len(outbox), 2) # one new message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=3))
|
||||
self.assertEqual(len(outbox), 3) # one new message
|
||||
|
||||
# expect a notification every other day
|
||||
empty_outbox()
|
||||
ReviewTeamSettings.objects.update(remind_days_unconfirmed_assignments=2)
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=0))
|
||||
self.assertEqual(len(outbox), 0) # no message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=1))
|
||||
self.assertEqual(len(outbox), 0) # no message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=2))
|
||||
self.assertEqual(len(outbox), 1) # one new message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=3))
|
||||
self.assertEqual(len(outbox), 1) # no new message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=4))
|
||||
self.assertEqual(len(outbox), 2) # one new message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=5))
|
||||
self.assertEqual(len(outbox), 2) # no new message
|
||||
send_reminder_unconfirmed_assignments(assigned_on + datetime.timedelta(days=6))
|
||||
self.assertEqual(len(outbox), 3) # no new message
|
||||
|
||||
def test_send_unavailability_period_ending_reminder(self):
|
||||
secretary = self.make_secretary(username='reviewsecretary')
|
||||
empty_outbox()
|
||||
today = datetime.date.today()
|
||||
UnavailablePeriod.objects.create(
|
||||
team=self.team,
|
||||
person=self.reviewer,
|
||||
start_date=today - datetime.timedelta(days=40),
|
||||
end_date=today + datetime.timedelta(days=3),
|
||||
availability="unavailable",
|
||||
)
|
||||
UnavailablePeriod.objects.create(
|
||||
team=self.team,
|
||||
person=self.reviewer,
|
||||
# This object should be ignored, length is too short
|
||||
start_date=today - datetime.timedelta(days=20),
|
||||
end_date=today + datetime.timedelta(days=3),
|
||||
availability="unavailable",
|
||||
)
|
||||
UnavailablePeriod.objects.create(
|
||||
team=self.team,
|
||||
person=self.reviewer,
|
||||
start_date=today - datetime.timedelta(days=40),
|
||||
# This object should be ignored, end date is too far away
|
||||
end_date=today + datetime.timedelta(days=4),
|
||||
availability="unavailable",
|
||||
)
|
||||
UnavailablePeriod.objects.create(
|
||||
team=self.team,
|
||||
person=self.reviewer,
|
||||
# This object should be ignored, end date is too close
|
||||
start_date=today - datetime.timedelta(days=40),
|
||||
end_date=today + datetime.timedelta(days=2),
|
||||
availability="unavailable",
|
||||
)
|
||||
log = send_unavailability_period_ending_reminder(today)
|
||||
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertTrue(self.reviewer.email_address() in outbox[0]["To"])
|
||||
self.assertTrue(secretary.person.email_address() in outbox[0]["To"])
|
||||
message = get_payload_text(outbox[0])
|
||||
self.assertTrue(self.reviewer.name in message)
|
||||
self.assertTrue(self.team.acronym in message)
|
||||
self.assertEqual(len(log), 1)
|
||||
self.assertTrue(self.reviewer.name in log[0])
|
||||
self.assertTrue(self.team.acronym in log[0])
|
||||
|
||||
def test_send_review_reminder_overdue_assignment(self):
|
||||
"""An overdue assignment reminder should be sent to the secretary
|
||||
|
||||
This tests that a second set of assignments for the same reviewer but a different
|
||||
review team does not cause cross-talk between teams. To do this, it removes the
|
||||
ReviewTeamSettings instance for the second review team. At the moment, this has
|
||||
the effect of disabling these reminders. This is a bit of a hack, because I'm not
|
||||
sure that review teams without the ReviewTeamSettings should exist. It has the
|
||||
needed effect but might require rethinking in the future.
|
||||
"""
|
||||
secretary = self.make_secretary(username='reviewsecretary')
|
||||
|
||||
# Set the remind_date to be exactly one grace period after self.deadline
|
||||
remind_date = self.deadline + datetime.timedelta(days=5)
|
||||
# Create a second request for a second team that will not be sent reminders
|
||||
second_team = ReviewAssignmentFactory(
|
||||
review_request__state_id='assigned',
|
||||
review_request__deadline=self.deadline,
|
||||
state_id='assigned',
|
||||
assigned_on=self.deadline,
|
||||
reviewer=self.reviewer.email_set.first(),
|
||||
).review_request.team
|
||||
second_team.reviewteamsettings.delete() # prevent it from being sent reminders
|
||||
|
||||
# An assignment that is not yet overdue
|
||||
not_overdue = remind_date + datetime.timedelta(days=1)
|
||||
ReviewAssignmentFactory(
|
||||
review_request__team=self.team,
|
||||
review_request__state_id='assigned',
|
||||
review_request__deadline=not_overdue,
|
||||
state_id='assigned',
|
||||
assigned_on=not_overdue,
|
||||
reviewer=self.reviewer.email_set.first(),
|
||||
)
|
||||
ReviewAssignmentFactory(
|
||||
review_request__team=second_team,
|
||||
review_request__state_id='assigned',
|
||||
review_request__deadline=not_overdue,
|
||||
state_id='assigned',
|
||||
assigned_on=not_overdue,
|
||||
reviewer=self.reviewer.email_set.first(),
|
||||
)
|
||||
|
||||
# An assignment that is overdue but is not past the grace period
|
||||
in_grace_period = remind_date - datetime.timedelta(days=1)
|
||||
ReviewAssignmentFactory(
|
||||
review_request__team=self.team,
|
||||
review_request__state_id='assigned',
|
||||
review_request__deadline=in_grace_period,
|
||||
state_id='assigned',
|
||||
assigned_on=in_grace_period,
|
||||
reviewer=self.reviewer.email_set.first(),
|
||||
)
|
||||
ReviewAssignmentFactory(
|
||||
review_request__team=second_team,
|
||||
review_request__state_id='assigned',
|
||||
review_request__deadline=in_grace_period,
|
||||
state_id='assigned',
|
||||
assigned_on=in_grace_period,
|
||||
reviewer=self.reviewer.email_set.first(),
|
||||
)
|
||||
|
||||
empty_outbox()
|
||||
log = send_review_reminder_overdue_assignment(remind_date)
|
||||
self.assertEqual(len(log), 1)
|
||||
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertTrue(secretary.person.email_address() in outbox[0]["To"])
|
||||
self.assertEqual(outbox[0]["Subject"], "1 Overdue review for team {}".format(self.team.acronym))
|
||||
message = get_payload_text(outbox[0])
|
||||
self.assertIn(
|
||||
self.team.acronym + ' has 1 accepted or assigned review overdue by at least 5 days.',
|
||||
message,
|
||||
)
|
||||
self.assertIn('Review of {} by {}'.format(self.review_req.doc.name, self.reviewer.plain_name()), message)
|
||||
self.assertEqual(len(log), 1)
|
||||
self.assertIn(secretary.person.email_address(), log[0])
|
||||
self.assertIn('1 overdue review', log[0])
|
||||
|
||||
def test_send_reminder_all_open_reviews(self):
|
||||
self.make_secretary(username='reviewsecretary')
|
||||
ReviewerSettingsFactory(team=self.team, person=self.reviewer, remind_days_open_reviews=1)
|
||||
|
||||
# Create another assignment for this reviewer in a different team.
|
||||
# Configure so that a reminder should not be sent for the date we test. It should not
|
||||
# be included in the reminder that's sent - only one open review assignment should be
|
||||
# reported.
|
||||
second_req = ReviewRequestFactory(state_id='assigned', deadline=self.deadline)
|
||||
second_team = second_req.team
|
||||
ReviewAssignmentFactory(
|
||||
review_request=second_req,
|
||||
state_id='assigned',
|
||||
assigned_on=second_req.time,
|
||||
reviewer=self.reviewer.email(),
|
||||
)
|
||||
ReviewerSettingsFactory(team=second_team, person=self.reviewer, remind_days_open_reviews=13)
|
||||
|
||||
empty_outbox()
|
||||
today = datetime.date.today()
|
||||
log = send_reminder_all_open_reviews(today)
|
||||
|
||||
self.assertEqual(len(outbox), 1)
|
||||
self.assertTrue(self.reviewer.email_address() in outbox[0]["To"])
|
||||
self.assertEqual(outbox[0]["Subject"], "Reminder: you have 1 open review assignment")
|
||||
message = get_payload_text(outbox[0])
|
||||
self.assertTrue(self.team.acronym in message)
|
||||
self.assertTrue('you have 1 open review' in message)
|
||||
self.assertTrue(self.review_req.doc.name in message)
|
||||
self.assertTrue(self.review_req.deadline.strftime('%Y-%m-%d') in message)
|
||||
self.assertEqual(len(log), 1)
|
||||
self.assertTrue(self.reviewer.email_address() in log[0])
|
||||
self.assertTrue('1 open review' in log[0])
|
||||
|
||||
|
|
|
|||
|
|
@ -702,7 +702,7 @@ def get_default_filter_re(person):
|
|||
return '^draft-(%s|%s)-.*$' % ( person.last_name().lower(), '|'.join(['ietf-%s' % g.acronym for g in groups_to_avoid]))
|
||||
|
||||
|
||||
def send_unavaibility_period_ending_reminder(remind_date):
|
||||
def send_unavailability_period_ending_reminder(remind_date):
|
||||
reminder_days = 3
|
||||
end_date = remind_date + datetime.timedelta(days=reminder_days)
|
||||
min_start_date = end_date - datetime.timedelta(days=30)
|
||||
|
|
@ -771,6 +771,7 @@ def send_reminder_all_open_reviews(remind_date):
|
|||
assignments = ReviewAssignment.objects.filter(
|
||||
state__in=("assigned", "accepted"),
|
||||
reviewer__person=reviewer_settings.person,
|
||||
review_request__team=reviewer_settings.team,
|
||||
)
|
||||
if not assignments:
|
||||
continue
|
||||
|
|
@ -800,14 +801,10 @@ def send_reminder_unconfirmed_assignments(remind_date):
|
|||
accepted or rejected, if enabled in ReviewTeamSettings.
|
||||
"""
|
||||
log = []
|
||||
days_since_origin = (remind_date - ORIGIN_DATE_PERIODIC_REMINDERS).days
|
||||
relevant_review_team_settings = ReviewTeamSettings.objects.filter(
|
||||
remind_days_unconfirmed_assignments__isnull=False)
|
||||
|
||||
for review_team_settings in relevant_review_team_settings:
|
||||
if days_since_origin % review_team_settings.remind_days_unconfirmed_assignments != 0:
|
||||
continue
|
||||
|
||||
assignments = ReviewAssignment.objects.filter(
|
||||
state='assigned',
|
||||
review_request__team=review_team_settings.group,
|
||||
|
|
@ -816,6 +813,9 @@ def send_reminder_unconfirmed_assignments(remind_date):
|
|||
continue
|
||||
|
||||
for assignment in assignments:
|
||||
days_old = (remind_date - assignment.assigned_on.date()).days
|
||||
if days_old == 0 or (days_old % review_team_settings.remind_days_unconfirmed_assignments) != 0:
|
||||
continue # skip those created today or not due for a reminder today
|
||||
to = assignment.reviewer.formatted_email()
|
||||
subject = "Reminder: you have not responded to a review assignment"
|
||||
domain = Site.objects.get_current().domain
|
||||
|
|
@ -823,19 +823,33 @@ def send_reminder_unconfirmed_assignments(remind_date):
|
|||
"name": assignment.review_request.doc.name,
|
||||
"request_id": assignment.review_request.pk
|
||||
})
|
||||
cc = [secr_role.formatted_email()
|
||||
for secr_role in assignment.review_request.team.role_set.filter(name__slug='secr')]
|
||||
|
||||
send_mail(None, to, None, subject, "review/reviewer_reminder_unconfirmed_assignments.txt", {
|
||||
"review_request_url": "https://{}{}".format(domain, review_request_url),
|
||||
"assignment": assignment,
|
||||
"team": assignment.review_request.team,
|
||||
"remind_days": review_team_settings.remind_days_unconfirmed_assignments,
|
||||
})
|
||||
send_mail(
|
||||
request=None,
|
||||
to=to,
|
||||
cc=cc,
|
||||
frm=None,
|
||||
subject=subject,
|
||||
template="review/reviewer_reminder_unconfirmed_assignments.txt",
|
||||
context={
|
||||
"review_request_url": "https://{}{}".format(domain, review_request_url),
|
||||
"assignment": assignment,
|
||||
"team": assignment.review_request.team,
|
||||
"remind_days": review_team_settings.remind_days_unconfirmed_assignments,
|
||||
},
|
||||
)
|
||||
log.append("Emailed reminder to {} about not accepted/rejected review assignment {}".format(to, assignment.pk))
|
||||
|
||||
return log
|
||||
|
||||
|
||||
def review_assignments_needing_reviewer_reminder(remind_date):
|
||||
"""Get review assignments needing reviewer reminders
|
||||
|
||||
Returns a queryset of ReviewAssignments whose reviewers should be notified.
|
||||
"""
|
||||
assignment_qs = ReviewAssignment.objects.filter(
|
||||
state__in=("assigned", "accepted"),
|
||||
reviewer__person__reviewersettings__remind_days_before_deadline__isnull=False,
|
||||
|
|
@ -876,23 +890,44 @@ def email_reviewer_reminder(assignment):
|
|||
})
|
||||
|
||||
def review_assignments_needing_secretary_reminder(remind_date):
|
||||
"""Find ReviewAssignments whose secretary should be sent a reminder today"""
|
||||
# Get ReviewAssignments for teams whose secretaries have a non-null remind_days_before_deadline
|
||||
# setting.
|
||||
assignment_qs = ReviewAssignment.objects.filter(
|
||||
state__in=("assigned", "accepted"),
|
||||
review_request__team__role__name__slug='secr',
|
||||
review_request__team__role__person__reviewsecretarysettings__remind_days_before_deadline__isnull=False,
|
||||
review_request__team__role__person__reviewsecretarysettings__team=F("review_request__team"),
|
||||
).exclude(
|
||||
reviewer=None
|
||||
).values_list("pk", "review_request__deadline", "review_request__team__role", "review_request__team__role__person__reviewsecretarysettings__remind_days_before_deadline").distinct()
|
||||
|
||||
assignment_pks = {}
|
||||
# For each assignment, find all secretaries who should be reminded today
|
||||
assignment_pks = set()
|
||||
secretary_pks = set()
|
||||
notifications = []
|
||||
for a_pk, deadline, secretary_role_pk, remind_days in assignment_qs:
|
||||
if (deadline - remind_date).days == remind_days:
|
||||
assignment_pks[a_pk] = secretary_role_pk
|
||||
notifications.append((a_pk, secretary_role_pk))
|
||||
assignment_pks.add(a_pk)
|
||||
secretary_pks.add(secretary_role_pk)
|
||||
|
||||
review_assignments = { a.pk: a for a in ReviewAssignment.objects.filter(pk__in=list(assignment_pks.keys())).select_related("reviewer", "reviewer__person", "state", "review_request__team") }
|
||||
secretary_roles = { r.pk: r for r in Role.objects.filter(pk__in=list(assignment_pks.values())).select_related("email", "person") }
|
||||
review_assignments = {
|
||||
a.pk: a
|
||||
for a in ReviewAssignment.objects.filter(pk__in=assignment_pks).select_related(
|
||||
"reviewer", "reviewer__person", "state", "review_request__team"
|
||||
)
|
||||
}
|
||||
secretary_roles = {
|
||||
r.pk: r
|
||||
for r in Role.objects.filter(pk__in=secretary_pks).select_related("email", "person")
|
||||
}
|
||||
|
||||
return [
|
||||
(review_assignments[a_pk], secretary_roles[secretary_role_pk])
|
||||
for a_pk, secretary_role_pk in notifications
|
||||
]
|
||||
|
||||
return [ (review_assignments[a_pk], secretary_roles[secretary_role_pk]) for a_pk, secretary_role_pk in assignment_pks.items() ]
|
||||
|
||||
def email_secretary_reminder(assignment, secretary_role):
|
||||
review_request = assignment.review_request
|
||||
|
|
@ -912,7 +947,7 @@ def email_secretary_reminder(assignment, secretary_role):
|
|||
settings = ReviewSecretarySettings.objects.filter(person=secretary_role.person_id, team=team).first()
|
||||
remind_days = settings.remind_days_before_deadline if settings else 0
|
||||
|
||||
send_mail(None, [assignment.reviewer.formatted_email()], None, subject, "review/secretary_reminder.txt", {
|
||||
send_mail(None, [secretary_role.email.formatted_email()], None, subject, "review/secretary_reminder.txt", {
|
||||
"review_request_url": "https://{}{}".format(domain, request_url),
|
||||
"settings_url": "https://{}{}".format(domain, settings_url),
|
||||
"review_request": review_request,
|
||||
|
|
|
|||
Loading…
Reference in a new issue