diff --git a/src/api/management/src/request/oncall/mod.rs b/src/api/management/src/request/oncall/mod.rs index 6e8543f908..d108b623b5 100644 --- a/src/api/management/src/request/oncall/mod.rs +++ b/src/api/management/src/request/oncall/mod.rs @@ -1674,29 +1674,22 @@ pub async fn set_policy( other => internal_error("set_policy node versions", &other), }; } - if let Some(template) = body.template.clone() { - let template = template - .map(|t| t.trim().to_string()) - .filter(|t| !t.is_empty()); - if let Some(name) = template.as_deref() - && let Err(e) = openobserve_core::alerts::templates::check_binding( - &org_id, - name, - config::meta::alerts::notification_kind::NotificationKind::OnCall, - ) - .await - { - return MetaHttpResponse::bad_request(e.as_str()); - } - if let Err(e) = o2_enterprise::enterprise::oncall::service::set_policy_template( - &org_id, &team_id, template, + // Checked before any write, written after the policy, so a refused policy leaves no half. + let template = body + .template + .clone() + .map(|t| t.map(|t| t.trim().to_string()).filter(|t| !t.is_empty())); + if let Some(Some(name)) = template.as_ref() + && let Err(e) = openobserve_core::alerts::templates::check_binding( + &org_id, + name, + config::meta::alerts::notification_kind::NotificationKind::OnCall, ) .await - { - return to_response(e); - } + { + return MetaHttpResponse::bad_request(e.as_str()); } - match o2_enterprise::enterprise::oncall::service::set_policy( + let policy = match o2_enterprise::enterprise::oncall::service::set_policy( &org_id, &team_id, body.rungs, @@ -1704,6 +1697,17 @@ pub async fn set_policy( body.l0, ) .await + { + Ok(policy) => policy, + Err(e) => return to_response(e), + }; + let Some(template) = template else { + return MetaHttpResponse::json(policy); + }; + match o2_enterprise::enterprise::oncall::service::set_policy_template( + &org_id, &team_id, template, + ) + .await { Ok(policy) => MetaHttpResponse::json(policy), Err(e) => to_response(e), diff --git a/src/core/src/alerts/notifications/oncall.rs b/src/core/src/alerts/notifications/oncall.rs index 8414beaff8..812dd7d368 100644 --- a/src/core/src/alerts/notifications/oncall.rs +++ b/src/core/src/alerts/notifications/oncall.rs @@ -20,7 +20,7 @@ use o2_enterprise::enterprise::oncall::sender::{OnCallNotification, OnCallSendEr use super::{ LegacyPayload, NotificationContext, STATUS_FIRING, - notify::{NotifyError, SkipReason, notify}, + notify::{NotifyError, notify}, }; /// Sends one on-call message to one destination through the shared path. @@ -44,7 +44,16 @@ pub async fn log_mail_destination_policies() { }; for policy in policies { let mut mail = Vec::new(); - for name in &policy.destinations { + // The team's own channel pages too, so its destinations count as well (D9). + let channel = infra::table::oncall_teams::get_channel(&policy.org_id, &policy.team_id) + .await + .ok() + .flatten() + .unwrap_or_default(); + let mut names: Vec<&String> = policy.destinations.iter().chain(channel.iter()).collect(); + names.sort(); + names.dedup(); + for name in names { if let Ok(Some(dest)) = infra::table::destinations::get(&policy.org_id, name).await && let Module::Alert { destination_type: DestinationType::Email(_) | DestinationType::Sns(_), @@ -119,7 +128,8 @@ pub fn on_call_context(n: &OnCallNotification) -> NotificationContext { /// A Retryable error is retried by the engine; a platform skip is neither delivered nor failed. fn send_result(result: Result<(), NotifyError>) -> Result<(), OnCallSendError> { match result { - Ok(()) | Err(NotifyError::Skipped(SkipReason::PlatformNotForKind)) => Ok(()), + Ok(()) => Ok(()), + // A skip is not a delivery, so a send that only reached platforms reports nothing sent. Err(NotifyError::Skipped(reason)) => Err(OnCallSendError::Skipped(reason.as_str().into())), Err(e) if e.is_retryable() => Err(OnCallSendError::Retryable(e.to_string())), Err(e) => Err(OnCallSendError::Permanent(e.to_string())), @@ -132,7 +142,7 @@ mod tests { use super::*; use crate::alerts::notifications::{ - notify::{Route, route}, + notify::{Route, SkipReason, route}, org_default::{EffectiveTemplate, TemplateSource, compiled_fallback_for}, }; @@ -245,10 +255,10 @@ mod tests { #[test] fn send_results_map_to_what_the_engine_retries() { assert_eq!(send_result(Ok(())), Ok(())); - assert_eq!( + assert!(matches!( send_result(Err(NotifyError::Skipped(SkipReason::PlatformNotForKind))), - Ok(()) - ); + Err(OnCallSendError::Skipped(_)) + )); assert!(matches!( send_result(Err(NotifyError::Skipped(SkipReason::NoOrgMemberRecipient))), Err(OnCallSendError::Skipped(_))