From 4c1639c944f193717e891b604e9ef3d8d031ddd1 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 1 Aug 2024 08:48:39 -0600 Subject: [PATCH] Fix retrying email notification task (#4772) # What this PR does Email tasks are failing and retrying when they use the fallback default notification policy as it does not get saved in the DB. This PR uses the same way as UserNotificationPolicyLogRecord to set that field to null to avoid `ValueError("save() prohibited to prevent data loss due to unsaved related object 'notification_policy'.").` when that is the case. ## Which issue(s) this PR closes Related to [issue link here] ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/email/models.py | 13 +++++++-- engine/apps/email/tasks.py | 9 ++++-- engine/apps/email/tests/test_notify_user.py | 32 +++++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/engine/apps/email/models.py b/engine/apps/email/models.py index 3c74929722..946113199f 100644 --- a/engine/apps/email/models.py +++ b/engine/apps/email/models.py @@ -1,12 +1,21 @@ -import logging import uuid from django.db import models -logger = logging.getLogger(__name__) + +class EmailMessageQuerySet(models.QuerySet): + def create(self, **kwargs): + from apps.base.models.user_notification_policy_log_record import ( + _check_if_notification_policy_is_transient_fallback, + ) + + _check_if_notification_policy_is_transient_fallback(kwargs) + return super().create(**kwargs) class EmailMessage(models.Model): + objects = EmailMessageQuerySet.as_manager() + message_uuid = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False) exceeded_limit = models.BooleanField(null=True, default=None) diff --git a/engine/apps/email/tasks.py b/engine/apps/email/tasks.py index 8aaff7f3f7..64587eb238 100644 --- a/engine/apps/email/tasks.py +++ b/engine/apps/email/tasks.py @@ -62,6 +62,11 @@ def _create_user_notification_policy_log_record(**kwargs): **kwargs, using_fallback_default_notification_policy_step=using_fallback_default_notification_policy_step ) + def _create_email_message(**kwargs): + return EmailMessage.objects.create( + **kwargs, using_fallback_default_notification_policy_step=using_fallback_default_notification_policy_step + ) + # create an error log in case EMAIL_HOST is not specified if not live_settings.EMAIL_HOST: _create_user_notification_policy_log_record( @@ -88,7 +93,7 @@ def _create_user_notification_policy_log_record(**kwargs): notification_channel=notification_policy.notify_by, notification_error_code=UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_MAIL_LIMIT_EXCEEDED, ) - EmailMessage.objects.create( + _create_email_message( represents_alert_group=alert_group, notification_policy=notification_policy, receiver=user, @@ -115,7 +120,7 @@ def _create_user_notification_policy_log_record(**kwargs): try: send_mail(subject, message, from_email, recipient_list, html_message=html_message, connection=connection) - EmailMessage.objects.create( + _create_email_message( represents_alert_group=alert_group, notification_policy=notification_policy, receiver=user, diff --git a/engine/apps/email/tests/test_notify_user.py b/engine/apps/email/tests/test_notify_user.py index f33cdaaf56..0fd01b67d6 100644 --- a/engine/apps/email/tests/test_notify_user.py +++ b/engine/apps/email/tests/test_notify_user.py @@ -7,6 +7,7 @@ from apps.base.models import UserNotificationPolicy, UserNotificationPolicyLogRecord from apps.email.alert_rendering import build_subject_and_message +from apps.email.models import EmailMessage from apps.email.tasks import get_from_email, notify_user_async from apps.user_management.subscription_strategy.free_public_beta_subscription_strategy import ( FreePublicBetaSubscriptionStrategy, @@ -212,3 +213,34 @@ def test_subject_newlines_removed( subject, _ = build_subject_and_message(alert_group, 1) assert subject == "testnewlines" + + +@pytest.mark.django_db +def test_notify_user_fallback_default_policy( + settings, + make_organization, + make_user_for_organization, + make_token_for_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + make_user_notification_policy, +): + settings.EMAIL_BACKEND = "django.core.mail.backends.locmem.EmailBackend" + settings.EMAIL_HOST = "test" + + organization = make_organization() + user = make_user_for_organization(organization) + + alert_receive_channel = make_alert_receive_channel(organization) + alert_group = make_alert_group(alert_receive_channel) + + make_alert(alert_group=alert_group, raw_request_data=alert_receive_channel.config.example_payload) + + notify_user_async(user.pk, alert_group.pk, None) + assert len(mail.outbox) == 1 + + log_record = UserNotificationPolicyLogRecord.objects.filter(author=user, alert_group=alert_group).first() + assert log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS + + EmailMessage.objects.get(receiver=user, represents_alert_group=alert_group, notification_policy=None)