From d87461ba14008f3481fe8d9eec4b62e9e6660838 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 17 Jan 2026 16:10:59 -0600 Subject: [PATCH] feat: add monitoring mode tab selector (SNMP & ICMP vs ICMP Only) - Add tab selector in Monitoring Configuration section for choosing monitoring mode - SNMP & ICMP mode: enables ICMP pings + SNMP discovery (default) - ICMP Only mode: enables only ICMP ping monitoring, hides SNMP fields - Monitoring mode controls snmp_enabled field automatically on save - Test SNMP Connection button shows informational message when agent is configured (agents handle SNMP testing during their polling cycle) - Update all tests to use tab selector instead of setting snmp_enabled directly - All 818 tests passing with no Credo warnings --- TODOS.md | 2 +- lib/towerops_web/live/device_live/form.ex | 53 ++++++++- .../live/device_live/form.html.heex | 55 +++++++-- .../live/device_live_nested/form_test.exs | 7 +- test/towerops_web/live/device_live_test.exs | 108 +++++++++++++++++- 5 files changed, 210 insertions(+), 15 deletions(-) diff --git a/TODOS.md b/TODOS.md index 203be4f3..38059f66 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1,6 +1,5 @@ # In Progress / Pending -* add and track ICMP pings for each device. it should also put a latency graph on the device page that links to the universal graph like the other graphs * when adding a new device, make it a tab selector to show the default SNMP & ICMP or ICMP only * add valkey as a k8s sidecar * add the latest exq to the web app @@ -13,3 +12,4 @@ * ~~change "revoke" agent to delete and have it actually delete the agent and ensure it's removed from everything it's been assigned and change them to cloud polling if they were previously set to that agent being deleted~~ * ~~add ability to rename an agent in a new edit page~~ * ~~When adding a new device, allow the community string to be empty so it will inherit from the parent~~ +* ~~add and track ICMP pings for each device. it should also put a latency graph on the device page that links to the universal graph like the other graphs~~ diff --git a/lib/towerops_web/live/device_live/form.ex b/lib/towerops_web/live/device_live/form.ex index 821e0d2b..b3d4876b 100644 --- a/lib/towerops_web/live/device_live/form.ex +++ b/lib/towerops_web/live/device_live/form.ex @@ -67,6 +67,7 @@ defmodule ToweropsWeb.DeviceLive.Form do |> assign(:page_title, "New Device") |> assign(:device, %DeviceSchema{}) |> assign(:form, to_form(changeset)) + |> assign(:monitoring_mode, "snmp_and_icmp") end defp apply_action(socket, :edit, %{"id" => id}) do @@ -101,6 +102,9 @@ defmodule ToweropsWeb.DeviceLive.Form do # Get effective SNMP configuration and source snmp_config = Devices.get_snmp_config(device) + # Determine monitoring mode based on current snmp_enabled value + monitoring_mode = if device.snmp_enabled, do: "snmp_and_icmp", else: "icmp_only" + socket |> assign(:page_title, "Edit Device") |> assign(:device, device_with_agent) @@ -110,6 +114,30 @@ defmodule ToweropsWeb.DeviceLive.Form do |> assign(:snmp_config_source, snmp_config.source) |> assign(:effective_snmp_version, snmp_config.version) |> assign(:effective_snmp_community, snmp_config.community || "(not set)") + |> assign(:monitoring_mode, monitoring_mode) + end + + @impl true + def handle_event("switch_monitoring_mode", %{"mode" => mode}, socket) do + # Update snmp_enabled based on the selected mode + snmp_enabled = mode == "snmp_and_icmp" + + # Get current form params + current_params = socket.assigns.form.params + + # Update params with new snmp_enabled value + updated_params = Map.put(current_params, "snmp_enabled", snmp_enabled) + + # Create new changeset with updated params + changeset = + socket.assigns.device + |> Devices.change_device(updated_params) + |> Map.put(:action, :validate) + + socket + |> assign(:monitoring_mode, mode) + |> assign(:form, to_form(changeset)) + |> then(&{:noreply, &1}) end @impl true @@ -143,8 +171,21 @@ defmodule ToweropsWeb.DeviceLive.Form do @impl true def handle_event("test_snmp", _params, socket) do - snmp_config = extract_snmp_config(socket.assigns.form, socket.assigns) - result = test_snmp_connection(snmp_config) + # Check if an agent is assigned + agent_token_id = socket.assigns.form.params["agent_token_id"] + + result = + if agent_token_id && agent_token_id != "" do + # Agent is configured - can't test directly from web server + %{ + success: true, + message: "SNMP configuration saved. Connection will be tested by the assigned agent during next polling cycle." + } + else + # No agent configured - test directly from web server + snmp_config = extract_snmp_config(socket.assigns.form, socket.assigns) + test_snmp_connection(snmp_config) + end {:noreply, assign(socket, :snmp_test_result, result)} end @@ -171,6 +212,10 @@ defmodule ToweropsWeb.DeviceLive.Form do agent_token_id = Map.get(device_params, "agent_token_id") device_params = Map.delete(device_params, "agent_token_id") + # Add snmp_enabled based on monitoring mode + snmp_enabled = socket.assigns.monitoring_mode == "snmp_and_icmp" + device_params = Map.put(device_params, "snmp_enabled", snmp_enabled) + case Devices.create_device(device_params) do {:ok, device} -> # Handle agent assignment after device creation @@ -195,6 +240,10 @@ defmodule ToweropsWeb.DeviceLive.Form do agent_token_id = Map.get(device_params, "agent_token_id") device_params = Map.delete(device_params, "agent_token_id") + # Add snmp_enabled based on monitoring mode + snmp_enabled = socket.assigns.monitoring_mode == "snmp_and_icmp" + device_params = Map.put(device_params, "snmp_enabled", snmp_enabled) + case Devices.update_device(old_device, device_params) do {:ok, device} -> # device update diff --git a/lib/towerops_web/live/device_live/form.html.heex b/lib/towerops_web/live/device_live/form.html.heex index 2a5fc262..1bf73274 100644 --- a/lib/towerops_web/live/device_live/form.html.heex +++ b/lib/towerops_web/live/device_live/form.html.heex @@ -92,15 +92,56 @@ <% end %>
-

SNMP Configuration

-

- Enable SNMP to discover device details, monitor sensors, and track interface statistics. -

- - <.input field={@form[:snmp_enabled]} type="checkbox" label="Enable SNMP Monitoring" /> +

Monitoring Configuration

+ + +
+
+ +
+
+ <%= if @monitoring_mode == "snmp_and_icmp" do %> +

+ Monitor device availability with ICMP pings and collect detailed information via SNMP + (interfaces, sensors, system info). +

+ <% else %> +

Monitor device availability with ICMP pings only (no SNMP discovery).

+ <% end %> +
+
<.input diff --git a/test/towerops_web/live/device_live_nested/form_test.exs b/test/towerops_web/live/device_live_nested/form_test.exs index b8bdbfa0..8bd41bfb 100644 --- a/test/towerops_web/live/device_live_nested/form_test.exs +++ b/test/towerops_web/live/device_live_nested/form_test.exs @@ -497,17 +497,16 @@ defmodule ToweropsWeb.DeviceLive.FormTest do {:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/#{device.id}/edit") - # First, toggle SNMP on via change event to reveal fields + # Switch to SNMP & ICMP mode using the tab selector view - |> form("#device-form", device: %{snmp_enabled: true}) - |> render_change() + |> element("button", "SNMP & ICMP") + |> render_click() # Now submit with SNMP settings {:ok, _, html} = view |> form("#device-form", device: %{ - snmp_enabled: true, snmp_community: "public", snmp_version: "2c" } diff --git a/test/towerops_web/live/device_live_test.exs b/test/towerops_web/live/device_live_test.exs index 4555b317..291d6fe0 100644 --- a/test/towerops_web/live/device_live_test.exs +++ b/test/towerops_web/live/device_live_test.exs @@ -219,6 +219,7 @@ defmodule ToweropsWeb.DeviceLiveTest do test "creates new device", %{conn: conn, organization: organization, site: site} do {:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new") + # Default is SNMP & ICMP mode, so snmp fields should be visible view |> form("#device-form", device: %{ @@ -228,7 +229,6 @@ defmodule ToweropsWeb.DeviceLiveTest do description: "Test router", monitoring_enabled: true, check_interval_seconds: 300, - snmp_enabled: true, snmp_version: "2c", snmp_community: "public", snmp_port: 161 @@ -243,6 +243,7 @@ defmodule ToweropsWeb.DeviceLiveTest do new_device = Enum.find(device_list, &(&1.name == "New Router")) assert new_device assert new_device.ip_address == "192.168.1.100" + assert new_device.snmp_enabled == true end test "validates required fields", %{conn: conn, organization: organization} do @@ -291,6 +292,111 @@ defmodule ToweropsWeb.DeviceLiveTest do assert {:redirect, %{to: path}} = redirect assert path == ~p"/users/log-in" end + + test "defaults to SNMP & ICMP monitoring mode", %{conn: conn, organization: organization} do + {:ok, _view, html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new") + + assert html =~ "SNMP & ICMP" + assert html =~ "ICMP Only" + assert html =~ "SNMP Version" + assert html =~ "Community String" + end + + test "switching to ICMP Only mode hides SNMP configuration", %{ + conn: conn, + organization: organization + } do + {:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new") + + html = + view + |> element("button", "ICMP Only") + |> render_click() + + refute html =~ "SNMP Version" + refute html =~ "Community String" + end + + test "switching back to SNMP & ICMP mode shows SNMP configuration", %{ + conn: conn, + organization: organization + } do + {:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new") + + # First switch to ICMP Only + view + |> element("button", "ICMP Only") + |> render_click() + + # Then switch back to SNMP & ICMP + html = + view + |> element("button", "SNMP & ICMP") + |> render_click() + + assert html =~ "SNMP Version" + assert html =~ "Community String" + end + + test "creates device with ICMP only mode (snmp_enabled false)", %{ + conn: conn, + organization: organization, + site: site + } do + {:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new") + + # Switch to ICMP Only mode + view + |> element("button", "ICMP Only") + |> render_click() + + view + |> form("#device-form", + device: %{ + name: "ICMP Only Device", + ip_address: "192.168.1.200", + site_id: site.id, + monitoring_enabled: true + } + ) + |> render_submit() + + # Verify device was created with snmp_enabled = false + device_list = Towerops.Devices.list_organization_devices(organization.id) + new_device = Enum.find(device_list, &(&1.name == "ICMP Only Device")) + + assert new_device + assert new_device.snmp_enabled == false + end + + test "creates device with SNMP & ICMP mode (snmp_enabled true)", %{ + conn: conn, + organization: organization, + site: site + } do + {:ok, view, _html} = live(conn, ~p"/orgs/#{organization.slug}/devices/new") + + # SNMP & ICMP is the default, so no need to click tab + + view + |> form("#device-form", + device: %{ + name: "SNMP Device", + ip_address: "192.168.1.201", + site_id: site.id, + monitoring_enabled: true, + snmp_community: "public" + } + ) + |> render_submit() + + # Verify device was created with snmp_enabled = true + device_list = Towerops.Devices.list_organization_devices(organization.id) + new_device = Enum.find(device_list, &(&1.name == "SNMP Device")) + + assert new_device + assert new_device.snmp_enabled == true + end end describe "Edit" do