From ce1948645aa1f8fea83cdc8836883da286e5a855 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Sat, 17 Jan 2026 17:11:39 -0600
Subject: [PATCH] Add graph_live tests and reorder traffic/latency charts

- Added comprehensive tests for GraphLive.Show to ensure:
  * Page renders with correct assigns
  * Uses @current_organization (not @organization)
  * Latency, processor, memory, and traffic graphs work
  * Time range selection works correctly
  * All required assigns are present
- Reordered device show page so Overall Traffic appears above Ping Latency
---
 .../live/device_live/show.html.heex           |  60 +--
 .../live/graph_live/show_test.exs             | 343 ++++++++++++++++++
 2 files changed, 373 insertions(+), 30 deletions(-)
 create mode 100644 test/towerops_web/live/graph_live/show_test.exs

diff --git a/lib/towerops_web/live/device_live/show.html.heex b/lib/towerops_web/live/device_live/show.html.heex
index a81e6411..867c33a6 100644
--- a/lib/towerops_web/live/device_live/show.html.heex
+++ b/lib/towerops_web/live/device_live/show.html.heex
@@ -211,36 +211,6 @@
                 </dl>
               </div>
             </div>
-            <!-- Ping Latency Chart -->
-            <%= if @latency_chart_data do %>
-              <div class="bg-white dark:bg-zinc-800 rounded-lg border border-zinc-200 dark:border-zinc-700">
-                <.link
-                  navigate={
-                    ~p"/orgs/#{@current_organization.slug}/devices/#{@device.id}/graph/latency"
-                  }
-                  class="block px-4 py-3 border-b border-zinc-200 dark:border-zinc-700 hover:bg-zinc-50 dark:hover:bg-zinc-750 transition-colors"
-                >
-                  <div class="flex items-center justify-between">
-                    <h3 class="text-sm font-semibold text-zinc-900 dark:text-zinc-100">
-                      Ping Latency
-                    </h3>
-                    <.icon name="hero-arrow-right" class="h-4 w-4 text-zinc-400" />
-                  </div>
-                </.link>
-                <div class="p-4">
-                  <div
-                    id="latency-chart"
-                    phx-hook="SensorChart"
-                    data-chart={@latency_chart_data}
-                    data-unit="ms"
-                    data-auto-scale="true"
-                    style="height: 300px;"
-                  >
-                    <canvas></canvas>
-                  </div>
-                </div>
-              </div>
-            <% end %>
             <!-- Overall Traffic Chart -->
             <%= if @traffic_chart_data do %>
               <div class="bg-white dark:bg-zinc-800 rounded-lg border border-zinc-200 dark:border-zinc-700">
@@ -272,6 +242,36 @@
                 </div>
               </div>
             <% end %>
+            <!-- Ping Latency Chart -->
+            <%= if @latency_chart_data do %>
+              <div class="bg-white dark:bg-zinc-800 rounded-lg border border-zinc-200 dark:border-zinc-700">
+                <.link
+                  navigate={
+                    ~p"/orgs/#{@current_organization.slug}/devices/#{@device.id}/graph/latency"
+                  }
+                  class="block px-4 py-3 border-b border-zinc-200 dark:border-zinc-700 hover:bg-zinc-50 dark:hover:bg-zinc-750 transition-colors"
+                >
+                  <div class="flex items-center justify-between">
+                    <h3 class="text-sm font-semibold text-zinc-900 dark:text-zinc-100">
+                      Ping Latency
+                    </h3>
+                    <.icon name="hero-arrow-right" class="h-4 w-4 text-zinc-400" />
+                  </div>
+                </.link>
+                <div class="p-4">
+                  <div
+                    id="latency-chart"
+                    phx-hook="SensorChart"
+                    data-chart={@latency_chart_data}
+                    data-unit="ms"
+                    data-auto-scale="true"
+                    style="height: 300px;"
+                  >
+                    <canvas></canvas>
+                  </div>
+                </div>
+              </div>
+            <% end %>
           </div>
           <!-- Right Column: Graphs and Metrics -->
           <div class="space-y-6">
diff --git a/test/towerops_web/live/graph_live/show_test.exs b/test/towerops_web/live/graph_live/show_test.exs
new file mode 100644
index 00000000..43dfa099
--- /dev/null
+++ b/test/towerops_web/live/graph_live/show_test.exs
@@ -0,0 +1,343 @@
+defmodule ToweropsWeb.GraphLive.ShowTest do
+  use ToweropsWeb.ConnCase
+
+  import Phoenix.LiveViewTest
+
+  alias Towerops.Monitoring
+  alias Towerops.Organizations
+  alias Towerops.Sites
+  alias Towerops.Snmp
+
+  setup :register_and_log_in_user
+
+  setup %{user: user} do
+    {:ok, organization} = Organizations.create_organization(%{name: "Test Org"}, user.id)
+
+    {:ok, site} =
+      Sites.create_site(%{
+        name: "Test Site",
+        organization_id: organization.id
+      })
+
+    {:ok, device} =
+      Towerops.Devices.create_device(%{
+        name: "Test Router",
+        ip_address: "192.168.1.1",
+        site_id: site.id,
+        snmp_enabled: true
+      })
+
+    # Create SNMP device for sensor tests
+    {:ok, snmp_device} =
+      Snmp.create_snmp_device(%{
+        device_id: device.id,
+        sys_name: "test-router",
+        sys_descr: "Test Router",
+        sys_object_id: "1.3.6.1.4.1.9"
+      })
+
+    %{organization: organization, site: site, device: device, snmp_device: snmp_device}
+  end
+
+  describe "Latency graph" do
+    test "renders latency graph page with correct assigns", %{
+      conn: conn,
+      device: device,
+      organization: org
+    } do
+      # Create monitoring checks
+      Monitoring.create_check(%{
+        device_id: device.id,
+        status: :success,
+        response_time_ms: 10.5,
+        checked_at: DateTime.utc_now()
+      })
+
+      {:ok, view, html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency")
+
+      # Verify page renders
+      assert html =~ "Ping Latency"
+      assert html =~ device.name
+
+      # Verify correct assigns are set
+      assert view.assigns.current_organization.id == org.id
+      assert view.assigns.device_id == device.id
+      assert view.assigns.sensor_type == "latency"
+      assert view.assigns.range == "24h"
+    end
+
+    test "uses current_organization in back link", %{
+      conn: conn,
+      device: device,
+      organization: org
+    } do
+      {:ok, _view, html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency")
+
+      # Verify back link uses organization slug (this was the bug)
+      assert html =~ ~s[/orgs/#{org.slug}/devices/#{device.id}]
+    end
+
+    test "displays empty state when no latency data", %{
+      conn: conn,
+      device: device,
+      organization: org
+    } do
+      {:ok, _view, html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency")
+
+      assert html =~ "No sensor data available"
+    end
+  end
+
+  describe "Processor graph" do
+    test "renders processor graph page", %{
+      conn: conn,
+      device: device,
+      organization: org,
+      snmp_device: snmp_device
+    } do
+      # Create a CPU sensor
+      {:ok, sensor} =
+        Snmp.create_sensor(%{
+          snmp_device_id: snmp_device.id,
+          sensor_type: "cpu_load",
+          sensor_index: 1,
+          sensor_descr: "CPU 1",
+          sensor_unit: "%"
+        })
+
+      # Create sensor reading
+      Snmp.create_sensor_reading(%{
+        sensor_id: sensor.id,
+        value: 45.2,
+        checked_at: DateTime.utc_now()
+      })
+
+      {:ok, view, html} =
+        live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/processors")
+
+      assert html =~ "Processor Usage"
+      assert html =~ device.name
+
+      # Verify correct assigns
+      assert view.assigns.current_organization.id == org.id
+      assert view.assigns.sensor_type == "processors"
+      assert view.assigns.unit == "%"
+    end
+  end
+
+  describe "Memory graph" do
+    test "renders memory graph page", %{
+      conn: conn,
+      device: device,
+      organization: org,
+      snmp_device: snmp_device
+    } do
+      # Create a memory sensor
+      {:ok, sensor} =
+        Snmp.create_sensor(%{
+          snmp_device_id: snmp_device.id,
+          sensor_type: "memory_usage",
+          sensor_index: 1,
+          sensor_descr: "Memory",
+          sensor_unit: "%"
+        })
+
+      # Create sensor reading
+      Snmp.create_sensor_reading(%{
+        sensor_id: sensor.id,
+        value: 75.5,
+        checked_at: DateTime.utc_now()
+      })
+
+      {:ok, view, html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/memory")
+
+      assert html =~ "Memory Usage"
+      assert html =~ device.name
+
+      # Verify correct assigns
+      assert view.assigns.current_organization.id == org.id
+      assert view.assigns.sensor_type == "memory"
+      assert view.assigns.unit == "%"
+    end
+  end
+
+  describe "Traffic graph" do
+    test "renders overall traffic graph page", %{
+      conn: conn,
+      device: device,
+      organization: org,
+      snmp_device: snmp_device
+    } do
+      # Create an interface
+      {:ok, interface} =
+        Snmp.create_interface(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_descr: "eth0",
+          if_name: "eth0",
+          if_oper_status: "up",
+          if_speed: 1_000_000_000
+        })
+
+      # Create interface stats
+      Snmp.create_interface_stat(%{
+        interface_id: interface.id,
+        if_in_octets: 1000,
+        if_out_octets: 2000,
+        checked_at: DateTime.utc_now()
+      })
+
+      {:ok, view, html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/traffic")
+
+      assert html =~ "Overall Traffic"
+      assert html =~ device.name
+
+      # Verify correct assigns
+      assert view.assigns.current_organization.id == org.id
+      assert view.assigns.sensor_type == "traffic"
+      assert view.assigns.unit == "bps"
+      assert view.assigns.show_zero_line == true
+    end
+
+    test "renders specific interface traffic graph", %{
+      conn: conn,
+      device: device,
+      organization: org,
+      snmp_device: snmp_device
+    } do
+      # Create an interface
+      {:ok, interface} =
+        Snmp.create_interface(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_descr: "eth0",
+          if_name: "eth0",
+          if_oper_status: "up",
+          if_speed: 1_000_000_000
+        })
+
+      # Create interface stats
+      Snmp.create_interface_stat(%{
+        interface_id: interface.id,
+        if_in_octets: 1000,
+        if_out_octets: 2000,
+        checked_at: DateTime.utc_now()
+      })
+
+      {:ok, view, html} =
+        live(
+          conn,
+          ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/traffic?interface_id=#{interface.id}"
+        )
+
+      assert html =~ "Overall Traffic"
+      assert html =~ device.name
+
+      # Verify correct assigns including interface_id
+      assert view.assigns.current_organization.id == org.id
+      assert view.assigns.interface_id == interface.id
+    end
+  end
+
+  describe "Time range selection" do
+    test "defaults to 24h range", %{conn: conn, device: device, organization: org} do
+      {:ok, view, _html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency")
+
+      assert view.assigns.range == "24h"
+    end
+
+    test "accepts custom range parameter", %{conn: conn, device: device, organization: org} do
+      {:ok, view, _html} =
+        live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency?range=1h")
+
+      assert view.assigns.range == "1h"
+    end
+
+    test "changes range when user clicks range button", %{
+      conn: conn,
+      device: device,
+      organization: org
+    } do
+      {:ok, view, _html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency")
+
+      # Initially 24h
+      assert view.assigns.range == "24h"
+
+      # Change to 1h
+      view
+      |> element("button", "1 Hour")
+      |> render_click()
+
+      assert view.assigns.range == "1h"
+    end
+
+    test "preserves interface_id when changing range", %{
+      conn: conn,
+      device: device,
+      organization: org,
+      snmp_device: snmp_device
+    } do
+      # Create an interface
+      {:ok, interface} =
+        Snmp.create_interface(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_descr: "eth0",
+          if_name: "eth0",
+          if_oper_status: "up",
+          if_speed: 1_000_000_000
+        })
+
+      {:ok, view, _html} =
+        live(
+          conn,
+          ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/traffic?interface_id=#{interface.id}"
+        )
+
+      # Change range
+      view
+      |> element("button", "1 Hour")
+      |> render_click()
+
+      # Interface ID should be preserved
+      assert view.assigns.interface_id == interface.id
+      assert view.assigns.range == "1h"
+    end
+  end
+
+  describe "Assign validation" do
+    test "ensures current_organization assign exists, not organization", %{
+      conn: conn,
+      device: device,
+      organization: org
+    } do
+      {:ok, view, _html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency")
+
+      # current_organization should exist
+      assert Map.has_key?(view.assigns, :current_organization)
+      assert view.assigns.current_organization.id == org.id
+
+      # organization should NOT exist (this was the bug)
+      refute Map.has_key?(view.assigns, :organization)
+    end
+
+    test "has all required assigns for rendering", %{
+      conn: conn,
+      device: device,
+      organization: org
+    } do
+      {:ok, view, _html} = live(conn, ~p"/orgs/#{org.slug}/devices/#{device.id}/graph/latency")
+
+      # All required assigns should be present
+      assert Map.has_key?(view.assigns, :current_organization)
+      assert Map.has_key?(view.assigns, :device)
+      assert Map.has_key?(view.assigns, :device_id)
+      assert Map.has_key?(view.assigns, :sensor_type)
+      assert Map.has_key?(view.assigns, :range)
+      assert Map.has_key?(view.assigns, :chart_title)
+      assert Map.has_key?(view.assigns, :unit)
+      assert Map.has_key?(view.assigns, :auto_scale)
+      assert Map.has_key?(view.assigns, :show_zero_line)
+    end
+  end
+end