From 7777a6003a13b0a7ffe398863a302c20865c72c9 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 18 Jul 2024 11:15:48 -0300 Subject: [PATCH] Handle multiple unpage event logs when getting paged users (#4698) Related to https://github.com/grafana/oncall-private/issues/2816 --- engine/apps/alerts/models/alert_group.py | 6 ++++-- engine/apps/alerts/tests/test_alert_group.py | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 8eea0c0e..3429361c 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -554,7 +554,7 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. log_records = self.log_records.filter( type__in=(AlertGroupLogRecord.TYPE_DIRECT_PAGING, AlertGroupLogRecord.TYPE_UNPAGE_USER) - ) + ).order_by("created_at") for log_record in log_records: # filter paging events, track still active escalations @@ -592,7 +592,9 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models. } else: # user was unpaged at some point, remove them - del users[user_id] + # there could be multiple unpage log records if API was hit several times + if user_id in users: + del users[user_id] return list(users.values()) diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index caecaa8e..ae2aecee 100644 --- a/engine/apps/alerts/tests/test_alert_group.py +++ b/engine/apps/alerts/tests/test_alert_group.py @@ -589,6 +589,24 @@ def test_alert_group_get_paged_users( assert len(paged_users) == 1 assert alert_group.get_paged_users()[0]["pk"] == user.public_primary_key + # user was paged and then paged again, then unpaged - they should not show up + alert_group = make_alert_group(alert_receive_channel) + _make_log_record(alert_group, user, AlertGroupLogRecord.TYPE_DIRECT_PAGING) + _make_log_record(alert_group, user, AlertGroupLogRecord.TYPE_DIRECT_PAGING) + _make_log_record(alert_group, user, AlertGroupLogRecord.TYPE_UNPAGE_USER) + + paged_users = alert_group.get_paged_users() + assert len(paged_users) == 0 + + # adding extra unpage events should not break things + _make_log_record(alert_group, user, AlertGroupLogRecord.TYPE_UNPAGE_USER) + _make_log_record(alert_group, user, AlertGroupLogRecord.TYPE_UNPAGE_USER) + _make_log_record(alert_group, user, AlertGroupLogRecord.TYPE_DIRECT_PAGING) + + paged_users = alert_group.get_paged_users() + assert len(paged_users) == 1 + assert alert_group.get_paged_users()[0]["pk"] == user.public_primary_key + @patch("apps.alerts.models.AlertGroup.start_unsilence_task", return_value=None) @pytest.mark.django_db