From 83f5ac73b2fda8eccb0db5976e77a062c46efaf8 Mon Sep 17 00:00:00 2001 From: Julien Cougnaud Date: Fri, 8 Dec 2023 12:18:17 +0100 Subject: [PATCH] [OSIS-8748] Better manage the errors when sending the emails --- README.md | 2 +- .../commands/send_email_notifications.py | 10 ++++- osis_notification/tests/test_commands.py | 45 ++++++++++++++----- setup.py | 2 +- 4 files changed, 46 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 183c1f6..54f4492 100644 --- a/README.md +++ b/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) ``` diff --git a/osis_notification/management/commands/send_email_notifications.py b/osis_notification/management/commands/send_email_notifications.py index 789da83..25918a1 100644 --- a/osis_notification/management/commands/send_email_notifications.py +++ b/osis_notification/management/commands/send_email_notifications.py @@ -8,5 +8,13 @@ class Command(BaseCommand): help = "Send all the email notifications." def handle(self, *args, **options): + errors = [] + for notification in EmailNotification.objects.pending(): - EmailNotificationHandler.process(notification) + try: + EmailNotificationHandler.process(notification) + except Exception as e: + errors.append(e) + + if errors: + raise errors[0] diff --git a/osis_notification/tests/test_commands.py b/osis_notification/tests/test_commands.py index fb19cc1..5d6e254 100644 --- a/osis_notification/tests/test_commands.py +++ b/osis_notification/tests/test_commands.py @@ -1,8 +1,9 @@ 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 @@ -10,7 +11,7 @@ from osis_notification.contrib.notification import ( EmailNotification as EmailNotificationType, ) -from osis_notification.models import EmailNotification, WebNotification +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 ( @@ -18,11 +19,12 @@ 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,7 +35,7 @@ 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"], ) @@ -41,27 +43,50 @@ def setUpTestData(cls): 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, ) + original_send_mail = Notification.save + with patch.object(Notification, 'save') as sender_mock: + sender_mock.side_effect = [original_send_mail, ValueError('Invalid value')] + + with self.assertRaises(ValueError): + 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) + 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, ) diff --git a/setup.py b/setup.py index 8fe5609..8fa35a1 100644 --- a/setup.py +++ b/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',