From 37314bd1543c7735ff02124ca1a60dc6d052a14a Mon Sep 17 00:00:00 2001 From: Graham McIntie Date: Fri, 13 Mar 2026 15:26:25 -0500 Subject: [PATCH] Add alert routing setting (builtin/pagerduty/both) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - New 'alert_routing' field on organizations (default: builtin) - AlertNotificationWorker respects the setting: - builtin: only built-in escalation policies - pagerduty: only PagerDuty events - both: fire both (useful during migration) - Radio button UI in Settings → General with visual cards - Warning shown when PagerDuty selected but not configured - Migration, schema validation, 6 tests --- lib/towerops/organizations/organization.ex | 9 +- .../workers/alert_notification_worker.ex | 87 ++++++++----- lib/towerops_web/live/org/settings_live.ex | 5 + .../live/org/settings_live.html.heex | 114 ++++++++++++++++++ ...705_add_alert_routing_to_organizations.exs | 9 ++ .../alert_notification_worker_test.exs | 47 ++++++++ 6 files changed, 242 insertions(+), 29 deletions(-) create mode 100644 priv/repo/migrations/20260313194705_add_alert_routing_to_organizations.exs create mode 100644 test/towerops/workers/alert_notification_worker_test.exs diff --git a/lib/towerops/organizations/organization.ex b/lib/towerops/organizations/organization.ex index db842256..1fd8f3c7 100644 --- a/lib/towerops/organizations/organization.ex +++ b/lib/towerops/organizations/organization.ex @@ -27,6 +27,9 @@ defmodule Towerops.Organizations.Organization do field :subscription_plan, :string, default: "free" field :use_sites, :boolean, default: false + # Alert routing: "builtin" (schedules/escalation), "pagerduty", or "both" + field :alert_routing, :string, default: "builtin" + # device) field :snmp_version, :string, default: "2c" field :snmp_community, :string @@ -151,11 +154,15 @@ defmodule Towerops.Organizations.Organization do :payment_method_status, :last_billing_sync_at, :last_synced_device_count, - :default_escalation_policy_id + :default_escalation_policy_id, + :alert_routing ]) |> validate_required([:name]) |> validate_length(:name, min: 2, max: 100) |> validate_inclusion(:subscription_plan, ["free", "paid"]) + |> validate_inclusion(:alert_routing, ["builtin", "pagerduty", "both"], + message: "must be builtin, pagerduty, or both" + ) |> validate_inclusion(:snmp_version, ["1", "2c", "3"], message: "must be 1, 2c, or 3") |> validate_number(:snmp_port, greater_than: 0, less_than: 65_536) |> validate_snmpv3_fields() diff --git a/lib/towerops/workers/alert_notification_worker.ex b/lib/towerops/workers/alert_notification_worker.ex index 4df611b5..62c9b4de 100644 --- a/lib/towerops/workers/alert_notification_worker.ex +++ b/lib/towerops/workers/alert_notification_worker.ex @@ -21,9 +21,27 @@ defmodule Towerops.Workers.AlertNotificationWorker do def perform(%Oban.Job{args: %{"action" => "trigger", "alert_id" => alert_id}}) do with {:ok, alert} <- fetch_alert(alert_id), {:ok, device} <- fetch_device(alert.device_id) do - pagerduty_result = Notifier.notify_trigger(alert, device) - maybe_start_escalation(alert, device) - pagerduty_result + routing = alert_routing(device) + + pd_result = + if routing in ["pagerduty", "both"] do + case Notifier.notify_trigger(alert, device) do + {:error, :not_configured} -> + Logger.debug("PagerDuty not configured for alert #{alert_id}, skipping") + :ok + + result -> + result + end + else + :ok + end + + if routing in ["builtin", "both"] do + maybe_start_escalation(alert, device) + end + + pd_result else {:error, :alert_not_found} -> Logger.warning("Alert #{alert_id} not found for notification") @@ -33,17 +51,6 @@ defmodule Towerops.Workers.AlertNotificationWorker do Logger.warning("Device not found for alert #{alert_id}") :ok - {:error, :not_configured} -> - Logger.debug("PagerDuty not configured for alert #{alert_id}, skipping notification") - - # Still try built-in escalation even if PagerDuty isn't configured - with {:ok, alert} <- fetch_alert(alert_id), - {:ok, device} <- fetch_device(alert.device_id) do - maybe_start_escalation(alert, device) - end - - :ok - {:error, reason} -> Logger.error("Failed to send trigger notification for alert #{alert_id}: #{inspect(reason)}") {:error, reason} @@ -53,15 +60,21 @@ defmodule Towerops.Workers.AlertNotificationWorker do def perform(%Oban.Job{args: %{"action" => "acknowledge", "alert_id" => alert_id}}) do case fetch_alert(alert_id) do {:ok, alert} -> - maybe_acknowledge_incident(alert) + routing = alert_routing_for_alert(alert) - case Notifier.notify_acknowledge(alert) do - {:error, :not_configured} -> - Logger.debug("PagerDuty not configured for alert #{alert_id}, skipping notification") - :ok + if routing in ["builtin", "both"], do: maybe_acknowledge_incident(alert) - result -> - result + if routing in ["pagerduty", "both"] do + case Notifier.notify_acknowledge(alert) do + {:error, :not_configured} -> + Logger.debug("PagerDuty not configured for alert #{alert_id}, skipping") + :ok + + result -> + result + end + else + :ok end {:error, :alert_not_found} -> @@ -77,15 +90,21 @@ defmodule Towerops.Workers.AlertNotificationWorker do def perform(%Oban.Job{args: %{"action" => "resolve", "alert_id" => alert_id}}) do case fetch_alert(alert_id) do {:ok, alert} -> - maybe_resolve_incident(alert) + routing = alert_routing_for_alert(alert) - case Notifier.notify_resolve(alert) do - {:error, :not_configured} -> - Logger.debug("PagerDuty not configured for alert #{alert_id}, skipping notification") - :ok + if routing in ["builtin", "both"], do: maybe_resolve_incident(alert) - result -> - result + if routing in ["pagerduty", "both"] do + case Notifier.notify_resolve(alert) do + {:error, :not_configured} -> + Logger.debug("PagerDuty not configured for alert #{alert_id}, skipping") + :ok + + result -> + result + end + else + :ok end {:error, :alert_not_found} -> @@ -157,6 +176,18 @@ defmodule Towerops.Workers.AlertNotificationWorker do end end + # Get alert routing from the device's organization (preloaded) + defp alert_routing(%{organization: %{alert_routing: routing}}) when is_binary(routing), do: routing + defp alert_routing(_device), do: "builtin" + + # Get alert routing for acknowledge/resolve (need to look up from alert's device) + defp alert_routing_for_alert(alert) do + case fetch_device(alert.device_id) do + {:ok, device} -> alert_routing(device) + _ -> "builtin" + end + end + defp fetch_alert(alert_id) do case Alerts.get_alert(alert_id) do nil -> {:error, :alert_not_found} diff --git a/lib/towerops_web/live/org/settings_live.ex b/lib/towerops_web/live/org/settings_live.ex index 09aa8ea2..0ce813e9 100644 --- a/lib/towerops_web/live/org/settings_live.ex +++ b/lib/towerops_web/live/org/settings_live.ex @@ -197,6 +197,11 @@ defmodule ToweropsWeb.Org.SettingsLive do {:noreply, socket} end + defp maybe_load_integrations(socket, "general") do + integrations = load_integrations(socket.assigns.organization.id) + assign(socket, :integrations, integrations) + end + defp maybe_load_integrations(socket, "members") do org_id = socket.assigns.organization.id diff --git a/lib/towerops_web/live/org/settings_live.html.heex b/lib/towerops_web/live/org/settings_live.html.heex index 5c384397..b430bd91 100644 --- a/lib/towerops_web/live/org/settings_live.html.heex +++ b/lib/towerops_web/live/org/settings_live.html.heex @@ -227,6 +227,120 @@ <% end %> + + +
+
+

+ {t("Alert Routing")} +

+

+ {t("Choose how alerts are routed for on-call notifications.")} +

+
+ +
+
+ {t("Alert routing method")} +
+ <% routing_value = to_string(@form[:alert_routing].value || "builtin") %> + + + + + + + + +
+
+
+
<% end %> <%= if @active_tab == "snmp" do %> diff --git a/priv/repo/migrations/20260313194705_add_alert_routing_to_organizations.exs b/priv/repo/migrations/20260313194705_add_alert_routing_to_organizations.exs new file mode 100644 index 00000000..a55e5e8d --- /dev/null +++ b/priv/repo/migrations/20260313194705_add_alert_routing_to_organizations.exs @@ -0,0 +1,9 @@ +defmodule Towerops.Repo.Migrations.AddAlertRoutingToOrganizations do + use Ecto.Migration + + def change do + alter table(:organizations) do + add :alert_routing, :string, default: "builtin", null: false + end + end +end diff --git a/test/towerops/workers/alert_notification_worker_test.exs b/test/towerops/workers/alert_notification_worker_test.exs new file mode 100644 index 00000000..8ab3182a --- /dev/null +++ b/test/towerops/workers/alert_notification_worker_test.exs @@ -0,0 +1,47 @@ +defmodule Towerops.Workers.AlertNotificationWorkerTest do + use Towerops.DataCase, async: false + + alias Towerops.Organizations + + setup do + user = Towerops.AccountsFixtures.user_fixture() + + {:ok, organization} = + Organizations.create_organization(%{name: "Test Org"}, user.id) + + %{organization: organization, user: user} + end + + describe "alert_routing organization setting" do + test "defaults to builtin routing", %{organization: org} do + assert org.alert_routing == "builtin" + end + + test "can be set to pagerduty", %{organization: org} do + {:ok, updated} = Organizations.update_organization(org, %{alert_routing: "pagerduty"}) + assert updated.alert_routing == "pagerduty" + end + + test "can be set to both", %{organization: org} do + {:ok, updated} = Organizations.update_organization(org, %{alert_routing: "both"}) + assert updated.alert_routing == "both" + end + + test "can be set back to builtin", %{organization: org} do + {:ok, pd} = Organizations.update_organization(org, %{alert_routing: "pagerduty"}) + {:ok, builtin} = Organizations.update_organization(pd, %{alert_routing: "builtin"}) + assert builtin.alert_routing == "builtin" + end + + test "rejects invalid routing values", %{organization: org} do + {:error, changeset} = Organizations.update_organization(org, %{alert_routing: "invalid"}) + assert {"must be builtin, pagerduty, or both", _} = changeset.errors[:alert_routing] + end + + test "persists through reload", %{organization: org} do + {:ok, _} = Organizations.update_organization(org, %{alert_routing: "pagerduty"}) + reloaded = Towerops.Repo.get!(Towerops.Organizations.Organization, org.id) + assert reloaded.alert_routing == "pagerduty" + end + end +end