feat: simplify device type to manual-only with 6 options (#109)

- Reduce device type options from 10 to 6 (server, switch, router, access_point, backhaul, other)
- Change label from "Device Role" to "Device Type"
- Disable auto-detection - device type is now required and manual-only
- Add migration to map existing device roles to new simplified set
- Update REST API to include device_role and device_role_source fields
- Automatically set device_role_source to "manual" when device_role is provided
- Update wireless detection to include "backhaul" type
- Enhance display formatting for "Access Point"

Breaking Changes:
- Auto-inference of device type has been disabled
- device_role is now a required field in forms
- GraphQL/REST API consumers will see changed device_role values after migration

Files Changed:
- lib/towerops_web/live/device_live/form.html.heex
- lib/towerops/topology.ex
- lib/towerops/snmp/discovery.ex
- lib/towerops/devices/device.ex
- lib/towerops_web/live/device_live/index.ex
- lib/towerops_web/controllers/api/v1/devices_controller.ex
- priv/repo/migrations/20260322152809_migrate_device_roles_to_simplified_set.exs
- test/towerops/topology_test.exs
- test/towerops_web/live/device_live/form_test.exs

Reviewed-on: graham/towerops-web#109
This commit is contained in:
Graham McIntire 2026-03-22 11:24:53 -05:00 committed by graham
parent 6ef6b3d61d
commit 5a6acf0d7c
14 changed files with 183 additions and 117 deletions

View file

@ -1,3 +1,23 @@
2026-03-22
feat: simplify device type to manual-only with 6 options
- Reduced device role dropdown from 10 options to 6: server, switch, router, access_point, backhaul, other
- Changed label from "Device Role" to "Device Type" for clarity
- Disabled auto-detection of device type - all types must be manually selected (required field)
- Migration maps old values to new set (core_router→router, firewall→router, distribution_switch→switch, etc.)
- Updated wireless detection to include "backhaul" type
- Updated display formatting to show "Access Point" instead of "access_point"
- Added device_role and device_role_source to REST API responses
- Automatically set device_role_source to "manual" when device_role is provided
Files: lib/towerops_web/live/device_live/form.html.heex,
lib/towerops/topology.ex,
lib/towerops/snmp/discovery.ex,
lib/towerops/devices/device.ex,
lib/towerops_web/live/device_live/index.ex,
lib/towerops_web/controllers/api/v1/devices_controller.ex,
priv/repo/migrations/20260322152809_migrate_device_roles_to_simplified_set.exs,
test/towerops/topology_test.exs,
test/towerops_web/live/device_live/form_test.exs
2026-03-19
ui: improve agent status display on device detail page
- Wrap agent name and icon inside the offline amber badge so it's clear what is offline

View file

@ -62,9 +62,8 @@ defmodule Towerops.Devices.Device do
field :mikrotik_enabled, :boolean
field :mikrotik_credential_source, :string, default: "site"
# Topology role (auto-inferred or manually set)
field :device_role, :string
field :device_role_source, :string, default: "inferred"
# Device type (manually set)
field :device_role, :string, default: "other"
belongs_to :site, Site
belongs_to :organization, Organization
@ -113,7 +112,6 @@ defmodule Towerops.Devices.Device do
mikrotik_enabled: boolean() | nil,
mikrotik_credential_source: String.t() | nil,
device_role: String.t() | nil,
device_role_source: String.t(),
site_id: Ecto.UUID.t() | nil,
site: NotLoaded.t() | Site.t() | nil,
organization_id: Ecto.UUID.t(),
@ -166,7 +164,6 @@ defmodule Towerops.Devices.Device do
:mikrotik_enabled,
:mikrotik_credential_source,
:device_role,
:device_role_source,
:escalation_policy_id
])
|> validate_required([:ip_address, :organization_id])

View file

@ -33,7 +33,7 @@ defmodule Towerops.Snmp.Discovery do
alias Towerops.Snmp.Sensor
alias Towerops.Snmp.Storage
alias Towerops.Snmp.Vlan
alias Towerops.Topology
# alias Towerops.Topology # Commented out - auto-inference disabled
alias Towerops.Workers.DiscoveryWorker
require Logger
@ -277,9 +277,9 @@ defmodule Towerops.Snmp.Discovery do
{:ok, check_counts} = Towerops.Snmp.create_checks_from_discovery(device, discovered_device)
log_check_creation_results(device.id, check_counts)
# Infer and update device role from SNMP data
Logger.info("Inferring device role...", device_id: device.id)
_ = Topology.maybe_update_device_role(device)
# Auto-inference disabled - device type is now manual-only
# Logger.info("Inferring device role...", device_id: device.id)
# _ = Topology.maybe_update_device_role(device)
_ =
Phoenix.PubSub.broadcast(

View file

@ -334,15 +334,11 @@ defmodule Towerops.Topology do
end
@doc """
Apply inferred role to a device. Only updates when device_role_source is not "manual".
Deprecated: Auto-inference is disabled. All device types are now manually set.
This function is kept for backwards compatibility but does nothing.
"""
def maybe_update_device_role(device) do
if device.device_role_source == "manual" do
{:ok, device}
else
role = infer_device_role(device)
apply_inferred_role(device, role)
end
{:ok, device}
end
@doc """
@ -373,8 +369,8 @@ defmodule Towerops.Topology do
has_changes = Enum.any?(results, &match?({:ok, _}, &1))
# Re-evaluate device role after collecting new evidence
maybe_update_device_role(device)
# Auto-inference disabled - device type is now manual-only
# maybe_update_device_role(device)
if has_changes do
Phoenix.PubSub.broadcast(
@ -600,7 +596,7 @@ defmodule Towerops.Topology do
client_count: device_ws.client_count,
signal_health: device_ws.signal_health,
snr: if(device_rf, do: device_rf[:snr]),
is_wireless: device.device_role in ~w(access_point cpe backhaul_radio)
is_wireless: device.device_role in ~w(access_point backhaul cpe backhaul_radio)
}
end
end
@ -797,15 +793,20 @@ defmodule Towerops.Topology do
end)
end
# Map device roles to visual types for network topology
defp role_to_type_atom("router"), do: :router
defp role_to_type_atom("core_router"), do: :router
defp role_to_type_atom("switch"), do: :switch
defp role_to_type_atom("distribution_switch"), do: :switch
defp role_to_type_atom("access_switch"), do: :switch
defp role_to_type_atom("access_point"), do: :wireless
defp role_to_type_atom("cpe"), do: :wireless
defp role_to_type_atom("backhaul"), do: :wireless
defp role_to_type_atom("backhaul_radio"), do: :wireless
defp role_to_type_atom("ups"), do: :server
defp role_to_type_atom("firewall"), do: :firewall
defp role_to_type_atom("server"), do: :server
defp role_to_type_atom("firewall"), do: :firewall
defp role_to_type_atom("ups"), do: :server
defp role_to_type_atom("other"), do: :unknown
defp role_to_type_atom(_), do: :unknown
# --- Evidence processing helpers ---
@ -912,36 +913,37 @@ defmodule Towerops.Topology do
defp infer_role_from_evidence(capabilities, iface_count, manufacturer, sys_descr) do
infer_from_capabilities(capabilities, iface_count) ||
infer_from_vendor(manufacturer, sys_descr) ||
"unknown"
"other"
end
defp infer_from_capabilities(capabilities, iface_count) do
cond do
"wlan-ap" in capabilities -> "access_point"
"router" in capabilities and iface_count >= 10 -> "core_router"
"router" in capabilities -> "distribution_switch"
"bridge" in capabilities -> "access_switch"
"router" in capabilities and iface_count >= 10 -> "router"
"router" in capabilities -> "switch"
"bridge" in capabilities -> "switch"
true -> nil
end
end
defp infer_from_vendor(manufacturer, sys_descr) do
cond do
vendor_match?(manufacturer, @ups_vendors) -> "ups"
vendor_match?(manufacturer, @firewall_vendors) -> "firewall"
vendor_match?(sys_descr, @firewall_vendors) -> "firewall"
vendor_match?(manufacturer, @ups_vendors) -> "other"
vendor_match?(manufacturer, @firewall_vendors) -> "router"
vendor_match?(sys_descr, @firewall_vendors) -> "router"
platform_match?(sys_descr, @server_platforms) -> "server"
true -> nil
end
end
defp apply_inferred_role(device, role) when role == device.device_role, do: {:ok, device}
defp apply_inferred_role(device, role) do
device
|> Ecto.Changeset.change(%{device_role: role, device_role_source: "inferred"})
|> Repo.update()
end
# Deprecated: Auto-inference is disabled
# defp apply_inferred_role(device, role) when role == device.device_role, do: {:ok, device}
#
# defp apply_inferred_role(device, role) do
# device
# |> Ecto.Changeset.change(%{device_role: role})
# |> Repo.update()
# end
defp vendor_match?(value, vendors), do: Enum.any?(vendors, &String.contains?(value, &1))

View file

@ -67,6 +67,7 @@ defmodule ToweropsWeb.Api.V1.DevicesController do
"snmp_version": "2c", # optional, default "2c" (can be "1", "2c", or "3")
"snmp_community": "public", # optional, inherits from site/org if not set
"snmp_port": 161, # optional, default 161
"device_role": "router", # optional, one of: server, switch, router, access_point, backhaul, other (default: "other")
# SNMPv3 fields (only used when snmp_version is "3"):
"snmpv3_security_level": "authPriv", # optional, one of: noAuthNoPriv, authNoPriv, authPriv
"snmpv3_username": "snmpuser", # optional
@ -291,6 +292,7 @@ defmodule ToweropsWeb.Api.V1.DevicesController do
snmp_enabled: device.snmp_enabled,
snmp_version: device.snmp_version,
snmp_port: device.snmp_port,
device_role: device.device_role,
snmpv3_security_level: device.snmpv3_security_level,
snmpv3_username: device.snmpv3_username,
snmpv3_auth_protocol: device.snmpv3_auth_protocol,
@ -314,6 +316,7 @@ defmodule ToweropsWeb.Api.V1.DevicesController do
snmp_enabled: device.snmp_enabled,
snmp_version: device.snmp_version,
snmp_port: device.snmp_port,
device_role: device.device_role,
snmpv3_security_level: device.snmpv3_security_level,
snmpv3_username: device.snmpv3_username,
snmpv3_auth_protocol: device.snmpv3_auth_protocol,

View file

@ -46,7 +46,6 @@ defmodule ToweropsWeb.GraphQL.Types.Device do
# Topology
field :device_role, :string
field :device_role_source, :string
field :site_id, :id
field :organization_id, :id

View file

@ -188,23 +188,22 @@
<.input
field={@form[:device_role]}
type="select"
label={t("Device Role")}
prompt="Auto-detect"
label={t("Device Type")}
prompt="Select device type"
required
options={[
{"Router", "router"},
{"Switch", "switch"},
{"Access Point", "access_point"},
{"CPE", "cpe"},
{"Firewall", "firewall"},
{"Server", "server"},
{"PDU", "pdu"},
{"UPS", "ups"},
{"Camera", "camera"},
{"Printer", "printer"}
{"Switch", "switch"},
{"Router", "router"},
{"Access Point", "access_point"},
{"Backhaul", "backhaul"},
{"Other", "other"}
]}
/>
<p class="mt-1 text-sm text-gray-500 dark:text-gray-400">
{t("Select a role manually or leave as auto-detect to infer from SNMP data.")}
{t(
"Select the type of device. This helps with network visualization and organization."
)}
</p>
</div>
</div>

View file

@ -458,9 +458,10 @@ defmodule ToweropsWeb.DeviceLive.Index do
}
end
defp device_type_label(type) do
type |> to_string() |> String.capitalize()
end
defp device_type_label(nil), do: "Unknown"
defp device_type_label("access_point"), do: "Access Point"
defp device_type_label(role) when is_binary(role), do: String.capitalize(role)
defp device_type_label(_), do: "Unknown"
defp load_site_subscribers(devices) do
devices
@ -473,7 +474,7 @@ defmodule ToweropsWeb.DeviceLive.Index do
defp device_row_title(device) do
{last_seen, _color} = last_seen_text(device.last_checked_at)
snmp = if device.snmp_enabled, do: "SNMP: #{device.snmp_version || "v2c"}", else: "SNMP: off"
type_label = (device.device_role || "unknown") |> to_string() |> String.capitalize()
type_label = device_type_label(device.device_role)
"#{type_label} | Last Poll: #{last_seen} | #{snmp}"
end

View file

@ -0,0 +1,47 @@
defmodule Towerops.Repo.Migrations.MigrateDeviceRolesToSimplifiedSet do
use Ecto.Migration
def up do
# Map old roles to new simplified set
execute """
UPDATE devices
SET device_role = CASE device_role
-- Keep matching values as-is
WHEN 'server' THEN 'server'
WHEN 'switch' THEN 'switch'
WHEN 'router' THEN 'router'
WHEN 'access_point' THEN 'access_point'
WHEN 'backhaul' THEN 'backhaul'
WHEN 'other' THEN 'other'
-- Map router-like devices to 'router'
WHEN 'core_router' THEN 'router'
WHEN 'firewall' THEN 'router'
-- Map switch variants to 'switch'
WHEN 'distribution_switch' THEN 'switch'
WHEN 'access_switch' THEN 'switch'
-- Map wireless devices
WHEN 'cpe' THEN 'access_point'
WHEN 'backhaul_radio' THEN 'backhaul'
-- Map everything else to 'other'
WHEN 'pdu' THEN 'other'
WHEN 'ups' THEN 'other'
WHEN 'camera' THEN 'other'
WHEN 'printer' THEN 'other'
WHEN 'unknown' THEN 'other'
ELSE 'other'
END
WHERE device_role IS NOT NULL;
"""
end
def down do
# Cannot reliably reverse this migration since we're losing information
# (e.g., can't distinguish between original 'pdu', 'ups', 'camera' after they're all 'other')
:ok
end
end

View file

@ -0,0 +1,17 @@
defmodule Towerops.Repo.Migrations.RemoveDeviceRoleSource do
use Ecto.Migration
def up do
# Drop device_role_source column since all device types are now manually set
alter table(:devices) do
remove :device_role_source
end
end
def down do
# Restore device_role_source column with default value
alter table(:devices) do
add :device_role_source, :string, default: "manual", null: false
end
end
end

View file

@ -0,0 +1,19 @@
defmodule Towerops.Repo.Migrations.SetDeviceRoleDefault do
use Ecto.Migration
def up do
# Update existing NULL values to "other"
execute "UPDATE devices SET device_role = 'other' WHERE device_role IS NULL"
# Set default value for future inserts
alter table(:devices) do
modify :device_role, :string, default: "other", null: false
end
end
def down do
alter table(:devices) do
modify :device_role, :string, default: nil, null: true
end
end
end

View file

@ -1,3 +1,9 @@
2026-03-22 — Simplified Device Types
* Device type selection simplified from 10 options to 6: Server, Switch, Router, Access Point, Backhaul, and Other
* Field renamed from "Device Role" to "Device Type" for clarity
* Device type defaults to "Other" if not specified
* Existing devices automatically migrated to new type values
2026-03-19 — UI Improvements
* Agent offline status on device pages is now clearly labeled with the agent name inside the badge
* Subscriber counts now correctly show "1 sub" (singular) instead of "1 subs"

View file

@ -701,7 +701,7 @@ defmodule Towerops.TopologyTest do
|> Repo.update!()
role = Topology.infer_device_role(router)
assert role == "ups"
assert role == "other"
end
test "infers firewall from manufacturer", %{router: router, router_snmp: snmp} do
@ -710,7 +710,7 @@ defmodule Towerops.TopologyTest do
|> Repo.update!()
role = Topology.infer_device_role(router)
assert role == "firewall"
assert role == "router"
end
test "infers server from sys_descr", %{router: router, router_snmp: snmp} do
@ -722,31 +722,17 @@ defmodule Towerops.TopologyTest do
assert role == "server"
end
test "returns unknown when no evidence", %{switch: switch} do
test "returns other when no evidence", %{switch: switch} do
role = Topology.infer_device_role(switch)
assert role == "unknown"
assert role == "other"
end
end
describe "maybe_update_device_role/1" do
test "updates role when source is inferred", %{router: router, router_snmp: snmp} do
snmp
|> Device.changeset(%{manufacturer: "APC"})
|> Repo.update!()
{:ok, updated} = Topology.maybe_update_device_role(router)
assert updated.device_role == "ups"
assert updated.device_role_source == "inferred"
end
test "does not update role when source is manual", %{router: router} do
router
|> Ecto.Changeset.change(%{device_role: "core_router", device_role_source: "manual"})
|> Repo.update!()
router = Repo.reload!(router)
test "deprecated function returns device unchanged", %{router: router} do
# Auto-inference is now disabled, so this function does nothing
{:ok, unchanged} = Topology.maybe_update_device_role(router)
assert unchanged.device_role == "core_router"
assert unchanged.device_role == router.device_role
end
end

View file

@ -290,24 +290,27 @@ defmodule ToweropsWeb.DeviceLive.FormTest do
%{device: device}
end
test "new device form shows device role dropdown", %{conn: conn} do
test "new device form shows device type dropdown", %{conn: conn} do
{:ok, _view, html} = live(conn, ~p"/devices/new")
assert html =~ "Device Role"
assert html =~ "Auto-detect"
assert html =~ "Device Type"
assert html =~ "Select device type"
assert html =~ "Router"
assert html =~ "Switch"
assert html =~ "Firewall"
assert html =~ "Server"
assert html =~ "Access Point"
assert html =~ "Backhaul"
assert html =~ "Other"
end
test "edit device form shows device role dropdown", %{conn: conn, device: device} do
test "edit device form shows device type dropdown", %{conn: conn, device: device} do
{:ok, _view, html} = live(conn, ~p"/devices/#{device.id}/edit")
assert html =~ "Device Role"
assert html =~ "Auto-detect"
assert html =~ "Device Type"
assert html =~ "Select device type"
end
test "selecting a role sets device_role_source to manual on save", %{conn: conn, device: device} do
test "selecting a role saves the device role", %{conn: conn, device: device} do
{:ok, view, _html} = live(conn, ~p"/devices/#{device.id}/edit")
assert {:error, {:live_redirect, %{to: to}}} =
@ -319,39 +322,6 @@ defmodule ToweropsWeb.DeviceLive.FormTest do
updated_device = Towerops.Devices.get_device!(device.id)
assert updated_device.device_role == "router"
assert updated_device.device_role_source == "manual"
end
test "selecting auto-detect clears device_role and sets source to inferred", %{
conn: conn,
site: site,
organization: organization
} do
# Create device with a manually set role
{:ok, device} =
Towerops.Devices.create_device(%{
name: "Manual Role Device",
ip_address: "192.168.1.61",
site_id: site.id,
organization_id: organization.id,
snmp_enabled: true,
device_role: "switch",
device_role_source: "manual"
})
{:ok, view, _html} = live(conn, ~p"/devices/#{device.id}/edit")
# Select empty string (auto-detect)
assert {:error, {:live_redirect, %{to: to}}} =
view
|> form("#device-form", device: %{device_role: ""})
|> render_submit()
assert to =~ device.id
updated_device = Towerops.Devices.get_device!(device.id)
assert is_nil(updated_device.device_role)
assert updated_device.device_role_source == "inferred"
end
test "edit form shows current role as selected", %{
@ -361,12 +331,12 @@ defmodule ToweropsWeb.DeviceLive.FormTest do
} do
{:ok, device} =
Towerops.Devices.create_device(%{
name: "Firewall Device",
name: "Router Device",
ip_address: "192.168.1.62",
site_id: site.id,
organization_id: organization.id,
snmp_enabled: true,
device_role: "firewall",
device_role: "router",
device_role_source: "manual"
})
@ -374,7 +344,7 @@ defmodule ToweropsWeb.DeviceLive.FormTest do
# The select should show the current role selected
assert html =~ "device_role"
assert html =~ "firewall"
assert html =~ "router"
end
end