Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Send tags header to mandrill #9332

Merged
merged 1 commit into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion h/emails/flag_notification.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from pyramid.renderers import render

from h.i18n import TranslationString as _
from h.services.email import EmailTag


def generate(request, email, incontext_link):
Expand All @@ -27,4 +28,4 @@ def generate(request, email, incontext_link):
"h:templates/emails/flag_notification.html.jinja2", context, request=request
)

return [email], subject, text, html
return [email], subject, text, EmailTag.FLAG_NOTIFICATION, html
mtomilov marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 8 additions & 1 deletion h/emails/reply_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from h.models import Subscriptions
from h.notification.reply import Notification
from h.services import SubscriptionService
from h.services.email import EmailTag


def generate(request: Request, notification: Notification):
Expand Down Expand Up @@ -49,7 +50,13 @@ def generate(request: Request, notification: Notification):
"h:templates/emails/reply_notification.html.jinja2", context, request=request
)

return [notification.parent_user.email], subject, text, html
return (
[notification.parent_user.email],
subject,
text,
EmailTag.REPLY_NOTIFICATION,
html,
)


def _get_user_url(user, request):
Expand Down
3 changes: 2 additions & 1 deletion h/emails/reset_password.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from pyramid.renderers import render

from h.i18n import TranslationString as _
from h.services.email import EmailTag


def generate(request, user):
Expand Down Expand Up @@ -31,4 +32,4 @@ def generate(request, user):
"h:templates/emails/reset_password.html.jinja2", context, request=request
)

return [user.email], subject, text, html
return [user.email], subject, text, EmailTag.RESET_PASSWORD, html
3 changes: 2 additions & 1 deletion h/emails/signup.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from pyramid.renderers import render

from h.i18n import TranslationString as _
from h.services.email import EmailTag


def generate(request, user_id, email, activation_code):
Expand All @@ -27,4 +28,4 @@ def generate(request, user_id, email, activation_code):
text = render("h:templates/emails/signup.txt.jinja2", context, request=request)
html = render("h:templates/emails/signup.html.jinja2", context, request=request)

return [email], subject, text, html
return [email], subject, text, EmailTag.ACTIVATION, html
3 changes: 2 additions & 1 deletion h/emails/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pyramid.renderers import render

from h import __version__
from h.services.email import EmailTag


def generate(request, recipient):
Expand All @@ -28,4 +29,4 @@ def generate(request, recipient):
text = render("h:templates/emails/test.txt.jinja2", context, request=request)
html = render("h:templates/emails/test.html.jinja2", context, request=request)

return [recipient], "Test mail", text, html
return [recipient], "Test mail", text, EmailTag.TEST, html
25 changes: 22 additions & 3 deletions h/services/email.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# noqa: A005

import smtplib
from enum import StrEnum

import pyramid_mailer
import pyramid_mailer.message
Expand All @@ -12,6 +13,14 @@
logger = get_task_logger(__name__)


class EmailTag(StrEnum):
ACTIVATION = "activation"
FLAG_NOTIFICATION = "flag_notification"
REPLY_NOTIFICATION = "reply_notification"
RESET_PASSWORD = "reset_password" # noqa: S105
TEST = "test"
mtomilov marked this conversation as resolved.
Show resolved Hide resolved


class EmailService:
"""A service for sending emails."""

Expand All @@ -20,10 +29,20 @@ def __init__(self, request: Request, mailer: IMailer) -> None:
self._mailer = mailer

def send(
self, recipients: list[str], subject: str, body: str, html: str | None = None
):
self,
recipients: list[str],
subject: str,
body: str,
tag: EmailTag,
html: str | None = None,
) -> None:
extra_headers = {"X-MC-Tags": tag}
email = pyramid_mailer.message.Message(
subject=subject, recipients=recipients, body=body, html=html
subject=subject,
recipients=recipients,
body=body,
html=html,
extra_headers=extra_headers,
)
if self._request.debug: # pragma: no cover
logger.info("emailing in debug mode: check the `mail/` directory")
Expand Down
15 changes: 12 additions & 3 deletions h/tasks/mailer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,21 @@

import smtplib

from h.services.email import EmailService
from h.services.email import EmailService, EmailTag
from h.tasks.celery import celery

__all__ = ("send",)


@celery.task(bind=True, max_retries=3, acks_late=True)
def send(self, recipients, subject, body, html=None):
def send( # noqa: PLR0913
mtomilov marked this conversation as resolved.
Show resolved Hide resolved
self,
recipients: list[str],
subject: str,
body: str,
tag: EmailTag,
html: str | None = None,
) -> None:
"""
Send an email.

Expand All @@ -28,7 +35,9 @@ def send(self, recipients, subject, body, html=None):
"""
service = celery.request.find_service(EmailService)
try:
service.send(recipients=recipients, subject=subject, body=body, html=html)
service.send(
recipients=recipients, subject=subject, body=body, html=html, tag=tag
)
except smtplib.socket.error as exc:
# Exponential backoff in case the SMTP service is having problems.
countdown = self.default_retry_delay * 2**self.request.retries
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/h/emails/flag_notification_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

from h.emails.flag_notification import generate
from h.services.email import EmailTag


class TestGenerate:
Expand All @@ -23,7 +24,7 @@ def test_appropriate_return_values(
html_renderer.string_response = "HTML output"
text_renderer.string_response = "Text output"

recipients, subject, text, html = generate(
recipients, subject, text, tag, html = generate(
pyramid_request,
email="[email protected]",
incontext_link="http://hyp.is/a/ann1",
Expand All @@ -32,6 +33,7 @@ def test_appropriate_return_values(
assert recipients == ["[email protected]"]
assert subject == "An annotation has been flagged"
assert html == "HTML output"
assert tag == EmailTag.FLAG_NOTIFICATION
assert text == "Text output"

@pytest.fixture
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/h/emails/reply_notification_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def test_supports_non_ascii_display_names(
parent_user.display_name = "Parent 👩"
reply_user.display_name = "Child 👧"

(_, subject, _, _) = generate(pyramid_request, notification)
(_, subject, _, _, _) = generate(pyramid_request, notification)

assert subject == "Child 👧 has replied to your annotation"

Expand Down Expand Up @@ -130,28 +130,28 @@ def test_returns_text_and_body_results_from_renderers(
html_renderer.string_response = "HTML output"
text_renderer.string_response = "Text output"

_, _, text, html = generate(pyramid_request, notification)
_, _, text, _, html = generate(pyramid_request, notification)

assert html == "HTML output"
assert text == "Text output"

def test_returns_subject_with_reply_display_name(
self, notification, pyramid_request
):
_, subject, _, _ = generate(pyramid_request, notification)
_, subject, _, _, _ = generate(pyramid_request, notification)

assert subject == "Ron Burgundy has replied to your annotation"

def test_returns_subject_with_reply_username(
self, notification, pyramid_request, reply_user
):
reply_user.display_name = None
_, subject, _, _ = generate(pyramid_request, notification)
_, subject, _, _, _ = generate(pyramid_request, notification)

assert subject == "ron has replied to your annotation"

def test_returns_parent_email_as_recipients(self, notification, pyramid_request):
recipients, _, _, _ = generate(pyramid_request, notification)
recipients, _, _, _, _ = generate(pyramid_request, notification)

assert recipients == ["[email protected]"]

Expand Down
4 changes: 3 additions & 1 deletion tests/unit/h/emails/reset_password_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest

from h.emails.reset_password import generate
from h.services.email import EmailTag


@pytest.mark.usefixtures("routes")
Expand Down Expand Up @@ -38,11 +39,12 @@ def test_appropriate_return_values(
html_renderer.string_response = "HTML output"
text_renderer.string_response = "Text output"

recipients, subject, text, html = generate(pyramid_request, user)
recipients, subject, text, tag, html = generate(pyramid_request, user)

assert recipients == [user.email]
assert subject == "Reset your password"
assert html == "HTML output"
assert tag == EmailTag.RESET_PASSWORD
assert text == "Text output"

def test_jinja_templates_render(
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/h/emails/signup_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest

from h.emails.signup import generate
from h.services.email import EmailTag


@pytest.mark.usefixtures("routes")
Expand All @@ -27,7 +28,7 @@ def test_appropriate_return_values(
html_renderer.string_response = "HTML output"
text_renderer.string_response = "Text output"

recipients, subject, text, html = generate(
recipients, subject, text, tag, html = generate(
pyramid_request,
user_id=1234,
email="[email protected]",
Expand All @@ -37,6 +38,7 @@ def test_appropriate_return_values(
assert recipients == ["[email protected]"]
assert subject == "Please activate your account"
assert html == "HTML output"
assert tag == EmailTag.ACTIVATION
assert text == "Text output"

def test_jinja_templates_render(self, pyramid_config, pyramid_request):
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/h/emails/test_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from h import __version__
from h.emails.test import generate
from h.services.email import EmailTag


class TestGenerate:
Expand All @@ -26,13 +27,14 @@ def test_appropriate_return_values(
html_renderer.string_response = "HTML output"
text_renderer.string_response = "Text output"

recipients, subject, text, html = generate(
recipients, subject, text, tag, html = generate(
pyramid_request, "[email protected]"
)

assert recipients == ["[email protected]"]
assert subject == "Test mail"
assert html == "HTML output"
assert tag == EmailTag.TEST
assert text == "Text output"

def test_jinja_templates_render(self, pyramid_config, pyramid_request):
Expand Down
8 changes: 7 additions & 1 deletion tests/unit/h/services/email_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import pytest

from h.services.email import EmailService, factory
from h.services.email import EmailService, EmailTag, factory


class TestEmailService:
Expand All @@ -12,13 +12,15 @@ def test_send_creates_email_message(self, email_service, pyramid_mailer):
recipients=["[email protected]"],
subject="My email subject",
body="Some text body",
tag=EmailTag.TEST,
)

pyramid_mailer.message.Message.assert_called_once_with(
recipients=["[email protected]"],
subject="My email subject",
body="Some text body",
html=None,
extra_headers={"X-MC-Tags": EmailTag.TEST},
)

def test_send_creates_email_message_with_html_body(
Expand All @@ -28,6 +30,7 @@ def test_send_creates_email_message_with_html_body(
recipients=["[email protected]"],
subject="My email subject",
body="Some text body",
tag=EmailTag.TEST,
html="<p>An HTML body</p>",
)

Expand All @@ -36,6 +39,7 @@ def test_send_creates_email_message_with_html_body(
subject="My email subject",
body="Some text body",
html="<p>An HTML body</p>",
extra_headers={"X-MC-Tags": EmailTag.TEST},
)

def test_send_dispatches_email_using_request_mailer(
Expand All @@ -48,6 +52,7 @@ def test_send_dispatches_email_using_request_mailer(
recipients=["[email protected]"],
subject="My email subject",
body="Some text body",
tag=EmailTag.TEST,
)

request_mailer.send_immediately.assert_called_once_with(message)
Expand All @@ -61,6 +66,7 @@ def test_raises_smtplib_exception(self, email_service, pyramid_mailer):
recipients=["[email protected]"],
subject="My email subject",
body="Some text body",
tag=EmailTag.TEST,
)

@pytest.fixture
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/h/tasks/mailer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import pytest

from h.services.email import EmailTag
from h.tasks import mailer


Expand All @@ -14,6 +15,7 @@ def test_send_retries_if_mailing_fails(email_service):
recipients=["[email protected]"],
subject="My email subject",
body="Some text body",
tag=EmailTag.TEST,
)

assert mailer.send.retry.called
Expand Down
Loading