contact: read an empty property value as absent
A blank ADR position or an empty FN:/TEL:/NOTE: arrived as "" where
a missing one arrived as nil, so every caller had to tell the two apart.

Assisted-by: GLM-5.3 via pi
change swrktymluwnuwwnnwqlokrtpxzskyrry
commit 1a9d270c93c9fc8d39c3f596079c72fc77e196f2
author Alpha Chen <alpha@kejadlen.dev>
date
parent omuvvxku
diff --git a/lib/pro_tacts/admin/contacts_show.rb b/lib/pro_tacts/admin/contacts_show.rb
index aff0af1..5c38654 100644
--- a/lib/pro_tacts/admin/contacts_show.rb
+++ b/lib/pro_tacts/admin/contacts_show.rb
@@ -71,10 +71,10 @@ module ProTacts
       #: (Contact::Address address) -> Array[String]
       def address_lines(address)
         [
-          [address.extended, address.street].reject { |s| s.nil? || s.empty? }.join(" "),
-          [address.locality, address.region, address.postal_code].reject { |s| s.nil? || s.empty? }.join(", "),
+          [address.extended, address.street].compact.join(" "),
+          [address.locality, address.region, address.postal_code].compact.join(", "),
           address.country,
-        ].reject { |s| s.nil? || s.empty? }
+        ].compact.reject { it.empty? }
       end
 
       #: (String | Array[String] value) -> void
diff --git a/lib/pro_tacts/admin/format.rb b/lib/pro_tacts/admin/format.rb
index 6dc45bf..a562f92 100644
--- a/lib/pro_tacts/admin/format.rb
+++ b/lib/pro_tacts/admin/format.rb
@@ -16,7 +16,7 @@ module ProTacts
       #: (Contact contact) -> String
       def self.initials(contact)
         family, given = contact.name_components || []
-        letters = [given, family].filter_map { it[0] unless it.to_s.empty? }
+        letters = [given, family].filter_map { it[0] if it }
         return letters.join.upcase unless letters.empty?
 
         (contact.name || contact.id).to_s.strip[0].to_s.upcase
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
index 1507001..b31ec49 100644
--- a/lib/pro_tacts/contact.rb
+++ b/lib/pro_tacts/contact.rb
@@ -45,8 +45,8 @@ module ProTacts
     # for TEL (RFC 2426 section 3.3.1) and EMAIL (section 3.3.2), and
     # ADR's seven components (section 3.2.1: post office box, extended
     # address, street, locality, region, postal code, country — nil
-    # where the card's value stopped short of the position). Data
-    # classes, whose members the inline syntax cannot read; the
+    # where the card left the position blank or stopped short of it).
+    # Data classes, whose members the inline syntax cannot read; the
     # signatures live in sig/pro_tacts/contact.rbs.
     # @rbs skip
     Phone = Data.define(:value, :type)
@@ -109,8 +109,7 @@ module ProTacts
     # FN's value (RFC 2426 section 3.1.1), in text form.
     #: () -> String?
     def name
-      property = properties.find { it.name.casecmp?("FN") }
-      property && VCard.unescape(property.value)
+      text_of(properties.find { it.name.casecmp?("FN") })
     end
 
     # N's components (RFC 2426 section 3.1.2: family; given; additional;
@@ -118,25 +117,31 @@ module ProTacts
     # Structured data — reading initials off it is the real answer to
     # where a first/last split falls, rather than guessing in the
     # free-text FN.
-    #: () -> Array[String]?
+    #: () -> Array[String?]?
     def name_components
       property = properties.find { it.name.casecmp?("N") }
-      property && VCard.split_components(property.value)
+      property && components_of(property.value)
     end
 
     #: () -> Array[Phone]
     def phones
-      of_name("TEL").map { Phone.new(value: VCard.unescape(it.value), type: type_of(it)) }
+      of_name("TEL").filter_map do |property|
+        value = text_of(property)
+        Phone.new(value:, type: type_of(property)) if value
+      end
     end
 
     #: () -> Array[Email]
     def emails
-      of_name("EMAIL").map { Email.new(value: VCard.unescape(it.value), type: type_of(it)) }
+      of_name("EMAIL").filter_map do |property|
+        value = text_of(property)
+        Email.new(value:, type: type_of(property)) if value
+      end
     end
 
     #: () -> Array[Address]
     def addresses
-      of_name("ADR").map { address_of(it) }
+      of_name("ADR").filter_map { address_of(it) }
     end
 
     # The birthday the served card carries — the model the store
@@ -153,8 +158,7 @@ module ProTacts
     # The card's NOTE (RFC 2426 section 3.6.2), in text form.
     #: () -> String?
     def notes
-      property = properties.find { it.name.casecmp?("NOTE") }
-      property && VCard.unescape(property.value)
+      text_of(properties.find { it.name.casecmp?("NOTE") })
     end
 
     private
@@ -164,10 +168,33 @@ module ProTacts
       properties.select { it.name.casecmp?(name) }
     end
 
-    #: (VCard::Parser::Property property) -> Address
+    # A text property's value, with the empty one reading as absent:
+    # a property whose value carries nothing is one no caller has to
+    # tell apart from a property that is not there.
+    #: (VCard::Parser::Property? property) -> String?
+    def text_of(property)
+      return if property.nil?
+
+      value = VCard.unescape(property.value)
+      value.empty? ? nil : value
+    end
+
+    # A structured value's components, nil in the positions the card
+    # left blank: an empty component is as absent as one past the end
+    # of the value, and reads the same.
+    #: (String value) -> Array[String?]
+    def components_of(value)
+      VCard.split_components(value).map { it.empty? ? nil : it }
+    end
+
+    # ADR's components, with a value that is blank throughout reading
+    # as no address at all.
+    #: (VCard::Parser::Property property) -> Address?
     def address_of(property)
-      po_box, extended, street, locality, region, postal_code, country =
-        VCard.split_components(property.value)
+      components = components_of(property.value)
+      return if components.none?
+
+      po_box, extended, street, locality, region, postal_code, country = components
       Address.new(po_box:, extended:, street:, locality:, region:, postal_code:, country:, type: type_of(property))
     end
 
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index 8a97c4a..2c2c737 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -133,6 +133,20 @@ class AdminContactsPagesTest < Minitest::Test
     end
   end
 
+  # Empty values are absent by the time the view reads them (see
+  # Contact), so a present-but-empty property renders no row either.
+  def test_show_hides_attributes_whose_values_are_empty
+    empty = ADA.sub("TEL;TYPE=mobile:+1-555-0100", "TEL;TYPE=mobile:")
+      .sub("NOTE:Countess of Lovelace.", "NOTE:")
+
+    with_contacts({"ada" => empty}) do
+      get "/contacts/ada"
+
+      refute_includes last_response.body, '<dt class="type-label">phone</dt>'
+      refute_includes last_response.body, "notes"
+    end
+  end
+
   # No properties at all means no grid, not an empty one — an empty
   # <dl> would still eat the gap card-body puts before it, leaving the
   # name pinned near the top of the card instead of centered in it.
diff --git a/test/pro_tacts/admin/test_format.rb b/test/pro_tacts/admin/test_format.rb
index 1df9088..d222e25 100644
--- a/test/pro_tacts/admin/test_format.rb
+++ b/test/pro_tacts/admin/test_format.rb
@@ -31,10 +31,12 @@ class FormatTest < Minitest::Test
     assert_equal "Z", ProTacts::Admin::Format.initials(contact(bare, id: "znorth"))
   end
 
-  def test_initials_of_a_blank_name_is_blank
+  # A blank FN is no name — empty values read as absent (see
+  # Contact) — so initials fall back to the id like any nameless card.
+  def test_initials_of_a_blank_name_fall_back_to_the_id
     blank = "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:\r\nUID:b\r\nEND:VCARD\r\n"
 
-    assert_equal "", ProTacts::Admin::Format.initials(contact(blank))
+    assert_equal "A", ProTacts::Admin::Format.initials(contact(blank))
   end
 
   # N with only one of given/family present (a mononym, or a card that
diff --git a/test/pro_tacts/test_contact.rb b/test/pro_tacts/test_contact.rb
index 1dc3031..a076379 100644
--- a/test/pro_tacts/test_contact.rb
+++ b/test/pro_tacts/test_contact.rb
@@ -65,7 +65,7 @@ class ContactTest < Minitest::Test
   end
 
   def test_reads_the_name_components
-    assert_equal ["Lovelace", "Ada", "", "", ""], contact(STRUCTURED).name_components
+    assert_equal ["Lovelace", "Ada", nil, nil, nil], contact(STRUCTURED).name_components
   end
 
   def test_values_come_back_unescaped
@@ -88,8 +88,8 @@ class ContactTest < Minitest::Test
 
   def test_reads_an_address_as_its_seven_components
     address = contact(STRUCTURED).addresses.fetch(0)
-    assert_equal "", address.po_box
-    assert_equal "", address.extended
+    assert_nil address.po_box
+    assert_nil address.extended
     assert_equal "12 Analytical Way", address.street
     assert_equal "London", address.locality
     assert_equal "England", address.region
@@ -98,8 +98,8 @@ class ContactTest < Minitest::Test
     assert_equal "home", address.type
   end
 
-  # Components the card's value stopped short of are nil, not empty —
-  # only a delimiter the value actually carried makes an empty one.
+  # A blank position and one past the end of the value read the same:
+  # neither says anything the card did not.
   def test_address_components_beyond_the_value_are_nil
     short = STRUCTURED.sub(
       "ADR;TYPE=home:;;12 Analytical Way;London;England;NW1 1AA;United Kingdom",
@@ -113,6 +113,25 @@ class ContactTest < Minitest::Test
     assert_nil address.country
   end
 
+  # An empty value carries nothing a missing property would not, so
+  # the accessors read one as the other: FN:, NOTE:, TEL:, and EMAIL:
+  # come back absent, and an ADR blank throughout is no address.
+  def test_empty_values_read_as_absent
+    empty = STRUCTURED.sub("FN:Ada Lovelace", "FN:")
+      .sub("TEL;TYPE=mobile:+1-555-0100", "TEL;TYPE=mobile:")
+      .sub("TEL;TYPE=work:+1-555-0199", "TEL;TYPE=work:")
+      .sub("EMAIL;TYPE=home:ada@example.com", "EMAIL;TYPE=home:")
+      .sub("ADR;TYPE=home:;;12 Analytical Way;London;England;NW1 1AA;United Kingdom", "ADR;TYPE=home:;")
+      .sub("NOTE:Countess\\, mathematician.", "NOTE:")
+
+    contact = contact(empty)
+    assert_nil contact.name
+    assert_empty contact.phones
+    assert_empty contact.emails
+    assert_empty contact.addresses
+    assert_nil contact.notes
+  end
+
   def test_the_birthday_is_the_model_the_store_composed
     assert_equal ProTacts::Birthday.new(year: 1985, month: 12, day: 10), contact(STRUCTURED).birthday
   end