diff --git a/CHANGELOG.md b/CHANGELOG.md index b521acd26f..68fcba0d40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix schedules invalid dates issue ([#support-escalations/issues/8084](https://github.com/grafana/support-escalations/issues/8084)) +- Fix issue related to updating alert group metrics when deleting an alert group via the public API by @joeyorlando ([#3544](https://github.com/grafana/oncall/pull/3544)) ## v1.3.76 (2023-12-11) diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 5a891a44c4..54e86764cb 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -1191,8 +1191,6 @@ def wipe_by_user(self, user: User) -> None: def delete_by_user(self, user: User): from apps.alerts.models import AlertGroupLogRecord - initial_state = self.state - self.stop_escalation() # prevent creating multiple logs # filter instead of get_or_create cause it can be multiple logs of this type due deleting error @@ -1222,10 +1220,9 @@ def delete_by_user(self, user: User): dependent_alerts = list(self.dependent_alert_groups.all()) self.hard_delete() - # Update alert group state metric cache - self._update_metrics(organization_id=user.organization_id, previous_state=initial_state, state=None) - for dependent_alert_group in dependent_alerts: # unattach dependent incidents + # unattach dependent incidents + for dependent_alert_group in dependent_alerts: dependent_alert_group.un_attach_by_delete() def hard_delete(self): diff --git a/engine/apps/alerts/tasks/delete_alert_group.py b/engine/apps/alerts/tasks/delete_alert_group.py index f8a1d2ae6a..e97ce515fc 100644 --- a/engine/apps/alerts/tasks/delete_alert_group.py +++ b/engine/apps/alerts/tasks/delete_alert_group.py @@ -24,6 +24,8 @@ def delete_alert_group(alert_group_pk, user_pk): logger.debug("User not found, skipping delete_alert_group") return + logger.debug(f"User {user} is deleting alert group {alert_group} (channel: {alert_group.channel})") + try: alert_group.delete_by_user(user) except SlackAPIRatelimitError as e: diff --git a/engine/apps/alerts/tests/test_alert_group.py b/engine/apps/alerts/tests/test_alert_group.py index 819b6af918..d2ba04d2d2 100644 --- a/engine/apps/alerts/tests/test_alert_group.py +++ b/engine/apps/alerts/tests/test_alert_group.py @@ -577,3 +577,47 @@ def test_filter_active_alert_groups( assert active_alert_groups.count() == 2 assert alert_group_active in active_alert_groups assert alert_group_active_silenced in active_alert_groups + + +@patch("apps.alerts.models.AlertGroup.hard_delete") +@patch("apps.alerts.models.AlertGroup.un_attach_by_delete") +@patch("apps.alerts.models.AlertGroup.stop_escalation") +@patch("apps.alerts.models.alert_group.alert_group_action_triggered_signal") +@pytest.mark.django_db +def test_delete_by_user( + mock_alert_group_action_triggered_signal, + _mock_stop_escalation, + _mock_un_attach_by_delete, + _mock_hard_delete, + make_organization_and_user, + make_alert_receive_channel, + make_alert_group, +): + organization, user = make_organization_and_user() + alert_receive_channel = make_alert_receive_channel(organization) + + alert_group = make_alert_group(alert_receive_channel) + + # make a few dependent alert groups + dependent_alert_groups = [make_alert_group(alert_receive_channel, root_alert_group=alert_group) for _ in range(3)] + + assert alert_group.log_records.filter(type=AlertGroupLogRecord.TYPE_DELETED).count() == 0 + + alert_group.delete_by_user(user) + + assert alert_group.log_records.filter(type=AlertGroupLogRecord.TYPE_DELETED).count() == 1 + deleted_log_record = alert_group.log_records.get(type=AlertGroupLogRecord.TYPE_DELETED) + + alert_group.stop_escalation.assert_called_once_with() + + mock_alert_group_action_triggered_signal.send.assert_called_once_with( + sender=alert_group.delete_by_user, + log_record=deleted_log_record.pk, + action_source=None, + force_sync=True, + ) + + alert_group.hard_delete.assert_called_once_with() + + for dependent_alert_group in dependent_alert_groups: + dependent_alert_group.un_attach_by_delete.assert_called_with()