From f22fb2159af59f69a32639e02f8c446b806e3b49 Mon Sep 17 00:00:00 2001 From: faucomte97 Date: Wed, 8 Nov 2023 16:22:35 +0000 Subject: [PATCH] fix: Send verification email to new email on email change request --- cfl_common/common/email_messages.py | 10 --- cfl_common/common/helpers/emails.py | 81 ++++++++++++++---------- portal/tests/test_independent_student.py | 9 ++- portal/tests/test_teacher.py | 9 ++- 4 files changed, 61 insertions(+), 48 deletions(-) diff --git a/cfl_common/common/email_messages.py b/cfl_common/common/email_messages.py index e67ea2d5d..c6b70c968 100644 --- a/cfl_common/common/email_messages.py +++ b/cfl_common/common/email_messages.py @@ -72,16 +72,6 @@ def emailChangeNotificationEmail(request, new_email_address): } -def emailChangeDuplicateNotificationEmail(request, email): - return { - "subject": f"Duplicate account", - "message": ( - f"A user is already registered with this email address: {email}.\n" - f"Please change your email address to something else." - ), - } - - def userAlreadyRegisteredEmail(request, email, is_independent_student=False): if is_independent_student: login_url = reverse("independent_student_login") diff --git a/cfl_common/common/helpers/emails.py b/cfl_common/common/helpers/emails.py index 508cdadf7..0383e698d 100644 --- a/cfl_common/common/helpers/emails.py +++ b/cfl_common/common/helpers/emails.py @@ -10,7 +10,6 @@ from common.email_messages import ( emailChangeNotificationEmail, emailChangeVerificationEmail, - emailChangeDuplicateNotificationEmail, emailVerificationNeededEmail, parentsEmailVerificationNeededEmail, ) @@ -61,8 +60,8 @@ def send_email( if replace_url: verify_url = replace_url["verify_url"] - verify_replace_url = re.sub(f'(.*/verify_email/)(.*)', f'\\1', verify_url) - html_body = re.sub(f'({verify_url})(.*){verify_url}', f'\\1\\2{verify_replace_url}', original_html_body) + verify_replace_url = re.sub(f"(.*/verify_email/)(.*)", f"\\1", verify_url) + html_body = re.sub(f"({verify_url})(.*){verify_url}", f"\\1\\2{verify_replace_url}", original_html_body) # make message using templates message = EmailMultiAlternatives(subject, plaintext_body, sender, recipients) @@ -78,6 +77,7 @@ def generate_token(user, new_email="", preverified=False): return generate_token_for_email(user.email, new_email) + def generate_token_for_email(email: str, new_email: str = ""): return jwt.encode( { @@ -124,7 +124,14 @@ def send_verification_email(request, user, data, new_email=None, age=None): # if the user is a teacher if age is None: message = emailVerificationNeededEmail(request, verification) - send_email(VERIFICATION_EMAIL, [user.email], message["subject"], message["message"], message["subject"], replace_url=message["url"]) + send_email( + VERIFICATION_EMAIL, + [user.email], + message["subject"], + message["message"], + message["subject"], + replace_url=message["url"], + ) if _newsletter_ticked(data): add_to_dotmailer(user.first_name, user.last_name, user.email, DotmailerUserType.TEACHER) @@ -132,10 +139,24 @@ def send_verification_email(request, user, data, new_email=None, age=None): else: if age < 13: message = parentsEmailVerificationNeededEmail(request, user, verification) - send_email(VERIFICATION_EMAIL, [user.email], message["subject"], message["message"], message["subject"], replace_url=message["url"]) + send_email( + VERIFICATION_EMAIL, + [user.email], + message["subject"], + message["message"], + message["subject"], + replace_url=message["url"], + ) else: message = emailVerificationNeededEmail(request, verification) - send_email(VERIFICATION_EMAIL, [user.email], message["subject"], message["message"], message["subject"], replace_url=message["url"]) + send_email( + VERIFICATION_EMAIL, + [user.email], + message["subject"], + message["message"], + message["subject"], + replace_url=message["url"], + ) if _newsletter_ticked(data): add_to_dotmailer(user.first_name, user.last_name, user.email, DotmailerUserType.STUDENT) @@ -144,10 +165,14 @@ def send_verification_email(request, user, data, new_email=None, age=None): verification = generate_token(user, new_email) message = emailChangeVerificationEmail(request, verification) - send_email(VERIFICATION_EMAIL, [user.email], message["subject"], message["message"], message["subject"], replace_url=message["url"]) - - message = emailChangeNotificationEmail(request, new_email) - send_email(VERIFICATION_EMAIL, [user.email], message["subject"], message["message"], message["subject"]) + send_email( + VERIFICATION_EMAIL, + [new_email], + message["subject"], + message["message"], + message["subject"], + replace_url=message["url"], + ) def add_to_dotmailer(first_name: str, last_name: str, email: str, user_type: DotmailerUserType): @@ -255,17 +280,12 @@ def update_indy_email(user, request, data): if new_email != "" and new_email != user.email: changing_email = True users_with_email = User.objects.filter(email=new_email) - # email is already taken - if users_with_email.exists(): - email_message = emailChangeDuplicateNotificationEmail(request, new_email) - send_email( - NOTIFICATION_EMAIL, - [user.email], - email_message["subject"], - email_message["message"], - email_message["subject"], - ) - else: + + message = emailChangeNotificationEmail(request, new_email) + send_email(VERIFICATION_EMAIL, [user.email], message["subject"], message["message"], message["subject"]) + + # email is available + if not users_with_email.exists(): # new email to set and verify send_verification_email(request, user, data, new_email) return changing_email, new_email @@ -278,17 +298,14 @@ def update_email(user: Teacher or Student, request, data): if new_email != "" and new_email != user.new_user.email: changing_email = True users_with_email = User.objects.filter(email=new_email) - # email is already taken - if users_with_email.exists(): - email_message = emailChangeDuplicateNotificationEmail(request, new_email) - send_email( - NOTIFICATION_EMAIL, - [user.new_user.email], - email_message["subject"], - email_message["message"], - email_message["subject"], - ) - else: + + message = emailChangeNotificationEmail(request, new_email) + send_email( + VERIFICATION_EMAIL, [user.new_user.email], message["subject"], message["message"], message["subject"] + ) + + # email is available + if not users_with_email.exists(): # new email to set and verify send_verification_email(request, user.new_user, data, new_email) return changing_email, new_email diff --git a/portal/tests/test_independent_student.py b/portal/tests/test_independent_student.py index 4af7b0562..0bf2a0095 100644 --- a/portal/tests/test_independent_student.py +++ b/portal/tests/test_independent_student.py @@ -354,7 +354,7 @@ def test_change_email(self): assert is_email_updated_message_showing(self.selenium) subject = str(mail.outbox[0].subject) - assert subject == "Duplicate account" + assert subject == "Email address update" mail.outbox = [] # Try changing email to an existing teacher's email @@ -372,7 +372,7 @@ def test_change_email(self): assert is_email_updated_message_showing(self.selenium) subject = str(mail.outbox[0].subject) - assert subject == "Duplicate account" + assert subject == "Email address update" mail.outbox = [] page = ( @@ -401,7 +401,10 @@ def test_change_email(self): page = page.logout() - page = email_utils.follow_change_email_link_to_independent_dashboard(page, mail.outbox[0]) + subject = str(mail.outbox[0].subject) + assert subject == "Email address update" + + page = email_utils.follow_change_email_link_to_independent_dashboard(page, mail.outbox[1]) mail.outbox = [] page = page.independent_student_login(new_email, password) diff --git a/portal/tests/test_teacher.py b/portal/tests/test_teacher.py index eec97dcef..2b01e3bb3 100644 --- a/portal/tests/test_teacher.py +++ b/portal/tests/test_teacher.py @@ -553,7 +553,7 @@ def test_change_email(self): assert is_email_updated_message_showing(self.selenium) subject = str(mail.outbox[0].subject) - assert subject == "Duplicate account" + assert subject == "Email address update" mail.outbox = [] # Try changing email to an existing indy student's email, should fail @@ -566,7 +566,7 @@ def test_change_email(self): assert is_email_updated_message_showing(self.selenium) subject = str(mail.outbox[0].subject) - assert subject == "Duplicate account" + assert subject == "Email address update" mail.outbox = [] page = self.go_to_homepage() @@ -585,7 +585,10 @@ def test_change_email(self): page = page.logout() - page = email_utils.follow_change_email_link_to_dashboard(page, mail.outbox[0]) + subject = str(mail.outbox[0].subject) + assert subject == "Email address update" + + page = email_utils.follow_change_email_link_to_dashboard(page, mail.outbox[1]) mail.outbox = [] page = page.login(new_email, password).open_account_tab()