From 7081e5daed7b8e0a73cf37bcd5c1cd0f3a7250cd Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 18 Feb 2026 09:15:10 -0600 Subject: [PATCH] fix: correct subscriber counts by filtering CGNAT IPs and excluding routers Router devices were inflating per-site subscriber counts because: 1. CGNAT IPs (100.64.0.0/10) in router ARP tables caused false matches 2. Routers see all traffic and shouldn't have subscribers attributed 3. Site totals were computed from raw links without filtering Changes: - Filter CGNAT (RFC 6598) IPs from device subscriber links - Deduplicate accounts across devices by match method priority - Detect routers via SNMP sys_descr and zero out per-device counts - Derive site totals from filtered per-device impacts - Add per-device Subs/MRR columns to site device tables - Add device link fallback for sites without IP blocks - Fix Gaiia sync case mismatch and execution order - Improve device list toolbar button icons and tooltips --- lib/towerops/gaiia.ex | 126 ++++-- lib/towerops/gaiia/site_aggregation.ex | 56 ++- lib/towerops/gaiia/sync.ex | 6 +- .../live/device_live/index.html.heex | 11 +- lib/towerops_web/live/site_live/show.ex | 15 + .../live/site_live/show.html.heex | 20 + test/towerops/gaiia/site_aggregation_test.exs | 64 +++ test/towerops/gaiia/sync_test.exs | 104 ++++- test/towerops/gaiia_test.exs | 382 ++++++++++++++++++ test/towerops_web/live/site_live_test.exs | 6 +- 10 files changed, 741 insertions(+), 49 deletions(-) diff --git a/lib/towerops/gaiia.ex b/lib/towerops/gaiia.ex index a8801bda..edd88aa5 100644 --- a/lib/towerops/gaiia.ex +++ b/lib/towerops/gaiia.ex @@ -13,6 +13,7 @@ defmodule Towerops.Gaiia do alias Towerops.Gaiia.NetworkSite alias Towerops.Repo alias Towerops.Sites.Site + alias Towerops.Snmp.Device, as: SnmpDevice # --- Accounts --- @@ -303,33 +304,94 @@ defmodule Towerops.Gaiia do @doc """ Get subscriber impact for multiple devices (batch). Returns `%{device_id => %{subscriber_count: integer, mrr: Decimal.t()}}`. + + Deduplicates accounts across devices: if the same account is linked to + multiple devices, it is attributed only to the device with the + highest-priority match method (wireless > ARP > site fallback). """ def get_devices_impact(device_ids) when is_list(device_ids) do if device_ids == [] do %{} else - # Get account links per device + # Fetch all links with account gaiia_id for MRR lookup links = DeviceSubscriberLink |> where([l], l.device_id in ^device_ids) |> join(:inner, [l], a in Account, on: l.gaiia_account_id == a.id) - |> join(:left, [l, a], bs in BillingSubscription, - on: bs.account_gaiia_id == a.gaiia_id and bs.organization_id == a.organization_id and bs.status == "ACTIVE" - ) - |> group_by([l, a, bs], l.device_id) - |> select([l, a, bs], %{ + |> select([l, a], %{ device_id: l.device_id, - subscriber_count: count(a.id, :distinct), - mrr: coalesce(sum(bs.mrr_amount), 0) + gaiia_account_id: l.gaiia_account_id, + account_gaiia_id: a.gaiia_id, + organization_id: a.organization_id, + match_method: l.match_method, + subscriber_ip: l.subscriber_ip }) |> Repo.all() + |> Enum.reject(&cgnat_ip?/1) - Map.new(links, fn row -> - {row.device_id, %{subscriber_count: row.subscriber_count, mrr: row.mrr}} + # Deduplicate: per account, keep only the link with the best match method + best_links = + links + |> Enum.group_by(& &1.gaiia_account_id) + |> Enum.map(fn {_acct_id, acct_links} -> + Enum.min_by(acct_links, &match_method_priority/1) + end) + + # Group by device and compute subscriber counts + MRR + best_links + |> Enum.group_by(& &1.device_id) + |> Map.new(fn {device_id, device_links} -> + account_gaiia_ids = + device_links + |> Enum.map(& &1.account_gaiia_id) + |> Enum.uniq() + + org_id = hd(device_links).organization_id + + mrr = compute_mrr_for_accounts(org_id, account_gaiia_ids) + + {device_id, %{subscriber_count: length(account_gaiia_ids), mrr: mrr}} end) end end + defp match_method_priority(%{match_method: "wireless_mac"}), do: 1 + defp match_method_priority(%{match_method: "wireless_ip"}), do: 2 + defp match_method_priority(%{match_method: "arp_mac"}), do: 3 + defp match_method_priority(%{match_method: "arp_ip"}), do: 4 + defp match_method_priority(%{match_method: "site_fallback"}), do: 5 + defp match_method_priority(_), do: 6 + + # RFC 6598 CGNAT range: 100.64.0.0/10 (100.64.0.0 – 100.127.255.255) + # These IPs appear in router ARP tables but not on APs, causing false matches. + defp cgnat_ip?(%{subscriber_ip: nil}), do: false + defp cgnat_ip?(%{subscriber_ip: ""}), do: false + + defp cgnat_ip?(%{subscriber_ip: ip}) do + case String.split(ip, ".") do + [first, second | _] -> + with {100, ""} <- Integer.parse(first), + {octet2, ""} <- Integer.parse(second) do + octet2 >= 64 and octet2 <= 127 + else + _ -> false + end + + _ -> + false + end + end + + defp compute_mrr_for_accounts(_org_id, []), do: Decimal.new(0) + + defp compute_mrr_for_accounts(org_id, account_gaiia_ids) do + BillingSubscription + |> where(organization_id: ^org_id) + |> where([bs], bs.account_gaiia_id in ^account_gaiia_ids) + |> where([bs], bs.status == "ACTIVE") + |> Repo.aggregate(:sum, :mrr_amount) || Decimal.new(0) + end + @doc """ Get subscriber impact for a site (all devices at site). Returns `%{subscriber_count: integer, mrr: Decimal.t(), devices: [%{device_id, name, count, mrr}]}`. @@ -343,38 +405,38 @@ defmodule Towerops.Gaiia do impact = get_devices_impact(device_ids) + # Detect router devices via SNMP sys_descr + router_device_ids = + SnmpDevice + |> where([sd], sd.device_id in ^device_ids) + |> where([sd], ilike(sd.sys_descr, "%RouterOS%")) + |> select([sd], sd.device_id) + |> Repo.all() + |> MapSet.new() + devices_with_names = Device |> where([d], d.id in ^device_ids) |> select([d], {d.id, d.name}) |> Repo.all() |> Enum.map(fn {id, name} -> - di = Map.get(impact, id, %{subscriber_count: 0, mrr: Decimal.new(0)}) - %{device_id: id, name: name, count: di.subscriber_count, mrr: di.mrr} + if MapSet.member?(router_device_ids, id) do + %{device_id: id, name: name, count: 0, mrr: Decimal.new(0)} + else + di = Map.get(impact, id, %{subscriber_count: 0, mrr: Decimal.new(0)}) + %{device_id: id, name: name, count: di.subscriber_count, mrr: di.mrr} + end end) - total_account_ids = - DeviceSubscriberLink - |> where([l], l.device_id in ^device_ids) - |> select([l], l.gaiia_account_id) - |> Repo.all() - |> Enum.uniq() - - total_mrr = - if total_account_ids == [] do - Decimal.new(0) - else - BillingSubscription - |> join(:inner, [bs], a in Account, - on: bs.account_gaiia_id == a.gaiia_id and bs.organization_id == a.organization_id - ) - |> where([bs, a], a.id in ^total_account_ids) - |> where([bs], bs.status == "ACTIVE") - |> Repo.aggregate(:sum, :mrr_amount) || Decimal.new(0) - end + # Site totals = sum of non-router per-device impacts + # (already CGNAT-filtered and deduplicated) + {total_subs, total_mrr} = + Enum.reduce(devices_with_names, {0, Decimal.new(0)}, fn dev, {subs, mrr} -> + {subs + dev.count, Decimal.add(mrr, dev.mrr)} + end) %{ - subscriber_count: length(total_account_ids), + subscriber_count: total_subs, mrr: total_mrr, devices: devices_with_names } diff --git a/lib/towerops/gaiia/site_aggregation.ex b/lib/towerops/gaiia/site_aggregation.ex index c374e361..90e398fe 100644 --- a/lib/towerops/gaiia/site_aggregation.ex +++ b/lib/towerops/gaiia/site_aggregation.ex @@ -1,13 +1,20 @@ defmodule Towerops.Gaiia.SiteAggregation do @moduledoc """ - Computes per-site subscriber counts and MRR by matching inventory item - IP addresses against network site CIDR blocks. + Computes per-site subscriber counts and MRR. + + Two strategies: + 1. **CIDR matching** — for sites with IP blocks, matches inventory item IPs + against CIDR ranges to find accounts. + 2. **Device link fallback** — for sites without IP blocks that are mapped to + a Towerops site, aggregates from `device_subscriber_links`. """ import Ecto.Query + alias Towerops.Devices.Device alias Towerops.Gaiia.Account alias Towerops.Gaiia.BillingSubscription + alias Towerops.Gaiia.DeviceSubscriberLink alias Towerops.Gaiia.InventoryItem alias Towerops.Gaiia.NetworkSite alias Towerops.Repo @@ -15,13 +22,17 @@ defmodule Towerops.Gaiia.SiteAggregation do require Logger @doc """ - For each network site with IP blocks, finds inventory items whose IP addresses - fall within those blocks, then counts unique accounts with active subscriptions - and sums their MRR. - - Updates each network site's `account_count` and `total_mrr` fields. + Computes and stores subscriber counts and MRR for all network sites + in the organization. Uses CIDR matching when IP blocks are available, + falls back to device_subscriber_links for mapped sites without IP blocks. """ def compute_and_store(organization_id) do + compute_cidr_sites(organization_id) + compute_linked_sites(organization_id) + end + + # Sites with IP blocks: match inventory item IPs against CIDR ranges. + defp compute_cidr_sites(organization_id) do sites = load_sites_with_ip_blocks(organization_id) items = load_assigned_items(organization_id) @@ -33,6 +44,18 @@ defmodule Towerops.Gaiia.SiteAggregation do end) end + # Sites without IP blocks but mapped to a Towerops site: aggregate from + # device_subscriber_links for devices at that Towerops site. + defp compute_linked_sites(organization_id) do + sites = load_mapped_sites_without_ip_blocks(organization_id) + + Enum.each(sites, fn site -> + account_gaiia_ids = linked_account_gaiia_ids(organization_id, site.site_id) + {account_count, total_mrr} = compute_totals(organization_id, account_gaiia_ids) + update_site_totals(site, account_count, total_mrr) + end) + end + defp load_sites_with_ip_blocks(organization_id) do NetworkSite |> where(organization_id: ^organization_id) @@ -40,6 +63,25 @@ defmodule Towerops.Gaiia.SiteAggregation do |> Repo.all() end + defp load_mapped_sites_without_ip_blocks(organization_id) do + NetworkSite + |> where(organization_id: ^organization_id) + |> where([ns], not is_nil(ns.site_id)) + |> where([ns], fragment("cardinality(?) = 0", ns.ip_blocks)) + |> Repo.all() + end + + defp linked_account_gaiia_ids(organization_id, towerops_site_id) do + DeviceSubscriberLink + |> join(:inner, [l], d in Device, on: l.device_id == d.id) + |> join(:inner, [l, _d], a in Account, on: l.gaiia_account_id == a.id) + |> where([l, d, _a], d.site_id == ^towerops_site_id) + |> where([l, _d, _a], l.organization_id == ^organization_id) + |> select([_l, _d, a], a.gaiia_id) + |> distinct(true) + |> Repo.all() + end + defp load_assigned_items(organization_id) do InventoryItem |> where(organization_id: ^organization_id) diff --git a/lib/towerops/gaiia/sync.ex b/lib/towerops/gaiia/sync.ex index 593ebb88..80ff1898 100644 --- a/lib/towerops/gaiia/sync.ex +++ b/lib/towerops/gaiia/sync.ex @@ -39,8 +39,8 @@ defmodule Towerops.Gaiia.Sync do Integrations.update_sync_status(integration, "success", message) persist_billing_totals(integration) - Gaiia.SiteAggregation.compute_and_store(org_id) SubscriberMatching.refresh_device_links(org_id) + Gaiia.SiteAggregation.compute_and_store(org_id) {:ok, %{ @@ -119,8 +119,8 @@ defmodule Towerops.Gaiia.Sync do {account_id, site_id} = case assignation do - %{"assigneeType" => "Account", "assignee" => %{"id" => id}} -> {id, nil} - %{"assigneeType" => "NetworkSite", "assignee" => %{"id" => id}} -> {nil, id} + %{"assigneeType" => "ACCOUNT", "assignee" => %{"id" => id}} -> {id, nil} + %{"assigneeType" => "NETWORK_SITE", "assignee" => %{"id" => id}} -> {nil, id} _ -> {nil, nil} end diff --git a/lib/towerops_web/live/device_live/index.html.heex b/lib/towerops_web/live/device_live/index.html.heex index 55a1adf4..0fbeb3d7 100644 --- a/lib/towerops_web/live/device_live/index.html.heex +++ b/lib/towerops_web/live/device_live/index.html.heex @@ -91,15 +91,19 @@ <.icon name="hero-check" class="h-4 w-4" /> Done <% else %> - <.button type="button" phx-click="toggle_reorder_mode"> - <.icon name="hero-bars-3" class="h-4 w-4" /> + <.button type="button" phx-click="toggle_reorder_mode" title="Reorder devices and sites"> + <.icon name="hero-bars-arrow-up" class="h-4 w-4" /> <.button type="button" phx-click="toggle_compact_mode" + title={if @compact_mode, do: "Normal view", else: "Compact view"} variant={if @compact_mode, do: "primary", else: nil} > - <.icon name="hero-bars-2" class="h-4 w-4" /> + <.icon + name={if @compact_mode, do: "hero-view-columns", else: "hero-table-cells"} + class="h-4 w-4" + /> <% end %> <% end %> @@ -110,6 +114,7 @@ <.button type="button" phx-click="force_rediscover_all" + title="Rediscover all devices" data-confirm={ t("This will trigger SNMP discovery for all SNMP-enabled devices. Continue?") } diff --git a/lib/towerops_web/live/site_live/show.ex b/lib/towerops_web/live/site_live/show.ex index 29ee1303..769313eb 100644 --- a/lib/towerops_web/live/site_live/show.ex +++ b/lib/towerops_web/live/site_live/show.ex @@ -51,12 +51,19 @@ defmodule ToweropsWeb.SiteLive.Show do _ -> %{subscriber_count: 0, mrr: nil, devices: []} end + # Build per-device impact lookup for device health table + device_impact = Map.new(site_impact.devices, fn d -> {d.device_id, d} end) + + # Use site_impact as fallback when network site isn't mapped + site_summary = merge_impact_into_summary(site_summary, site_impact) + {:noreply, socket |> assign(:page_title, site.name) |> assign(:site, site) |> assign(:device, devices) |> assign(:site_impact, site_impact) + |> assign(:device_impact, device_impact) |> assign(:latency_chart_data, latency_chart_data) |> assign(:site_summary, site_summary) |> assign(:qoe_summary, qoe_summary) @@ -125,6 +132,14 @@ defmodule ToweropsWeb.SiteLive.Show do } end + defp merge_impact_into_summary(summary, impact) do + if impact.subscriber_count > 0 do + %{summary | subscribers: impact.subscriber_count, mrr: impact.mrr} + else + summary + end + end + defp format_mrr(nil), do: "$0" defp format_mrr(%Decimal{} = amount) do diff --git a/lib/towerops_web/live/site_live/show.html.heex b/lib/towerops_web/live/site_live/show.html.heex index 64a7a1cf..814fb294 100644 --- a/lib/towerops_web/live/site_live/show.html.heex +++ b/lib/towerops_web/live/site_live/show.html.heex @@ -381,6 +381,10 @@ {t("Device")} {t("IP Address")} + {t("Subs")} + <%= if @can_view_financials do %> + {t("MRR")} + <% end %> {t("Latency")} @@ -404,6 +408,22 @@ {eq.ip_address} + + <%= if di = @device_impact[eq.id] do %> + {if di.count > 0, do: di.count, else: "—"} + <% else %> + — + <% end %> + + <%= if @can_view_financials do %> + + <%= if di = @device_impact[eq.id] do %> + {if Decimal.gt?(di.mrr, 0), do: format_mrr(di.mrr), else: "—"} + <% else %> + — + <% end %> + + <% end %> "item-ns", + "status" => "FUNCTIONAL", + "ipAddressV4" => "10.0.0.50", + "model" => %{ + "name" => "Switch", + "manufacturer" => %{"name" => "Cambium"}, + "category" => %{"name" => "Infrastructure"} + }, + "assignation" => %{ + "assigneeType" => "NETWORK_SITE", + "assignee" => %{"id" => "site-456"} + }, + "fields" => %{"edges" => []} + } + ] + ) + + {:ok, _} = Sync.sync_organization(integration) + + [item] = Gaiia.list_inventory_items(org.id) + assert item.assigned_network_site_gaiia_id == "site-456" + assert is_nil(item.assigned_account_gaiia_id) + end + + test "ignores INVENTORY_LOCATION assigneeType", %{org: org, integration: integration} do + stub_gaiia_api( + inventory_items: [ + %{ + "id" => "item-loc", + "status" => "IN_STOCK", + "ipAddressV4" => nil, + "model" => %{ + "name" => "Spare Router", + "manufacturer" => %{"name" => "MikroTik"}, + "category" => %{"name" => "Router"} + }, + "assignation" => %{ + "assigneeType" => "INVENTORY_LOCATION", + "assignee" => %{"id" => "loc-789"} + }, + "fields" => %{"edges" => []} + } + ] + ) + + {:ok, _} = Sync.sync_organization(integration) + + [item] = Gaiia.list_inventory_items(org.id) + assert is_nil(item.assigned_account_gaiia_id) + assert is_nil(item.assigned_network_site_gaiia_id) + end + test "handles nil root key in API response gracefully", %{integration: integration} do Req.Test.stub(Client, fn conn -> {:ok, body, _conn} = Plug.Conn.read_body(conn) @@ -179,6 +238,49 @@ defmodule Towerops.Gaiia.SyncTest do end end + defp stub_gaiia_api(opts) do + items = Keyword.get(opts, :inventory_items, []) + + Req.Test.stub(Client, fn conn -> + {:ok, body, _conn} = Plug.Conn.read_body(conn) + decoded = Jason.decode!(body) + query = decoded["query"] + + response = + cond do + String.contains?(query, "ListAccounts") -> + empty_relay_response(query) + + String.contains?(query, "ListNetworkSites") -> + empty_relay_response(query) + + String.contains?(query, "ListInventoryItems") -> + paginated_response("inventoryItems", items) + + String.contains?(query, "ListBillingSubscriptions") -> + empty_relay_response(query) + + true -> + %{"data" => %{}} + end + + Req.Test.json(conn, response) + end) + end + + defp paginated_response(root_key, nodes) do + edges = Enum.map(nodes, fn node -> %{"node" => node} end) + + %{ + "data" => %{ + root_key => %{ + "edges" => edges, + "pageInfo" => %{"hasNextPage" => false, "endCursor" => nil} + } + } + } + end + defp stub_all_endpoints do Req.Test.stub(Client, fn conn -> {:ok, body, _conn} = Plug.Conn.read_body(conn) @@ -286,7 +388,7 @@ defmodule Towerops.Gaiia.SyncTest do "category" => %{"name" => "AP"} }, "assignation" => %{ - "assigneeType" => "Account", + "assigneeType" => "ACCOUNT", "assignee" => %{"id" => "acct-1"} }, "fields" => %{ diff --git a/test/towerops/gaiia_test.exs b/test/towerops/gaiia_test.exs index 1c951ac1..ef9ac31b 100644 --- a/test/towerops/gaiia_test.exs +++ b/test/towerops/gaiia_test.exs @@ -6,6 +6,7 @@ defmodule Towerops.GaiiaTest do import Towerops.OrganizationsFixtures alias Towerops.Gaiia + alias Towerops.Snmp.Device setup do user = user_fixture() @@ -169,6 +170,387 @@ defmodule Towerops.GaiiaTest do end end + describe "get_devices_impact/1" do + test "deduplicates accounts across devices, preferring higher-confidence matches", %{org: org} do + site = create_site(org.id) + + # Create two devices: a router and an AP + router = device_fixture(%{organization_id: org.id, site: site, name: "Router"}) + ap = device_fixture(%{organization_id: org.id, site: site, name: "AP-1"}) + + # Create an account with an active subscription + {:ok, account} = + Gaiia.upsert_account(org.id, %{ + gaiia_id: "acct-1", + name: "Alice", + subscription_count: 1 + }) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-1", + account_gaiia_id: "acct-1", + status: "ACTIVE", + mrr_amount: Decimal.new("50.00") + }) + + now = DateTime.truncate(DateTime.utc_now(), :second) + + # Same account linked to AP via wireless_mac (high confidence) + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: ap.id, + gaiia_account_id: account.id, + match_method: "wireless_mac", + confidence: "high", + subscriber_mac: "aa:bb:cc:dd:ee:ff", + inserted_at: now, + updated_at: now + }) + + # Same account linked to Router via arp_ip (medium confidence) + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: router.id, + gaiia_account_id: account.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "10.0.0.5", + inserted_at: now, + updated_at: now + }) + + impact = Gaiia.get_devices_impact([router.id, ap.id]) + + # Account should be counted only on the AP (wireless_mac beats arp_ip) + assert impact[ap.id].subscriber_count == 1 + assert Decimal.equal?(impact[ap.id].mrr, Decimal.new("50.00")) + + # Router should have 0 subscribers (deduped to AP) + router_impact = Map.get(impact, router.id, %{subscriber_count: 0, mrr: Decimal.new(0)}) + assert router_impact.subscriber_count == 0 + end + + test "keeps ARP match on router when no higher-confidence match exists elsewhere", %{org: org} do + site = create_site(org.id) + router = device_fixture(%{organization_id: org.id, site: site, name: "Router"}) + ap = device_fixture(%{organization_id: org.id, site: site, name: "AP-1"}) + + {:ok, account} = + Gaiia.upsert_account(org.id, %{ + gaiia_id: "acct-only-router", + name: "Bob", + subscription_count: 1 + }) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-bob", + account_gaiia_id: "acct-only-router", + status: "ACTIVE", + mrr_amount: Decimal.new("75.00") + }) + + now = DateTime.truncate(DateTime.utc_now(), :second) + + # Account only linked to router via ARP (no AP match) + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: router.id, + gaiia_account_id: account.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "10.0.0.10", + inserted_at: now, + updated_at: now + }) + + impact = Gaiia.get_devices_impact([router.id, ap.id]) + + # Router should keep the account since no better match exists + assert impact[router.id].subscriber_count == 1 + assert Decimal.equal?(impact[router.id].mrr, Decimal.new("75.00")) + end + + test "deduplicates across same-method matches, keeping first device", %{org: org} do + site = create_site(org.id) + router = device_fixture(%{organization_id: org.id, site: site, name: "Router"}) + ap = device_fixture(%{organization_id: org.id, site: site, name: "AP-1"}) + + {:ok, account} = + Gaiia.upsert_account(org.id, %{ + gaiia_id: "acct-both-arp", + name: "Charlie", + subscription_count: 1 + }) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-charlie", + account_gaiia_id: "acct-both-arp", + status: "ACTIVE", + mrr_amount: Decimal.new("60.00") + }) + + now = DateTime.truncate(DateTime.utc_now(), :second) + + # Same account linked to both devices via arp_ip (same method) + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: ap.id, + gaiia_account_id: account.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "10.0.0.20", + inserted_at: now, + updated_at: now + }) + + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: router.id, + gaiia_account_id: account.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "100.64.0.20", + inserted_at: now, + updated_at: now + }) + + impact = Gaiia.get_devices_impact([router.id, ap.id]) + + # Account should be counted on exactly one device (not both) + router_count = get_in(impact, [router.id, :subscriber_count]) || 0 + ap_count = get_in(impact, [ap.id, :subscriber_count]) || 0 + assert router_count + ap_count == 1 + end + + test "excludes links matched through CGNAT IPs (100.64.0.0/10)", %{org: org} do + site = create_site(org.id) + router = device_fixture(%{organization_id: org.id, site: site, name: "Router"}) + + {:ok, account} = + Gaiia.upsert_account(org.id, %{ + gaiia_id: "acct-cgnat", + name: "CGNAT Sub", + subscription_count: 1 + }) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-cgnat", + account_gaiia_id: "acct-cgnat", + status: "ACTIVE", + mrr_amount: Decimal.new("50.00") + }) + + now = DateTime.truncate(DateTime.utc_now(), :second) + + # Link via CGNAT IP — should be excluded from per-device counts + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: router.id, + gaiia_account_id: account.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "100.64.62.50", + inserted_at: now, + updated_at: now + }) + + impact = Gaiia.get_devices_impact([router.id]) + + router_impact = Map.get(impact, router.id, %{subscriber_count: 0, mrr: Decimal.new(0)}) + assert router_impact.subscriber_count == 0 + end + + test "keeps links with non-CGNAT IPs", %{org: org} do + site = create_site(org.id) + ap = device_fixture(%{organization_id: org.id, site: site, name: "AP-1"}) + + {:ok, account} = + Gaiia.upsert_account(org.id, %{ + gaiia_id: "acct-lan", + name: "LAN Sub", + subscription_count: 1 + }) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-lan", + account_gaiia_id: "acct-lan", + status: "ACTIVE", + mrr_amount: Decimal.new("80.00") + }) + + now = DateTime.truncate(DateTime.utc_now(), :second) + + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: ap.id, + gaiia_account_id: account.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "10.0.0.5", + inserted_at: now, + updated_at: now + }) + + impact = Gaiia.get_devices_impact([ap.id]) + + assert impact[ap.id].subscriber_count == 1 + assert Decimal.equal?(impact[ap.id].mrr, Decimal.new("80.00")) + end + end + + describe "get_site_impact/1" do + test "excludes router devices from per-device impact", %{org: org} do + site = create_site(org.id) + router = device_fixture(%{organization_id: org.id, site: site, name: "Router"}) + ap = device_fixture(%{organization_id: org.id, site: site, name: "AP-1"}) + + # Mark router as a RouterOS device via SNMP + Repo.insert!(%Device{ + device_id: router.id, + sys_descr: "RouterOS CCR1009-7G-1C-1S+", + sys_object_id: "1.3.6.1.4.1.14988.1", + sys_name: "router" + }) + + {:ok, account} = + Gaiia.upsert_account(org.id, %{ + gaiia_id: "acct-r", + name: "Router Sub", + subscription_count: 1 + }) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-r", + account_gaiia_id: "acct-r", + status: "ACTIVE", + mrr_amount: Decimal.new("50.00") + }) + + now = DateTime.truncate(DateTime.utc_now(), :second) + + # Link to router via LAN IP + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: router.id, + gaiia_account_id: account.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "10.0.0.5", + inserted_at: now, + updated_at: now + }) + + result = Gaiia.get_site_impact(site.id) + + # Site totals should exclude router-only subscribers + assert result.subscriber_count == 0 + assert Decimal.equal?(result.mrr, Decimal.new(0)) + + # Router's per-device entry should show 0 + router_device = Enum.find(result.devices, &(&1.device_id == router.id)) + assert router_device.count == 0 + + # AP should show 0 (no links to it) + ap_device = Enum.find(result.devices, &(&1.device_id == ap.id)) + assert ap_device.count == 0 + end + + test "site totals equal sum of non-router per-device impacts", %{org: org} do + site = create_site(org.id) + router = device_fixture(%{organization_id: org.id, site: site, name: "Router"}) + ap1 = device_fixture(%{organization_id: org.id, site: site, name: "AP-1"}) + ap2 = device_fixture(%{organization_id: org.id, site: site, name: "AP-2"}) + + Repo.insert!(%Device{ + device_id: router.id, + sys_descr: "RouterOS CCR1009-7G-1C-1S+", + sys_object_id: "1.3.6.1.4.1.14988.1", + sys_name: "router" + }) + + {:ok, acct1} = + Gaiia.upsert_account(org.id, %{gaiia_id: "acct-a1", name: "Sub A", subscription_count: 1}) + + {:ok, acct2} = + Gaiia.upsert_account(org.id, %{gaiia_id: "acct-a2", name: "Sub B", subscription_count: 1}) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-a1", + account_gaiia_id: "acct-a1", + status: "ACTIVE", + mrr_amount: Decimal.new("40.00") + }) + + {:ok, _} = + Gaiia.upsert_billing_subscription(org.id, %{ + gaiia_id: "sub-a2", + account_gaiia_id: "acct-a2", + status: "ACTIVE", + mrr_amount: Decimal.new("60.00") + }) + + now = DateTime.truncate(DateTime.utc_now(), :second) + + # acct1 on AP-1 and router (AP wins via wireless_mac) + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: ap1.id, + gaiia_account_id: acct1.id, + match_method: "wireless_mac", + confidence: "high", + subscriber_mac: "aa:bb:cc:dd:ee:01", + inserted_at: now, + updated_at: now + }) + + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: router.id, + gaiia_account_id: acct1.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "10.0.0.1", + inserted_at: now, + updated_at: now + }) + + # acct2 on AP-2 only + Repo.insert!(%Gaiia.DeviceSubscriberLink{ + organization_id: org.id, + device_id: ap2.id, + gaiia_account_id: acct2.id, + match_method: "arp_ip", + confidence: "medium", + subscriber_ip: "10.0.0.2", + inserted_at: now, + updated_at: now + }) + + result = Gaiia.get_site_impact(site.id) + + # Site totals = AP-1 (1 sub, $40) + AP-2 (1 sub, $60) = 2 subs, $100 + assert result.subscriber_count == 2 + assert Decimal.equal?(result.mrr, Decimal.new("100.00")) + + # Per-device + ap1_dev = Enum.find(result.devices, &(&1.device_id == ap1.id)) + assert ap1_dev.count == 1 + + ap2_dev = Enum.find(result.devices, &(&1.device_id == ap2.id)) + assert ap2_dev.count == 1 + + router_dev = Enum.find(result.devices, &(&1.device_id == router.id)) + assert router_dev.count == 0 + end + end + describe "get_site_subscriber_summary/1" do test "returns subscriber data for a mapped site", %{org: org} do site = create_site(org.id) diff --git a/test/towerops_web/live/site_live_test.exs b/test/towerops_web/live/site_live_test.exs index 5dc08251..3f8d0037 100644 --- a/test/towerops_web/live/site_live_test.exs +++ b/test/towerops_web/live/site_live_test.exs @@ -225,7 +225,7 @@ defmodule ToweropsWeb.SiteLiveTest do assert html =~ "$3,750" end - test "hides subscriber badge when no Gaiia mapping", %{ + test "shows zero subscribers when no Gaiia mapping", %{ conn: conn, organization: organization } do @@ -242,8 +242,8 @@ defmodule ToweropsWeb.SiteLiveTest do {:ok, _view, html} = live(conn, ~p"/sites/#{site.id}") - refute html =~ "subscribers" - refute html =~ "MRR" + assert html =~ "Subscribers" + assert html =~ "$0" end end