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

Log message when email sent #9337

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
21 changes: 15 additions & 6 deletions h/emails/flag_notification.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
from pyramid.renderers import render
from pyramid.request import Request

from h.i18n import TranslationString as _
from h.models import User
from h.services.email import EmailLogData, EmailTag


def generate(request, email, incontext_link):
def generate(
request: Request, users: list[User], incontext_link: str, annotation_id: str
):
"""
Generate an email to notify the group admin when a group member flags an annotation.

:param request: the current request
:type request: pyramid.request.Request
:param email: the group admin's email address
:type email: text
:param users: the group admins
:param incontext_link: the direct link to the flagged annotation
:type incontext_link: text

:returns: a 4-element tuple containing: recipients, subject, text, html
"""
Expand All @@ -27,4 +29,11 @@ def generate(request, email, incontext_link):
"h:templates/emails/flag_notification.html.jinja2", context, request=request
)

return [email], subject, text, html
log_data = EmailLogData(
tag=EmailTag.FLAG_NOTIFICATION,
recipient_ids=[user.id for user in users],
annotation_id=annotation_id,
)

emails = [user.email for user in users]
return emails, subject, text, EmailTag.FLAG_NOTIFICATION, html, log_data
17 changes: 16 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 EmailLogData, EmailTag


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

return [notification.parent_user.email], subject, text, html
log_data = EmailLogData(
tag=EmailTag.REPLY_NOTIFICATION,
recipient_ids=[notification.parent_user.id],
sender_id=notification.reply_user.id,
annotation_id=notification.reply.id,
)

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


def _get_user_url(user, request):
Expand Down
7 changes: 6 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 EmailLogData, EmailTag


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

return [user.email], subject, text, html
log_data = EmailLogData(
tag=EmailTag.RESET_PASSWORD, recipient_ids=[user.id], sender_id=user.id
)

return [user.email], subject, text, EmailTag.RESET_PASSWORD, html, log_data
5 changes: 4 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 EmailLogData, EmailTag


def generate(request, user_id, email, activation_code):
Expand All @@ -27,4 +28,6 @@ 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
log_data = EmailLogData(tag=EmailTag.ACTIVATION, recipient_ids=[user_id])

return [email], subject, text, EmailTag.ACTIVATION, html, log_data
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
51 changes: 46 additions & 5 deletions h/services/email.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# noqa: A005

import json
import smtplib
from dataclasses import asdict, dataclass
from enum import StrEnum
from typing import Any, Self

import pyramid_mailer
import pyramid_mailer.message
Expand All @@ -12,23 +15,61 @@
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"


@dataclass
class EmailLogData:
tag: EmailTag
recipient_ids: list[int]
annotation_id: str | None = None
sender_id: int | None = None
additional_data: dict[str, Any] | None = None

def __repr__(self: Self) -> str:
data = asdict(self)
if self.additional_data:
data.update(self.additional_data)
data.pop("additional_data")
data = {k: v for k, v in data.items() if v is not None}
return json.dumps(data)


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

def __init__(self, request: Request, mailer: IMailer) -> None:
self._request = request
self._mailer = mailer

def send(
self, recipients: list[str], subject: str, body: str, html: str | None = None
):
def send( # noqa: PLR0913
self,
recipients: list[str],
subject: str,
body: str,
tag: EmailTag,
html: str | None = None,
log_data: EmailLogData | 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")
try:
self._mailer.send_immediately(email)
if log_data:
logger.info("Email sent: %s", log_data)
except smtplib.SMTPRecipientsRefused as exc: # pragma: no cover
logger.warning(
"Recipient was refused when trying to send an email. Does the user have an invalid email address?",
Expand Down
26 changes: 18 additions & 8 deletions h/tasks/mailer.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,39 @@

import smtplib

from h.services.email import EmailService
from h.services.email import EmailLogData, 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
self,
recipients: list[str],
subject: str,
body: str,
tag: EmailTag,
html: str | None = None,
log_data: EmailLogData | None = None,
) -> None:
"""
Send an email.

:param recipients: the list of email addresses to send the email to
:type recipients: list of unicode strings

:param subject: the subject of the email
:type subject: unicode

:param body: the body of the email
:type body: unicode
"""
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,
log_data=log_data,
)
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
11 changes: 7 additions & 4 deletions h/views/api/flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ def _email_group_moderators(request, annotation):
memberships = group_members_service.get_memberships(
annotation.group, roles=list(GROUP_MODERATE_PREDICATES.keys())
)
users = [membership.user for membership in memberships]
if not users:
return

for membership in memberships:
if email := membership.user.email:
send_params = flag_notification.generate(request, email, incontext_link)
mailer.send.delay(*send_params)
send_params = flag_notification.generate(
request, users, incontext_link, annotation.id
)
mailer.send.delay(*send_params)
25 changes: 18 additions & 7 deletions tests/unit/h/emails/flag_notification_test.py
Original file line number Diff line number Diff line change
@@ -1,38 +1,49 @@
import pytest

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


class TestGenerate:
def test_calls_renderers_with_appropriate_context(
self, pyramid_request, html_renderer, text_renderer
self, pyramid_request, html_renderer, text_renderer, factories
):
generate(
pyramid_request,
email="[email protected]",
[factories.User.build()],
incontext_link="http://hyp.is/a/ann1",
annotation_id="ann1",
)

expected_context = {"incontext_link": "http://hyp.is/a/ann1"}
html_renderer.assert_(**expected_context) # noqa: PT009
text_renderer.assert_(**expected_context) # noqa: PT009

def test_appropriate_return_values(
self, pyramid_request, html_renderer, text_renderer
self, pyramid_request, html_renderer, text_renderer, factories
):
html_renderer.string_response = "HTML output"
text_renderer.string_response = "Text output"

recipients, subject, text, html = generate(
annotation_id = "ann1"
users = [factories.User.build(email="[email protected]")]
recipients, subject, text, tag, html, log_data = generate(
pyramid_request,
email="[email protected]",
incontext_link="http://hyp.is/a/ann1",
users,
incontext_link=f"http://hyp.is/a/{annotation_id}",
annotation_id=annotation_id,
)

assert recipients == ["[email protected]"]
assert recipients == [user.email for user in users]
assert subject == "An annotation has been flagged"
assert html == "HTML output"
assert tag == EmailTag.FLAG_NOTIFICATION
assert text == "Text output"
assert log_data == EmailLogData(
tag=EmailTag.FLAG_NOTIFICATION,
recipient_ids=[user.id for user in users],
annotation_id=annotation_id,
)

@pytest.fixture
def html_renderer(self, pyramid_config):
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
8 changes: 7 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 EmailLogData, EmailTag


@pytest.mark.usefixtures("routes")
Expand Down Expand Up @@ -38,12 +39,17 @@ 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, log_data = 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"
assert log_data == EmailLogData(
tag=EmailTag.RESET_PASSWORD,
recipient_ids=[user.id],
)

def test_jinja_templates_render(
self, pyramid_config, pyramid_request, serializer, user
Expand Down
Loading
Loading