Integrate mb-mail

config.py.example

@@ -97,6 +97,7 @@ ADMINS = [
 SMTP_SERVER = "metabrainz-mail"
 SMTP_PORT = 25
 MAIL_FROM_DOMAIN = "metabrainz.org"
+MB_MAIL_SERVER_URI = "http://localhost:3000"
 
 # OTHER STUFF
 

metabrainz/db/notification.py

@@ -4,7 +4,7 @@
 from datetime import datetime
 from typing import List, Tuple, Optional
 from flask import current_app
-from psycopg2.extras import execute_values
+from psycopg2.extras import execute_values, RealDictCursor
 
 from metabrainz import db
 
@@ -125,14 +125,14 @@ def delete_notifications(user_id: int, delete_ids: Tuple[int, ...]):
         connection.execute(delete_query, {'user_id': user_id, 'delete_ids': delete_ids})
 
 
-def insert_notifications(notifications: List[dict]) -> List[tuple[int]]:
+def insert_notifications(notifications: List[dict]) -> List[dict]:
     """
     Inserts a batch of notifications into the table.
     Args:
         notifications (List[dict]): List of notifications to be inserted.
 
     Returns:
-        List[tuple[int]]: List of tuples with each tuple contaning ID of an inserted notification.
+        List[dict]: List of inserted notifications.
     """
     params = _prepare_notifications(notifications)
     insert_query = """
@@ -147,7 +147,7 @@ def insert_notifications(notifications: List[dict]) -> List[tuple[int]]:
                         VALUES
                             %s
                         RETURNING 
-                            id
+                            *
         """
     template = """
         (
@@ -160,14 +160,14 @@ def insert_notifications(notifications: List[dict]) -> List[tuple[int]]:
 
     conn = db.engine.raw_connection()
     try:
-        with conn.cursor() as cur:
+        with conn.cursor(cursor_factory=RealDictCursor) as cur:
             execute_values(cur, insert_query, params, template)
-            notification_ids = cur.fetchall()
+            inserted_notifications = cur.fetchall()
         conn.commit()
     finally:
         conn.close()
 
-    return notification_ids
+    return inserted_notifications
 
 
 def _prepare_notifications(notifications: List[dict]) -> List[dict]:

metabrainz/mail.py

@@ -1,9 +1,10 @@
 import uuid
 import orjson
+import requests
 from flask import current_app
-from typing import List
 from brainzutils import cache
 from brainzutils.mail import send_mail
+
 from metabrainz.db.notification import (
     filter_non_digest_notifications,
     get_digest_notifications,
@@ -17,27 +18,35 @@ class NotificationSender:
     def __init__(self, notifications):
         self.notifications = notifications
 
-    def _send_notifications(self, notifications: List[dict]):
+    def _send_notifications(self, notifications: list[dict]):
         """Helper function to send immediate notifications through mail.
         If sending fails, the notification is stored in cache for later retry.
         """
         for notification in notifications:
             try:
-                send_mail(
-                    subject=notification["subject"],
-                    text=notification["body"],
-                    recipients=[notification["to"]],
-                    from_addr=notification["sent_from"],
-                )
+                if notification.get("subject") and notification.get("body"):
+                    send_mail(
+                        subject=notification["subject"],
+                        text=notification["body"],
+                        recipients=[notification["recipient"]],
+                        from_addr=notification["sent_from"],
+                    )
+                elif notification.get("template_id") and notification.get("template_params"):
+                    self.send_html_mail(
+                        from_addr=notification["sent_from"],
+                        template_id=notification["template_id"],
+                        template_params=notification["template_params"],
+                        recipient=notification["recipient"],
+                        message_id=notification["email_id"],
+                    )
+
             except Exception as err:
                 current_app.logger.error(
                     "Failed to send mail to recipient %s, %s",
-                    notification["to"],
+                    notification["recipient"],
                     str(err),
                 )
-                cache.hset(
-                    FAILED_MAIL_HASH_KEY, str(uuid.uuid4()), orjson.dumps(notification)
-                )
+                cache.hset(FAILED_MAIL_HASH_KEY, str(uuid.uuid4()), orjson.dumps(notification))
 
     def send_immediate_notifications(self):
         """Sends notifications that are marked as important or are for users with notifications enabled and digest disabled."""
@@ -47,10 +56,7 @@ def send_immediate_notifications(self):
 
         for notification in self.notifications:
             if notification["important"] and notification["send_email"]:
-                if notification.get("subject") and notification.get("body"):
-                    immediate_notifications.append(notification)
-                else:
-                    pass  # TODO: integrate mb-mail
+                immediate_notifications.append(notification)
 
             elif not notification["important"] and notification["send_email"]:
                 unimportant_notifications.append(notification)
@@ -69,3 +75,62 @@ def send_digest_notifications(self):
         notifications = get_digest_notifications()
         self._send_notifications(notifications)
         mark_notifications_sent(notifications)
+
+    def send_html_mail(
+        self,
+        from_addr: str,
+        recipient: str,
+        template_id: str,
+        template_params: dict,
+        language: str | None = None,
+        in_reply_to: list[str] | None = [],
+        sender: str | None = None,
+        references: list[str] | None = [],
+        reply_to: str | None = None,
+        message_id: str | None = None,
+    ):
+        """
+        Send HTML email through MB mail service.
+
+        Args:
+            from_addr (str):
+                The email address of the sender.
+            recipient (str):
+                The email address of the recipient.
+            template_id (str):
+                MB mail service template id for the required template.
+            template_params (dict):
+                Dictionary containing the data to be passed into the template.
+            language (str) :
+                Optional. The language preference of the recipient.
+            in_reply_to (list[str]):
+                Optional. list of Message ID of the emails thats being replied to.
+            sender (str):
+                Optional. The address ultimately sending the email Should not be set if same as from address.
+            references (list[str]):
+                Optional. list of Message ID of the emails that this email references.
+            reply_to (str):
+                Optional. Reply-To email header.
+            message_id (str):
+                Optional. UUID for the email.
+
+        Raises:
+            HTTPError: If sending email fails.
+        """
+
+        single_mail_endpoint = current_app.config["MB_MAIL_SERVER_URI"] + "/send_single"
+        data = {
+            "from": from_addr,
+            "to": recipient,
+            "template_id": template_id,
+            "params": template_params,
+            "language": language,
+            "in_reply_to": in_reply_to,
+            "sender": sender,
+            "references": references,
+            "reply_to": reply_to,
+            "message_id": message_id,
+        }
+
+        response = requests.post(url=single_mail_endpoint, json=data)
+        response.raise_for_status()

metabrainz/mail_test.py

@@ -0,0 +1,154 @@
+from unittest import mock
+import requests
+import requests_mock
+import orjson
+
+from flask import current_app
+from metabrainz.testing import FlaskTestCase
+from metabrainz.mail import NotificationSender, FAILED_MAIL_HASH_KEY
+
+
+class NotificationSenderTest(FlaskTestCase):
+    def setUp(self):
+        super(NotificationSenderTest, self).setUp()
+        self.notifications = [
+            {
+                "musicbrainz_row_id": 3,
+                "project": "metabrainz",
+                "recipient": "[email protected]",
+                "sent_from": "[email protected]",
+                "reply_to": "[email protected]",
+                "send_email": True,
+                "subject": "test",
+                "body": "test-123",
+                "important": False,
+                "expire_age": 30,
+                "email_id": "c7d7b431-7130-4a3c-ad8a-17cdd5ebdf2d",
+                "id": 6,
+            },
+            {
+                "musicbrainz_row_id": 1,
+                "project": "musicbrainz",
+                "recipient": "[email protected]",
+                "sent_from": "[email protected]",
+                "reply_to": "[email protected]",
+                "send_email": True,
+                "template_id": "verify-email",
+                "template_params": {"reason": "verify"},
+                "important": True,
+                "expire_age": 30,
+                "email_id": "c512e637-21ff-45b1-a58a-03412224b48b",
+                "id": 4,
+            },
+        ]
+        self.service = NotificationSender(self.notifications)
+
+    @requests_mock.Mocker()
+    def test_send_html_mail(self, mock_request):
+        single_mail_endpoint = current_app.config["MB_MAIL_SERVER_URI"] + "/send_single"
+        mock_request.post(single_mail_endpoint, status_code=200)
+        expected_payload = {
+            "from": self.notifications[1]["sent_from"],
+            "to": self.notifications[1]["recipient"],
+            "template_id": self.notifications[1]["template_id"],
+            "params": self.notifications[1]["template_params"],
+            "language": None,
+            "in_reply_to": [],
+            "sender": None,
+            "references": [],
+            "reply_to": None,
+            "message_id": self.notifications[1]["email_id"],
+        }
+
+        self.service.send_html_mail(
+            from_addr=self.notifications[1]["sent_from"],
+            recipient=self.notifications[1]["recipient"],
+            template_id=self.notifications[1]["template_id"],
+            template_params=self.notifications[1]["template_params"],
+            message_id=self.notifications[1]["email_id"]
+        )
+
+        self.assertEqual(mock_request.last_request.json(), expected_payload)
+
+        # Error on the mb-mail server side.
+        mock_request.post(single_mail_endpoint, status_code=500)
+
+        with self.assertRaises(requests.exceptions.HTTPError) as err:
+            self.service.send_html_mail(
+                from_addr=self.notifications[1]["sent_from"],
+                recipient=self.notifications[1]["recipient"],
+                template_id=self.notifications[1]["template_id"],
+                template_params=self.notifications[1]["template_params"],
+                message_id=self.notifications[1]["email_id"]
+            )
+
+        self.assertEqual(err.exception.response.status_code, 500)
+
+    @mock.patch("metabrainz.mail.cache")
+    @mock.patch("metabrainz.mail.NotificationSender.send_html_mail")
+    @mock.patch("metabrainz.mail.send_mail")
+    def test_send_notifications(self, mock_send_mail, mock_send_html_mail, mock_cache):
+        mock_send_mail.return_value = None
+        mock_send_html_mail.return_value = None
+
+        self.service._send_notifications(self.notifications)
+
+        # Plain text mail notification.
+        mock_send_mail.assert_called_once_with(
+            subject=self.notifications[0]["subject"],
+            text=self.notifications[0]["body"],
+            recipients=[self.notifications[0]["recipient"]],
+            from_addr=self.notifications[0]["sent_from"],
+        )
+        # HTML mail notification.
+        mock_send_html_mail.assert_called_once_with(
+            from_addr=self.notifications[1]["sent_from"],
+            recipient=self.notifications[1]["recipient"],
+            template_id=self.notifications[1]["template_id"],
+            template_params=self.notifications[1]["template_params"],
+            message_id=self.notifications[1]["email_id"]            
+        )
+
+        # MB-mail failure
+        mock_send_html_mail.side_effect = Exception()
+        mock_cache.return_value = 1
+
+        self.service._send_notifications(self.notifications)
+
+        args, _ = mock_cache.hset.call_args
+        self.assertEqual(args[0], FAILED_MAIL_HASH_KEY)
+        self.assertIsInstance(args[1], str)
+        self.assertEqual(args[2], orjson.dumps(self.notifications[1]))
+
+
+
+    @mock.patch("metabrainz.mail.NotificationSender._send_notifications")
+    @mock.patch("metabrainz.mail.mark_notifications_sent")
+    @mock.patch("metabrainz.mail.filter_non_digest_notifications")
+    def test_send_immediate_notifications(self, mock_filter, mock_mark_sent, mock_send):
+        mock_filter.return_value = []
+        mock_mark_sent.return_value = None
+        mock_send.return_value = None
+
+        self.service.send_immediate_notifications()
+
+        # Non-important notification
+        mock_filter.assert_called_once_with([self.notifications[0]])
+        # Immediate notification
+        mock_send.assert_called_once_with([self.notifications[1]])
+        mock_mark_sent.has_called_with([self.notifications[1]])
+
+    @mock.patch("metabrainz.mail.mark_notifications_sent")
+    @mock.patch("metabrainz.mail.get_digest_notifications")
+    @mock.patch("metabrainz.mail.NotificationSender._send_notifications")
+    def test_send_digest_notifications(self, mock_send, mock_get, mock_mark):
+        mock_send.return_value = None
+        mock_get.return_value = [self.notifications[0]]
+        mock_mark.return_value = None
+
+        self.service.send_digest_notifications()
+
+        mock_get.assert_called_once()
+        mock_send.assert_called_once_with([self.notifications[0]])
+        mock_mark.assert_called_once_with([self.notifications[0]])
+

metabrainz/notifications/views.py

@@ -327,12 +327,9 @@ def send_notifications():
             raise APIBadRequest(f'Notification {idx} should include either subject and body or template_id and template_params.')
 
     try:
-        notification_ids = insert_notifications(data)
+        notifications = insert_notifications(data)
 
-        for index, id_tuple in enumerate(notification_ids):
-            data[index]["id"] = id_tuple[0]
-
-        sender = NotificationSender(data)
+        sender = NotificationSender(notifications)
         sender.send_immediate_notifications()
 
     except Exception as err: