admin: phone rows on the edit screen, saved by digest
Assisted-by: GLM-5.3 via pi
change plklqoopszlmvszqkrtxomluxmxqmyyx
commit 828b2593be9cbe9ce0a66d8d3cb24b4f1fabf90f
author Alpha Chen <alpha@kejadlen.dev>
date
parent utrtmnuq
diff --git a/lib/pro_tacts/admin/contacts_edit.rb b/lib/pro_tacts/admin/contacts_edit.rb
index 8344fe5..038e70a 100644
--- a/lib/pro_tacts/admin/contacts_edit.rb
+++ b/lib/pro_tacts/admin/contacts_edit.rb
@@ -7,10 +7,11 @@ module ProTacts
     # GET /contacts/:id/edit — the editor
     # (docs/plans/2026-09-05-web-card-editor.md): an explicit mode
     # rather than an always-editable page, one form over the
-    # properties the save knows how to address. This stage carries the
-    # cardinality-1 set — name, nickname, note — whose save is
-    # VCard#replace under each field; phones, emails, and addresses
-    # arrive with their own stages, birthday last.
+    # properties the save knows how to address. The cardinality-1 set
+    # — name, nickname, note — saves through VCard#replace under each
+    # field; the phone rows save through VCard#substitute, each named
+    # by its line's digest. Emails and addresses arrive with their
+    # own stage, birthday last.
     #
     # The fields prefill from the accessors' unescaped readings, and
     # blank equals absent on the way back (Web#edited_card), so write
@@ -68,6 +69,31 @@ module ProTacts
                     plain "Nickname"
                     input(type: "text", name: "nickname", value: @contact.nickname)
                   end
+                  # A phone row edits its value and nothing else: the
+                  # TYPE parameters ride in the line's own header,
+                  # which the save keeps (VCard.header_of) — a header
+                  # rebuilt from form fields would drop the parameters
+                  # no field models, and macOS writes three TYPE
+                  # parameters on one TEL. The digest in the field's
+                  # name is the row's address, and a blank value
+                  # removes the line. Identical duplicate lines share a
+                  # digest and therefore a field name, and Rack keeps
+                  # the last value of a duplicated name — editing one
+                  # of a pair of byte-identical rows means blanking
+                  # one, saving, then editing the other.
+                  @contact.phones.each do |phone|
+                    label(class: "field") do
+                      plain phone.type || "phone"
+                      input(type: "tel", name: "phone[#{phone.line.digest}]", value: phone.value)
+                    end
+                  end
+                  # One add-row: a value lands as a bare TEL before
+                  # END:VCARD, and blank inserts nothing — inserting
+                  # absence is a no-op, the plan's rule for new rows.
+                  label(class: "field") do
+                    plain "add phone"
+                    input(type: "tel", name: "new_phone")
+                  end
                   label(class: "field") do
                     plain "Note"
                     textarea(name: "note", rows: 4) { @contact.notes.to_s }
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index cddc905..d9fea19 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -706,7 +706,44 @@ module ProTacts
       card = card.replace("FN", ["FN:#{VCard.escape([first, last].reject(&:empty?).join(" "))}\r\n"])
       card = card.replace("NICKNAME", text_lines("NICKNAME", nickname))
       card = card.replace("NOTE", text_lines("NOTE", note))
-      card
+      edited_phones(contact, card, params)
+    end
+
+    # The phones' half of the surgical save: each row names its line
+    # by digest, so a row the request leaves out is a line the save
+    # never mentions, and an unchanged row is skipped — the line keeps
+    # its own bytes by construction, not by careful re-rendering. A
+    # blank row removes its line; a changed row swaps the value under
+    # the line's own header (VCard.header_of), where the TYPE
+    # parameters no field models ride. The addresses come from the
+    # contact, never the request, so a doctored digest names nothing
+    # and a missing one touches nothing.
+    #: (Contact contact, VCard card, Hash[String, untyped] params) -> VCard
+    def edited_phones(contact, card, params)
+      rows = params["phone"]
+      if rows.is_a?(Hash)
+        contact.phones.each do |phone|
+          # The property is nil only for a line that would not read,
+          # and phones read from lines that did; the guard is the
+          # type's honesty, not a reachable case (n_line's own shape).
+          property = phone.line.property
+          next if property.nil?
+
+          submitted = rows[phone.line.digest]
+          next if submitted.nil? || submitted.to_s.strip == phone.value
+
+          value = submitted.to_s.strip
+          card = card.substitute(
+            phone.line.digest,
+            value.empty? ? [] : ["#{VCard.header_of(property)}#{VCard.escape(value)}"]
+          )
+        end
+      end
+
+      new_phone = params["new_phone"].to_s.strip
+      added = [] #: Array[String]
+      added.replace(["TEL:#{VCard.escape(new_phone)}\r\n"]) unless new_phone.empty?
+      card.insert(added)
     end
 
     # A text property's replacement lines: the escaped value (RFC 2426
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index 8af593d..ac2f85c 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -744,6 +744,133 @@ class AdminContactsPagesTest < Minitest::Test
     end
   end
 
+  ## Editing phones
+
+  # The phone rows: one value field per line, named by the line's
+  # digest — the address the save substitutes — under the type's own
+  # caption, and one add-row beneath them.
+  def test_the_edit_screen_renders_a_digest_named_field_per_phone
+    with_contacts({"ada" => ADA}) do |store|
+      get "/contacts/ada/edit"
+
+      digest = store.contact("ada").phones.first.line.digest
+      body = last_response.body
+      assert_includes body, '<label class="field">mobile<input type="tel" name="phone[' + digest + ']" value="+1-555-0100"></label>'
+      assert_includes body, '<input type="tel" name="new_phone">'
+    end
+  end
+
+  # The addressed edit at work: the submitted row swaps its line's
+  # value under the line's own header, and every other byte of the
+  # stored card — the other TEL-able lines included — stands
+  # untouched.
+  def test_saving_a_changed_phone_replaces_only_its_line
+    two = ADA.sub("TEL;TYPE=mobile:+1-555-0100\r\n", "TEL;TYPE=mobile:+1-555-0100\r\nTEL;TYPE=work:+1-555-0199\r\n")
+
+    with_contacts({"ada" => two}) do |store|
+      digest = store.contact("ada").phones.first.line.digest
+      post "/contacts/ada", first: "Ada", last: "Lovelace", etag: store.contact("ada").etag,
+                               phone: {digest => "+1-555-0150"}
+
+      assert_equal 303, last_response.status
+      card = store.contact("ada").vcard.to_s
+      assert_includes card, "TEL;TYPE=mobile:+1-555-0150\r\n"
+      assert_includes card, "TEL;TYPE=work:+1-555-0199\r\n"
+      refute_includes card, "+1-555-0100"
+
+      follow_redirect!
+      assert_includes last_response.body, "+1-555-0150"
+    end
+  end
+
+  # The header is the line's own, kept byte for byte: macOS writes
+  # three TYPE parameters on one TEL, and a save that rebuilt the
+  # header from a type field would drop the two the form never
+  # showed. The value is the only thing that moves.
+  def test_a_saved_phone_keeps_its_lines_own_parameters
+    apple = ADA.sub("TEL;TYPE=mobile:+1-555-0100", "TEL;type=CELL;type=VOICE;type=pref:+1-555-0100")
+
+    with_contacts({"ada" => apple}) do |store|
+      digest = store.contact("ada").phones.first.line.digest
+      post "/contacts/ada", first: "Ada", last: "Lovelace", etag: store.contact("ada").etag,
+                               phone: {digest => "+1-555-0150"}
+
+      assert_equal 303, last_response.status
+      assert_includes store.contact("ada").vcard.to_s,
+        "TEL;type=CELL;type=VOICE;type=pref:+1-555-0150\r\n"
+    end
+  end
+
+  # Blank equals absent for a row with a digest: the blank removes
+  # the line, the reader's own rule in the other direction. The FN
+  # assertion keeps an empty-card regression from passing vacuously.
+  def test_a_blank_phone_row_removes_the_line
+    with_contacts({"ada" => ADA}) do |store|
+      digest = store.contact("ada").phones.first.line.digest
+      post "/contacts/ada", first: "Ada", last: "Lovelace", etag: store.contact("ada").etag,
+                               phone: {digest => " "}
+
+      assert_equal 303, last_response.status
+      card = store.contact("ada").vcard.to_s
+      refute_includes card, "TEL"
+      assert_includes card, "FN:Ada Lovelace\r\n"
+    end
+  end
+
+  # An unchanged row never rewrites its line — byte identity by
+  # construction, not by faithful re-rendering — and the whole save
+  # leaves the stored card byte for byte as it lay.
+  def test_an_unchanged_phone_leaves_the_stored_card_byte_identical
+    with_contacts({"ada" => ADA}) do |store|
+      before = store.contact("ada").stored.to_s
+      digest = store.contact("ada").phones.first.line.digest
+      post "/contacts/ada", first: "Ada", last: "Lovelace", note: "Countess of Lovelace.",
+                               etag: store.contact("ada").etag, new_phone: "",
+                               phone: {digest => " +1-555-0100 "}
+
+      assert_equal 303, last_response.status
+      assert_equal before, store.contact("ada").stored.to_s
+    end
+  end
+
+  # The add-row: a value lands as a bare TEL before END:VCARD — the
+  # served card composes the BDAY in under it — and a blank one
+  # inserts nothing, inserting absence being a no-op.
+  def test_an_added_phone_lands_as_a_bare_tel_before_the_end
+    with_contacts({"ada" => ADA}) do |store|
+      post "/contacts/ada", first: "Ada", last: "Lovelace", etag: store.contact("ada").etag,
+                               new_phone: "+1 (555) 020-0700, x9"
+
+      assert_equal 303, last_response.status
+      card = store.contact("ada").vcard.to_s
+      assert_includes card, "TEL:+1 (555) 020-0700\\, x9\r\nBDAY:1985-12-10\r\nEND:VCARD\r\n"
+      assert_includes card, "TEL;TYPE=mobile:+1-555-0100\r\n"
+    end
+  end
+
+  def test_a_blank_add_row_inserts_nothing
+    with_contacts({"ada" => ADA}) do |store|
+      post "/contacts/ada", first: "Ada", last: "Lovelace", etag: store.contact("ada").etag,
+                               new_phone: " "
+
+      assert_equal 303, last_response.status
+      assert_equal 1, store.contact("ada").phones.length
+    end
+  end
+
+  # The addresses come from the contact, never the request: a
+  # doctored digest names no row and touches nothing, where a
+  # doctored index into a list would have rewritten some other line.
+  def test_a_digest_the_card_never_had_is_ignored
+    with_contacts({"ada" => ADA}) do |store|
+      post "/contacts/ada", first: "Ada", last: "Lovelace", etag: store.contact("ada").etag,
+                               phone: {"0" * 64 => "+1-555-9999"}
+
+      assert_equal 303, last_response.status
+      assert_includes store.contact("ada").vcard.to_s, "TEL;TYPE=mobile:+1-555-0100\r\n"
+    end
+  end
+
   # The snapshot guard: a save whose etag names a card the store no
   # longer holds is refused with a toast, and nothing is written —
   # another tab or a client sync edited the contact in between, and