Make log_data required in mailer.send

Author: mtomilov

Diff for: h/services/email.py

@@ -57,7 +57,7 @@ def __init__(self, request: Request, mailer: IMailer) -> None:
         self._request = request
         self._mailer = mailer
 
-    def send(self, email_data: EmailData, log_data: LogData | None = None) -> None:
+    def send(self, email_data: EmailData, log_data: LogData) -> None:
         if self._request.debug:  # pragma: no cover
             logger.info("emailing in debug mode: check the `mail/` directory")
         try:
@@ -70,14 +70,13 @@ def send(self, email_data: EmailData, log_data: LogData | None = None) -> None:
         except smtplib.SMTPException:
             raise
 
-        if log_data:
-            logger.info(
-                "Sent email: tag=%r, sender_id=%s, recipient_ids=%s, annotation_id=%r",
-                log_data.tag,
-                log_data.sender_id,
-                log_data.recipient_ids,
-                log_data.annotation_id,
-            )
+        logger.info(
+            "Sent email: tag=%r, sender_id=%s, recipient_ids=%s, annotation_id=%r",
+            log_data.tag,
+            log_data.sender_id,
+            log_data.recipient_ids,
+            log_data.annotation_id,
+        )
 
 
 def factory(_context, request: Request) -> EmailService:

Diff for: h/tasks/mailer.py

@@ -24,11 +24,11 @@
 def send(
     self,  # noqa: ARG001
     email_data: dict[str, Any],
-    log_data: dict[str, Any] | None = None,
+    log_data: dict[str, Any],
 ) -> None:
     """Send an email.
 
     :param email_data: A dictionary containing email data compatible with EmailData class.
     """
     service: EmailService = celery.request.find_service(EmailService)
-    service.send(EmailData(**email_data), LogData(**log_data) if log_data else None)
+    service.send(EmailData(**email_data), LogData(**log_data))

Diff for: tests/unit/h/services/email_test.py

@@ -7,14 +7,10 @@
 
 
 class TestEmailService:
-    def test_send_creates_email_message(self, email_service, pyramid_mailer):
-        email = EmailData(
-            recipients=["[email protected]"],
-            subject="My email subject",
-            body="Some text body",
-            tag=EmailTag.TEST,
-        )
-        email_service.send(email)
+    def test_send_creates_email_message(
+        self, email_data, log_data, email_service, pyramid_mailer
+    ):
+        email_service.send(email_data, log_data)
 
         pyramid_mailer.message.Message.assert_called_once_with(
             recipients=["[email protected]"],
@@ -25,16 +21,16 @@ def test_send_creates_email_message(self, email_service, pyramid_mailer):
         )
 
     def test_send_creates_email_message_with_html_body(
-        self, email_service, pyramid_mailer
+        self, log_data, email_service, pyramid_mailer
     ):
-        email = EmailData(
+        email_data = EmailData(
             recipients=["[email protected]"],
             subject="My email subject",
             body="Some text body",
             tag=EmailTag.TEST,
             html="<p>An HTML body</p>",
         )
-        email_service.send(email)
+        email_service.send(email_data, log_data)
 
         pyramid_mailer.message.Message.assert_called_once_with(
             recipients=["[email protected]"],
@@ -45,41 +41,25 @@ def test_send_creates_email_message_with_html_body(
         )
 
     def test_send_dispatches_email_using_request_mailer(
-        self, email_service, pyramid_mailer
+        self, email_data, log_data, email_service, pyramid_mailer
     ):
         request_mailer = pyramid_mailer.get_mailer.return_value
         message = pyramid_mailer.message.Message.return_value
 
-        email = EmailData(
-            recipients=["[email protected]"],
-            subject="My email subject",
-            body="Some text body",
-            tag=EmailTag.TEST,
-        )
-        email_service.send(email)
+        email_service.send(email_data, log_data)
 
         request_mailer.send_immediately.assert_called_once_with(message)
 
-    def test_raises_smtplib_exception(self, email_service, pyramid_mailer):
+    def test_raises_smtplib_exception(
+        self, email_data, log_data, email_service, pyramid_mailer
+    ):
         request_mailer = pyramid_mailer.get_mailer.return_value
         request_mailer.send_immediately.side_effect = smtplib.SMTPException()
 
-        email = EmailData(
-            recipients=["[email protected]"],
-            subject="My email subject",
-            body="Some text body",
-            tag=EmailTag.TEST,
-        )
         with pytest.raises(smtplib.SMTPException):
-            email_service.send(email)
+            email_service.send(email_data, log_data)
 
-    def test_send_logging(self, email_service, info_caplog):
-        email_data = EmailData(
-            recipients=["[email protected]"],
-            subject="My email subject",
-            body="Some text body",
-            tag=EmailTag.TEST,
-        )
+    def test_send_logging(self, email_service, info_caplog, email_data):
         user_id = 123
         annotation_id = "annotation_id"
         log_data = LogData(
@@ -94,6 +74,23 @@ def test_send_logging(self, email_service, info_caplog):
             f"Sent email: tag={email_data.tag!r}, sender_id={user_id}, recipient_ids=[{user_id}], annotation_id={annotation_id!r}"
         ]
 
+    @pytest.fixture
+    def email_data(self):
+        return EmailData(
+            recipients=["[email protected]"],
+            subject="My email subject",
+            body="Some text body",
+            tag=EmailTag.TEST,
+        )
+
+    @pytest.fixture
+    def log_data(self):
+        return LogData(
+            sender_id=123,
+            tag=EmailTag.TEST,
+            recipient_ids=[456],
+        )
+
     @pytest.fixture
     def pyramid_request(self, pyramid_request):
         pyramid_request.debug = False

Diff for: tests/unit/h/tasks/mailer_test.py

@@ -6,19 +6,7 @@
 from h.tasks import mailer
 
 
-def test_send_without_log_data(email_service):
-    email_data = {
-        "recipients": ["[email protected]"],
-        "subject": "My email subject",
-        "body": "Some text body",
-        "tag": EmailTag.TEST,
-    }
-    mailer.send(email_data)
-
-    email_service.send.assert_called_once_with(EmailData(**email_data), None)
-
-
-def test_send_with_log_data(email_service):
+def test_send(email_service):
     email_data = {
         "recipients": ["[email protected]"],
         "subject": "My email subject",