diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 800d8b47..8f64a5a9 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -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 diff --git a/lib/towerops/devices/device.ex b/lib/towerops/devices/device.ex index 59f7832d..fedc0952 100644 --- a/lib/towerops/devices/device.ex +++ b/lib/towerops/devices/device.ex @@ -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]) diff --git a/lib/towerops/snmp/discovery.ex b/lib/towerops/snmp/discovery.ex index 0275ecf3..206c8cbd 100644 --- a/lib/towerops/snmp/discovery.ex +++ b/lib/towerops/snmp/discovery.ex @@ -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( diff --git a/lib/towerops/topology.ex b/lib/towerops/topology.ex index d042578c..afdf1451 100644 --- a/lib/towerops/topology.ex +++ b/lib/towerops/topology.ex @@ -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)) diff --git a/lib/towerops_web/controllers/api/v1/devices_controller.ex b/lib/towerops_web/controllers/api/v1/devices_controller.ex index ce2726a9..1efc3157 100644 --- a/lib/towerops_web/controllers/api/v1/devices_controller.ex +++ b/lib/towerops_web/controllers/api/v1/devices_controller.ex @@ -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, diff --git a/lib/towerops_web/graphql/types/device.ex b/lib/towerops_web/graphql/types/device.ex index 5f7805c5..06edf0a8 100644 --- a/lib/towerops_web/graphql/types/device.ex +++ b/lib/towerops_web/graphql/types/device.ex @@ -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 diff --git a/lib/towerops_web/live/device_live/form.html.heex b/lib/towerops_web/live/device_live/form.html.heex index 552abfcd..1a0fbccc 100644 --- a/lib/towerops_web/live/device_live/form.html.heex +++ b/lib/towerops_web/live/device_live/form.html.heex @@ -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"} ]} />

- {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." + )}

diff --git a/lib/towerops_web/live/device_live/index.ex b/lib/towerops_web/live/device_live/index.ex index f3898b5a..3c589639 100644 --- a/lib/towerops_web/live/device_live/index.ex +++ b/lib/towerops_web/live/device_live/index.ex @@ -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 diff --git a/priv/repo/migrations/20260322152809_migrate_device_roles_to_simplified_set.exs b/priv/repo/migrations/20260322152809_migrate_device_roles_to_simplified_set.exs new file mode 100644 index 00000000..5fa5ca92 --- /dev/null +++ b/priv/repo/migrations/20260322152809_migrate_device_roles_to_simplified_set.exs @@ -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 diff --git a/priv/repo/migrations/20260322161037_remove_device_role_source.exs b/priv/repo/migrations/20260322161037_remove_device_role_source.exs new file mode 100644 index 00000000..f4f651c9 --- /dev/null +++ b/priv/repo/migrations/20260322161037_remove_device_role_source.exs @@ -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 diff --git a/priv/repo/migrations/20260322161750_set_device_role_default.exs b/priv/repo/migrations/20260322161750_set_device_role_default.exs new file mode 100644 index 00000000..5530d8ba --- /dev/null +++ b/priv/repo/migrations/20260322161750_set_device_role_default.exs @@ -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 diff --git a/priv/static/changelog.txt b/priv/static/changelog.txt index 4661e4c5..da71b7b3 100644 --- a/priv/static/changelog.txt +++ b/priv/static/changelog.txt @@ -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" diff --git a/test/towerops/topology_test.exs b/test/towerops/topology_test.exs index 2f935b43..fbd4e82b 100644 --- a/test/towerops/topology_test.exs +++ b/test/towerops/topology_test.exs @@ -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 diff --git a/test/towerops_web/live/device_live/form_test.exs b/test/towerops_web/live/device_live/form_test.exs index f5516e77..6adc2159 100644 --- a/test/towerops_web/live/device_live/form_test.exs +++ b/test/towerops_web/live/device_live/form_test.exs @@ -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