Don't update alert group metrics when deleting an alert group (#3544)
# Which issue(s) this PR fixes Fixes https://github.com/grafana/oncall-private/issues/2376 ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required)
This commit is contained in:
parent
58de3f8458
commit
16ba87bff6
4 changed files with 49 additions and 5 deletions
|
|
@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
- Fix schedules invalid dates issue ([#support-escalations/issues/8084](https://github.com/grafana/support-escalations/issues/8084))
|
- 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)
|
## v1.3.76 (2023-12-11)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1191,8 +1191,6 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models.
|
||||||
def delete_by_user(self, user: User):
|
def delete_by_user(self, user: User):
|
||||||
from apps.alerts.models import AlertGroupLogRecord
|
from apps.alerts.models import AlertGroupLogRecord
|
||||||
|
|
||||||
initial_state = self.state
|
|
||||||
|
|
||||||
self.stop_escalation()
|
self.stop_escalation()
|
||||||
# prevent creating multiple logs
|
# prevent creating multiple logs
|
||||||
# filter instead of get_or_create cause it can be multiple logs of this type due deleting error
|
# filter instead of get_or_create cause it can be multiple logs of this type due deleting error
|
||||||
|
|
@ -1222,10 +1220,9 @@ class AlertGroup(AlertGroupSlackRenderingMixin, EscalationSnapshotMixin, models.
|
||||||
dependent_alerts = list(self.dependent_alert_groups.all())
|
dependent_alerts = list(self.dependent_alert_groups.all())
|
||||||
|
|
||||||
self.hard_delete()
|
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()
|
dependent_alert_group.un_attach_by_delete()
|
||||||
|
|
||||||
def hard_delete(self):
|
def hard_delete(self):
|
||||||
|
|
|
||||||
|
|
@ -24,6 +24,8 @@ def delete_alert_group(alert_group_pk, user_pk):
|
||||||
logger.debug("User not found, skipping delete_alert_group")
|
logger.debug("User not found, skipping delete_alert_group")
|
||||||
return
|
return
|
||||||
|
|
||||||
|
logger.debug(f"User {user} is deleting alert group {alert_group} (channel: {alert_group.channel})")
|
||||||
|
|
||||||
try:
|
try:
|
||||||
alert_group.delete_by_user(user)
|
alert_group.delete_by_user(user)
|
||||||
except SlackAPIRatelimitError as e:
|
except SlackAPIRatelimitError as e:
|
||||||
|
|
|
||||||
|
|
@ -577,3 +577,47 @@ def test_filter_active_alert_groups(
|
||||||
assert active_alert_groups.count() == 2
|
assert active_alert_groups.count() == 2
|
||||||
assert alert_group_active in active_alert_groups
|
assert alert_group_active in active_alert_groups
|
||||||
assert alert_group_active_silenced 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()
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue