From b79c5d0c1c8c10e34019f8cff2ad69f36e5e29bf Mon Sep 17 00:00:00 2001
From: Matias Bordese <mbordese@gmail.com>
Date: Wed, 9 Oct 2024 15:10:39 -0300
Subject: [PATCH] Skip user notification if alert group is already resolved
 (#5145)

Sometimes a task is queued and scheduled for later (or for a retry too),
but the alert group is resolved by then. Skip notification in that case.
---
 engine/apps/alerts/tasks/notify_user.py      | 14 +++++++++
 engine/apps/alerts/tests/test_notify_user.py | 32 ++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/engine/apps/alerts/tasks/notify_user.py b/engine/apps/alerts/tasks/notify_user.py
index 10f62b25..53b88129 100644
--- a/engine/apps/alerts/tasks/notify_user.py
+++ b/engine/apps/alerts/tasks/notify_user.py
@@ -419,6 +419,20 @@ def perform_notification(log_record_pk, use_default_notification_policy_fallback
         ).save()
         return
 
+    if alert_group.resolved:
+        # skip notification if alert group was resolved
+        UserNotificationPolicyLogRecord(
+            author=user,
+            type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED,
+            notification_policy=notification_policy,
+            reason="Skipped notification because alert group is resolved",
+            alert_group=alert_group,
+            notification_step=notification_policy.step if notification_policy else None,
+            notification_channel=notification_channel,
+            notification_error_code=None,
+        ).save()
+        return
+
     if notification_channel == UserNotificationPolicy.NotificationChannel.SMS:
         phone_backend = PhoneBackend()
         phone_backend.notify_by_sms(user, alert_group, notification_policy)
diff --git a/engine/apps/alerts/tests/test_notify_user.py b/engine/apps/alerts/tests/test_notify_user.py
index 5c0d6381..7124f957 100644
--- a/engine/apps/alerts/tests/test_notify_user.py
+++ b/engine/apps/alerts/tests/test_notify_user.py
@@ -186,6 +186,38 @@ def test_notify_user_error_if_viewer(
     assert error_log_record.notification_error_code == UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_FORBIDDEN
 
 
+@pytest.mark.django_db
+def test_notify_user_perform_notification_skip_if_resolved(
+    make_organization,
+    make_user,
+    make_user_notification_policy,
+    make_alert_receive_channel,
+    make_alert_group,
+    make_user_notification_policy_log_record,
+):
+    organization = make_organization()
+    user_1 = make_user(organization=organization, _verified_phone_number="1234567890")
+    user_notification_policy = make_user_notification_policy(
+        user=user_1,
+        step=UserNotificationPolicy.Step.NOTIFY,
+        notify_by=UserNotificationPolicy.NotificationChannel.SMS,
+    )
+    alert_receive_channel = make_alert_receive_channel(organization=organization)
+    alert_group = make_alert_group(alert_receive_channel=alert_receive_channel, resolved=True)
+    log_record = make_user_notification_policy_log_record(
+        author=user_1,
+        alert_group=alert_group,
+        notification_policy=user_notification_policy,
+        type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED,
+    )
+
+    perform_notification(log_record.pk, False)
+
+    error_log_record = UserNotificationPolicyLogRecord.objects.last()
+    assert error_log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED
+    assert error_log_record.reason == "Skipped notification because alert group is resolved"
+
+
 @pytest.mark.django_db
 @pytest.mark.parametrize(
     "reason_to_skip_escalation,error_code",