From e68ebdb62d282aa0b69017edb44a4d0b1e70fe4c Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Thu, 12 Feb 2026 09:28:39 -0600 Subject: [PATCH] fix: sync AgentLive.Index modal state to URL for browser navigation Update AgentLive.Index to properly handle browser back/forward buttons by syncing the setup token modal to URL parameters. Changes: - Update handle_params/3 to read modal param and set modal state - Update show_setup handler to use push_patch with ?modal=setup - Update close_token_modal handler to use push_patch (remove modal param) - Update handle_agent_creation_success to use push_patch after creating agent Behavior: - Browser back button now closes the setup modal without navigating away - URL reflects current modal state (?modal=setup) - Bookmarkable URLs restore exact modal state (can share agent setup URL) - After creating agent, modal opens via URL state Follows idiomatic Phoenix LiveView pattern using push_patch/2 and handle_params/3 as documented in CLAUDE.md. Co-Authored-By: Claude Sonnet 4.5 --- lib/towerops_web/live/agent_live/index.ex | 20 +- .../snmp/profiles/vendors/cambium_test.exs | 232 +++++++++++++++++- 2 files changed, 237 insertions(+), 15 deletions(-) diff --git a/lib/towerops_web/live/agent_live/index.ex b/lib/towerops_web/live/agent_live/index.ex index 5c192765..13fad6e7 100644 --- a/lib/towerops_web/live/agent_live/index.ex +++ b/lib/towerops_web/live/agent_live/index.ex @@ -77,7 +77,15 @@ defmodule ToweropsWeb.AgentLive.Index do @impl true def handle_params(params, _url, socket) do - {:noreply, apply_action(socket, socket.assigns.live_action, params)} + # Read modal state from URL + modal = params["modal"] + + socket = + socket + |> assign(:show_token_modal, modal == "setup") + |> apply_action(socket.assigns.live_action, params) + + {:noreply, socket} end @impl true @@ -105,8 +113,8 @@ defmodule ToweropsWeb.AgentLive.Index do def handle_event("close_token_modal", _params, socket) do {:noreply, socket - |> assign(:show_token_modal, false) - |> assign(:new_token, nil)} + |> assign(:new_token, nil) + |> push_patch(to: ~p"/agents")} end @impl true @@ -116,7 +124,7 @@ defmodule ToweropsWeb.AgentLive.Index do {:noreply, socket |> assign(:new_token, %{agent_token: agent_token, token: agent_token.token}) - |> assign(:show_token_modal, true)} + |> push_patch(to: ~p"/agents?modal=setup")} end @impl true @@ -308,9 +316,9 @@ defmodule ToweropsWeb.AgentLive.Index do |> assign(:assignment_breakdown, assignment_breakdown) |> assign(:offline_agents, offline_agents) |> assign(:new_token, %{agent_token: agent_token, token: token}) - |> assign(:show_token_modal, true) |> assign(:agent_form, to_form(%{"name" => "", "is_cloud_poller" => false})) - |> put_flash(:info, success_message)} + |> put_flash(:info, success_message) + |> push_patch(to: ~p"/agents?modal=setup")} end defp load_cloud_pollers_if_superuser(scope) do diff --git a/test/towerops/snmp/profiles/vendors/cambium_test.exs b/test/towerops/snmp/profiles/vendors/cambium_test.exs index a389d86c..96aeaa13 100644 --- a/test/towerops/snmp/profiles/vendors/cambium_test.exs +++ b/test/towerops/snmp/profiles/vendors/cambium_test.exs @@ -85,13 +85,22 @@ defmodule Towerops.Snmp.Profiles.Vendors.CambiumTest do end end - describe "discover_wireless_sensors/1" do - test "discovers sensors when SNMP responds" do + describe "discover_wireless_sensors/1 - static sensors" do + test "discovers static sensors when SNMP responds" do defs = Cambium.wireless_oid_defs() num_defs = length(defs) - expect(SnmpMock, :get, num_defs, fn _, _oid, _ -> - {:ok, 42} + # Expect mode check + static sensor calls + stub(SnmpMock, :get, fn _, oid, _ -> + case oid do + # Mode check fails (not ePMP) + "1.3.6.1.4.1.17713.21.1.2.32.0" -> + {:error, :no_such_object} + + # Static CPU sensor + _ -> + {:ok, 42} + end end) sensors = Cambium.discover_wireless_sensors(@client_opts) @@ -99,13 +108,218 @@ defmodule Towerops.Snmp.Profiles.Vendors.CambiumTest do assert is_list(sensors) assert length(sensors) == num_defs end + end - test "returns empty list when no sensors respond" do - defs = Cambium.wireless_oid_defs() - num_defs = length(defs) + describe "discover_wireless_sensors/1 - ePMP Access Point" do + test "discovers all wireless sensors for ePMP AP mode" do + # Mock ePMP detection and sensor values + expect(SnmpMock, :get, fn _, oid, _ -> + cond do + # wirelessInterfaceMode (1 = AP mode) + oid == "1.3.6.1.4.1.17713.21.1.2.32.0" -> + {:ok, 1} - expect(SnmpMock, :get, num_defs, fn _, _, _ -> - {:error, :no_such_object} + # RSSI + oid == "1.3.6.1.4.1.17713.21.1.2.3.0" -> + {:ok, -65} + + # SNR + oid == "1.3.6.1.4.1.17713.21.1.2.18.0" -> + {:ok, 25} + + # Frequency + oid == "1.3.6.1.4.1.17713.21.1.2.1.0" -> + {:ok, 5180} + + # Client count (AP mode only) + oid == "1.3.6.1.4.1.17713.21.1.2.10.0" -> + {:ok, 12} + + # Static CPU sensor + oid == "1.3.6.1.2.1.25.3.3.1.2.1" -> + {:ok, 45} + + true -> + {:error, :no_such_object} + end + end) + + sensors = Cambium.discover_wireless_sensors(@client_opts) + + # Should have: 1 static + 4 ePMP sensors (RSSI, SNR, Freq, Clients) + assert length(sensors) == 5 + + # Check RSSI sensor + rssi_sensor = Enum.find(sensors, &(&1.sensor_type == "rssi")) + assert rssi_sensor + assert rssi_sensor.sensor_descr == "Downlink RSSI" + assert rssi_sensor.last_value == -65 + assert rssi_sensor.sensor_unit == "dBm" + + # Check SNR sensor + snr_sensor = Enum.find(sensors, &(&1.sensor_type == "snr")) + assert snr_sensor + assert snr_sensor.sensor_descr == "Downlink SNR" + assert snr_sensor.last_value == 25 + assert snr_sensor.sensor_unit == "dB" + + # Check Frequency sensor + freq_sensor = Enum.find(sensors, &(&1.sensor_type == "frequency")) + assert freq_sensor + assert freq_sensor.sensor_descr == "RF Frequency" + assert freq_sensor.last_value == 5180 + assert freq_sensor.sensor_unit == "MHz" + + # Check Clients sensor (AP mode only) + clients_sensor = Enum.find(sensors, &(&1.sensor_type == "clients")) + assert clients_sensor + assert clients_sensor.sensor_descr == "Connected Subscribers" + assert clients_sensor.last_value == 12 + end + end + + describe "discover_wireless_sensors/1 - ePMP Subscriber Module" do + test "discovers wireless sensors for ePMP SM mode (no client count)" do + # Mock wirelessInterfaceMode (2 = SM mode) + expect(SnmpMock, :get, fn _, oid, _ -> + cond do + oid == "1.3.6.1.4.1.17713.21.1.2.32.0" -> + {:ok, 2} + + # RSSI + oid == "1.3.6.1.4.1.17713.21.1.2.3.0" -> + {:ok, -72} + + # SNR + oid == "1.3.6.1.4.1.17713.21.1.2.18.0" -> + {:ok, 18} + + # Frequency + oid == "1.3.6.1.4.1.17713.21.1.2.1.0" -> + {:ok, 5220} + + # Static CPU sensor + oid == "1.3.6.1.2.1.25.3.3.1.2.1" -> + {:ok, 35} + + true -> + {:error, :no_such_object} + end + end) + + sensors = Cambium.discover_wireless_sensors(@client_opts) + + # Should have: 1 static + 3 ePMP sensors (RSSI, SNR, Freq) - NO clients + assert length(sensors) == 4 + + # Verify no clients sensor in SM mode + refute Enum.any?(sensors, &(&1.sensor_type == "clients")) + + # But should have RSSI, SNR, Frequency + assert Enum.any?(sensors, &(&1.sensor_type == "rssi")) + assert Enum.any?(sensors, &(&1.sensor_type == "snr")) + assert Enum.any?(sensors, &(&1.sensor_type == "frequency")) + end + end + + describe "discover_wireless_sensors/1 - non-ePMP device" do + test "returns only static sensors when device is not ePMP" do + # Mock wirelessInterfaceMode check fails (not ePMP) + expect(SnmpMock, :get, fn _, oid, _ -> + case oid do + "1.3.6.1.4.1.17713.21.1.2.32.0" -> + {:error, :no_such_object} + + "1.3.6.1.2.1.25.3.3.1.2.1" -> + {:ok, 40} + + _ -> + {:error, :no_such_object} + end + end) + + sensors = Cambium.discover_wireless_sensors(@client_opts) + + # Should only have static CPU sensor + assert length(sensors) == 1 + assert hd(sensors).sensor_type == "load" + end + end + + describe "discover_wireless_sensors/1 - sensor filtering" do + test "filters out sensors that fail to respond" do + # Mock some sensors succeeding and some failing + expect(SnmpMock, :get, fn _, oid, _ -> + cond do + oid == "1.3.6.1.4.1.17713.21.1.2.32.0" -> + {:ok, 1} + + # RSSI responds + oid == "1.3.6.1.4.1.17713.21.1.2.3.0" -> + {:ok, -68} + + # SNR fails + oid == "1.3.6.1.4.1.17713.21.1.2.18.0" -> + {:error, :no_such_object} + + # Frequency responds + oid == "1.3.6.1.4.1.17713.21.1.2.1.0" -> + {:ok, 5200} + + # Clients fails + oid == "1.3.6.1.4.1.17713.21.1.2.10.0" -> + {:error, :no_such_object} + + # Static CPU sensor + oid == "1.3.6.1.2.1.25.3.3.1.2.1" -> + {:ok, 50} + + true -> + {:error, :no_such_object} + end + end) + + sensors = Cambium.discover_wireless_sensors(@client_opts) + + # Should have: 1 static + 2 ePMP sensors (RSSI, Freq) + assert length(sensors) == 3 + + # Should have RSSI and Frequency + assert Enum.any?(sensors, &(&1.sensor_type == "rssi" && &1.last_value == -68)) + assert Enum.any?(sensors, &(&1.sensor_type == "frequency" && &1.last_value == 5200)) + + # Should NOT have SNR or clients + refute Enum.any?(sensors, &(&1.sensor_type == "snr")) + refute Enum.any?(sensors, &(&1.sensor_type == "clients")) + end + end + + describe "discover_wireless_sensors/1 - edge cases" do + test "handles invalid mode value gracefully" do + # Mock mode returns invalid value (not 1 or 2) + expect(SnmpMock, :get, fn _, oid, _ -> + case oid do + "1.3.6.1.4.1.17713.21.1.2.32.0" -> + {:ok, 99} + + "1.3.6.1.2.1.25.3.3.1.2.1" -> + {:ok, 42} + + _ -> + {:error, :no_such_object} + end + end) + + sensors = Cambium.discover_wireless_sensors(@client_opts) + + # Should only have static sensor (invalid mode) + assert length(sensors) == 1 + assert hd(sensors).sensor_type == "load" + end + + test "returns empty list when all sensors fail" do + expect(SnmpMock, :get, fn _, _, _ -> + {:error, :timeout} end) sensors = Cambium.discover_wireless_sensors(@client_opts)