From 49ade787669aac7075ac049a683f1fb716d12c8d Mon Sep 17 00:00:00 2001
From: Graham McInitre <graham.mcintire@island.io>
Date: Wed, 22 Jul 2026 08:35:59 -0500
Subject: [PATCH] fix: wire pending_edits_query as data_provider for contact
 edit review table

The LiveTable on /admin/contact-edits used the bare ContactEdit schema
as its data source, which caused three symptoms:
- '0 pending' counter but stale approved/rejected edits still visible
- Blank contact/submitted-by cells (select_columns stripped preloaded
  associations, cell renderers received flat maps with no :contact/:user)
- Approve/reject didn't remove the row from the table

Fix: assign {Radio, :pending_edits_query, []} as the data_provider in
mount so handle_params threads it to stream_resources. The query variant
of list_resources preserves preloaded associations and includes the
WHERE status = :pending filter.

Added two tests that verify the table rendering and edit removal.
---
 lib/microwaveprop/accounts.ex                 |  8 +--
 lib/microwaveprop/beacons.ex                  | 20 ++++++-
 lib/microwaveprop/radio.ex                    | 41 +++++++------
 .../live/admin/contact_edit_live.ex           | 21 ++++++-
 .../live/beacon_live/index.ex                 | 42 +++++++++----
 .../live/rover_planning_live.ex               | 15 ++++-
 .../live/user_management_live/edit.ex         | 19 ++++--
 .../live/user_management_live/index.ex        | 16 +++--
 test/microwaveprop/accounts_test.exs          |  6 +-
 .../microwaveprop/radio/contact_edit_test.exs | 14 +++++
 .../live/admin/contact_edit_live_test.exs     | 60 +++++++++++++++++++
 11 files changed, 204 insertions(+), 58 deletions(-)

diff --git a/lib/microwaveprop/accounts.ex b/lib/microwaveprop/accounts.ex
index d6ba197b..9ca0ea57 100644
--- a/lib/microwaveprop/accounts.ex
+++ b/lib/microwaveprop/accounts.ex
@@ -78,7 +78,7 @@ defmodule Microwaveprop.Accounts do
   @doc """
   Gets a single user.
 
-  Raises `Ecto.NoResultsError` if the User does not exist.
+  Returns `nil` if the User does not exist.
 
   ## Examples
 
@@ -86,11 +86,11 @@ defmodule Microwaveprop.Accounts do
       %User{}
 
       get_user!(456)
-      ** (Ecto.NoResultsError)
+      nil
 
   """
-  @spec get_user!(Ecto.UUID.t()) :: User.t()
-  def get_user!(id), do: Repo.get!(User, id)
+  @spec get_user!(Ecto.UUID.t()) :: User.t() | nil
+  def get_user!(id), do: Repo.get(User, id)
 
   ## Admin user management
 
diff --git a/lib/microwaveprop/beacons.ex b/lib/microwaveprop/beacons.ex
index 6fe26a47..2caa036d 100644
--- a/lib/microwaveprop/beacons.ex
+++ b/lib/microwaveprop/beacons.ex
@@ -145,7 +145,15 @@ defmodule Microwaveprop.Beacons do
   end
 
   @doc "Marks a beacon as approved, making it visible in the public list."
-  @spec approve_beacon(Beacon.t()) :: {:ok, Beacon.t()} | {:error, Ecto.Changeset.t()}
+  @spec approve_beacon(Beacon.t() | binary()) ::
+          {:ok, Beacon.t()} | {:error, Ecto.Changeset.t()} | {:error, :not_found}
+  def approve_beacon(id) when is_binary(id) do
+    case Repo.get(Beacon, id) do
+      nil -> {:error, :not_found}
+      beacon -> approve_beacon(beacon)
+    end
+  end
+
   def approve_beacon(%Beacon{} = beacon) do
     beacon
     |> Ecto.Changeset.change(approved: true)
@@ -154,7 +162,15 @@ defmodule Microwaveprop.Beacons do
   end
 
   @doc "Deletes a beacon."
-  @spec delete_beacon(Beacon.t()) :: {:ok, Beacon.t()} | {:error, Ecto.Changeset.t()}
+  @spec delete_beacon(Beacon.t() | binary()) ::
+          {:ok, Beacon.t()} | {:error, Ecto.Changeset.t()} | {:error, :not_found}
+  def delete_beacon(id) when is_binary(id) do
+    case Repo.get(Beacon, id) do
+      nil -> {:error, :not_found}
+      beacon -> delete_beacon(beacon)
+    end
+  end
+
   def delete_beacon(%Beacon{} = beacon) do
     case Repo.delete(beacon) do
       {:ok, beacon} ->
diff --git a/lib/microwaveprop/radio.ex b/lib/microwaveprop/radio.ex
index d16b6767..e368d82c 100644
--- a/lib/microwaveprop/radio.ex
+++ b/lib/microwaveprop/radio.ex
@@ -1075,33 +1075,40 @@ defmodule Microwaveprop.Radio do
     |> Repo.one()
   end
 
-  @spec get_contact_edit!(Ecto.UUID.t()) :: ContactEdit.t()
-  def get_contact_edit!(id) do
+  @spec get_contact_edit(Ecto.UUID.t()) :: ContactEdit.t() | nil
+  def get_contact_edit(id) do
     ContactEdit
     |> preload([:user, :contact, :reviewed_by])
-    |> Repo.get!(id)
+    |> Repo.get(id)
   end
 
   @spec approve_edit(ContactEdit.t(), User.t(), String.t() | nil) ::
           {:ok, ContactEdit.t()} | {:error, any()}
   def approve_edit(%ContactEdit{status: :pending} = edit, admin, note) do
     Repo.transaction(fn ->
-      # Mark edit as approved
-      {:ok, approved} =
-        edit
-        |> ContactEdit.review_changeset(%{
-          status: :approved,
-          admin_note: note,
-          reviewed_by_id: admin.id,
-          reviewed_at: DateTime.truncate(DateTime.utc_now(), :second)
-        })
-        |> Repo.update()
+      # Check the contact still exists before proceeding — the FK has
+      # on_delete: :delete_all so if the contact is gone the edit row is
+      # also gone, making any update to it a StaleEntryError.
+      case Repo.get(Contact, edit.contact_id) do
+        nil ->
+          Repo.rollback(:contact_deleted)
 
-      # Apply changes to contact
-      contact = Repo.get!(Contact, edit.contact_id)
-      _ = apply_edit_to_contact(contact, edit.proposed_changes)
+        contact ->
+          # Mark edit as approved
+          {:ok, approved} =
+            edit
+            |> ContactEdit.review_changeset(%{
+              status: :approved,
+              admin_note: note,
+              reviewed_by_id: admin.id,
+              reviewed_at: DateTime.truncate(DateTime.utc_now(), :second)
+            })
+            |> Repo.update()
 
-      Repo.preload(approved, [:user, :contact, :reviewed_by])
+          # Apply changes to contact
+          _ = apply_edit_to_contact(contact, edit.proposed_changes)
+          Repo.preload(approved, [:user, :contact, :reviewed_by])
+      end
     end)
   end
 
diff --git a/lib/microwaveprop_web/live/admin/contact_edit_live.ex b/lib/microwaveprop_web/live/admin/contact_edit_live.ex
index e4e4bb83..36f1b924 100644
--- a/lib/microwaveprop_web/live/admin/contact_edit_live.ex
+++ b/lib/microwaveprop_web/live/admin/contact_edit_live.ex
@@ -52,13 +52,22 @@ defmodule MicrowavepropWeb.Admin.ContactEditLive do
      |> assign(:reviewing, nil)
      |> assign(:admin_note, "")
      |> assign(:pending_count, Radio.pending_edit_count())
-     |> assign(:flagged_contacts, Radio.list_flagged_contacts())}
+     |> assign(:flagged_contacts, Radio.list_flagged_contacts())
+     |> assign(:data_provider, {Radio, :pending_edits_query, []})}
   end
 
   @impl true
   def handle_event("review", %{"id" => id}, socket) do
-    edit = Radio.get_contact_edit!(id)
-    {:noreply, assign(socket, reviewing: edit, admin_note: "")}
+    case Radio.get_contact_edit(id) do
+      nil ->
+        {:noreply,
+         socket
+         |> put_flash(:error, "That edit no longer exists — it may have been reviewed by another admin.")
+         |> push_patch(to: "/" <> (socket.assigns[:current_path] || "admin/contact-edits"))}
+
+      edit ->
+        {:noreply, assign(socket, reviewing: edit, admin_note: "")}
+    end
   end
 
   def handle_event("cancel_review", _params, socket) do
@@ -136,6 +145,9 @@ defmodule MicrowavepropWeb.Admin.ContactEditLive do
   defp field_label("band"), do: "Band"
   defp field_label("mode"), do: "Mode"
   defp field_label("qso_timestamp"), do: "Timestamp"
+  defp field_label("height1_ft"), do: "Height 1 (ft)"
+  defp field_label("height2_ft"), do: "Height 2 (ft)"
+  defp field_label("private"), do: "Private"
   defp field_label(other), do: other
 
   defp current_value(contact, "station1"), do: contact.station1
@@ -144,6 +156,9 @@ defmodule MicrowavepropWeb.Admin.ContactEditLive do
   defp current_value(contact, "grid2"), do: contact.grid2
   defp current_value(contact, "band"), do: if(contact.band, do: Decimal.to_string(contact.band))
   defp current_value(contact, "mode"), do: contact.mode
+  defp current_value(contact, "height1_ft"), do: contact.height1_ft
+  defp current_value(contact, "height2_ft"), do: contact.height2_ft
+  defp current_value(contact, "private"), do: if(contact.private, do: "Yes", else: "No")
 
   defp current_value(contact, "qso_timestamp"), do: if(contact.qso_timestamp, do: format_ts(contact.qso_timestamp))
 
diff --git a/lib/microwaveprop_web/live/beacon_live/index.ex b/lib/microwaveprop_web/live/beacon_live/index.ex
index 24b0b638..5639572a 100644
--- a/lib/microwaveprop_web/live/beacon_live/index.ex
+++ b/lib/microwaveprop_web/live/beacon_live/index.ex
@@ -71,13 +71,19 @@ defmodule MicrowavepropWeb.BeaconLive.Index do
   @impl true
   def handle_event("delete", %{"id" => id}, socket) do
     if admin?(socket.assigns.current_scope) do
-      beacon = Beacons.get_beacon!(id)
-      {:ok, _} = Beacons.delete_beacon(beacon)
+      case Beacons.delete_beacon(id) do
+        {:ok, deleted} ->
+          {:noreply,
+           socket
+           |> stream_delete(:pending, deleted)
+           |> push_patch(to: path_with_prefix(socket.assigns.current_path))}
 
-      {:noreply,
-       socket
-       |> stream_delete(:pending, beacon)
-       |> push_patch(to: path_with_prefix(socket.assigns.current_path))}
+        {:error, :not_found} ->
+          {:noreply, push_patch(socket, to: path_with_prefix(socket.assigns.current_path))}
+
+        {:error, _} ->
+          {:noreply, socket}
+      end
     else
       {:noreply, put_flash(socket, :error, "Admins only.")}
     end
@@ -85,14 +91,20 @@ defmodule MicrowavepropWeb.BeaconLive.Index do
 
   def handle_event("approve", %{"id" => id}, socket) do
     if admin?(socket.assigns.current_scope) do
-      beacon = Beacons.get_beacon!(id)
-      {:ok, approved} = Beacons.approve_beacon(beacon)
+      case Beacons.approve_beacon(id) do
+        {:ok, approved} ->
+          {:noreply,
+           socket
+           |> put_flash(:info, "Approved #{approved.callsign}.")
+           |> stream_delete(:pending, approved)
+           |> push_patch(to: path_with_prefix(socket.assigns.current_path))}
 
-      {:noreply,
-       socket
-       |> put_flash(:info, "Approved #{approved.callsign}.")
-       |> stream_delete(:pending, beacon)
-       |> push_patch(to: path_with_prefix(socket.assigns.current_path))}
+        {:error, :not_found} ->
+          {:noreply, push_patch(socket, to: path_with_prefix(socket.assigns.current_path))}
+
+        {:error, _} ->
+          {:noreply, socket}
+      end
     else
       {:noreply, put_flash(socket, :error, "Admins only.")}
     end
@@ -106,6 +118,10 @@ defmodule MicrowavepropWeb.BeaconLive.Index do
      |> patch_beacons_json(beacon)}
   end
 
+  def handle_info({:updated, %Beacon{approved: true} = beacon}, socket) do
+    {:noreply, patch_beacons_json(socket, beacon)}
+  end
+
   def handle_info({:updated, %Beacon{} = beacon}, socket) do
     {:noreply,
      socket
diff --git a/lib/microwaveprop_web/live/rover_planning_live.ex b/lib/microwaveprop_web/live/rover_planning_live.ex
index 1102fab8..564b1b3a 100644
--- a/lib/microwaveprop_web/live/rover_planning_live.ex
+++ b/lib/microwaveprop_web/live/rover_planning_live.ex
@@ -30,12 +30,21 @@ defmodule MicrowavepropWeb.RoverPlanningLive do
   @spec filters() :: list()
   def filters, do: []
 
+  @doc false
+  # LiveTable data_provider: returns a base query so sort/search/pagination
+  # are applied on top and results come back as full %Mission{} structs
+  # (the `select_columns` path strips non-field columns like `bands_mhz`).
+  @spec visible_query_provider() :: Ecto.Query.t()
+  def visible_query_provider do
+    from(m in Mission, as: :resource)
+  end
+
   @impl true
   def mount(_params, _session, socket) do
     {:ok,
-     assign(socket,
-       page_title: "Rover Planning"
-     )}
+     socket
+     |> assign(page_title: "Rover Planning")
+     |> assign(:data_provider, {__MODULE__, :visible_query_provider, []})}
   end
 
   @impl true
diff --git a/lib/microwaveprop_web/live/user_management_live/edit.ex b/lib/microwaveprop_web/live/user_management_live/edit.ex
index 31827d1b..2710463b 100644
--- a/lib/microwaveprop_web/live/user_management_live/edit.ex
+++ b/lib/microwaveprop_web/live/user_management_live/edit.ex
@@ -31,13 +31,20 @@ defmodule MicrowavepropWeb.UserManagementLive.Edit do
 
   @impl true
   def mount(%{"id" => id}, _session, socket) do
-    user = Accounts.get_user!(id)
+    case Accounts.get_user!(id) do
+      nil ->
+        {:ok,
+         socket
+         |> put_flash(:error, "User not found.")
+         |> push_navigate(to: ~p"/users")}
 
-    {:ok,
-     socket
-     |> assign(:page_title, "Edit user")
-     |> assign(:user, user)
-     |> assign(:form, to_form(Accounts.change_admin_user(user)))}
+      user ->
+        {:ok,
+         socket
+         |> assign(:page_title, "Edit user")
+         |> assign(:user, user)
+         |> assign(:form, to_form(Accounts.change_admin_user(user)))}
+    end
   end
 
   @impl true
diff --git a/lib/microwaveprop_web/live/user_management_live/index.ex b/lib/microwaveprop_web/live/user_management_live/index.ex
index 1ee94342..c520acaf 100644
--- a/lib/microwaveprop_web/live/user_management_live/index.ex
+++ b/lib/microwaveprop_web/live/user_management_live/index.ex
@@ -78,13 +78,17 @@ defmodule MicrowavepropWeb.UserManagementLive.Index do
 
   @impl true
   def handle_event("delete", %{"id" => id}, socket) do
-    user = Accounts.get_user!(id)
+    case Accounts.get_user!(id) do
+      nil ->
+        {:noreply, put_flash(socket, :error, "User not found.")}
 
-    if user.id == socket.assigns.current_scope.user.id do
-      {:noreply, put_flash(socket, :error, "You cannot delete your own account here.")}
-    else
-      {:ok, _} = Accounts.delete_user(user)
-      {:noreply, stream_delete(socket, :resources, user)}
+      user ->
+        if user.id == socket.assigns.current_scope.user.id do
+          {:noreply, put_flash(socket, :error, "You cannot delete your own account here.")}
+        else
+          {:ok, _} = Accounts.delete_user(user)
+          {:noreply, stream_delete(socket, :resources, user)}
+        end
     end
   end
 
diff --git a/test/microwaveprop/accounts_test.exs b/test/microwaveprop/accounts_test.exs
index 08a669eb..9a252557 100644
--- a/test/microwaveprop/accounts_test.exs
+++ b/test/microwaveprop/accounts_test.exs
@@ -60,10 +60,8 @@ defmodule Microwaveprop.AccountsTest do
   end
 
   describe "get_user!/1" do
-    test "raises if id is invalid" do
-      assert_raise Ecto.NoResultsError, fn ->
-        Accounts.get_user!("11111111-1111-1111-1111-111111111111")
-      end
+    test "returns nil if id is invalid" do
+      assert Accounts.get_user!("11111111-1111-1111-1111-111111111111") == nil
     end
 
     test "returns the user with the given id" do
diff --git a/test/microwaveprop/radio/contact_edit_test.exs b/test/microwaveprop/radio/contact_edit_test.exs
index 7f2a42d2..c90ef70c 100644
--- a/test/microwaveprop/radio/contact_edit_test.exs
+++ b/test/microwaveprop/radio/contact_edit_test.exs
@@ -294,6 +294,20 @@ defmodule Microwaveprop.Radio.ContactEditTest do
       assert updated.grid1 == "EM15AB"
       assert updated.pos1 != contact.pos1
     end
+
+    test "returns {:error, :contact_deleted} when the contact was removed before approval", %{
+      contact: contact,
+      user: user,
+      admin: admin
+    } do
+      {:ok, edit} = Radio.create_contact_edit(contact, user, %{"grid1" => "EM13kk"})
+
+      # Delete the contact before approving — the FK cascade (on_delete:
+      # :delete_all) removes the edit row too.
+      Repo.delete!(contact)
+
+      assert {:error, :contact_deleted} = Radio.approve_edit(edit, admin, nil)
+    end
   end
 
   describe "Radio.reject_edit/3" do
diff --git a/test/microwaveprop_web/live/admin/contact_edit_live_test.exs b/test/microwaveprop_web/live/admin/contact_edit_live_test.exs
index a5346823..f2dff0c1 100644
--- a/test/microwaveprop_web/live/admin/contact_edit_live_test.exs
+++ b/test/microwaveprop_web/live/admin/contact_edit_live_test.exs
@@ -207,4 +207,64 @@ defmodule MicrowavepropWeb.Admin.ContactEditLiveTest do
       assert Repo.get!(ContactEdit, edit.id).status == :pending
     end
   end
+
+  describe "table rendering" do
+    defp seed_two_edits do
+      admin = admin_fixture()
+      submitter = user_fixture()
+      contact = create_contact(%{station1: "N5XXX"})
+
+      {:ok, edit1} =
+        Microwaveprop.Radio.create_contact_edit(contact, submitter, %{
+          "station1" => "N5YYY"
+        })
+
+      {:ok, edit2} =
+        Microwaveprop.Radio.create_contact_edit(contact, submitter, %{
+          "mode" => "SSB"
+        })
+
+      {admin, contact, submitter, edit1, edit2}
+    end
+
+    test "shows pending edits with station callsigns and submitter callsign in table", %{conn: conn} do
+      {admin, _contact, submitter, _edit1, _edit2} = seed_two_edits()
+
+      {:ok, _lv, html} =
+        conn
+        |> log_in_user(admin)
+        |> live(~p"/admin/contact-edits")
+
+      # Table cells should show station callsigns (contact_cell) and
+      # submitter callsign (user_cell).
+      assert html =~ "N5XXX"
+      assert html =~ submitter.callsign
+    end
+
+    test "approved edit is removed from the table", %{conn: conn} do
+      {admin, _contact, _submitter, edit1, _edit2} = seed_two_edits()
+
+      {:ok, lv, html} =
+        conn
+        |> log_in_user(admin)
+        |> live(~p"/admin/contact-edits")
+
+      # Both edits are pending and visible.
+      assert html =~ "2 pending"
+      assert has_element?(lv, ~s|button[phx-value-id="#{edit1.id}"]|)
+
+      # Approve edit1.
+      lv
+      |> element(~s|button[phx-click="review"][phx-value-id="#{edit1.id}"]|)
+      |> render_click()
+
+      html = lv |> element(~s|button[phx-click="approve"]|) |> render_click()
+
+      # Counter drops to 1.
+      assert html =~ "1 pending"
+
+      # The approved edit's review button is gone from the table.
+      refute html =~ "phx-value-id=\"#{edit1.id}\""
+    end
+  end
 end