towerops/test/towerops_web/live/device_live/form_test.exs
Graham McIntire 3583624e0d
fix: prevent nil boolean error in device edit page
Fixed production bug where visiting the edit page for a device with no
SNMP discovery data would crash with "expected boolean on left-side of
'and', got: nil" error.

The mikrotik_device?/1 function was using && operator which returns nil
when device.snmp_device is nil, instead of returning false. The template
conditional on line 455 requires a proper boolean value.

Changed from:
  device.snmp_device && (...)

To:
  not is_nil(device.snmp_device) and (...)

This ensures the function always returns a boolean (true/false) instead
of potentially returning nil.

Added regression test to verify the page renders correctly when a device
has no SNMP discovery data yet.

Fixes: https://app.honeybadger.io/projects/136860/faults/127120431
2026-02-03 15:32:51 -06:00

173 lines
5.5 KiB
Elixir

defmodule ToweropsWeb.DeviceLive.FormTest do
use ToweropsWeb.ConnCase, async: true
import Phoenix.LiveViewTest
setup :register_and_log_in_user
setup %{user: user} do
{:ok, organization} =
Towerops.Organizations.create_organization(%{name: "Test Org"}, user.id)
{:ok, site} =
Towerops.Sites.create_site(%{
name: "Test Site",
organization_id: organization.id
})
%{organization: organization, site: site}
end
describe "edit page rendering" do
test "renders edit page when device has no SNMP discovery data (bug fix for nil boolean)", %{
conn: conn,
site: site
} do
# Regression test for production bug where visiting edit page for a device
# with no SNMP discovery data would crash with "expected boolean, got nil"
# because mikrotik_device?/1 could return nil instead of false.
# Create a device without any snmp_device association (before SNMP discovery)
{:ok, device} =
Towerops.Devices.create_device(%{
name: "Test Device",
ip_address: "192.168.1.1",
site_id: site.id,
snmp_enabled: true,
snmp_version: "2c",
snmp_port: 161,
monitoring_enabled: true,
check_interval_seconds: 300
})
# Ensure device has no snmp_device (this is the bug condition)
device_with_snmp = Towerops.Repo.preload(device, :snmp_device)
assert is_nil(device_with_snmp.snmp_device)
# Visit edit page - should not crash with "expected boolean, got nil" error
assert {:ok, _view, html} = live(conn, ~p"/devices/#{device.id}/edit")
# Page should render successfully
assert html =~ "Edit Device"
assert html =~ device.name
end
test "renders MikroTik section for superuser when device is MikroTik", %{
conn: conn,
site: site,
user: user
} do
alias Towerops.Snmp.Device, as: SnmpDevice
# Make user a superuser using Ecto.Changeset (standard test pattern)
_user =
user
|> Ecto.Changeset.change(%{is_superuser: true})
|> Towerops.Repo.update!()
# Create a device with MikroTik SNMP discovery data
{:ok, device} =
Towerops.Devices.create_device(%{
name: "MikroTik Router",
ip_address: "192.168.1.254",
site_id: site.id,
snmp_enabled: true,
snmp_version: "2c",
snmp_port: 161,
monitoring_enabled: true,
check_interval_seconds: 300
})
# Create SNMP device with MikroTik manufacturer (using Towerops.Snmp.Device schema)
_snmp_device =
%SnmpDevice{}
|> SnmpDevice.changeset(%{
device_id: device.id,
manufacturer: "MikroTik",
sys_descr: "RouterOS 7.8",
sys_object_id: "1.3.6.1.4.1.14988.1",
sys_uptime: 0,
sys_name: "test-router"
})
|> Towerops.Repo.insert!()
# Visit edit page
{:ok, _view, html} = live(conn, ~p"/devices/#{device.id}/edit")
# Page should render successfully
assert html =~ "Edit Device"
assert html =~ device.name
# MikroTik section SHOULD be visible (superuser + MikroTik device)
assert html =~ "MikroTik API Configuration"
end
end
describe "non_routable_ip?/1" do
# Access private function via Module.get_attribute or test via public interface
# Since these are private functions, we test through the validation behavior
test "RFC1918 10.0.0.0/8 range" do
assert non_routable?("10.0.0.1")
assert non_routable?("10.255.255.255")
assert non_routable?("10.100.50.25")
end
test "RFC1918 172.16.0.0/12 range" do
assert non_routable?("172.16.0.1")
assert non_routable?("172.31.255.255")
assert non_routable?("172.20.10.5")
refute non_routable?("172.15.0.1")
refute non_routable?("172.32.0.1")
end
test "RFC1918 192.168.0.0/16 range" do
assert non_routable?("192.168.0.1")
assert non_routable?("192.168.255.255")
assert non_routable?("192.168.1.100")
refute non_routable?("192.167.1.1")
refute non_routable?("192.169.1.1")
end
test "RFC6598 CGNAT 100.64.0.0/10 range" do
assert non_routable?("100.64.0.1")
assert non_routable?("100.127.255.255")
assert non_routable?("100.100.50.25")
refute non_routable?("100.63.255.255")
refute non_routable?("100.128.0.1")
end
test "public IPs are not flagged" do
refute non_routable?("8.8.8.8")
refute non_routable?("1.1.1.1")
refute non_routable?("203.0.113.1")
refute non_routable?("198.51.100.1")
end
test "invalid IPs are not flagged" do
refute non_routable?("invalid")
refute non_routable?("999.999.999.999")
refute non_routable?("")
end
test "IPv6 addresses are not flagged" do
refute non_routable?("::1")
refute non_routable?("fe80::1")
refute non_routable?("2001:db8::1")
end
end
# Helper to test the private function logic
# Duplicated here for testing since the actual function is private
defp non_routable?(ip_string) do
case ip_string |> String.to_charlist() |> :inet.parse_address() do
{:ok, {a, b, _c, _d}} -> non_routable_ipv4_range?(a, b)
_ -> false
end
end
defp non_routable_ipv4_range?(10, _b), do: true
defp non_routable_ipv4_range?(172, b) when b >= 16 and b <= 31, do: true
defp non_routable_ipv4_range?(192, 168), do: true
defp non_routable_ipv4_range?(100, b) when b >= 64 and b <= 127, do: true
defp non_routable_ipv4_range?(_a, _b), do: false
end