From b51fd428b9086aff42d651f87bb2e3e460317890 Mon Sep 17 00:00:00 2001 From: faucomte97 Date: Wed, 8 Nov 2023 16:23:53 +0000 Subject: [PATCH] Revert "fix: Send verification email to new email on email change request" This reverts commit f22fb2159af59f69a32639e02f8c446b806e3b49. --- 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, 48 insertions(+), 61 deletions(-) diff --git a/cfl_common/common/email_messages.py b/cfl_common/common/email_messages.py index c6b70c968..e67ea2d5d 100644 --- a/cfl_common/common/email_messages.py +++ b/cfl_common/common/email_messages.py @@ -72,6 +72,16 @@ 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 0383e698d..508cdadf7 100644 --- a/cfl_common/common/helpers/emails.py +++ b/cfl_common/common/helpers/emails.py @@ -10,6 +10,7 @@ from common.email_messages import ( emailChangeNotificationEmail, emailChangeVerificationEmail, + emailChangeDuplicateNotificationEmail, emailVerificationNeededEmail, parentsEmailVerificationNeededEmail, ) @@ -60,8 +61,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) @@ -77,7 +78,6 @@ 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,14 +124,7 @@ 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) @@ -139,24 +132,10 @@ 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) @@ -165,14 +144,10 @@ 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, - [new_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"]) + + message = emailChangeNotificationEmail(request, new_email) + send_email(VERIFICATION_EMAIL, [user.email], message["subject"], message["message"], message["subject"]) def add_to_dotmailer(first_name: str, last_name: str, email: str, user_type: DotmailerUserType): @@ -280,12 +255,17 @@ 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) - - 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(): + # 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: # new email to set and verify send_verification_email(request, user, data, new_email) return changing_email, new_email @@ -298,14 +278,17 @@ 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) - - 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(): + # 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: # 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 0bf2a0095..4af7b0562 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 == "Email address update" + assert subject == "Duplicate account" 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 == "Email address update" + assert subject == "Duplicate account" mail.outbox = [] page = ( @@ -401,10 +401,7 @@ def test_change_email(self): page = page.logout() - 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]) + page = email_utils.follow_change_email_link_to_independent_dashboard(page, mail.outbox[0]) 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 2b01e3bb3..eec97dcef 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 == "Email address update" + assert subject == "Duplicate account" 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 == "Email address update" + assert subject == "Duplicate account" mail.outbox = [] page = self.go_to_homepage() @@ -585,10 +585,7 @@ def test_change_email(self): page = page.logout() - subject = str(mail.outbox[0].subject) - assert subject == "Email address update" - - page = email_utils.follow_change_email_link_to_dashboard(page, mail.outbox[1]) + page = email_utils.follow_change_email_link_to_dashboard(page, mail.outbox[0]) mail.outbox = [] page = page.login(new_email, password).open_account_tab()