Skip to content

Commit

Permalink
fix: move remaining emails to dotdigital (#2295)
Browse files Browse the repository at this point in the history
* move remaining emails to dotdigital

* Merge branch 'master' into move-remaining-emails-to-dotdigital

* fix typos

* update tests

* fix tests

* fix tests

* get rid of unnecessary email messages

* address PR comments

* fix patch

* testing some changes to the tests

* add import

* debug tests

* debug tests

* debug tests

* debug tests

* change arg order

* debug tests

* debug tests

* debug tests

* debug tests

* debug tests

* debug tests

* debug tests

* remove import

* add import

* debug tests

* debug tests

* debug tests

* debug tests

* fix login_link url creation

* debug tests

* finalize some tests

* debug tests

* debug tests

* debug tests

* reset things I tried while debugging

* fix invite email condition

* add one more email ID

* address PR comment
  • Loading branch information
evemartin committed May 3, 2024
1 parent ba49895 commit 3abb334
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 123 deletions.
50 changes: 0 additions & 50 deletions cfl_common/common/email_messages.py

This file was deleted.

5 changes: 5 additions & 0 deletions cfl_common/common/mail.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,15 @@
"delete_account": 1567477,
"email_change_notification": 1551600,
"email_change_verification": 1551594,
"invite_teacher_with_account": 1569599,
"invite_teacher_without_account": 1569607,
"level_creation": 1570259,
"reset_password": 1557153,
"student_join_request_notification": 1569486,
"student_join_request_rejected": 1569470,
"student_join_request_sent": 1569477,
"teacher_released": 1569537,
"user_already_registered": 1569539,
"verify_new_user": 1551577,
"verify_new_user_first_reminder": 1557170,
"verify_new_user_second_reminder": 1557173,
Expand Down
13 changes: 2 additions & 11 deletions cfl_common/common/tests/utils/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,15 @@ def follow_verify_email_link_to_login(page, url, user_type):
return go_to_independent_student_login_page(page.browser)


def follow_duplicate_account_link_to_login(page, email, user_type):
_follow_duplicate_account_email_link(page, email)
def follow_duplicate_account_link_to_login(page, url, user_type):
page.browser.get(url)

if user_type == "teacher":
return go_to_teacher_login_page(page.browser)
elif user_type == "independent":
return go_to_independent_student_login_page(page.browser)


def _follow_duplicate_account_email_link(page, email):
message = str(email.message())
prefix = 'please login: <a href="'
i = str.find(message, prefix) + len(prefix)
suffix = '" rel="nofollow">'
j = str.find(message, suffix, i)
page.browser.get(message[i:j])


def follow_reset_email_link(browser, link):
browser.get(link)

Expand Down
18 changes: 0 additions & 18 deletions cfl_common/common/tests/utils/student.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from common.helpers.emails import generate_token
from common.helpers.generators import generate_login_id
from common.models import Class, Student
from django.core import mail

from . import email

Expand Down Expand Up @@ -102,23 +101,6 @@ def generate_independent_student_details():
generate_independent_student_details.next_id = 1


def signup_duplicate_independent_student_fail(page, duplicate_email=None):
page = page.go_to_signup_page()

name, username, email_address, password = generate_independent_student_details()

if not duplicate_email:
duplicate_email = email_address

page = page.independent_student_signup(name, duplicate_email, password=password, confirm_password=password)

page = page.return_to_home_page()

page = email.follow_duplicate_account_link_to_login(page, mail.outbox[0], "independent")

return page, name, username, email_address, password


@patch("common.helpers.emails.send_dotdigital_email")
def create_independent_student(page, mock_send_dotdigital_email):
page = page.go_to_signup_page()
Expand Down
9 changes: 5 additions & 4 deletions cfl_common/common/tests/utils/teacher.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

from common.helpers.emails import generate_token
from common.models import Teacher
from django.core import mail

from . import email

Expand Down Expand Up @@ -34,16 +33,18 @@ def signup_teacher_directly(preverified=True, **kwargs):
return email_address, password


def signup_duplicate_teacher_fail(page, duplicate_email):
@patch("portal.views.home.send_dotdigital_email")
def signup_duplicate_teacher_fail(page, duplicate_email, mock_send_dotdigital_email):
page = page.go_to_signup_page()

first_name, last_name, email_address, password = generate_details()
page = page.signup(first_name, last_name, duplicate_email, password, password)

page = page.return_to_home_page()

page = email.follow_duplicate_account_link_to_login(page, mail.outbox[0], "teacher")
mail.outbox = []
login_link = mock_send_dotdigital_email.call_args.kwargs["personalization_values"]["LOGIN_URL"]

page = email.follow_duplicate_account_link_to_login(page, login_link, "teacher")

return page, email_address, password

Expand Down
29 changes: 20 additions & 9 deletions portal/tests/test_independent_student.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
create_independent_student,
create_independent_student_directly,
create_school_student_directly,
signup_duplicate_independent_student_fail,
generate_independent_student_details,
verify_email,
)
from common.tests.utils.teacher import signup_teacher_directly
Expand Down Expand Up @@ -222,20 +222,30 @@ def test_signup_without_newsletter(self):
page, _, _, _, _ = create_independent_student(page)
assert is_email_verified_message_showing(self.selenium)

def test_signup_duplicate_email_failure(self):
@patch("portal.views.home.send_dotdigital_email")
def test_signup_duplicate_email_failure(self, mock_send_dotdigital_email):
page = self.go_to_homepage()
page, _, _, email, _ = create_independent_student(page)
assert is_email_verified_message_showing(self.selenium)

page = self.go_to_homepage()
page, _, _, _, _ = signup_duplicate_independent_student_fail(page, duplicate_email=email)
page = self.go_to_homepage().go_to_signup_page()

name, username, email_address, password = generate_independent_student_details()
page = page.independent_student_signup(name, email, password=password, confirm_password=password)
page = page.return_to_home_page()

mock_send_dotdigital_email.assert_called_once_with(
campaign_ids["user_already_registered"], ANY, personalization_values=ANY
)

login_link = mock_send_dotdigital_email.call_args.kwargs["personalization_values"]["LOGIN_URL"]

assert len(mail.outbox) == 1
assert mail.outbox[0].subject == "Duplicate account"
page = email_utils.follow_duplicate_account_link_to_login(page, login_link, "independent")

assert self.is_login_page(page)

def test_signup_duplicate_email_with_teacher(self):
@patch("portal.views.home.send_dotdigital_email")
def test_signup_duplicate_email_with_teacher(self, mock_send_dotdigital_email: Mock):
teacher_email, _ = signup_teacher_directly()

page = self.go_to_homepage()
Expand All @@ -248,8 +258,9 @@ def test_signup_duplicate_email_with_teacher(self):

page.return_to_home_page()

assert len(mail.outbox) == 1
assert mail.outbox[0].subject == "Duplicate account"
mock_send_dotdigital_email.assert_called_once_with(
campaign_ids["user_already_registered"], ANY, personalization_values=ANY
)

def test_login_failure(self):
page = self.go_to_homepage()
Expand Down
32 changes: 16 additions & 16 deletions portal/views/home.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import logging
import math

from common import email_messages
from common.helpers.emails import (
NOTIFICATION_EMAIL,
send_email,
send_verification_email,
)
from common.models import Student, Teacher, DynamicElement, TotalActivity
from common.mail import campaign_ids, send_dotdigital_email
from common.models import DynamicElement, Student, Teacher, TotalActivity
from common.permissions import logged_in_as_student, logged_in_as_teacher
from common.utils import _using_two_factor
from django.contrib import messages as messages
Expand All @@ -34,8 +34,7 @@
from portal.strings.coding_club import CODING_CLUB_BANNER
from portal.strings.home_learning import HOME_LEARNING_BANNER
from portal.templatetags.app_tags import cloud_storage
from portal.views.teacher.teach import DownloadType
from portal.views.teacher.teach import count_student_pack_downloads_click
from portal.views.teacher.teach import DownloadType, count_student_pack_downloads_click

LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -126,7 +125,7 @@ def process_signup_form(request, data):
email = data["teacher_email"]

if email and User.objects.filter(email=email).exists():
email_message = email_messages.userAlreadyRegisteredEmail(request, email)

is_email_ratelimited = is_ratelimited(
request=request,
group=RATELIMIT_USER_ALREADY_REGISTERED_EMAIL_GROUP,
Expand All @@ -136,12 +135,13 @@ def process_signup_form(request, data):
)

if not is_email_ratelimited:
send_email(
NOTIFICATION_EMAIL,
send_dotdigital_email(
campaign_ids["user_already_registered"],
[email],
email_message["subject"],
email_message["message"],
email_message["subject"],
personalization_values={
"EMAIL": email,
"LOGIN_URL": request.build_absolute_uri(reverse("teacher_login")),
},
)
else:
LOGGER.warn(f"Ratelimit teacher {RATELIMIT_USER_ALREADY_REGISTERED_EMAIL_GROUP}: {email}")
Expand All @@ -164,7 +164,6 @@ def process_independent_student_signup_form(request, data):
email = data["email"]

if email and User.objects.filter(email=email).exists():
email_message = email_messages.userAlreadyRegisteredEmail(request, email, is_independent_student=True)
is_email_ratelimited = is_ratelimited(
request=request,
group=RATELIMIT_USER_ALREADY_REGISTERED_EMAIL_GROUP,
Expand All @@ -174,12 +173,13 @@ def process_independent_student_signup_form(request, data):
)

if not is_email_ratelimited:
send_email(
NOTIFICATION_EMAIL,
send_dotdigital_email(
campaign_ids["user_already_registered"],
[email],
email_message["subject"],
email_message["message"],
email_message["subject"],
personalization_values={
"EMAIL": email,
"LOGIN_URL": request.build_absolute_uri(reverse("independent_student_login")),
},
)
else:
LOGGER.warning(f"Ratelimit independent {RATELIMIT_USER_ALREADY_REGISTERED_EMAIL_GROUP}: {email}")
Expand Down
1 change: 0 additions & 1 deletion portal/views/student/play.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from typing import Any, Dict, List, Optional

from common import email_messages
from common.helpers.emails import NOTIFICATION_EMAIL, send_email
from common.mail import campaign_ids, send_dotdigital_email
from common.models import Student
Expand Down
46 changes: 32 additions & 14 deletions portal/views/teacher/dashboard.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from datetime import timedelta
from uuid import uuid4

from common import email_messages
from common.helpers.emails import (
INVITE_FROM,
NOTIFICATION_EMAIL,
Expand Down Expand Up @@ -167,9 +166,21 @@ def dashboard_teacher_view(request, is_admin):
)

account_exists = User.objects.filter(email=invited_teacher_email).exists()
message = email_messages.inviteTeacherEmail(request, school.name, token, account_exists)
send_email(
INVITE_FROM, [invited_teacher_email], message["subject"], message["message"], message["subject"]

registration_link = (
f"{request.build_absolute_uri(reverse('invited_teacher', kwargs={'token': token}))} "
)

campaign_id = (
campaign_ids["invite_teacher_with_account"]
if account_exists
else campaign_ids["invite_teacher_without_account"]
)

send_dotdigital_email(
campaign_id,
[invited_teacher_email],
personalization_values={"SCHOOL_NAME": school.name, "REGISTRATION_LINK": registration_link},
)

messages.success(
Expand Down Expand Up @@ -375,14 +386,10 @@ def organisation_kick(request, pk):

messages.success(request, success_message)

emailMessage = email_messages.kickedEmail(request, user.school.name)

send_email(
NOTIFICATION_EMAIL,
send_dotdigital_email(
campaign_ids["teacher_released"],
[teacher.new_user.email],
emailMessage["subject"],
emailMessage["message"],
emailMessage["subject"],
personalization_values={"SCHOOL_CLUB_NAME": user.school.name},
)

return HttpResponseRedirect(reverse_lazy("dashboard"))
Expand Down Expand Up @@ -589,10 +596,21 @@ def resend_invite_teacher(request, token):
teacher = Teacher.objects.filter(id=invite.from_teacher.id)[0]

messages.success(request, "Teacher re-invited!")
message = email_messages.inviteTeacherEmail(request, invite.school, token, not (invite.is_expired))
send_email(
INVITE_FROM, [invite.invited_teacher_email], message["subject"], message["message"], message["subject"]

registration_link = f"{request.build_absolute_uri(reverse('invited_teacher', kwargs={'token': token}))} "

campaign_id = (
campaign_ids["invite_teacher_with_account"]
if teacher.exists()
else campaign_ids["invite_teacher_without_account"]
)

send_dotdigital_email(
campaign_id,
[invite.invited_teacher_email],
personalization_values={"SCHOOL_NAME": invite.school, "REGISTRATION_LINK": registration_link},
)

return HttpResponseRedirect(reverse_lazy("dashboard"))


Expand Down

0 comments on commit 3abb334

Please sign in to comment.