From 2a27a2bffc5f0b2fc73002c7c8c042dc7ef28a4e Mon Sep 17 00:00:00 2001 From: Eric Vyncke Date: Thu, 18 May 2023 02:23:20 +0900 Subject: [PATCH] feat: Can we provide a better review assignment email subject. #3760 (#5415) * Specific email subject + requester/secretary in cc * Send the deadline in the subject * Use unicode rather than ASCII for reviewer's name * More log info in the test * Fix the closing parenthesis * Fix the email test when review is assigned * chore: address review comment --------- Co-authored-by: Robert Sparks --- ietf/group/tests_review.py | 2 +- ietf/review/utils.py | 10 +++++----- ietf/utils/test_utils.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index 43101cd378..6ca77a0e18 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -731,7 +731,7 @@ def test_rotation_queue_update(self): r = self.client.post(unassigned_url, postdict) self.assertEqual(r.status_code,302) self.assertEqual(expected_ending_head_of_rotation, policy.default_reviewer_rotation_list()[0]) - self.assertMailboxContains(outbox, subject='Last Call assignment', text='Requested by', count=4) + self.assertMailboxContains(outbox, subject='Last Call', text='Requested by', count=4) class ResetNextReviewerInTeamTests(TestCase): diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 032b1fd418..1e9a237b56 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -308,7 +308,7 @@ def email_review_assignment_change(request, review_assignment, subject, msg, by, doc=review_assignment.review_request.doc, group=review_assignment.review_request.team, review_assignment=review_assignment, - skip_review_secretary=not notify_secretary, + skip_secretary=not notify_secretary, skip_review_reviewer=not notify_reviewer, skip_review_requested_by=not notify_requested_by, ) @@ -328,11 +328,11 @@ def email_review_request_change(request, review_req, subject, msg, by, notify_se was done by that party.""" (to, cc) = gather_address_lists( 'review_req_changed', - skipped_recipients=[Person.objects.get(name="(System)").formatted_email(), by.email_address()], + skipped_recipients=[Person.objects.get(name="(System)").formatted_email()], doc=review_req.doc, group=review_req.team, review_request=review_req, - skip_review_secretary=not notify_secretary, + skip_secretary=not notify_secretary, skip_review_reviewer=not notify_reviewer, skip_review_requested_by=not notify_requested_by, ) @@ -430,9 +430,9 @@ def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=Fa email_review_request_change( request, review_req, - "%s %s assignment: %s" % (review_req.team.acronym.capitalize(), review_req.type.name,review_req.doc.name), + "For %s, %s %s review by %s assigned: %s" % (reviewer.person.name, review_req.team.acronym.capitalize(), review_req.type.name, review_req.deadline, review_req.doc.name), msg , - by=request.user.person, notify_secretary=False, notify_reviewer=True, notify_requested_by=False) + by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=True) def close_review_request(request, review_req, close_state, close_comment=''): diff --git a/ietf/utils/test_utils.py b/ietf/utils/test_utils.py index 2e6c5fffbb..ddd274a613 100644 --- a/ietf/utils/test_utils.py +++ b/ietf/utils/test_utils.py @@ -284,7 +284,7 @@ def assertMailboxContains(self, mailbox, subject=None, text=None, count=None): assert isinstance(text, str) mlist = [ m for m in mlist if text in get_payload_text(m) ] if count and len(mlist) != count: - sys.stderr.write("Wrong count in assertMailboxContains(). The complete mailbox contains %s emails:\n\n" % len(mailbox)) + sys.stderr.write("Wrong count in assertMailboxContains(). The complete mailbox contains %s messages, only %s of them contain the searched-for text:\n\n" % (len(mailbox), len(mlist))) for m in mailbox: sys.stderr.write(m.as_string()) sys.stderr.write('\n\n')