From 2365c1932119b29160982528edac43b227a1a891 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 25 Apr 2026 10:03:24 -0500 Subject: [PATCH] fix(radio): coerce qso_timestamp string in normalize_proposed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /contacts/:id edit form pre-fills qso_timestamp via `Calendar.strftime(ts, "%Y-%m-%d %H:%M")` and submits whatever the user typed back unchanged. Two failure modes resulted: 1. `diff_against_contact/2` compared that string against the stored `%DateTime{}`, never matched, and treated every untouched submit as a modification. 2. `build_contact_changes/2` then ran `DateTime.from_iso8601/1` on the space-separated, no-seconds, no-Z form and crashed with `MatchError` on `{:error, :invalid_format}` — direct owner/admin submits raised instead of saving. Adds `normalize_timestamp_field/2` to `normalize_proposed/1` that parses both the form's `"YYYY-MM-DD HH:MM[:SS]"` and the strict `"YYYY-MM-DDTHH:MM:SSZ"` shapes into a UTC `%DateTime{}` (with `NaiveDateTime.from_iso8601` doing the heavy lifting after a small seconds-padding pass). Empty strings drop the field; unparseable input drops it too so a typo becomes "no changes" rather than a 500. --- lib/microwaveprop/radio.ex | 45 +++++++++++++++++ .../microwaveprop/radio/contact_edit_test.exs | 49 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/lib/microwaveprop/radio.ex b/lib/microwaveprop/radio.ex index 5db7702e..6c4b5019 100644 --- a/lib/microwaveprop/radio.ex +++ b/lib/microwaveprop/radio.ex @@ -819,6 +819,51 @@ defmodule Microwaveprop.Radio do |> normalize_integer_field("height1_ft") |> normalize_integer_field("height2_ft") |> normalize_boolean_field("private") + |> normalize_timestamp_field("qso_timestamp") + end + + # The /contacts/:id edit form pre-fills qso_timestamp as + # `"YYYY-MM-DD HH:MM"` via Calendar.strftime, then submits whatever + # the user typed. Coerce to %DateTime{} (UTC) so the diff matches + # against the stored DateTime instead of always reading as a change, + # and so the apply path doesn't crash on a non-ISO string. Drop the + # field when empty or unparseable so a bogus value is treated as + # "no change" rather than raising. + defp normalize_timestamp_field(map, key) do + case Map.get(map, key, :not_provided) do + :not_provided -> map + nil -> Map.delete(map, key) + "" -> Map.delete(map, key) + %DateTime{} = dt -> Map.put(map, key, dt) + val when is_binary(val) -> put_or_drop_timestamp(map, key, parse_timestamp(val)) + _ -> Map.delete(map, key) + end + end + + defp put_or_drop_timestamp(map, key, nil), do: Map.delete(map, key) + defp put_or_drop_timestamp(map, key, %DateTime{} = dt), do: Map.put(map, key, dt) + + defp parse_timestamp(val) do + trimmed = String.trim(val) + + # Accept `"YYYY-MM-DD HH:MM[:SS][Z]"` (form pre-fill, space sep) and + # the strict ISO `"YYYY-MM-DDTHH:MM:SSZ"` form. NaiveDateTime handles + # both separators and an optional seconds field. + iso = trimmed |> String.replace(" ", "T") |> ensure_seconds() |> String.trim_trailing("Z") + + case NaiveDateTime.from_iso8601(iso) do + {:ok, ndt} -> DateTime.from_naive!(ndt, "Etc/UTC") + _ -> nil + end + end + + defp ensure_seconds(s) do + # Add ":00" before any trailing "Z" / end-of-string when only HH:MM + # is present, since NaiveDateTime.from_iso8601 requires seconds. + case Regex.run(~r/^(\d{4}-\d{2}-\d{2}T\d{2}:\d{2})(Z?)$/, s) do + [_, prefix, tail] -> prefix <> ":00" <> tail + _ -> s + end end # HTML checkboxes arrive as "true"/"false" strings; Ecto won't coerce diff --git a/test/microwaveprop/radio/contact_edit_test.exs b/test/microwaveprop/radio/contact_edit_test.exs index ab4b668a..c108b1a4 100644 --- a/test/microwaveprop/radio/contact_edit_test.exs +++ b/test/microwaveprop/radio/contact_edit_test.exs @@ -400,6 +400,55 @@ defmodule Microwaveprop.Radio.ContactEditTest do assert updated.height1_ft == 30 assert updated.terrain_status == :pending end + + test "untouched form (qso_timestamp pre-fill string) is treated as no-op", %{user: user} do + # The /contacts/:id edit form pre-fills qso_timestamp via + # `Calendar.strftime(ts, "%Y-%m-%d %H:%M")`. When the user submits + # without touching it, the diff used to compare a string against + # the stored DateTime, treat it as a change, and then crash in + # build_contact_changes on `DateTime.from_iso8601("2026-04-01 14:00")`. + contact = owned_contact(user) + pre_fill = Calendar.strftime(contact.qso_timestamp, "%Y-%m-%d %H:%M") + + params = %{ + "station1" => contact.station1, + "station2" => contact.station2, + "grid1" => contact.grid1, + "grid2" => contact.grid2, + "qso_timestamp" => pre_fill, + "mode" => contact.mode + } + + assert {:error, :no_changes} = Radio.apply_owner_edit(contact, user, params) + end + + test "owner can edit qso_timestamp via the form's space-separated input", %{user: user} do + contact = owned_contact(user) + + {:ok, updated} = + Radio.apply_owner_edit(contact, user, %{"qso_timestamp" => "2026-04-01 15:30"}) + + assert updated.qso_timestamp == ~U[2026-04-01 15:30:00Z] + end + + test "owner can edit qso_timestamp via ISO 8601 with seconds + Z", %{user: user} do + contact = owned_contact(user) + + {:ok, updated} = + Radio.apply_owner_edit(contact, user, %{"qso_timestamp" => "2026-04-01T15:30:00Z"}) + + assert updated.qso_timestamp == ~U[2026-04-01 15:30:00Z] + end + + test "unparseable qso_timestamp drops the field instead of crashing", %{user: user} do + contact = owned_contact(user) + + # Submitting just a bogus timestamp with no other fields means no + # other changes to apply — apply_owner_edit returns :no_changes + # rather than raising a MatchError on DateTime.from_iso8601. + assert {:error, :no_changes} = + Radio.apply_owner_edit(contact, user, %{"qso_timestamp" => "not a real date"}) + end end describe "Radio.owner?/2" do