fix: close review findings

lib/aprsme/partition_manager.ex

@@ -91,7 +91,7 @@ defmodule Aprsme.PartitionManager do
       list_partitions()
       |> Enum.filter(fn name ->
         case partition_date(name) do
-          {:ok, date} -> Date.before?(date, cutoff_date)
+          {:ok, date} -> Date.compare(date, cutoff_date) != :gt
           _ -> false
         end
       end)

lib/aprsme_web/channels/mobile_channel.ex

@@ -185,6 +185,8 @@ defmodule AprsmeWeb.MobileChannel do
     # Load historical packets for this callsign
     socket = load_callsign_history(socket, callsign, hours_back)
 
+    socket = ensure_callsign_subscription(socket)
+
     # Store tracked callsign in socket
     socket = assign(socket, :tracked_callsign, callsign)
 
@@ -197,7 +199,11 @@ defmodule AprsmeWeb.MobileChannel do
 
     if socket.assigns[:tracked_callsign] do
       callsign = socket.assigns.tracked_callsign
-      socket = assign(socket, :tracked_callsign, nil)
+
+      socket =
+        socket
+        |> remove_callsign_subscription()
+        |> assign(:tracked_callsign, nil)
 
       Logger.info("Mobile client #{socket.assigns[:client_id]} unsubscribed from callsign: #{callsign}")
 
@@ -209,6 +215,19 @@ defmodule AprsmeWeb.MobileChannel do
 
   @impl true
   def handle_info({:streaming_packet, packet}, socket) do
+    maybe_push_tracked_packet(socket, packet)
+  end
+
+  @impl true
+  def handle_info({:postgres_packet, packet}, socket) do
+    maybe_push_tracked_packet(socket, packet)
+  end
+
+  def handle_info(_message, socket) do
+    {:noreply, socket}
+  end
+
+  defp maybe_push_tracked_packet(socket, packet) do
     # Check if packet matches tracked callsign (if any)
     should_send =
       if tracked_callsign = socket.assigns[:tracked_callsign] do
@@ -237,6 +256,10 @@ defmodule AprsmeWeb.MobileChannel do
       Aprsme.StreamingPacketsPubSub.unsubscribe(self())
     end
 
+    if callsign_subscription_active?(socket) do
+      Phoenix.PubSub.unsubscribe(Aprsme.PubSub, "postgres:aprsme_packets")
+    end
+
     :ok
   end
 
@@ -275,7 +298,14 @@ defmodule AprsmeWeb.MobileChannel do
 
   defp ensure_float(value) when is_integer(value), do: value * 1.0
   defp ensure_float(value) when is_float(value), do: value
-  defp ensure_float(value) when is_binary(value), do: String.to_float(value)
+
+  defp ensure_float(value) when is_binary(value) do
+    case Float.parse(String.trim(value)) do
+      {float, ""} -> float
+      _ -> value
+    end
+  end
+
   defp ensure_float(value), do: value
 
   defp ensure_integer(value, _default) when is_integer(value), do: value
@@ -521,4 +551,26 @@ defmodule AprsmeWeb.MobileChannel do
 
     {:reply, {:ok, %{bounds: bounds, message: "Bounds updated"}}, socket}
   end
+
+  defp ensure_callsign_subscription(socket) do
+    if callsign_subscription_active?(socket) do
+      socket
+    else
+      Phoenix.PubSub.subscribe(Aprsme.PubSub, "postgres:aprsme_packets")
+      assign(socket, :callsign_subscription_active, true)
+    end
+  end
+
+  defp remove_callsign_subscription(socket) do
+    if callsign_subscription_active?(socket) do
+      Phoenix.PubSub.unsubscribe(Aprsme.PubSub, "postgres:aprsme_packets")
+      assign(socket, :callsign_subscription_active, false)
+    else
+      socket
+    end
+  end
+
+  defp callsign_subscription_active?(socket) do
+    Map.get(socket.assigns, :callsign_subscription_active, false)
+  end
 end

lib/aprsme_web/plugs/api_csrf.ex

@@ -1,6 +1,6 @@
 defmodule AprsmeWeb.Plugs.ApiCSRF do
   @moduledoc """
-  CSRF protection for API endpoints using X-Requested-With header or API tokens
+  CSRF protection for API endpoints using `X-Requested-With` or a valid CSRF token.
   """
   import Phoenix.Controller
   import Plug.Conn
@@ -22,16 +22,12 @@ defmodule AprsmeWeb.Plugs.ApiCSRF do
   end
 
   defp check_csrf_protection(conn) do
-    case {get_req_header(conn, "x-requested-with"), get_req_header(conn, "authorization")} do
-      {["XMLHttpRequest"], _} ->
+    case get_req_header(conn, "x-requested-with") do
+      ["XMLHttpRequest"] ->
         # Valid AJAX request
         conn
 
-      {_, ["Bearer " <> _token]} ->
-        # Has authorization header (API token) - would need validation in production
-        conn
-
-      {_, _} ->
+      _ ->
         # Check for CSRF token in header
         case get_req_header(conn, "x-csrf-token") do
           [token] when byte_size(token) > 0 ->

test/aprsme/partition_manager_test.exs

@@ -104,6 +104,20 @@ defmodule Aprsme.PartitionManagerTest do
                  [today_name]
                )
     end
+
+    test "drops the partition at the retention cutoff boundary" do
+      cutoff_date = Date.add(Date.utc_today(), -7)
+      name = PartitionManager.partition_name(cutoff_date)
+      {from_dt, to_dt} = PartitionManager.partition_range(cutoff_date)
+
+      Repo.query!(
+        "CREATE TABLE IF NOT EXISTS #{name} PARTITION OF packets FOR VALUES FROM ('#{DateTime.to_iso8601(from_dt)}') TO ('#{DateTime.to_iso8601(to_dt)}')"
+      )
+
+      {:ok, dropped} = PartitionManager.drop_old_partitions(7)
+
+      assert name in dropped
+    end
   end
 
   describe "list_partitions/0" do

test/aprsme/workers/packet_cleanup_worker_test.exs

@@ -37,6 +37,19 @@ defmodule Aprsme.Workers.PacketCleanupWorkerTest do
 
       assert today_name in PartitionManager.list_partitions()
     end
+
+    test "drops the partition exactly at the cleanup cutoff" do
+      cutoff_date = Date.add(Date.utc_today(), -7)
+      name = PartitionManager.partition_name(cutoff_date)
+      {from_dt, to_dt} = PartitionManager.partition_range(cutoff_date)
+
+      Repo.query!(
+        "CREATE TABLE IF NOT EXISTS #{name} PARTITION OF packets FOR VALUES FROM ('#{DateTime.to_iso8601(from_dt)}') TO ('#{DateTime.to_iso8601(to_dt)}')"
+      )
+
+      assert :ok = PacketCleanupWorker.perform(%{"cleanup_days" => 7})
+      refute name in PartitionManager.list_partitions()
+    end
   end
 
   describe "perform/1 without cleanup_days" do

test/aprsme_web/channels/mobile_channel_test.exs

@@ -126,6 +126,18 @@ defmodule AprsmeWeb.MobileChannelTest do
       assert is_float(bounds.east)
       assert is_float(bounds.west)
     end
+
+    test "rejects invalid numeric strings without crashing", %{socket: socket} do
+      ref =
+        push(socket, "subscribe_bounds", %{
+          "north" => "abc",
+          "south" => "33.0",
+          "east" => "-96.0",
+          "west" => "-96.2"
+        })
+
+      assert_reply ref, :error, %{message: "All bounds must be numeric"}
+    end
   end
 
   describe "update_bounds" do
@@ -191,6 +203,28 @@ defmodule AprsmeWeb.MobileChannelTest do
 
       assert_reply ref, :error, %{message: "North must be greater than south"}
     end
+
+    test "rejects invalid numeric strings on update without crashing", %{socket: socket} do
+      ref =
+        push(socket, "subscribe_bounds", %{
+          "north" => 33.2,
+          "south" => 33.0,
+          "east" => -96.0,
+          "west" => -96.2
+        })
+
+      assert_reply ref, :ok, _
+
+      ref =
+        push(socket, "update_bounds", %{
+          "north" => "bad",
+          "south" => "32.9",
+          "east" => "-95.9",
+          "west" => "-96.3"
+        })
+
+      assert_reply ref, :error, %{message: "All bounds must be numeric"}
+    end
   end
 
   describe "unsubscribe" do
@@ -409,6 +443,23 @@ defmodule AprsmeWeb.MobileChannelTest do
       refute_push "packet", _
     end
 
+    test "receives live postgres packets after subscribing by callsign only", %{socket: socket} do
+      ref = push(socket, "subscribe_callsign", %{"callsign" => "W5ISP-9"})
+      assert_reply ref, :ok, _
+
+      packet = %{
+        "sender" => "W5ISP-9",
+        "lat" => 33.1,
+        "lon" => -96.1,
+        "received_at" => DateTime.truncate(DateTime.utc_now(), :second)
+      }
+
+      send(socket.channel_pid, {:postgres_packet, packet})
+
+      assert_push "packet", pushed_packet
+      assert pushed_packet.callsign == "W5ISP-9"
+    end
+
     test "wildcard callsign matches multiple SSIDs", %{socket: socket} do
       # Subscribe to wildcard
       ref = push(socket, "subscribe_callsign", %{"callsign" => "W5ISP*"})

test/aprsme_web/plugs/api_csrf_test.exs

@@ -0,0 +1,39 @@
+defmodule AprsmeWeb.Plugs.ApiCSRFTest do
+  use AprsmeWeb.ConnCase, async: true
+
+  alias AprsmeWeb.Plugs.ApiCSRF
+
+  test "allows JSON requests with XMLHttpRequest header", %{conn: conn} do
+    conn =
+      conn
+      |> init_test_session(%{})
+      |> put_req_header("content-type", "application/json")
+      |> put_req_header("x-requested-with", "XMLHttpRequest")
+      |> ApiCSRF.call([])
+
+    refute conn.halted
+  end
+
+  test "rejects JSON requests with arbitrary bearer token", %{conn: conn} do
+    conn =
+      conn
+      |> init_test_session(%{})
+      |> put_req_header("content-type", "application/json")
+      |> put_req_header("authorization", "Bearer definitely-not-valid")
+      |> ApiCSRF.call([])
+
+    assert conn.halted
+    assert conn.status == 403
+    assert Jason.decode!(conn.resp_body)["error"] == "CSRF protection failed"
+  end
+
+  test "allows non-JSON requests without CSRF headers", %{conn: conn} do
+    conn =
+      conn
+      |> init_test_session(%{})
+      |> put_req_header("content-type", "text/plain")
+      |> ApiCSRF.call([])
+
+    refute conn.halted
+  end
+end