fix/traffic-graph-rendering-and-polarity (#130)

Reviewed-on: graham/towerops-web#130

assets/js/app.ts

@@ -28,6 +28,7 @@ import Chart from "../vendor/chart"
 import type { ChartData, ChartDataset, ChartDataPoint } from "../vendor/chart"
 import cytoscape from "../vendor/cytoscape"
 import type { SensorChartHook } from "./types/liveview"
+import { buildChartDatasets } from "./chart_dataset_config.mjs"
 import { DeviceListReorder } from "./device_list_reorder"
 import { initCookieConsent } from "./cookie_consent"
 
@@ -104,52 +105,7 @@ const SensorChart: SensorChartHook = {
     const range = this.el.dataset.range || '24h'
     const isTrafficChart = unit === 'bps'
 
-    // Generate colors for each dataset
+    const datasets = buildChartDatasets(data, { isTrafficChart, dualAxis }) as ChartDataset[]
-    const colors = [
-      'rgb(59, 130, 246)',   // blue
-      'rgb(239, 68, 68)',    // red
-      'rgb(34, 197, 94)',    // green
-      'rgb(234, 179, 8)',    // yellow
-      'rgb(168, 85, 247)',   // purple
-      'rgb(236, 72, 153)',   // pink
-      'rgb(14, 165, 233)',   // sky
-      'rgb(249, 115, 22)',   // orange
-    ]
-
-    const datasets: ChartDataset[] = data.datasets.map((dataset, index) => {
-      const color = colors[index % colors.length]
-      const baseConfig: ChartDataset = {
-        ...dataset,
-        borderColor: color,
-        backgroundColor: color.replace('rgb', 'rgba').replace(')', ', 0.2)'),
-        borderWidth: 2,
-        tension: 0.4,
-        pointRadius: 0,
-        pointHoverRadius: 4,
-      }
-
-      // Preserve yAxisID from server data for dual-axis charts
-      if ((dataset as any).yAxisID) {
-        (baseConfig as any).yAxisID = (dataset as any).yAxisID
-      }
-
-      // Status dataset (dual axis, right side) uses stepped line with fill
-      if (dualAxis && (dataset as any).yAxisID === 'y1') {
-        (baseConfig as any).stepped = 'before'
-        baseConfig.tension = 0
-        baseConfig.fill = 'origin'
-        baseConfig.borderWidth = 1
-        // Use green with low opacity for the status fill area
-        baseConfig.borderColor = 'rgb(34, 197, 94)'
-        baseConfig.backgroundColor = 'rgba(34, 197, 94, 0.1)'
-      } else if (isTrafficChart) {
-        baseConfig.fill = 'origin'
-      } else {
-        baseConfig.fill = false
-      }
-
-      return baseConfig
-    })
 
     // Format bits per second into human-readable units
     const formatBps = (bps: number): string => {

assets/js/chart_dataset_config.mjs

@@ -0,0 +1,45 @@
+const DEFAULT_COLORS = [
+  'rgb(59, 130, 246)',
+  'rgb(239, 68, 68)',
+  'rgb(34, 197, 94)',
+  'rgb(234, 179, 8)',
+  'rgb(168, 85, 247)',
+  'rgb(236, 72, 153)',
+  'rgb(14, 165, 233)',
+  'rgb(249, 115, 22)',
+]
+
+const hasOwn = (object, key) => Object.prototype.hasOwnProperty.call(object, key)
+
+export function buildChartDatasets(data, { isTrafficChart, dualAxis, colors = DEFAULT_COLORS }) {
+  return data.datasets.map((dataset, index) => {
+    const color = colors[index % colors.length]
+    const baseConfig = {
+      ...dataset,
+      borderColor: color,
+      backgroundColor: color.replace('rgb', 'rgba').replace(')', ', 0.2)'),
+      borderWidth: 2,
+      tension: 0.4,
+      pointRadius: 0,
+      pointHoverRadius: 4,
+    }
+
+    if (dualAxis && dataset.yAxisID === 'y1') {
+      baseConfig.stepped = 'before'
+      baseConfig.tension = 0
+      baseConfig.fill = 'origin'
+      baseConfig.borderWidth = 1
+      baseConfig.borderColor = 'rgb(34, 197, 94)'
+      baseConfig.backgroundColor = 'rgba(34, 197, 94, 0.1)'
+      return baseConfig
+    }
+
+    if (hasOwn(dataset, 'fill')) {
+      baseConfig.fill = dataset.fill
+      return baseConfig
+    }
+
+    baseConfig.fill = isTrafficChart ? 'origin' : false
+    return baseConfig
+  })
+}

lib/towerops_web/live/graph_live/show.ex

@@ -809,7 +809,7 @@ defmodule ToweropsWeb.GraphLive.Show do
 
         %{
           x: DateTime.to_unix(stat2.checked_at, :millisecond),
-          y: -bps
+          y: bps
         }
       end
     end)
@@ -834,7 +834,7 @@ defmodule ToweropsWeb.GraphLive.Show do
 
         %{
           x: DateTime.to_unix(stat2.checked_at, :millisecond),
-          y: bps
+          y: -bps
         }
       end
     end)

test/assets/chart_dataset_config_test.mjs

@@ -0,0 +1,34 @@
+import test from 'node:test'
+import assert from 'node:assert/strict'
+
+import { buildChartDatasets } from '../../assets/js/chart_dataset_config.mjs'
+
+test('preserves explicit fill settings on traffic datasets', () => {
+  const datasets = buildChartDatasets(
+    {
+      datasets: [
+        { label: 'Outbound', data: [{ x: 1, y: -100 }] },
+        { label: 'Capacity (Out)', data: [{ x: 1, y: -300 }], fill: false, borderDash: [5, 5] }
+      ]
+    },
+    { isTrafficChart: true, dualAxis: false }
+  )
+
+  assert.equal(datasets[0].fill, 'origin')
+  assert.equal(datasets[1].fill, false)
+  assert.deepEqual(datasets[1].borderDash, [5, 5])
+})
+
+test('uses stepped fill for dual-axis status datasets', () => {
+  const datasets = buildChartDatasets(
+    {
+      datasets: [
+        { label: 'Status', yAxisID: 'y1', data: [{ x: 1, y: 1 }] }
+      ]
+    },
+    { isTrafficChart: false, dualAxis: true }
+  )
+
+  assert.equal(datasets[0].stepped, 'before')
+  assert.equal(datasets[0].fill, 'origin')
+})

test/towerops_web/live/device_live/show_test.exs

@@ -4,6 +4,8 @@ defmodule ToweropsWeb.DeviceLive.ShowTest do
   import Phoenix.LiveViewTest
 
   alias Towerops.Snmp.Device
+  alias Towerops.Snmp.Interface
+  alias Towerops.Snmp.InterfaceStat
 
   setup do
     user = Towerops.AccountsFixtures.user_fixture(enable_totp: true)
@@ -443,8 +445,6 @@ defmodule ToweropsWeb.DeviceLive.ShowTest do
   describe "ports tab capacity" do
     setup %{device: device} do
       alias Device, as: SnmpDevice
-      alias Towerops.Snmp.Interface
-      alias Towerops.Snmp.InterfaceStat
 
       snmp_device =
         %SnmpDevice{}
@@ -537,4 +537,95 @@ defmodule ToweropsWeb.DeviceLive.ShowTest do
       refute html =~ "300.0 Mbps"
     end
   end
+
+  describe "overall traffic chart" do
+    setup %{device: device} do
+      snmp_device =
+        %Device{}
+        |> Device.changeset(%{device_id: device.id, sys_name: "radio", sys_descr: "PTP"})
+        |> Towerops.Repo.insert!()
+
+      device =
+        device
+        |> Ecto.Changeset.change(device_role: "backhaul")
+        |> Towerops.Repo.update!()
+
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          snmp_device_id: snmp_device.id,
+          if_index: 1,
+          if_name: "eth0",
+          if_descr: "eth0",
+          if_speed: 1_000_000_000,
+          if_type: 6,
+          configured_capacity_bps: 500_000_000,
+          capacity_source: "manual"
+        })
+        |> Towerops.Repo.insert!()
+
+      now = DateTime.utc_now()
+
+      %InterfaceStat{}
+      |> InterfaceStat.changeset(%{
+        interface_id: interface.id,
+        if_in_octets: 1_000,
+        if_out_octets: 2_000,
+        checked_at: DateTime.add(now, -60, :second)
+      })
+      |> Towerops.Repo.insert!()
+
+      %InterfaceStat{}
+      |> InterfaceStat.changeset(%{
+        interface_id: interface.id,
+        if_in_octets: 10_000,
+        if_out_octets: 14_000,
+        checked_at: now
+      })
+      |> Towerops.Repo.insert!()
+
+      %{device: device}
+    end
+
+    test "renders overall backhaul traffic and capacity with consistent polarity", %{
+      conn: conn,
+      user: user,
+      device: device
+    } do
+      conn = log_in_user(conn, user)
+      {:ok, _view, html} = live(conn, ~p"/devices/#{device.id}?tab=overview")
+
+      datasets = chart_datasets_from_html(html, "traffic-chart")
+
+      assert dataset_y_values(datasets, "Outbound") == [-1600.0]
+      assert dataset_y_values(datasets, "Inbound") == [1200.0]
+      assert dataset_y_values(datasets, "Capacity (In)") == [500_000_000.0]
+      assert dataset_y_values(datasets, "Capacity (Out)") == [-500_000_000.0]
+      assert dataset_fill(datasets, "Capacity (In)") == false
+      assert dataset_fill(datasets, "Capacity (Out)") == false
+    end
+  end
+
+  defp chart_datasets_from_html(html, chart_id) do
+    pattern = ~r/id="#{chart_id}"[^>]*data-chart="([^"]+)"/
+    [_, encoded_json] = Regex.run(pattern, html)
+
+    encoded_json
+    |> String.replace(""", "\"")
+    |> Jason.decode!()
+    |> Map.fetch!("datasets")
+  end
+
+  defp dataset_y_values(datasets, label) do
+    datasets
+    |> Enum.find(&(Map.get(&1, "label") == label))
+    |> Map.fetch!("data")
+    |> Enum.map(&Map.fetch!(&1, "y"))
+  end
+
+  defp dataset_fill(datasets, label) do
+    datasets
+    |> Enum.find(&(Map.get(&1, "label") == label))
+    |> Map.get("fill")
+  end
 end

test/towerops_web/live/graph_live/show_test.exs

@@ -367,6 +367,82 @@ defmodule ToweropsWeb.GraphLive.ShowTest do
       assert html =~ "Overall Traffic"
       assert html =~ device.name
     end
+
+    test "renders backhaul traffic and capacity with consistent polarity", %{
+      conn: conn,
+      device: device,
+      organization: _org,
+      snmp_device: snmp_device
+    } do
+      device =
+        device
+        |> Ecto.Changeset.change(device_role: "backhaul")
+        |> Repo.update!()
+
+      interface =
+        %Interface{}
+        |> Interface.changeset(%{
+          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,
+          configured_capacity_bps: 500_000_000,
+          capacity_source: "manual"
+        })
+        |> Repo.insert!()
+
+      base_time = DateTime.utc_now()
+
+      Snmp.create_interface_stat(%{
+        interface_id: interface.id,
+        if_in_octets: 1_000,
+        if_out_octets: 2_000,
+        checked_at: DateTime.add(base_time, -60, :second)
+      })
+
+      Snmp.create_interface_stat(%{
+        interface_id: interface.id,
+        if_in_octets: 10_000,
+        if_out_octets: 14_000,
+        checked_at: base_time
+      })
+
+      {:ok, _view, html} = live(conn, ~p"/devices/#{device.id}/graph/traffic")
+
+      datasets = chart_datasets_from_html(html, "detail-chart")
+
+      assert dataset_y_values(datasets, "eth0 Out") == [-1600.0]
+      assert dataset_y_values(datasets, "eth0 In") == [1200.0]
+      assert dataset_y_values(datasets, "Capacity (In)") == [500_000_000.0]
+      assert dataset_y_values(datasets, "Capacity (Out)") == [-500_000_000.0]
+      assert dataset_fill(datasets, "Capacity (In)") == false
+      assert dataset_fill(datasets, "Capacity (Out)") == false
+    end
+  end
+
+  defp chart_datasets_from_html(html, chart_id) do
+    pattern = ~r/id="#{chart_id}"[^>]*data-chart="([^"]+)"/
+    [_, encoded_json] = Regex.run(pattern, html)
+
+    encoded_json
+    |> String.replace(""", "\"")
+    |> Jason.decode!()
+    |> Map.fetch!("datasets")
+  end
+
+  defp dataset_y_values(datasets, label) do
+    datasets
+    |> Enum.find(&(Map.get(&1, "label") == label))
+    |> Map.fetch!("data")
+    |> Enum.map(&Map.fetch!(&1, "y"))
+  end
+
+  defp dataset_fill(datasets, label) do
+    datasets
+    |> Enum.find(&(Map.get(&1, "label") == label))
+    |> Map.get("fill")
   end
 
   describe "Time range selection" do