admin: keep FN when the save moved no name field
Assisted-by: Claude Opus 5 via Claude Code
change uqrotzxwyoyuopqonvmzqtmmvwqlqtwk
commit a8c80bd7b873d0a8de652af13d374238dd2e90b6
author Alpha Chen <alpha@kejadlen.dev>
date
parent pmzkpxyk
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index 6c57447..06027a7 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -754,7 +754,7 @@ module ProTacts
 
       card = contact.stored
       card = card.replace("N", [n_line(card, first, last)])
-      card = card.replace("FN", ["FN:#{VCard.escape([first, last].reject(&:empty?).join(" "))}\r\n"])
+      card = edited_fn(contact, card, first, last)
       card = card.replace("NICKNAME", text_lines("NICKNAME", nickname))
       card = card.replace("NOTE", text_lines("NOTE", note))
       card = edited_phones(contact, card, params)
@@ -962,6 +962,25 @@ module ProTacts
       ["#{name}:#{VCard.escape(value)}\r\n"]
     end
 
+    # FN's replacement, or the card untouched: the display name is the
+    # card's own text (RFC 2426 section 3.1.1), not a rendering of N,
+    # and "Dr. Ada B. Lovelace, Jr." holds parts no field on this form
+    # does — the same parts n_line rejoins byte for byte rather than
+    # dropping. Rebuilding it under a save that only moved a phone
+    # number would lose them, so FN is rewritten only when a name
+    # field moved under it, which is the one time the old display
+    # name is stale. A card carrying no FN is the other case: the
+    # property is mandatory (section 4), so a save fills it in rather
+    # than leaving the absence it found.
+    #: (Contact contact, VCard card, String first, String last) -> VCard
+    def edited_fn(contact, card, first, last)
+      family, given = contact.name_components || []
+      named = card.properties.any? { it.name.casecmp?("FN") }
+      return card if named && first == given.to_s && last == family.to_s
+
+      card.replace("FN", ["FN:#{VCard.escape([first, last].reject(&:empty?).join(" "))}\r\n"])
+    end
+
     # N's replacement line: a splice over the still-escaped value
     # (VCard.split_raw_components), not an unescape-then-re-escape round
     # trip, which is not byte-stable. The first two components are the
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index a1a8fe3..8758b06 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -731,6 +731,59 @@ class AdminContactsPagesTest < Minitest::Test
     end
   end
 
+  # FN is the card's own display name, not a rendering of N: a save
+  # that left both name fields alone must not rebuild it from them,
+  # or a card spelling the name in full loses every part the form has
+  # no field for — the parts N itself keeps.
+  def test_a_save_that_moves_no_name_field_keeps_the_cards_own_display_name
+    formal = ADA.sub("FN:Ada Lovelace", "FN:Dr. Ada B. Lovelace, Jr.")
+                .sub("N:Lovelace;Ada;;;", "N:Lovelace;Ada;B.;Dr.;Jr.")
+
+    with_contacts({"ada" => formal}) do |store|
+      digest = store.contact("ada").phones.first.line.digest
+      post "/contacts/ada", first: "Ada", last: "Lovelace", note: "Countess of Lovelace.",
+                               etag: store.contact("ada").etag, phone: {digest => "+1-555-0199"}
+
+      assert_equal 303, last_response.status
+      card = store.contact("ada").vcard.to_s
+      assert_includes card, "FN:Dr. Ada B. Lovelace, Jr.\r\n"
+      assert_includes card, "N:Lovelace;Ada;B.;Dr.;Jr.\r\n"
+      assert_includes card, "TEL;TYPE=mobile:+1-555-0199\r\n"
+    end
+  end
+
+  # A moved name field is the one time the old display name is stale,
+  # and then FN is the form's reading — N's other three components
+  # staying where they were, this being an edit of the name and not of
+  # the card's spelling of it.
+  def test_a_moved_name_field_rewrites_the_display_name
+    formal = ADA.sub("FN:Ada Lovelace", "FN:Dr. Ada B. Lovelace, Jr.")
+                .sub("N:Lovelace;Ada;;;", "N:Lovelace;Ada;B.;Dr.;Jr.")
+
+    with_contacts({"ada" => formal}) do |store|
+      post "/contacts/ada", first: "Ada", last: "King", etag: store.contact("ada").etag
+
+      assert_equal 303, last_response.status
+      card = store.contact("ada").vcard.to_s
+      assert_includes card, "FN:Ada King\r\n"
+      assert_includes card, "N:King;Ada;B.;Dr.;Jr.\r\n"
+    end
+  end
+
+  # The mandatory property is filled in rather than left absent (RFC
+  # 2426 section 4): a card with no FN has no display name to keep, so
+  # the untouched-name rule does not apply to it.
+  def test_a_save_writes_the_display_name_a_card_lacks
+    nameless = "BEGIN:VCARD\r\nVERSION:3.0\r\nN:Lovelace;Ada;;;\r\nUID:ada\r\nEND:VCARD\r\n"
+
+    with_contacts({"ada" => nameless}) do |store|
+      post "/contacts/ada", first: "Ada", last: "Lovelace", etag: store.contact("ada").etag
+
+      assert_equal 303, last_response.status
+      assert_includes store.contact("ada").vcard.to_s, "FN:Ada Lovelace\r\n"
+    end
+  end
+
   # Blank equals absent on the way back: a blank nickname or note
   # removes the property, as a blank value is absent to the reader
   # (Contact#text_of) — and filling one in adds it where the card