Skip to content

Commit 9a87a3e

Browse files
committed
Merged in [19848] from jennifer@painless-security.com:
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 ietf-tools#3482. Fixes ietf-tools#3324. - Legacy-Id: 19853 Note: SVN reference [19848] has been migrated to Git commit 6df0377
2 parents 6f682c3 + 6df0377 commit 9a87a3e

4 files changed

Lines changed: 496 additions & 223 deletions

File tree

ietf/bin/send-review-reminders

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ django.setup()
2323
from ietf.review.utils import (
2424
review_assignments_needing_reviewer_reminder, email_reviewer_reminder,
2525
review_assignments_needing_secretary_reminder, email_secretary_reminder,
26-
send_unavaibility_period_ending_reminder, send_reminder_all_open_reviews,
26+
send_unavailability_period_ending_reminder, send_reminder_all_open_reviews,
2727
send_review_reminder_overdue_assignment, send_reminder_unconfirmed_assignments)
2828
from ietf.utils.log import log
2929

@@ -38,7 +38,7 @@ for assignment, secretary_role in review_assignments_needing_secretary_reminder(
3838
review_req = assignment.review_request
3939
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))
4040

41-
period_end_reminders_sent = send_unavaibility_period_ending_reminder(today)
41+
period_end_reminders_sent = send_unavailability_period_ending_reminder(today)
4242
for msg in period_end_reminders_sent:
4343
log(msg)
4444

ietf/group/tests_review.py

Lines changed: 2 additions & 202 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,10 @@
1212
from ietf.review.policies import get_reviewer_queue_policy
1313
from ietf.utils.test_utils import login_testing_unauthorized, TestCase, reload_db_objects
1414
from ietf.doc.models import TelechatDocEvent, LastCallDocEvent, State
15-
from ietf.group.models import Role
1615
from ietf.iesg.models import TelechatDate
1716
from ietf.person.models import Person
18-
from ietf.review.models import ( ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings,
19-
ReviewTeamSettings, NextReviewerInTeam )
20-
from ietf.review.utils import (
21-
suggested_review_requests_for_team,
22-
review_assignments_needing_reviewer_reminder, email_reviewer_reminder,
23-
review_assignments_needing_secretary_reminder, email_secretary_reminder,
24-
send_unavaibility_period_ending_reminder, send_reminder_all_open_reviews,
25-
send_review_reminder_overdue_assignment, send_reminder_unconfirmed_assignments)
17+
from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings,NextReviewerInTeam
18+
from ietf.review.utils import suggested_review_requests_for_team
2619
from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName, \
2720
ReviewTypeName
2821
import ietf.group.views
@@ -703,199 +696,6 @@ def test_change_review_secretary_settings(self):
703696
self.assertEqual(settings.max_items_to_show_in_reviewer_list, 10)
704697
self.assertEqual(settings.days_to_show_in_reviewer_list, 365)
705698

706-
def test_review_reminders(self):
707-
review_req = ReviewRequestFactory(state_id='assigned')
708-
reviewer = RoleFactory(name_id='reviewer',group=review_req.team,person__user__username='reviewer').person
709-
assignment = ReviewAssignmentFactory(review_request=review_req, state_id='assigned', assigned_on = review_req.time, reviewer=reviewer.email_set.first())
710-
RoleFactory(name_id='secr',group=review_req.team,person__user__username='reviewsecretary')
711-
ReviewerSettingsFactory(team = review_req.team, person = reviewer)
712-
713-
remind_days = 6
714-
715-
reviewer_settings = ReviewerSettings.objects.get(team=review_req.team, person=reviewer)
716-
reviewer_settings.remind_days_before_deadline = remind_days
717-
reviewer_settings.save()
718-
719-
secretary = Person.objects.get(user__username="reviewsecretary")
720-
secretary_role = Role.objects.get(group=review_req.team, name="secr", person=secretary)
721-
722-
secretary_settings = ReviewSecretarySettings(team=review_req.team, person=secretary)
723-
secretary_settings.remind_days_before_deadline = remind_days
724-
secretary_settings.save()
725-
726-
today = datetime.date.today()
727-
728-
review_req.reviewer = reviewer.email_set.first()
729-
review_req.deadline = today + datetime.timedelta(days=remind_days)
730-
review_req.save()
731-
732-
# reviewer
733-
needing_reminders = review_assignments_needing_reviewer_reminder(today - datetime.timedelta(days=1))
734-
self.assertEqual(list(needing_reminders), [])
735-
736-
needing_reminders = review_assignments_needing_reviewer_reminder(today)
737-
self.assertEqual(list(needing_reminders), [assignment])
738-
739-
needing_reminders = review_assignments_needing_reviewer_reminder(today + datetime.timedelta(days=1))
740-
self.assertEqual(list(needing_reminders), [])
741-
742-
# secretary
743-
needing_reminders = review_assignments_needing_secretary_reminder(today - datetime.timedelta(days=1))
744-
self.assertEqual(list(needing_reminders), [])
745-
746-
needing_reminders = review_assignments_needing_secretary_reminder(today)
747-
self.assertEqual(list(needing_reminders), [(assignment, secretary_role)])
748-
749-
needing_reminders = review_assignments_needing_secretary_reminder(today + datetime.timedelta(days=1))
750-
self.assertEqual(list(needing_reminders), [])
751-
752-
# email reviewer
753-
empty_outbox()
754-
email_reviewer_reminder(assignment)
755-
self.assertEqual(len(outbox), 1)
756-
self.assertTrue(review_req.doc.name in get_payload_text(outbox[0]))
757-
758-
# email secretary
759-
empty_outbox()
760-
email_secretary_reminder(assignment, secretary_role)
761-
self.assertEqual(len(outbox), 1)
762-
self.assertTrue(review_req.doc.name in get_payload_text(outbox[0]))
763-
764-
def test_send_unavaibility_period_ending_reminder(self):
765-
review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review",
766-
list_email="reviewteam@ietf.org")
767-
reviewer = RoleFactory(group=review_team, person__user__username='reviewer',
768-
person__user__email='reviewer@example.com',
769-
person__name='Some Reviewer', name_id='reviewer')
770-
secretary = RoleFactory(group=review_team, person__user__username='reviewsecretary',
771-
person__user__email='reviewsecretary@example.com', name_id='secr')
772-
empty_outbox()
773-
today = datetime.date.today()
774-
UnavailablePeriod.objects.create(
775-
team=review_team,
776-
person=reviewer.person,
777-
start_date=today - datetime.timedelta(days=40),
778-
end_date=today + datetime.timedelta(days=3),
779-
availability="unavailable",
780-
)
781-
UnavailablePeriod.objects.create(
782-
team=review_team,
783-
person=reviewer.person,
784-
# This object should be ignored, length is too short
785-
start_date=today - datetime.timedelta(days=20),
786-
end_date=today + datetime.timedelta(days=3),
787-
availability="unavailable",
788-
)
789-
UnavailablePeriod.objects.create(
790-
team=review_team,
791-
person=reviewer.person,
792-
start_date=today - datetime.timedelta(days=40),
793-
# This object should be ignored, end date is too far away
794-
end_date=today + datetime.timedelta(days=4),
795-
availability="unavailable",
796-
)
797-
UnavailablePeriod.objects.create(
798-
team=review_team,
799-
person=reviewer.person,
800-
# This object should be ignored, end date is too close
801-
start_date=today - datetime.timedelta(days=40),
802-
end_date=today + datetime.timedelta(days=2),
803-
availability="unavailable",
804-
)
805-
log = send_unavaibility_period_ending_reminder(today)
806-
807-
self.assertEqual(len(outbox), 1)
808-
self.assertTrue(reviewer.person.email_address() in outbox[0]["To"])
809-
self.assertTrue(secretary.person.email_address() in outbox[0]["To"])
810-
message = get_payload_text(outbox[0])
811-
self.assertTrue(reviewer.person.name in message)
812-
self.assertTrue(review_team.acronym in message)
813-
self.assertEqual(len(log), 1)
814-
self.assertTrue(reviewer.person.name in log[0])
815-
self.assertTrue(review_team.acronym in log[0])
816-
817-
def test_send_review_reminder_overdue_assignment(self):
818-
today = datetime.date.today()
819-
820-
# An assignment that's exactly on the date at which the grace period expires
821-
review_req = ReviewRequestFactory(state_id='assigned', deadline=today - datetime.timedelta(5))
822-
reviewer = RoleFactory(name_id='reviewer', group=review_req.team,person__user__username='reviewer').person
823-
ReviewAssignmentFactory(review_request=review_req, state_id='assigned', assigned_on=review_req.time, reviewer=reviewer.email_set.first())
824-
secretary = RoleFactory(name_id='secr', group=review_req.team, person__user__username='reviewsecretary')
825-
826-
# A assignment that is not yet overdue
827-
not_overdue = today + datetime.timedelta(days=1)
828-
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())
829-
830-
# An assignment that is overdue but is not past the grace period
831-
in_grace_period = today - datetime.timedelta(days=1)
832-
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())
833-
834-
empty_outbox()
835-
log = send_review_reminder_overdue_assignment(today)
836-
self.assertEqual(len(log), 1)
837-
838-
self.assertEqual(len(outbox), 1)
839-
self.assertTrue(secretary.person.email_address() in outbox[0]["To"])
840-
self.assertEqual(outbox[0]["Subject"], "1 Overdue review for team {}".format(review_req.team.acronym))
841-
message = get_payload_text(outbox[0])
842-
self.assertIn(review_req.team.acronym + ' has 1 accepted or assigned review overdue by at least 5 days.', message)
843-
self.assertIn('Review of {} by {}'.format(review_req.doc.name, reviewer.plain_name()), message)
844-
self.assertEqual(len(log), 1)
845-
self.assertIn(secretary.person.email_address(), log[0])
846-
self.assertIn('1 overdue review', log[0])
847-
848-
def test_send_reminder_all_open_reviews(self):
849-
review_req = ReviewRequestFactory(state_id='assigned')
850-
reviewer = RoleFactory(name_id='reviewer', group=review_req.team,person__user__username='reviewer').person
851-
ReviewAssignmentFactory(review_request=review_req, state_id='assigned', assigned_on=review_req.time, reviewer=reviewer.email_set.first())
852-
RoleFactory(name_id='secr', group=review_req.team, person__user__username='reviewsecretary')
853-
ReviewerSettingsFactory(team=review_req.team, person=reviewer, remind_days_open_reviews=1)
854-
855-
empty_outbox()
856-
today = datetime.date.today()
857-
log = send_reminder_all_open_reviews(today)
858-
859-
self.assertEqual(len(outbox), 1)
860-
self.assertTrue(reviewer.email_address() in outbox[0]["To"])
861-
self.assertEqual(outbox[0]["Subject"], "Reminder: you have 1 open review assignment")
862-
message = get_payload_text(outbox[0])
863-
self.assertTrue(review_req.team.acronym in message)
864-
self.assertTrue('you have 1 open review' in message)
865-
self.assertTrue(review_req.doc.name in message)
866-
self.assertTrue(review_req.deadline.strftime('%Y-%m-%d') in message)
867-
self.assertEqual(len(log), 1)
868-
self.assertTrue(reviewer.email_address() in log[0])
869-
self.assertTrue('1 open review' in log[0])
870-
871-
def test_send_reminder_unconfirmed_assignments(self):
872-
review_req = ReviewRequestFactory(state_id='assigned')
873-
reviewer = RoleFactory(name_id='reviewer', group=review_req.team, person__user__username='reviewer').person
874-
ReviewAssignmentFactory(review_request=review_req, state_id='assigned', assigned_on=review_req.time, reviewer=reviewer.email_set.first())
875-
RoleFactory(name_id='secr', group=review_req.team, person__user__username='reviewsecretary')
876-
today = datetime.date.today()
877-
878-
# By default, these reminders are disabled for all teams.
879-
empty_outbox()
880-
log = send_reminder_unconfirmed_assignments(today)
881-
self.assertEqual(len(outbox), 0)
882-
self.assertFalse(log)
883-
884-
ReviewTeamSettings.objects.update(remind_days_unconfirmed_assignments=1)
885-
886-
empty_outbox()
887-
log = send_reminder_unconfirmed_assignments(today)
888-
self.assertEqual(len(outbox), 1)
889-
self.assertIn(reviewer.email_address(), outbox[0]["To"])
890-
self.assertEqual(outbox[0]["Subject"], "Reminder: you have not responded to a review assignment")
891-
message = get_payload_text(outbox[0])
892-
self.assertIn(review_req.team.acronym, message)
893-
self.assertIn('accept or reject the assignment on', message)
894-
self.assertIn(review_req.doc.name, message)
895-
self.assertEqual(len(log), 1)
896-
self.assertIn(reviewer.email_address(), log[0])
897-
self.assertIn('not accepted/rejected review assignment', log[0])
898-
899699

900700
class BulkAssignmentTests(TestCase):
901701

0 commit comments

Comments
 (0)