towerops/lib/towerops_web/live/device_live/components/preseem_tab.ex
Graham McIntire c4e301a84c fix: address code review findings - memory leaks and redundant queries (#192)
Fixed 5 critical issues from code review:

1. **GlobalSearchTrigger hook listener leak**
   - Added destroyed() callback to remove click listener
   - Prevents memory leak on LiveView unmount

2. **NetworkMap/WeathermapViewer hook listener leaks**
   - Store zoom/fit button handlers as properties
   - Remove DOM event listeners in destroyed() callback
   - Fixes leak on every LiveView remount

3. **SitesMap uses undocumented window.liveSocket.execJS**
   - Changed to proper LiveView pattern using pushEvent
   - Attach listener via Leaflet's popupopen event

4. **handle_info(:refresh_data) double DB fetch**
   - Removed redundant Devices.get_device call
   - Reuse device from assign_base_data

5. **Redundant DB queries in DeviceLive.Show overview**
   - Changed load_sensor_chart_data to accept snmp_device
   - Changed load_overall_traffic_chart_data to accept snmp_device
   - Eliminated 4 redundant Snmp.get_device_with_associations calls

Also removed unused functions:
- load_equipment_data/2
- reload_active_tab_data/1
- assign_subscriber_impact/1

Note: DeviceLive.Show module size (2252 lines) is acknowledged but
deferred as it requires larger architectural refactoring.

Reviewed-on: graham/towerops-web#192
2026-03-27 16:34:42 -05:00

291 lines
13 KiB
Elixir

defmodule ToweropsWeb.DeviceLive.Components.PreseemTab do
@moduledoc """
LiveComponent for the device Preseem integration tab.
Displays Preseem access point data, subscriber metrics, and device insights.
"""
use ToweropsWeb, :live_component
alias ToweropsWeb.DeviceLive.Helpers.DataLoaders
@impl true
def update(assigns, socket) do
{:ok,
socket
|> assign(assigns)
|> load_preseem_data()}
end
defp load_preseem_data(socket) do
device = socket.assigns.device
preseem_data = DataLoaders.load_preseem_data(device)
socket
|> assign(:preseem_access_point, preseem_data.preseem_access_point)
|> assign(:preseem_metrics, preseem_data.preseem_metrics)
|> assign(:preseem_insights, preseem_data.preseem_insights)
end
@impl true
def render(assigns) do
~H"""
<div id="preseem-tab">
<%= if @preseem_access_point do %>
<div class="space-y-6">
<%!-- Insights section --%>
<%= if @preseem_insights != [] do %>
<div class="space-y-3">
<%= for insight <- @preseem_insights do %>
<div class={[
"rounded-lg border p-4 flex items-start justify-between",
case insight.urgency do
"critical" ->
"bg-red-50 border-red-200 dark:bg-red-900/20 dark:border-red-800"
"warning" ->
"bg-yellow-50 border-yellow-200 dark:bg-yellow-900/20 dark:border-yellow-800"
_ ->
"bg-blue-50 border-blue-200 dark:bg-blue-900/20 dark:border-blue-800"
end
]}>
<div class="flex items-start gap-3">
<div class={[
"mt-0.5",
case insight.urgency do
"critical" -> "text-red-500"
"warning" -> "text-yellow-500"
_ -> "text-blue-500"
end
]}>
<%= case insight.urgency do %>
<% "critical" -> %>
<.icon name="hero-exclamation-triangle" class="h-5 w-5" />
<% "warning" -> %>
<.icon name="hero-exclamation-circle" class="h-5 w-5" />
<% _ -> %>
<.icon name="hero-information-circle" class="h-5 w-5" />
<% end %>
</div>
<div>
<p class="text-sm font-medium text-gray-900 dark:text-white">
{insight.title}
</p>
<p class="mt-1 text-sm text-gray-600 dark:text-gray-300">
{insight.description}
</p>
</div>
</div>
<button
phx-click="dismiss_insight"
phx-value-id={insight.id}
phx-target={@myself}
class="ml-4 text-gray-400 hover:text-gray-600 dark:hover:text-gray-300 flex-shrink-0"
title="Dismiss"
>
<.icon name="hero-x-mark" class="h-4 w-4" />
</button>
</div>
<% end %>
</div>
<% end %>
<%!-- Score cards --%>
<div class="grid grid-cols-1 sm:grid-cols-3 gap-4">
<div class="bg-white dark:bg-gray-800/50 rounded-lg border border-gray-200 dark:border-white/10 p-4">
<div class="text-sm font-medium text-gray-500 dark:text-gray-400">QoE Score</div>
<div class="mt-1 text-2xl font-semibold text-gray-900 dark:text-white">
{if @preseem_access_point.qoe_score,
do: Float.round(@preseem_access_point.qoe_score, 1),
else: "---"}
</div>
</div>
<div class="bg-white dark:bg-gray-800/50 rounded-lg border border-gray-200 dark:border-white/10 p-4">
<div class="text-sm font-medium text-gray-500 dark:text-gray-400">
{t("Capacity Score")}
</div>
<div class="mt-1 text-2xl font-semibold text-gray-900 dark:text-white">
{if @preseem_access_point.capacity_score,
do: Float.round(@preseem_access_point.capacity_score, 1),
else: "---"}
</div>
</div>
<div class="bg-white dark:bg-gray-800/50 rounded-lg border border-gray-200 dark:border-white/10 p-4">
<div class="text-sm font-medium text-gray-500 dark:text-gray-400">RF Score</div>
<div class="mt-1 text-2xl font-semibold text-gray-900 dark:text-white">
{if @preseem_access_point.rf_score,
do: Float.round(@preseem_access_point.rf_score, 1),
else: "---"}
</div>
</div>
</div>
<%!-- Details card --%>
<div class="bg-white dark:bg-gray-800/50 rounded-lg border border-gray-200 dark:border-white/10">
<div class="px-4 py-3 border-b border-gray-200 dark:border-white/10">
<h3 class="text-sm font-semibold text-gray-900 dark:text-white">
{t("Access Point Details")}
</h3>
</div>
<div class="px-4 py-3">
<dl class="grid grid-cols-1 sm:grid-cols-2 gap-x-4 gap-y-3">
<div>
<dt class="text-sm font-medium text-gray-500 dark:text-gray-400">Name</dt>
<dd class="mt-0.5 text-sm text-gray-900 dark:text-white">
{@preseem_access_point.name || "---"}
</dd>
</div>
<div>
<dt class="text-sm font-medium text-gray-500 dark:text-gray-400">
{t("Preseem ID")}
</dt>
<dd class="mt-0.5 text-sm text-gray-900 dark:text-white">
{@preseem_access_point.preseem_id}
</dd>
</div>
<div>
<dt class="text-sm font-medium text-gray-500 dark:text-gray-400">
{t("Subscribers")}
</dt>
<dd class="mt-0.5 text-sm text-gray-900 dark:text-white">
{@preseem_access_point.subscriber_count || "---"}
</dd>
</div>
<div>
<dt class="text-sm font-medium text-gray-500 dark:text-gray-400">
{t("Busy Hours")}
</dt>
<dd class="mt-0.5 text-sm text-gray-900 dark:text-white">
<%= if @preseem_access_point.busy_hours do %>
{@preseem_access_point.busy_hours} hrs/day
<% else %>
---
<% end %>
</dd>
</div>
<div>
<dt class="text-sm font-medium text-gray-500 dark:text-gray-400">
{t("Airtime Utilization")}
</dt>
<dd class="mt-0.5 text-sm text-gray-900 dark:text-white">
<%= if @preseem_access_point.airtime_utilization do %>
{Float.round(@preseem_access_point.airtime_utilization, 1)}%
<% else %>
---
<% end %>
</dd>
</div>
<div>
<dt class="text-sm font-medium text-gray-500 dark:text-gray-400">
{t("Match Type")}
</dt>
<dd class="mt-0.5 text-sm text-gray-900 dark:text-white">
{@preseem_access_point.match_confidence}
</dd>
</div>
</dl>
</div>
</div>
<%!-- Recent metrics table --%>
<%= if @preseem_metrics != [] do %>
<div class="bg-white dark:bg-gray-800/50 rounded-lg border border-gray-200 dark:border-white/10">
<div class="px-4 py-3 border-b border-gray-200 dark:border-white/10">
<h3 class="text-sm font-semibold text-gray-900 dark:text-white">
{t("Recent QoE Metrics")}
</h3>
</div>
<div class="overflow-x-auto">
<table class="min-w-full divide-y divide-gray-200 dark:divide-white/10">
<thead>
<tr>
<th class="px-4 py-2 text-left text-xs font-medium text-gray-500 dark:text-gray-400 uppercase">
{t("Time")}
</th>
<th class="px-4 py-2 text-right text-xs font-medium text-gray-500 dark:text-gray-400 uppercase">
{t("Latency (ms)")}
</th>
<th class="px-4 py-2 text-right text-xs font-medium text-gray-500 dark:text-gray-400 uppercase">
{t("P95 Latency")}
</th>
<th class="px-4 py-2 text-right text-xs font-medium text-gray-500 dark:text-gray-400 uppercase">
{t("Jitter (ms)")}
</th>
<th class="px-4 py-2 text-right text-xs font-medium text-gray-500 dark:text-gray-400 uppercase">
{t("Loss (%)")}
</th>
<th class="px-4 py-2 text-right text-xs font-medium text-gray-500 dark:text-gray-400 uppercase">
{t("Throughput")}
</th>
<th class="px-4 py-2 text-right text-xs font-medium text-gray-500 dark:text-gray-400 uppercase">
{t("Subscribers")}
</th>
</tr>
</thead>
<tbody class="divide-y divide-gray-200 dark:divide-white/10">
<%= for metric <- @preseem_metrics do %>
<tr>
<td class="px-4 py-2 text-sm text-gray-900 dark:text-white whitespace-nowrap">
{ToweropsWeb.TimeHelpers.format_iso8601(
metric.recorded_at,
@current_scope.timezone,
@current_scope.time_format
)}
</td>
<td class="px-4 py-2 text-sm text-right text-gray-900 dark:text-white">
{if metric.avg_latency,
do: Float.round(metric.avg_latency, 1),
else: "---"}
</td>
<td class="px-4 py-2 text-sm text-right text-gray-900 dark:text-white">
{if metric.p95_latency,
do: Float.round(metric.p95_latency, 1),
else: "---"}
</td>
<td class="px-4 py-2 text-sm text-right text-gray-900 dark:text-white">
{if metric.avg_jitter,
do: Float.round(metric.avg_jitter, 1),
else: "---"}
</td>
<td class="px-4 py-2 text-sm text-right text-gray-900 dark:text-white">
{if metric.avg_loss, do: Float.round(metric.avg_loss, 2), else: "---"}
</td>
<td class="px-4 py-2 text-sm text-right text-gray-900 dark:text-white">
{if metric.avg_throughput,
do: "#{Float.round(metric.avg_throughput, 1)} Mbps",
else: "---"}
</td>
<td class="px-4 py-2 text-sm text-right text-gray-900 dark:text-white">
{metric.subscriber_count || "---"}
</td>
</tr>
<% end %>
</tbody>
</table>
</div>
</div>
<% end %>
</div>
<% else %>
<div class="bg-white dark:bg-gray-800/50 rounded-lg border border-gray-200 dark:border-white/10 p-8 text-center">
<p class="text-sm text-gray-500 dark:text-gray-400">
{t("No Preseem data linked to this device.")}
</p>
</div>
<% end %>
</div>
"""
end
@impl true
def handle_event("dismiss_insight", %{"id" => insight_id}, socket) do
case Towerops.Preseem.dismiss_insight(insight_id) do
{:ok, _} ->
# Reload preseem data after dismissing insight
{:noreply, load_preseem_data(socket)}
{:error, _} ->
{:noreply, put_flash(socket, :error, "Failed to dismiss insight")}
end
end
end