From 13996583bd4a1e768eea0f5a8c215c47e1cce435 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Sat, 25 Apr 2026 10:04:39 -0500
Subject: [PATCH] fix(radio): add private clause to current_value/2
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The /contacts/:id edit form always submits the `private` checkbox, but
`current_value/2` had no `"private"` clause and fell through to the
catch-all `nil`. The Contact schema defaults `private: false`, so an
unchanged box compared `false != nil` and was always tagged as a
change — for non-owners that meant a spurious pending review queued
on every save, and for owners/admins it triggered a no-op direct
update.

Adds the missing clause so the diff sees boolean-vs-boolean and
correctly returns `:no_changes` when the box is untouched.
---
 lib/microwaveprop/radio.ex                    |  1 +
 .../microwaveprop/radio/contact_edit_test.exs | 32 +++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/lib/microwaveprop/radio.ex b/lib/microwaveprop/radio.ex
index 6c4b5019..2f4614f9 100644
--- a/lib/microwaveprop/radio.ex
+++ b/lib/microwaveprop/radio.ex
@@ -941,6 +941,7 @@ defmodule Microwaveprop.Radio do
   defp current_value(contact, "qso_timestamp"), do: contact.qso_timestamp
   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: contact.private
   defp current_value(_contact, _key), do: nil
 
   defp values_equal?(a, b) when is_binary(a) and is_binary(b) do
diff --git a/test/microwaveprop/radio/contact_edit_test.exs b/test/microwaveprop/radio/contact_edit_test.exs
index c108b1a4..147e0784 100644
--- a/test/microwaveprop/radio/contact_edit_test.exs
+++ b/test/microwaveprop/radio/contact_edit_test.exs
@@ -449,6 +449,38 @@ defmodule Microwaveprop.Radio.ContactEditTest do
       assert {:error, :no_changes} =
                Radio.apply_owner_edit(contact, user, %{"qso_timestamp" => "not a real date"})
     end
+
+    test "unchanged private checkbox is treated as no-op", %{user: user} do
+      contact = owned_contact(user)
+      # Schema default is `private: false`; the form always submits the
+      # current state. Without a current_value/2 clause the diff sees
+      # `false != nil` and treats it as a real change.
+      assert contact.private == false
+
+      assert {:error, :no_changes} =
+               Radio.apply_owner_edit(contact, user, %{"private" => false})
+    end
+
+    test "owner can flip private from false to true", %{user: user} do
+      contact = owned_contact(user)
+
+      {:ok, updated} = Radio.apply_owner_edit(contact, user, %{"private" => true})
+
+      assert updated.private == true
+    end
+
+    test "checkbox 'true'/'false' strings round-trip correctly", %{user: user} do
+      contact = owned_contact(user)
+
+      # Form delivers booleans as strings; the existing
+      # normalize_boolean_field coerces them, but the diff still has to
+      # use the boolean current_value for correctness.
+      assert {:error, :no_changes} =
+               Radio.apply_owner_edit(contact, user, %{"private" => "false"})
+
+      {:ok, updated} = Radio.apply_owner_edit(contact, user, %{"private" => "true"})
+      assert updated.private == true
+    end
   end
 
   describe "Radio.owner?/2" do