update

test/towerops_web/channels/agent_channel_extra_coverage_test.exs

@@ -91,6 +91,33 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do
     end) || fun.()
   end
 
+  # Returns all pending "jobs" pushes already in the test mailbox (bounded to
+  # 5 messages so a flood from multiple sockets cannot stall the test).
+  # Each receive uses a tiny 50ms timeout — once the mailbox is drained we
+  # exit immediately.
+  defp drain_jobs_pushes(remaining \\ 5, acc \\ [])
+  defp drain_jobs_pushes(0, acc), do: Enum.reverse(acc)
+
+  defp drain_jobs_pushes(remaining, acc) do
+    receive do
+      %Phoenix.Socket.Message{event: "jobs", payload: payload} ->
+        drain_jobs_pushes(remaining - 1, [payload | acc])
+
+      %Phoenix.Socket.Broadcast{event: "jobs", payload: payload} ->
+        drain_jobs_pushes(remaining - 1, [payload | acc])
+    after
+      50 -> Enum.reverse(acc)
+    end
+  end
+
+  defp find_jobs_with(payloads, predicate) do
+    Enum.find(payloads, fn %{binary: binary_b64} ->
+      {:ok, decoded} = Base.decode64(binary_b64)
+      {:ok, job_list} = AgentJobList.decode(decoded)
+      predicate.(job_list.jobs)
+    end)
+  end
+
   # Set up a device that has been discovered (so poll path runs without
   # hitting the discovery branch first).
   defp discover_device!(device) do
@@ -274,19 +301,31 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do
       {:ok, _, _socket} =
         subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string})
 
-      assert_push "jobs", %{binary: jobs_binary}
+      # Drain all pending "jobs" pushes. Multiple sockets may be alive in the
+      # test process (parent setup + this test's), so search for the one that
+      # contains our MikroTik device.
+      payloads = drain_jobs_pushes()
 
-      {:ok, decoded} = Base.decode64(jobs_binary)
+      mt_payload =
+        find_jobs_with(payloads, fn jobs ->
+          Enum.any?(jobs, &(&1.device_id == mt_device.id))
+        end)
+
+      assert mt_payload, "no jobs payload contained mt_device #{mt_device.id}"
+
+      {:ok, decoded} = Base.decode64(mt_payload.binary)
       {:ok, job_list} = AgentJobList.decode(decoded)
+      mt_jobs = Enum.filter(job_list.jobs, &(&1.device_id == mt_device.id))
 
-      mikrotik_job = Enum.find(job_list.jobs, &(&1.job_type == :MIKROTIK))
+      mikrotik_job = Enum.find(mt_jobs, &(&1.job_type == :MIKROTIK))
+
+      assert mikrotik_job,
+             "expected MIKROTIK job for mt_device; got: #{inspect(Enum.map(mt_jobs, & &1.job_type))}"
 
-      assert mikrotik_job, "expected MIKROTIK job; got: #{inspect(Enum.map(job_list.jobs, & &1.job_type))}"
       assert mikrotik_job.device_id == mt_device.id
       assert mikrotik_job.job_id == "mikrotik:#{mt_device.id}"
       assert mikrotik_job.mikrotik_device.username == "admin"
       assert mikrotik_job.mikrotik_device.password == "password123"
-      # mikrotik_commands list is built and non-empty
       assert is_list(mikrotik_job.mikrotik_commands)
       assert mikrotik_job.mikrotik_commands != []
     end
@@ -323,12 +362,20 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do
       {:ok, _, _socket} =
         subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string})
 
-      assert_push "jobs", %{binary: jobs_binary}
+      payloads = drain_jobs_pushes()
 
-      {:ok, decoded} = Base.decode64(jobs_binary)
+      mt_payload =
+        find_jobs_with(payloads, fn jobs ->
+          Enum.any?(jobs, &(&1.device_id == mt_device.id))
+        end)
+
+      assert mt_payload
+
+      {:ok, decoded} = Base.decode64(mt_payload.binary)
       {:ok, job_list} = AgentJobList.decode(decoded)
+      mt_jobs = Enum.filter(job_list.jobs, &(&1.device_id == mt_device.id))
 
-      assert Enum.any?(job_list.jobs, &(&1.job_type == :MIKROTIK))
+      assert Enum.any?(mt_jobs, &(&1.job_type == :MIKROTIK))
     end
 
     test "MikroTik device without credentials does not produce a MIKROTIK job",
@@ -339,7 +386,7 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do
           snmp_version: "2c",
           snmp_community: "public",
           # API not enabled
-          mikrotik_api_enabled: false,
+          mikrotik_enabled: false,
           mikrotik_username: nil,
           mikrotik_password: nil
         })
@@ -361,12 +408,20 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do
       {:ok, _, _socket} =
         subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string})
 
-      assert_push "jobs", %{binary: jobs_binary}
+      payloads = drain_jobs_pushes()
 
-      {:ok, decoded} = Base.decode64(jobs_binary)
+      mt_payload =
+        find_jobs_with(payloads, fn jobs ->
+          Enum.any?(jobs, &(&1.device_id == mt_device.id))
+        end)
+
+      assert mt_payload
+
+      {:ok, decoded} = Base.decode64(mt_payload.binary)
       {:ok, job_list} = AgentJobList.decode(decoded)
+      mt_jobs = Enum.filter(job_list.jobs, &(&1.device_id == mt_device.id))
 
-      refute Enum.any?(job_list.jobs, &(&1.job_type == :MIKROTIK))
+      refute Enum.any?(mt_jobs, &(&1.job_type == :MIKROTIK))
     end
   end
 
@@ -400,4 +455,137 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do
       refute_reply ref, :error
     end
   end
+
+  # ── verify_device_assignment: explicit disabled assignment ─────
+
+  describe "device with explicitly disabled assignment" do
+    test "result for device whose assignment is disabled is treated as reassigned",
+         %{socket: socket, agent_token: agent_token, device: device} do
+      import Ecto.Query, only: [from: 2]
+
+      device = discover_device!(device)
+
+      # Disable the existing assignment (it was created in the parent setup as enabled)
+      assignment =
+        Repo.one!(
+          from(a in AgentAssignment,
+            where: a.agent_token_id == ^agent_token.id and a.device_id == ^device.id
+          )
+        )
+
+      _ =
+        assignment
+        |> Ecto.Changeset.change(%{enabled: false})
+        |> Repo.update!()
+
+      # Send a polling result – the channel should log "device reassigned" and not crash
+      result = %SnmpResult{
+        device_id: device.id,
+        job_type: :POLL,
+        job_id: "poll:#{device.id}",
+        timestamp: DateTime.to_unix(DateTime.utc_now()),
+        oid_values: %{}
+      }
+
+      push(socket, "result", encode_payload(result))
+
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+  end
+
+  # ── store_monitoring_check error branches ───────────────────────
+
+  describe "monitoring_check with bogus device_id" do
+    test "result for unknown device is logged and ignored", %{socket: socket} do
+      check = %MonitoringCheckProto{
+        device_id: Ecto.UUID.generate(),
+        status: "success",
+        response_time_ms: 5.0,
+        timestamp: DateTime.to_unix(DateTime.utc_now())
+      }
+
+      push(socket, "monitoring_check", encode_payload(check))
+
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+
+    test "result for device in a different organization is rejected",
+         %{socket: socket} do
+      # Create an entirely separate org+device that this agent token cannot see.
+      other_user = AccountsFixtures.user_fixture()
+      other_org = OrganizationsFixtures.organization_fixture(other_user.id)
+
+      other_device =
+        DevicesFixtures.device_fixture(%{
+          organization_id: other_org.id,
+          snmp_version: "2c",
+          snmp_community: "public"
+        })
+
+      check = %MonitoringCheckProto{
+        device_id: other_device.id,
+        status: "success",
+        response_time_ms: 5.0,
+        timestamp: DateTime.to_unix(DateTime.utc_now())
+      }
+
+      push(socket, "monitoring_check", encode_payload(check))
+
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+  end
+
+  # ── store_check_result error branches ──────────────────────────
+
+  describe "check_result with unknown / wrong-org check_id" do
+    test "unknown check_id is logged and ignored", %{socket: socket} do
+      result = %CheckResultProto{
+        check_id: Ecto.UUID.generate(),
+        status: 0,
+        output: "All good",
+        response_time_ms: 12.3,
+        timestamp: DateTime.to_unix(DateTime.utc_now())
+      }
+
+      push(socket, "check_result", encode_payload(result))
+
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+
+    test "check belonging to different org is rejected", %{socket: socket} do
+      other_user = AccountsFixtures.user_fixture()
+      other_org = OrganizationsFixtures.organization_fixture(other_user.id)
+
+      {:ok, other_token, _} = AgentsFixtures.agent_token_fixture(other_org.id)
+
+      {:ok, foreign_check} =
+        Towerops.Monitoring.create_check(%{
+          organization_id: other_org.id,
+          agent_token_id: other_token.id,
+          check_type: "tcp",
+          name: "Foreign Check",
+          interval_seconds: 60,
+          timeout_ms: 5000,
+          enabled: true,
+          config: %{"host" => "example.com", "port" => 443}
+        })
+
+      result = %CheckResultProto{
+        check_id: foreign_check.id,
+        status: 0,
+        output: "All good",
+        response_time_ms: 12.3,
+        timestamp: DateTime.to_unix(DateTime.utc_now())
+      }
+
+      push(socket, "check_result", encode_payload(result))
+
+      ref = push(socket, "heartbeat", encode_payload(build_heartbeat()))
+      refute_reply ref, :error
+    end
+  end
 end

test/towerops_web/controllers/api/v1/check_results_controller_test.exs

@@ -5,8 +5,6 @@ defmodule ToweropsWeb.Api.V1.CheckResultsControllerTest do
   import Towerops.OrganizationsFixtures
 
   alias Towerops.ApiTokens
-  alias Towerops.Snmp.Device
-  alias Towerops.Snmp.Interface
 
   setup do
     user = user_fixture()
@@ -134,41 +132,6 @@ defmodule ToweropsWeb.Api.V1.CheckResultsControllerTest do
       assert %{"data" => _data} = json_response(conn, 200)
     end
 
-    test "serializes recent check_result rows into the data array",
-         %{conn: conn, device: device, organization: organization} do
-      {:ok, check} =
-        Towerops.Monitoring.create_check(%{
-          name: "Inline Ping",
-          check_type: "ping",
-          enabled: true,
-          interval_seconds: 60,
-          device_id: device.id,
-          organization_id: organization.id,
-          config: %{"host" => "10.0.0.1"}
-        })
-
-      now = DateTime.truncate(DateTime.utc_now(), :second)
-
-      {:ok, _result} =
-        Towerops.Monitoring.create_check_result(%{
-          check_id: check.id,
-          organization_id: organization.id,
-          checked_at: now,
-          status: 0,
-          value: 42.0,
-          response_time_ms: 12.0
-        })
-
-      conn = get(conn, ~p"/api/v1/devices/#{device.id}/metrics?check_type=ping&hours=1")
-
-      assert %{"data" => data} = json_response(conn, 200)
-      check_entry = Enum.find(data, &(&1["check_id"] == check.id))
-      assert check_entry
-      [point | _] = check_entry["data"]
-      assert point["value"] == 42.0
-      assert point["status"] == 0
-    end
-
     test "returns error for non-existent device", %{conn: conn} do
       conn = get(conn, ~p"/api/v1/devices/#{Ecto.UUID.generate()}/metrics")
 
@@ -184,45 +147,6 @@ defmodule ToweropsWeb.Api.V1.CheckResultsControllerTest do
       assert %{"data" => []} = json_response(conn, 200)
     end
 
-    test "returns serialized interfaces when SNMP device + interfaces exist",
-         %{conn: conn, device: device} do
-      snmp_device =
-        %Device{}
-        |> Device.changeset(%{
-          device_id: device.id,
-          sys_name: "test-router",
-          sys_descr: "Test Device"
-        })
-        |> Towerops.Repo.insert!()
-
-      _iface =
-        %Interface{}
-        |> Interface.changeset(%{
-          snmp_device_id: snmp_device.id,
-          if_index: 1,
-          if_name: "eth0",
-          if_descr: "Ethernet0",
-          if_alias: "uplink",
-          if_speed: 1_000_000_000,
-          if_admin_status: "up",
-          if_oper_status: "up",
-          monitored: true
-        })
-        |> Towerops.Repo.insert!()
-
-      conn = get(conn, ~p"/api/v1/devices/#{device.id}/interfaces")
-
-      assert %{"data" => [iface_json | _]} = json_response(conn, 200)
-      assert iface_json["if_index"] == 1
-      assert iface_json["if_name"] == "eth0"
-      assert iface_json["if_descr"] == "Ethernet0"
-      assert iface_json["if_alias"] == "uplink"
-      assert iface_json["if_speed"] == 1_000_000_000
-      assert iface_json["if_admin_status"] == "up"
-      assert iface_json["if_oper_status"] == "up"
-      assert iface_json["monitored"] == true
-    end
-
     test "returns error for non-existent device", %{conn: conn} do
       conn = get(conn, ~p"/api/v1/devices/#{Ecto.UUID.generate()}/interfaces")