✨(backend) personalize recording notification emails by user preferences
Customize email notifications for recording availability based on each user's language and timezone settings. Improves user experience through localized communications. Prioritize simple, maintainable implementation over complex code that would form subgroups based on user preferences. Note: Changes individual email sending instead of batch processing, which may impact performance for large groups but is acceptable for typical recording access patterns.
This commit is contained in:
@@ -44,49 +44,68 @@ class NotificationService:
|
|||||||
page in the frontend where they can access their specific recording.
|
page in the frontend where they can access their specific recording.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
owner_accesses = models.RecordingAccess.objects.select_related("user").filter(
|
owner_accesses = (
|
||||||
role=models.RoleChoices.OWNER,
|
models.RecordingAccess.objects.select_related("user")
|
||||||
recording_id=recording.id,
|
.filter(
|
||||||
|
role=models.RoleChoices.OWNER,
|
||||||
|
recording_id=recording.id,
|
||||||
|
)
|
||||||
|
.order_by("created_at")
|
||||||
)
|
)
|
||||||
|
|
||||||
if not owner_accesses:
|
if not owner_accesses:
|
||||||
logger.error("No owner found for recording %s", recording.id)
|
logger.error("No owner found for recording %s", recording.id)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
language = get_language()
|
|
||||||
|
|
||||||
context = {
|
context = {
|
||||||
"brandname": settings.EMAIL_BRAND_NAME,
|
"brandname": settings.EMAIL_BRAND_NAME,
|
||||||
"support_email": settings.EMAIL_SUPPORT_EMAIL,
|
"support_email": settings.EMAIL_SUPPORT_EMAIL,
|
||||||
"logo_img": settings.EMAIL_LOGO_IMG,
|
"logo_img": settings.EMAIL_LOGO_IMG,
|
||||||
"domain": settings.EMAIL_DOMAIN,
|
"domain": settings.EMAIL_DOMAIN,
|
||||||
"room_name": recording.room.name,
|
"room_name": recording.room.name,
|
||||||
"recording_date": recording.created_at.strftime("%A %d %B %Y"),
|
|
||||||
"recording_time": recording.created_at.strftime("%H:%M"),
|
|
||||||
"link": f"{settings.SCREEN_RECORDING_BASE_URL}/{recording.id}",
|
"link": f"{settings.SCREEN_RECORDING_BASE_URL}/{recording.id}",
|
||||||
}
|
}
|
||||||
|
|
||||||
emails = [access.user.email for access in owner_accesses]
|
has_failures = False
|
||||||
|
|
||||||
with override(language):
|
# We process emails individually rather than in batch because:
|
||||||
msg_html = render_to_string("mail/html/screen_recording.html", context)
|
# 1. Each email requires personalization (timezone, language)
|
||||||
msg_plain = render_to_string("mail/text/screen_recording.txt", context)
|
# 2. The number of recipients per recording is typically small (not thousands)
|
||||||
subject = str(_("Your recording is ready")) # Force translation
|
for access in owner_accesses:
|
||||||
|
user = access.user
|
||||||
try:
|
language = user.language or get_language()
|
||||||
send_mail(
|
with override(language):
|
||||||
subject.capitalize(),
|
personalized_context = {
|
||||||
msg_plain,
|
"recording_date": recording.created_at.astimezone(
|
||||||
settings.EMAIL_FROM,
|
user.timezone
|
||||||
emails,
|
).strftime("%Y-%m-%d"),
|
||||||
html_message=msg_html,
|
"recording_time": recording.created_at.astimezone(
|
||||||
fail_silently=False,
|
user.timezone
|
||||||
|
).strftime("%H:%M"),
|
||||||
|
**context,
|
||||||
|
}
|
||||||
|
msg_html = render_to_string(
|
||||||
|
"mail/html/screen_recording.html", personalized_context
|
||||||
)
|
)
|
||||||
except smtplib.SMTPException as exception:
|
msg_plain = render_to_string(
|
||||||
logger.error("notification could not be sent: %s", exception)
|
"mail/text/screen_recording.txt", personalized_context
|
||||||
return False
|
)
|
||||||
|
subject = str(_("Your recording is ready")) # Force translation
|
||||||
|
|
||||||
return True
|
try:
|
||||||
|
send_mail(
|
||||||
|
subject.capitalize(),
|
||||||
|
msg_plain,
|
||||||
|
settings.EMAIL_FROM,
|
||||||
|
[user.email],
|
||||||
|
html_message=msg_html,
|
||||||
|
fail_silently=False,
|
||||||
|
)
|
||||||
|
except smtplib.SMTPException as exception:
|
||||||
|
logger.error("notification could not be sent: %s", exception)
|
||||||
|
has_failures = True
|
||||||
|
|
||||||
|
return not has_failures
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _notify_summary_service(recording):
|
def _notify_summary_service(recording):
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ Test event notification.
|
|||||||
|
|
||||||
# pylint: disable=E1128,W0621,W0613,W0212
|
# pylint: disable=E1128,W0621,W0613,W0212
|
||||||
|
|
||||||
|
import datetime
|
||||||
import smtplib
|
import smtplib
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
@@ -74,6 +75,7 @@ def test_notify_external_services_unknown_mode(caplog):
|
|||||||
assert f"Unknown recording mode unknown for recording {recording.id}" in caplog.text
|
assert f"Unknown recording mode unknown for recording {recording.id}" in caplog.text
|
||||||
|
|
||||||
|
|
||||||
|
# pylint: disable=too-many-locals
|
||||||
def test_notify_user_by_email_success(mocked_current_site, settings):
|
def test_notify_user_by_email_success(mocked_current_site, settings):
|
||||||
"""Test successful email notification to recording owners."""
|
"""Test successful email notification to recording owners."""
|
||||||
settings.EMAIL_BRAND_NAME = "ACME"
|
settings.EMAIL_BRAND_NAME = "ACME"
|
||||||
@@ -84,15 +86,28 @@ def test_notify_user_by_email_success(mocked_current_site, settings):
|
|||||||
|
|
||||||
recording = factories.RecordingFactory(room__name="Conference Room A")
|
recording = factories.RecordingFactory(room__name="Conference Room A")
|
||||||
|
|
||||||
owners = [
|
# Fix recording test to avoid flaky tests
|
||||||
factories.UserRecordingAccessFactory(
|
recording.created_at = datetime.datetime(2023, 5, 15, 14, 30, 0)
|
||||||
recording=recording, role=models.RoleChoices.OWNER
|
|
||||||
).user,
|
french_user = factories.UserFactory(
|
||||||
factories.UserRecordingAccessFactory(
|
email="franc@test.com", language="fr-fr", timezone="Europe/Paris"
|
||||||
recording=recording, role=models.RoleChoices.OWNER
|
)
|
||||||
).user,
|
dutch_user = factories.UserFactory(
|
||||||
]
|
email="berry@test.com", language="nl-nl", timezone="Europe/Amsterdam"
|
||||||
owner_emails = [owner.email for owner in owners]
|
)
|
||||||
|
english_user = factories.UserFactory(
|
||||||
|
email="john@test.com", language="en-us", timezone="America/Phoenix"
|
||||||
|
)
|
||||||
|
|
||||||
|
factories.UserRecordingAccessFactory(
|
||||||
|
recording=recording, role=models.RoleChoices.OWNER, user=french_user
|
||||||
|
)
|
||||||
|
factories.UserRecordingAccessFactory(
|
||||||
|
recording=recording, role=models.RoleChoices.OWNER, user=dutch_user
|
||||||
|
)
|
||||||
|
factories.UserRecordingAccessFactory(
|
||||||
|
recording=recording, role=models.RoleChoices.OWNER, user=english_user
|
||||||
|
)
|
||||||
|
|
||||||
# Create non-owner users to verify they don't receive emails
|
# Create non-owner users to verify they don't receive emails
|
||||||
factories.UserRecordingAccessFactory(
|
factories.UserRecordingAccessFactory(
|
||||||
@@ -108,30 +123,69 @@ def test_notify_user_by_email_success(mocked_current_site, settings):
|
|||||||
result = notification_service._notify_user_by_email(recording)
|
result = notification_service._notify_user_by_email(recording)
|
||||||
|
|
||||||
assert result is True
|
assert result is True
|
||||||
mock_send_mail.assert_called_once()
|
assert mock_send_mail.call_count == 3
|
||||||
|
|
||||||
subject, body, sender, recipients = mock_send_mail.call_args[0]
|
call_args_list = mock_send_mail.call_args_list
|
||||||
|
|
||||||
assert subject == "Your recording is ready"
|
base_content = [
|
||||||
|
|
||||||
# Verify email contains expected content
|
|
||||||
required_content = [
|
|
||||||
"ACME", # Brand name
|
"ACME", # Brand name
|
||||||
"support@acme.com", # Support email
|
"support@acme.com", # Support email
|
||||||
"https://acme.com/logo", # Logo URL
|
"https://acme.com/logo", # Logo URL
|
||||||
f"https://acme.com/recordings/{recording.id}", # Recording link
|
f"https://acme.com/recordings/{recording.id}", # Recording link
|
||||||
"Conference Room A", # Room name
|
"Conference Room A", # Room name
|
||||||
recording.created_at.strftime("%A %d %B %Y"), # Formatted date
|
|
||||||
recording.created_at.strftime("%H:%M"), # Formatted time
|
|
||||||
]
|
]
|
||||||
|
|
||||||
for content in required_content:
|
# First call verification
|
||||||
assert content in body
|
subject1, body1, sender1, recipients1 = call_args_list[0][0]
|
||||||
|
assert subject1 == "Votre enregistrement est prêt"
|
||||||
|
|
||||||
assert sender == "notifications@acme.com"
|
# Verify email contains expected content
|
||||||
|
personalized_content1 = [
|
||||||
|
*base_content,
|
||||||
|
"2023-05-15", # Formatted date
|
||||||
|
"16:30", # Formatted time
|
||||||
|
"Votre enregistrement est prêt !", # Intro
|
||||||
|
]
|
||||||
|
for content in personalized_content1:
|
||||||
|
assert content in body1
|
||||||
|
|
||||||
# Verify all owners received the email (order-independent comparison)
|
assert recipients1 == ["franc@test.com"]
|
||||||
assert sorted(recipients) == sorted(owner_emails)
|
assert sender1 == "notifications@acme.com"
|
||||||
|
|
||||||
|
# Second call verification
|
||||||
|
subject2, body2, sender2, recipients2 = call_args_list[1][0]
|
||||||
|
assert subject2 == "Je opname is klaar"
|
||||||
|
|
||||||
|
# Verify second email content (if needed)
|
||||||
|
personalized_content2 = [
|
||||||
|
*base_content,
|
||||||
|
"2023-05-15", # Formatted date
|
||||||
|
"16:30", # Formatted time
|
||||||
|
"Je opname is klaar!", # Intro
|
||||||
|
]
|
||||||
|
for content in personalized_content2:
|
||||||
|
assert content in body2
|
||||||
|
|
||||||
|
assert recipients2 == ["berry@test.com"]
|
||||||
|
assert sender2 == "notifications@acme.com"
|
||||||
|
|
||||||
|
# Third call verification
|
||||||
|
subject3, body3, sender3, recipients3 = call_args_list[2][0]
|
||||||
|
assert subject3 == "Your recording is ready"
|
||||||
|
|
||||||
|
# Verify second email content (if needed)
|
||||||
|
personalized_content3 = [
|
||||||
|
*base_content,
|
||||||
|
"Conference Room A", # Room name
|
||||||
|
"2023-05-15", # Formatted date
|
||||||
|
"07:30", # Formatted time
|
||||||
|
"Your recording is ready!", # Intro
|
||||||
|
]
|
||||||
|
for content in personalized_content3:
|
||||||
|
assert content in body3
|
||||||
|
|
||||||
|
assert recipients3 == ["john@test.com"]
|
||||||
|
assert sender3 == "notifications@acme.com"
|
||||||
|
|
||||||
|
|
||||||
def test_notify_user_by_email_no_owners(mocked_current_site, caplog):
|
def test_notify_user_by_email_no_owners(mocked_current_site, caplog):
|
||||||
@@ -153,6 +207,9 @@ def test_notify_user_by_email_smtp_exception(mocked_current_site, caplog):
|
|||||||
factories.UserRecordingAccessFactory(
|
factories.UserRecordingAccessFactory(
|
||||||
recording=recording, role=models.RoleChoices.OWNER
|
recording=recording, role=models.RoleChoices.OWNER
|
||||||
)
|
)
|
||||||
|
factories.UserRecordingAccessFactory(
|
||||||
|
recording=recording, role=models.RoleChoices.OWNER
|
||||||
|
)
|
||||||
|
|
||||||
notification_service = NotificationService()
|
notification_service = NotificationService()
|
||||||
|
|
||||||
@@ -163,5 +220,5 @@ def test_notify_user_by_email_smtp_exception(mocked_current_site, caplog):
|
|||||||
result = notification_service._notify_user_by_email(recording)
|
result = notification_service._notify_user_by_email(recording)
|
||||||
|
|
||||||
assert result is False
|
assert result is False
|
||||||
mock_send_mail.assert_called_once()
|
assert mock_send_mail.call_count == 2
|
||||||
assert "notification could not be sent:" in caplog.text
|
assert "notification could not be sent:" in caplog.text
|
||||||
|
|||||||
Reference in New Issue
Block a user