towerops/lib/towerops_web/live/device_live/components/backups_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

315 lines
15 KiB
Elixir

defmodule ToweropsWeb.DeviceLive.Components.BackupsTab do
@moduledoc """
LiveComponent for the device MikroTik backups tab.
Displays MikroTik configuration backups with download, compare,
and backup-now functionality.
"""
use ToweropsWeb, :live_component
alias ToweropsWeb.DeviceLive.Helpers.DataLoaders
alias ToweropsWeb.DeviceLive.Helpers.Formatters
@impl true
def update(assigns, socket) do
{:ok,
socket
|> assign(assigns)
|> load_backups_data()}
end
defp load_backups_data(socket) do
device = socket.assigns.device
mikrotik_backups = DataLoaders.load_mikrotik_backups(device)
socket
|> assign(:mikrotik_backups, mikrotik_backups)
|> assign(:selected_backup_ids, MapSet.new())
end
@impl true
def render(assigns) do
~H"""
<div id="backups-tab">
<%= if false and @device.mikrotik_enabled 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">
<div class="flex items-center justify-between">
<div class="flex items-center gap-2">
<h3 class="text-sm font-semibold text-gray-900 dark:text-white">
{t("Configuration Backups")}
<%= if Enum.any?(@mikrotik_backups) do %>
<span class="ml-2 text-xs font-normal text-gray-500 dark:text-gray-400">
({length(@mikrotik_backups)})
</span>
<% end %>
</h3>
<span class="inline-flex items-center gap-1 px-2 py-0.5 text-xs font-medium text-blue-700 bg-blue-100 rounded dark:bg-blue-900 dark:text-blue-200">
<.icon name="hero-beaker" class="h-3 w-3" /> Experimental
</span>
</div>
<%= if @agent_info.agent_token_id do %>
<button
phx-click="backup_now"
phx-throttle="2000"
class="inline-flex items-center gap-1.5 px-3 py-1.5 text-sm font-medium text-white bg-blue-600 hover:bg-blue-700 rounded-lg transition-colors"
>
<.icon name="hero-arrow-down-tray" class="h-4 w-4" /> Backup Now
</button>
<% else %>
<div class="text-sm text-amber-600 dark:text-amber-400">
{t("Assign an agent to enable backups")}
</div>
<% end %>
</div>
</div>
<%= if Enum.any?(@mikrotik_backups) do %>
<%!-- Instructions for comparing backups --%>
<%= if length(@mikrotik_backups) > 1 do %>
<div class="px-4 py-3 bg-blue-50 dark:bg-blue-950/30 border-b border-blue-100 dark:border-blue-900/50">
<div class="flex items-start gap-2">
<.icon
name="hero-information-circle"
class="h-5 w-5 text-blue-600 dark:text-blue-400 flex-shrink-0 mt-0.5"
/>
<div class="text-sm text-blue-900 dark:text-blue-200">
<span class="font-medium">Compare configurations:</span>
{t("Select any 2 backups using the checkboxes to compare their differences")}
</div>
</div>
</div>
<% end %>
<div class="overflow-x-auto">
<table class="min-w-full divide-y divide-gray-200 dark:divide-white/10">
<thead class="bg-gray-50 dark:bg-gray-800/75">
<tr class="text-xs text-gray-600 dark:text-gray-400">
<th
class="px-4 py-3 text-center font-medium w-12"
title="Select backups to compare"
>
<.icon name="hero-check-circle" class="h-4 w-4 mx-auto" />
</th>
<th class="px-4 py-3 text-left font-medium">Date</th>
<th class="px-4 py-3 text-left font-medium">Original Size</th>
<th class="px-4 py-3 text-left font-medium">Compressed Size</th>
<th class="px-4 py-3 text-left font-medium">Ratio</th>
<th class="px-4 py-3 text-left font-medium">Source</th>
<th class="px-4 py-3 text-left font-medium">Actions</th>
</tr>
</thead>
<tbody class="divide-y divide-gray-200 dark:divide-white/10">
<%= for backup <- @mikrotik_backups do %>
<% compression_ratio =
if backup.config_size_bytes && backup.config_size_bytes > 0 do
Float.round(
(1 - backup.compressed_size_bytes / backup.config_size_bytes) * 100,
1
)
else
0.0
end %>
<tr class="text-sm hover:bg-gray-50 dark:hover:bg-gray-800">
<td class="px-4 py-3 text-center">
<input
type="checkbox"
phx-click="toggle_backup_selection"
phx-value-id={backup.id}
phx-target={@myself}
checked={MapSet.member?(@selected_backup_ids, backup.id)}
disabled={
!MapSet.member?(@selected_backup_ids, backup.id) &&
MapSet.size(@selected_backup_ids) >= 2
}
title={
if MapSet.member?(@selected_backup_ids, backup.id),
do: "Selected for comparison",
else:
if(MapSet.size(@selected_backup_ids) >= 2,
do: "Maximum 2 backups can be selected",
else: "Select to compare"
)
}
class="h-4 w-4 rounded border-gray-300 text-blue-600 focus:ring-blue-600 disabled:opacity-50 disabled:cursor-not-allowed dark:border-gray-600 dark:bg-gray-700"
/>
</td>
<td class="px-4 py-3 text-gray-900 dark:text-white whitespace-nowrap">
{ToweropsWeb.TimeHelpers.format_iso8601(backup.backed_up_at, @timezone)}
</td>
<td class="px-4 py-3 text-gray-600 dark:text-gray-400 font-mono">
{format_bytes(backup.config_size_bytes)}
</td>
<td class="px-4 py-3 text-gray-600 dark:text-gray-400 font-mono">
{format_bytes(backup.compressed_size_bytes)}
</td>
<td class="px-4 py-3 text-gray-600 dark:text-gray-400">
{compression_ratio}%
</td>
<td class="px-4 py-3">
<span class={[
"inline-flex items-center px-2 py-0.5 rounded text-xs font-medium",
backup.trigger_source == "daily_cron" &&
"bg-blue-100 text-blue-800 dark:bg-blue-900 dark:text-blue-200",
backup.trigger_source == "manual" &&
"bg-green-100 text-green-800 dark:bg-green-900 dark:text-green-200",
backup.trigger_source == "pre_change" &&
"bg-yellow-100 text-yellow-800 dark:bg-yellow-900 dark:text-yellow-200"
]}>
{String.replace(backup.trigger_source, "_", " ") |> String.capitalize()}
</span>
</td>
<td class="px-4 py-3">
<div class="flex items-center gap-2">
<button
phx-click="download_backup"
phx-value-id={backup.id}
class="inline-flex items-center gap-1 px-2 py-1 text-xs font-medium text-blue-600 hover:text-blue-800 dark:text-blue-400 dark:hover:text-blue-300"
>
<.icon name="hero-arrow-down-tray" class="h-4 w-4" /> Download
</button>
<%= if ToweropsWeb.Permissions.owner?(@current_scope) do %>
<.button
phx-click="delete_backup"
phx-value-id={backup.id}
data-confirm={
t(
"Are you sure you want to delete this backup? This action cannot be undone."
)
}
variant="danger"
>
<.icon name="hero-trash" class="h-4 w-4" /> Delete
</.button>
<% end %>
</div>
</td>
</tr>
<% end %>
</tbody>
</table>
</div>
<%!-- Pagination --%>
<%= if assigns[:pagination] do %>
<div class="px-4 py-3 border-t border-gray-200 dark:border-white/10">
<.pagination
meta={@pagination}
path={~p"/devices/#{@device.id}"}
params={%{"tab" => "backups"}}
/>
</div>
<% end %>
<%!-- Floating Compare Button --%>
<%= if MapSet.size(@selected_backup_ids) > 0 do %>
<div class="fixed bottom-8 right-8 z-50">
<div class="bg-white dark:bg-gray-800 rounded-lg shadow-lg border border-gray-200 dark:border-white/10 p-4">
<div class="flex flex-col gap-2">
<%!-- Selection status --%>
<div class="flex items-center gap-2 text-sm">
<.icon
name="hero-check-circle"
class="h-5 w-5 text-blue-600 dark:text-blue-400"
/>
<div class="text-gray-700 dark:text-gray-300">
<span class="font-medium">{MapSet.size(@selected_backup_ids)}</span>
of 2 backups selected
</div>
</div>
<%!-- Instructions when only 1 selected --%>
<%= if MapSet.size(@selected_backup_ids) == 1 do %>
<div class="text-xs text-gray-500 dark:text-gray-400 pl-7">
{t("Select one more backup to compare")}
</div>
<% end %>
<%!-- Action buttons --%>
<div class="flex items-center gap-2 mt-1">
<button
phx-click="clear_selection"
phx-target={@myself}
class="inline-flex items-center gap-1.5 px-3 py-2 text-sm font-medium text-gray-700 bg-white hover:bg-gray-50 border border-gray-300 rounded-lg transition-colors dark:bg-gray-700 dark:text-gray-300 dark:border-gray-600 dark:hover:bg-gray-600"
>
{t("Clear")}
</button>
<button
phx-click="compare_selected"
disabled={MapSet.size(@selected_backup_ids) != 2}
class="inline-flex items-center gap-1.5 px-4 py-2 text-sm font-medium text-white bg-blue-600 hover:bg-blue-700 disabled:bg-gray-400 disabled:cursor-not-allowed rounded-lg transition-colors shadow-sm"
>
<.icon name="hero-arrows-right-left" class="h-4 w-4" />
{if MapSet.size(@selected_backup_ids) == 2,
do: "Compare Selected",
else: "Compare"}
</button>
</div>
</div>
</div>
</div>
<% end %>
<% else %>
<div class="p-8 text-center">
<.icon name="hero-document-text" class="mx-auto h-12 w-12 text-gray-400" />
<h3 class="mt-2 text-sm font-semibold text-gray-900 dark:text-white">
{t("No backups yet")}
</h3>
<%= if @agent_info.agent_token_id do %>
<p class="mt-1 text-sm text-gray-500 dark:text-gray-400">
Automatic backups run daily at {ToweropsWeb.TimeHelpers.format_utc_hour(
7,
@timezone
)}.
</p>
<% else %>
<p class="mt-1 text-sm text-amber-600 dark:text-amber-400">
{t("Assign an agent to enable automatic backups.")}
</p>
<% end %>
</div>
<% end %>
</div>
<% else %>
<div class="bg-white dark:bg-gray-800/50 rounded-lg border border-gray-200 dark:border-white/10 p-12">
<div class="text-center">
<.icon name="hero-server" class="mx-auto h-12 w-12 text-gray-400" />
<h3 class="mt-2 text-sm font-semibold text-gray-900 dark:text-white">
{t("MikroTik API not enabled")}
</h3>
<p class="mt-1 text-sm text-gray-500 dark:text-gray-400">
{t("Enable MikroTik API in device settings to enable configuration backups.")}
</p>
<div class="mt-6">
<.button navigate={~p"/devices/#{@device.id}/edit"}>
{t("Edit Device Settings")}
</.button>
</div>
</div>
</div>
<% end %>
</div>
"""
end
@impl true
def handle_event("toggle_backup_selection", %{"id" => backup_id}, socket) do
selected = socket.assigns.selected_backup_ids
updated_selected =
if MapSet.member?(selected, backup_id) do
MapSet.delete(selected, backup_id)
else
# Limit to 2 selections max
if MapSet.size(selected) >= 2 do
selected
else
MapSet.put(selected, backup_id)
end
end
{:noreply, assign(socket, :selected_backup_ids, updated_selected)}
end
@impl true
def handle_event("clear_selection", _params, socket) do
{:noreply, assign(socket, :selected_backup_ids, MapSet.new())}
end
# Helper functions
defp format_bytes(bytes), do: Formatters.format_bytes(bytes)
end