From 31ebd6f6fe785b12ee5a2d07a865331b938c821c Mon Sep 17 00:00:00 2001 From: Talha Date: Wed, 7 Oct 2026 18:53:07 +0500 Subject: [PATCH] fix: include Administrator when alerting System Managers in the desk --- frappe_restic/restic_backup/backup_control.py | 9 +++++---- frappe_restic/restic_backup/test_backup_control.py | 8 ++++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/frappe_restic/restic_backup/backup_control.py b/frappe_restic/restic_backup/backup_control.py index 2946b3f..bc818c5 100644 --- a/frappe_restic/restic_backup/backup_control.py +++ b/frappe_restic/restic_backup/backup_control.py @@ -17,7 +17,6 @@ from frappe.utils import cint, get_time, now_datetime from frappe.utils.background_jobs import enqueue from frappe.utils.backups import BackupGenerator, get_backup_path -from frappe.utils.user import get_users_with_role from redis.exceptions import LockError from frappe_restic.config import namespace @@ -475,10 +474,12 @@ def _send_alert(subject: str, message: str, *, succeeded: bool = False, run_name if succeeded and not _settings_int("email_on_success", 0): return "Not requested" try: + # Frappe's own role lookups leave out Administrator, the only manager on a small site. + names = frappe.get_all( + "Has Role", filters={"role": SYSTEM_MANAGER_ROLE, "parenttype": "User"}, pluck="parent" + ) managers = frappe.get_all( - "User", - filters={"name": ["in", get_users_with_role(SYSTEM_MANAGER_ROLE)]}, - fields=["name", "email"], + "User", filters={"name": ["in", names], "enabled": 1}, fields=["name", "email"] ) except Exception: # notification failure must not replace the operation outcome frappe.logger("restic_backup").exception("Could not find the System Managers to notify") diff --git a/frappe_restic/restic_backup/test_backup_control.py b/frappe_restic/restic_backup/test_backup_control.py index 385a5a3..1f55d51 100644 --- a/frappe_restic/restic_backup/test_backup_control.py +++ b/frappe_restic/restic_backup/test_backup_control.py @@ -170,8 +170,7 @@ def _alert(self, *, succeeded: bool, email_on_success: int = 0, desk=None, email ] with ( patch.object(control, "_settings_int", return_value=email_on_success), - patch.object(control, "get_users_with_role", return_value=["Administrator", "ops@example.com"]), - patch("frappe.get_all", return_value=managers), + patch("frappe.get_all", side_effect=[["Administrator", "ops@example.com"], managers]) as get_all, patch("frappe.logger"), patch.object(control, "enqueue_create_notification", side_effect=desk) as notify, patch("frappe.sendmail", side_effect=email) as sendmail, @@ -179,6 +178,11 @@ def _alert(self, *, succeeded: bool, email_on_success: int = 0, desk=None, email status = control._send_alert( "FAILED: backup", "disk full", succeeded=succeeded, run_name="run123" ) + if get_all.call_count: + self.assertEqual( + get_all.call_args.kwargs["filters"], + {"name": ["in", ["Administrator", "ops@example.com"]], "enabled": 1}, + ) return status, notify, sendmail def test_failure_reaches_every_system_manager_in_the_desk_and_by_email(self) -> None: