Merge pull request #45 from uclouvain/technical/OSIS-8748

[OSIS-8748] Better manage the errors when sending the emails

README.md

@@ -136,7 +136,7 @@ from osis_notification.contrib.handlers import EmailNotificationHandler
 recipient = Person.objects.get(user__username="jmr")
 language = recipient.language
 tokens = {"username": person.user.username}
-email_message = generate_email(your_mail_template_id, language, tokens, recipients=[recipient])
+email_message = generate_email(your_mail_template_id, language, tokens, recipients=[recipient.email])
 email_notification = EmailNotificationHandler.create(email_message)
 ```
 

osis_notification/api/utils.py

@@ -23,6 +23,7 @@
 #    see http://www.gnu.org/licenses/.
 #
 # ##############################################################################
+import base64
 import re
 from urllib.parse import urlparse
 
@@ -51,8 +52,10 @@ def wrapped(request, *args, **kwargs):
         url = settings.OSIS_NOTIFICATION_BASE_URL + url_for_remote_api.replace(local_base_url, '')
         headers = {
             'accept-language': request.user.person.language or settings.LANGUAGE_CODE,
-            'x-user-firstname': request.user.person.first_name or '',
-            'x-user-lastname': request.user.person.last_name or '',
+            'x-user-firstname': convert_str_to_base64_str(request.user.person.first_name)
+            if request.user.person.first_name else '',
+            'x-user-lastname': convert_str_to_base64_str(request.user.person.last_name)
+            if request.user.person.last_name else '',
             'x-user-email': request.user.email or '',
             'x-user-globalid': request.user.person.global_id,
             'authorization': f"ESB {settings.REST_FRAMEWORK_ESB_AUTHENTICATION_SECRET_KEY}",
@@ -99,6 +102,16 @@ def wrapped(request, *args, **kwargs):
     return wrapped
 
 
+def convert_str_to_base64_str(s: str) -> str:
+    """
+    This utility allow to convert an str to a base64 represtention of this str (Allow to bypass char issues)
+    but need to be decode on the server-side !
+    """
+    return base64.b64encode(
+        bytes(s, 'utf-8')
+    ).decode('utf-8')
+
+
 def make_absolute_location(base_url, location):
     """
     Convert a location header into an absolute URL.

osis_notification/contrib/exceptions.py

@@ -0,0 +1,41 @@
+# ##############################################################################
+#
+#  OSIS stands for Open Student Information System. It's an application
+#  designed to manage the core business of higher education institutions,
+#  such as universities, faculties, institutes and professional schools.
+#  The core business involves the administration of students, teachers,
+#  courses, programs and so on.
+#
+#  Copyright (C) 2015-2023 Université catholique de Louvain (http://www.uclouvain.be)
+#
+#  This program is free software: you can redistribute it and/or modify
+#  it under the terms of the GNU General Public License as published by
+#  the Free Software Foundation, either version 3 of the License, or
+#  (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#  GNU General Public License for more details.
+#
+#  A copy of this license - GNU General Public License - is available
+#  at the root of the source code of this program.  If not,
+#  see http://www.gnu.org/licenses/.
+#
+# ##############################################################################
+import uuid
+from typing import Dict
+
+
+class EmailNotificationSendingException(Exception):
+    def __init__(self, exceptions: Dict[uuid.UUID, Exception]):
+        message = (
+            'An error occurred while sending the email notifications:'
+            if len(exceptions) == 1
+            else 'Errors occurred while sending the email notifications:'
+        )
+
+        for id_notification, exception in exceptions.items():
+            message += f'\n{id_notification}: {exception}.'
+
+        super().__init__(message)

osis_notification/contrib/handlers.py

@@ -47,6 +47,7 @@
 
 UNKNOWN_PERSON = object()
 
+
 class EmailNotificationHandler:
     @staticmethod
     def build(notification: EmailNotificationType) -> EmailMessage:
@@ -112,6 +113,7 @@ def process(notification: EmailNotification):
 
         subject = make_header(decode_header(email_message.get("subject")))
         cc = email_message.get("Cc")
+        from_email = email_message.get('from', settings.DEFAULT_FROM_EMAIL)
         if cc:
             cc = [Person(email=cc_email) for cc_email in cc.split(',')]
         for mail_sender_class in settings.MAIL_SENDER_CLASSES:
@@ -123,7 +125,7 @@ def process(notification: EmailNotification):
                 subject=unescape(strip_tags(str(subject))),
                 message=plain_text_content.rstrip(),
                 html_message=html_content.rstrip(),
-                from_email=settings.DEFAULT_FROM_EMAIL,
+                from_email=from_email,
                 attachment=None,
                 cc=cc,
             )

osis_notification/management/commands/send_email_notifications.py

@@ -1,12 +1,21 @@
 from django.core.management.base import BaseCommand
 
 from osis_notification.contrib.handlers import EmailNotificationHandler
+from osis_notification.contrib.exceptions import EmailNotificationSendingException
 from osis_notification.models import EmailNotification
 
 
 class Command(BaseCommand):
     help = "Send all the email notifications."
 
     def handle(self, *args, **options):
+        exceptions = {}
+
         for notification in EmailNotification.objects.pending():
-            EmailNotificationHandler.process(notification)
+            try:
+                EmailNotificationHandler.process(notification)
+            except Exception as exception:
+                exceptions[notification.uuid] = exception
+
+        if exceptions:
+            raise EmailNotificationSendingException(exceptions)

osis_notification/tests/test_commands.py

@@ -1,28 +1,31 @@
 from datetime import timedelta
+from unittest.mock import patch
 
 from django.conf import settings
 from django.core.management import call_command
-from django.test import override_settings, TestCase
+from django.test import override_settings
 from django.utils.timezone import now
 
 from base.tests.factories.person import PersonFactory
 from osis_notification.contrib.handlers import EmailNotificationHandler
 from osis_notification.contrib.notification import (
     EmailNotification as EmailNotificationType,
 )
-from osis_notification.models import EmailNotification, WebNotification
+from osis_notification.contrib.exceptions import EmailNotificationSendingException
+from osis_notification.models import EmailNotification, WebNotification, Notification
 from osis_notification.models.enums import NotificationStates
 from osis_notification.tests import TestCase
 from osis_notification.tests.factories import (
     EmailNotificationFactory,
     WebNotificationFactory,
 )
 
+from osis_common.messaging.mail_sender_classes import MessageHistorySender
+
 
 class SendNotificationsTest(TestCase):
-    @classmethod
-    def setUpTestData(cls):
-        cls.web_notification = WebNotificationFactory()
+    def setUp(self):
+        self.web_notification = WebNotificationFactory()
         # It's seems a bit more complicated to create the email message payload in a
         # factory so I leave it like this for the moment
         email_notification_data = {
@@ -33,35 +36,71 @@ def setUpTestData(cls):
         }
         email_notification = EmailNotificationType(**email_notification_data)
         email_message = EmailNotificationHandler.build(email_notification)
-        cls.email_notification = EmailNotificationFactory(
+        self.email_notification = EmailNotificationFactory(
             payload=email_message.as_string(),
             person=email_notification_data["recipient"],
         )
 
     def test_send_email_notifications(self):
         # ensure email notification is in pending state after creation
         self.assertEqual(
-            self.email_notification.state, NotificationStates.PENDING_STATE.name
+            self.email_notification.state,
+            NotificationStates.PENDING_STATE.name,
         )
         with self.assertNumQueriesLessThan(6):
             call_command("send_email_notifications")
         self.email_notification.refresh_from_db()
         # now email notification should be in sent state
         self.assertEqual(
-            self.email_notification.state, NotificationStates.SENT_STATE.name
+            self.email_notification.state,
+            NotificationStates.SENT_STATE.name,
+        )
+
+    def test_send_email_notifications_with_failure(self):
+        second_email_notification = EmailNotificationFactory(
+            payload=self.email_notification.payload,
+            person=self.email_notification.person,
+        )
+        third_email_notification = EmailNotificationFactory(
+            payload=self.email_notification.payload,
+            person=self.email_notification.person,
         )
 
+        original_send_mail = Notification.save
+        with patch.object(Notification, 'save') as sender_mock:
+            sender_mock.side_effect = [original_send_mail, ValueError('Invalid value'), Exception('Custom error')]
+
+            with self.assertRaisesRegex(
+                EmailNotificationSendingException,
+                f'Errors occurred while sending the email notifications:'
+                f'\n{second_email_notification.uuid}: Invalid value.'
+                f'\n{third_email_notification.uuid}: Custom error.'
+            ):
+                call_command("send_email_notifications")
+
+                self.email_notification.refresh_from_db()
+                self.assertEqual(self.email_notification.state, NotificationStates.SENT_STATE.name)
+
+                second_email_notification.refresh_from_db()
+                self.assertEqual(second_email_notification.state, NotificationStates.PENDING_STATE.name)
+
+                third_email_notification.refresh_from_db()
+                self.assertEqual(second_email_notification.state, NotificationStates.PENDING_STATE.name)
+
+
     def test_send_web_notifications(self):
         # ensure web notification is in pending state after creation
         self.assertEqual(
-            self.web_notification.state, NotificationStates.PENDING_STATE.name
+            self.web_notification.state,
+            NotificationStates.PENDING_STATE.name,
         )
         with self.assertNumQueriesLessThan(3):
             call_command("send_web_notifications")
         self.web_notification.refresh_from_db()
         # now web notification should be in sent state
         self.assertEqual(
-            self.web_notification.state, NotificationStates.SENT_STATE.name
+            self.web_notification.state,
+            NotificationStates.SENT_STATE.name,
         )
 
 

setup.py

@@ -27,7 +27,7 @@
 
 setup(
     name='OSIS Notification',
-    version='0.1',
+    version='0.1.2',
     description='Notification management API and UI',
     url='http://github.com/uclouvain/osis-notification',
     author='Université catholique de Louvain',