From 757f0d1ce0e5790679bb14ced2946b4956396527 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Wed, 20 Nov 2024 10:14:14 -0700 Subject: [PATCH] fix: remove notification failure policy log record when prevent posting is set (#5260) # What this PR does Changes UserNotificationPolicyLogRecord to success when slack_prevent_posting is set as the user has already been notified in slack or another method in their personal notification preferences. These entries have also been filtered out of the alert group history timeline as they were causing confusion to users thinking notifications failed when in fact they had already been sent. ## Which issue(s) this PR closes https://github.com/grafana/support-escalations/issues/13236 ## 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. --- .../alerts/incident_log_builder/incident_log_builder.py | 9 +++++++++ engine/apps/alerts/tasks/notify_user.py | 2 +- engine/apps/alerts/tests/test_notify_user.py | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/engine/apps/alerts/incident_log_builder/incident_log_builder.py b/engine/apps/alerts/incident_log_builder/incident_log_builder.py index 9fb1f002..d291d4fa 100644 --- a/engine/apps/alerts/incident_log_builder/incident_log_builder.py +++ b/engine/apps/alerts/incident_log_builder/incident_log_builder.py @@ -152,6 +152,15 @@ class IncidentLogBuilder: Q(type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_TRIGGERED) & Q(notification_policy__step=UserNotificationPolicy.Step.WAIT) ) + # Exclude SUCCESS + ERROR_NOTIFICATION_POSTING_TO_SLACK_IS_DISABLED, these cause confusions as the user + # has already been notified by another path so this step should not be displayed, although it is kept + # for auditing. + | Q( + Q(type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS) + & Q( + notification_error_code=UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_POSTING_TO_SLACK_IS_DISABLED + ) + ) ) .select_related("author") .distinct() diff --git a/engine/apps/alerts/tasks/notify_user.py b/engine/apps/alerts/tasks/notify_user.py index 88f46f6e..2cc8b89e 100644 --- a/engine/apps/alerts/tasks/notify_user.py +++ b/engine/apps/alerts/tasks/notify_user.py @@ -528,7 +528,7 @@ def perform_notification(log_record_pk, use_default_notification_policy_fallback ) UserNotificationPolicyLogRecord( author=user, - type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED, + type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS, notification_policy=notification_policy, reason="Prevented from posting in Slack", alert_group=alert_group, diff --git a/engine/apps/alerts/tests/test_notify_user.py b/engine/apps/alerts/tests/test_notify_user.py index a24003df..2b885ced 100644 --- a/engine/apps/alerts/tests/test_notify_user.py +++ b/engine/apps/alerts/tests/test_notify_user.py @@ -315,7 +315,7 @@ def test_perform_notification_slack_prevent_posting( mocked_send_slack_notification.assert_not_called() last_log_record = UserNotificationPolicyLogRecord.objects.last() - assert last_log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED + assert last_log_record.type == UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS assert last_log_record.reason == "Prevented from posting in Slack" assert ( last_log_record.notification_error_code