Fix acknowledge reminder (#3345)
# What this PR does Fix acknowledge reminder: - check if organization was deleted - improve logging ## Which issue(s) this PR fixes ## 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] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required)
This commit is contained in:
Yulya Artyukhina
GitHub
parent
e8bd71bbab
commit
d7d5c3aa28
3 changed files with 73 additions and 16 deletions
|
|
@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
- Forward headers for Amazon SNS when organizations are moved @mderynck ([#3326](https://github.com/grafana/oncall/pull/3326))
|
||||
- Fix styling when light theme is turned on via system preferences
|
||||
by excluding dark theme css vars in this case ([#3336](https://github.com/grafana/oncall/pull/3336))
|
||||
- Fix issue when acknowledge reminder works for deleted organizations @Ferril ([#3345](https://github.com/grafana/oncall/pull/3345))
|
||||
|
||||
## v1.3.57 (2023-11-10)
|
||||
|
||||
|
|
|
|||
|
|
@ -26,13 +26,11 @@ def acknowledge_reminder_task(alert_group_pk: int, unacknowledge_process_id: str
|
|||
if unacknowledge_process_id != alert_group.last_unique_unacknowledge_process_id:
|
||||
return
|
||||
|
||||
organization = alert_group.channel.organization
|
||||
|
||||
# Get timeout values
|
||||
acknowledge_reminder_timeout = Organization.ACKNOWLEDGE_REMIND_DELAY[
|
||||
alert_group.channel.organization.acknowledge_remind_timeout
|
||||
]
|
||||
unacknowledge_timeout = Organization.UNACKNOWLEDGE_TIMEOUT_DELAY[
|
||||
alert_group.channel.organization.unacknowledge_timeout
|
||||
]
|
||||
acknowledge_reminder_timeout = Organization.ACKNOWLEDGE_REMIND_DELAY[organization.acknowledge_remind_timeout]
|
||||
unacknowledge_timeout = Organization.UNACKNOWLEDGE_TIMEOUT_DELAY[organization.unacknowledge_timeout]
|
||||
|
||||
# Don't proceed if the alert group is not in a state for acknowledgement reminder
|
||||
acknowledge_reminder_required = (
|
||||
|
|
@ -41,10 +39,18 @@ def acknowledge_reminder_task(alert_group_pk: int, unacknowledge_process_id: str
|
|||
and alert_group.acknowledged_by == AlertGroup.USER
|
||||
and acknowledge_reminder_timeout
|
||||
)
|
||||
if not acknowledge_reminder_required:
|
||||
task_logger.info("AlertGroup is not in a state for acknowledgement reminder")
|
||||
is_organization_deleted = organization.deleted_at is not None
|
||||
log_info = (
|
||||
f"acknowledge_reminder_timeout option: {acknowledge_reminder_timeout},"
|
||||
f"organization ppk: {organization.public_primary_key},"
|
||||
f"organization is deleted: {is_organization_deleted}"
|
||||
)
|
||||
if not acknowledge_reminder_required or is_organization_deleted:
|
||||
task_logger.info(f"alert group {alert_group_pk} is not in a state for acknowledgement reminder. {log_info}")
|
||||
return
|
||||
|
||||
task_logger.info(f"alert group {alert_group_pk} is in a state for acknowledgement reminder. {log_info}")
|
||||
|
||||
# unacknowledge_timeout_task uses acknowledged_by_confirmed to check if acknowledgement reminder has been confirmed
|
||||
# by the user. Setting to None here to indicate that the user has not confirmed the acknowledgement reminder
|
||||
alert_group.acknowledged_by_confirmed = None
|
||||
|
|
@ -80,13 +86,11 @@ def unacknowledge_timeout_task(alert_group_pk: int, unacknowledge_process_id: st
|
|||
if unacknowledge_process_id != alert_group.last_unique_unacknowledge_process_id:
|
||||
return
|
||||
|
||||
organization = alert_group.channel.organization
|
||||
|
||||
# Get timeout values
|
||||
acknowledge_reminder_timeout = Organization.ACKNOWLEDGE_REMIND_DELAY[
|
||||
alert_group.channel.organization.acknowledge_remind_timeout
|
||||
]
|
||||
unacknowledge_timeout = Organization.UNACKNOWLEDGE_TIMEOUT_DELAY[
|
||||
alert_group.channel.organization.unacknowledge_timeout
|
||||
]
|
||||
acknowledge_reminder_timeout = Organization.ACKNOWLEDGE_REMIND_DELAY[organization.acknowledge_remind_timeout]
|
||||
unacknowledge_timeout = Organization.UNACKNOWLEDGE_TIMEOUT_DELAY[organization.unacknowledge_timeout]
|
||||
|
||||
# Don't proceed if the alert group is not in a state for auto-unacknowledge
|
||||
unacknowledge_required = (
|
||||
|
|
@ -96,16 +100,28 @@ def unacknowledge_timeout_task(alert_group_pk: int, unacknowledge_process_id: st
|
|||
and acknowledge_reminder_timeout
|
||||
and unacknowledge_timeout
|
||||
)
|
||||
if not unacknowledge_required:
|
||||
task_logger.info("AlertGroup is not in a state for unacknowledge")
|
||||
is_organization_deleted = organization.deleted_at is not None
|
||||
log_info = (
|
||||
f"acknowledge_reminder_timeout option: {acknowledge_reminder_timeout},"
|
||||
f"unacknowledge_timeout option: {unacknowledge_timeout},"
|
||||
f"organization ppk: {organization.public_primary_key},"
|
||||
f"organization is deleted: {is_organization_deleted}"
|
||||
)
|
||||
if not unacknowledge_required or is_organization_deleted:
|
||||
task_logger.info(f"alert group {alert_group_pk} is not in a state for unacknowledge by timeout. {log_info}")
|
||||
return
|
||||
|
||||
if alert_group.acknowledged_by_confirmed: # acknowledgement reminder was confirmed by the user
|
||||
acknowledge_reminder_task.apply_async(
|
||||
(alert_group_pk, unacknowledge_process_id), countdown=acknowledge_reminder_timeout - unacknowledge_timeout
|
||||
)
|
||||
task_logger.info(
|
||||
f"Acknowledgement reminder was confirmed by user. Rescheduling acknowledge_reminder_task..."
|
||||
f"alert group: {alert_group_pk}, {log_info}"
|
||||
)
|
||||
return
|
||||
|
||||
task_logger.info(f"alert group {alert_group_pk} is in a state for unacknowledge by timeout. {log_info}")
|
||||
# If acknowledgement reminder wasn't confirmed by the user, unacknowledge the alert group and start escalation again
|
||||
log_record = alert_group.log_records.create(
|
||||
type=AlertGroupLogRecord.TYPE_AUTO_UN_ACK, author=alert_group.acknowledged_by_user
|
||||
|
|
|
|||
|
|
@ -299,3 +299,43 @@ def test_unacknowledge_timeout_task_no_unacknowledge(
|
|||
)
|
||||
|
||||
assert not alert_group.log_records.exists()
|
||||
|
||||
|
||||
@patch.object(acknowledge_reminder_task, "apply_async")
|
||||
@patch.object(unacknowledge_timeout_task, "apply_async")
|
||||
@pytest.mark.django_db
|
||||
def test_ack_reminder_skip_deleted_org(
|
||||
mock_acknowledge_reminder_task,
|
||||
mock_unacknowledge_timeout_task,
|
||||
ack_reminder_test_setup,
|
||||
):
|
||||
organization, alert_group, user = ack_reminder_test_setup()
|
||||
organization.deleted_at = timezone.now()
|
||||
organization.save()
|
||||
|
||||
acknowledge_reminder_task(alert_group.pk, TASK_ID)
|
||||
|
||||
mock_unacknowledge_timeout_task.assert_not_called()
|
||||
mock_acknowledge_reminder_task.assert_not_called()
|
||||
|
||||
assert not alert_group.log_records.exists()
|
||||
|
||||
|
||||
@patch.object(acknowledge_reminder_task, "apply_async")
|
||||
@patch.object(unacknowledge_timeout_task, "apply_async")
|
||||
@pytest.mark.django_db
|
||||
def test_unacknowledge_timeout_task_skip_deleted_org(
|
||||
mock_acknowledge_reminder_task,
|
||||
mock_unacknowledge_timeout_task,
|
||||
ack_reminder_test_setup,
|
||||
):
|
||||
organization, alert_group, user = ack_reminder_test_setup()
|
||||
organization.deleted_at = timezone.now()
|
||||
organization.save()
|
||||
|
||||
unacknowledge_timeout_task(alert_group.pk, TASK_ID)
|
||||
|
||||
mock_unacknowledge_timeout_task.assert_not_called()
|
||||
mock_acknowledge_reminder_task.assert_not_called()
|
||||
|
||||
assert not alert_group.log_records.exists()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue