contact: phones and addresses keep every type
The first TYPE was the label, so work,fax showed as work;
now the key lists every kind but pref and voice, the client's quirk.
Assisted-by: Claude Opus 5 via Claude Code
diff --git a/lib/pro_tacts/admin/contacts_edit.rb b/lib/pro_tacts/admin/contacts_edit.rb
index 6fcea8d..9315030 100644
--- a/lib/pro_tacts/admin/contacts_edit.rb
+++ b/lib/pro_tacts/admin/contacts_edit.rb
@@ -2,6 +2,7 @@ require "date"
require "pro_tacts/admin/phlex"
+require "pro_tacts/admin/format"
require "pro_tacts/admin/icons"
require "pro_tacts/admin/layout"
@@ -169,7 +170,7 @@ module ProTacts
# one, saving, then editing the other.
@own.phones.each do |phone|
label(class: "field", data: {blank_removes: true}) do
- span { phone.type || "phone" }
+ span { Format.type_label(phone.types, "phone") }
input(type: "tel", name: "phone[#{phone.line.digest}]",
value: phone.value, placeholder: "removed on save")
end
@@ -177,7 +178,7 @@ module ProTacts
# An email row, the phone row's own shape over EMAIL.
@own.emails.each do |email|
label(class: "field", data: {blank_removes: true}) do
- span { email.type || "email" }
+ span { Format.type_label(email.types, "email") }
input(type: "email", name: "email[#{email.line.digest}]",
value: email.value, placeholder: "removed on save")
end
@@ -354,7 +355,7 @@ module ProTacts
#: (Contact::Address address) -> void
def address_row(address)
div(class: "field", data: {blank_removes: address.po_box.nil?}) do
- span { address.type || "address" }
+ span { Format.type_label(address.types, "address") }
div(class: "field-stack") do
ADDRESS_FIELDS.each do |component, label|
input(type: "text", name: "address[#{address.line.digest}][#{component}]",
diff --git a/lib/pro_tacts/admin/contacts_show.rb b/lib/pro_tacts/admin/contacts_show.rb
index c6afb61..8ac09c4 100644
--- a/lib/pro_tacts/admin/contacts_show.rb
+++ b/lib/pro_tacts/admin/contacts_show.rb
@@ -133,13 +133,13 @@ module ProTacts
def rows
groups_row if @groups.any?
@contact.phones.each do |phone|
- row(phone.type || "phone", phone.value, phone.line)
+ row(Format.type_label(phone.types, "phone"), phone.value, phone.line)
end
@contact.emails.each do |email|
- row(email.type || "email", email.value, email.line)
+ row(Format.type_label(email.types, "email"), email.value, email.line)
end
@contact.addresses.each do |address|
- row(address.type || "address", Format.address_lines(address), address.line)
+ row(Format.type_label(address.types, "address"), Format.address_lines(address), address.line)
end
row("birthday", @birthday) if @birthday
@contact.notes.each do |note|
diff --git a/lib/pro_tacts/admin/format.rb b/lib/pro_tacts/admin/format.rb
index 8e12aab..10e8c71 100644
--- a/lib/pro_tacts/admin/format.rb
+++ b/lib/pro_tacts/admin/format.rb
@@ -56,6 +56,13 @@ module ProTacts
].compact.reject { it.empty? }
end
+ # A row's types as its key, or the kind of value when it has
+ # none, so no row renders unlabeled in the grid.
+ #: (Array[String] types, String kind) -> String
+ def self.type_label(types, kind)
+ types.empty? ? kind : types.join(", ")
+ end
+
# How much of an etag's hash is shown before it is cut. Twelve
# hex digits is git's own longest short-hash, and this address
# book will never hold enough cards for two to collide in a
diff --git a/lib/pro_tacts/admin/groups_edit.rb b/lib/pro_tacts/admin/groups_edit.rb
index c45aff0..070fec1 100644
--- a/lib/pro_tacts/admin/groups_edit.rb
+++ b/lib/pro_tacts/admin/groups_edit.rb
@@ -1,6 +1,7 @@
require "pro_tacts/admin/phlex"
require "pro_tacts/admin/contacts_edit"
+require "pro_tacts/admin/format"
require "pro_tacts/admin/group_label"
require "pro_tacts/admin/layout"
@@ -84,7 +85,7 @@ module ProTacts
#: (Contact::Address address) -> void
def address_row(address)
div(class: "field", data: {blank_removes: address.po_box.nil?}) do
- span { address.type || "address" }
+ span { Format.type_label(address.types, "address") }
div(class: "field-stack") do
ContactsEdit::ADDRESS_FIELDS.each do |component, label|
input(type: "text", name: "address[#{address.line.digest}][#{component}]",
diff --git a/lib/pro_tacts/admin/groups_show.rb b/lib/pro_tacts/admin/groups_show.rb
index 587f4c7..9183380 100644
--- a/lib/pro_tacts/admin/groups_show.rb
+++ b/lib/pro_tacts/admin/groups_show.rb
@@ -53,7 +53,7 @@ module ProTacts
def rows
members_row
@reading.addresses.each do |address|
- row(address.type || "address") do
+ row(Format.type_label(address.types, "address")) do
Format.address_lines(address).each { |line| div { line } }
end
end
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
index b5065bb..d730bc5 100644
--- a/lib/pro_tacts/contact.rb
+++ b/lib/pro_tacts/contact.rb
@@ -61,7 +61,7 @@ module ProTacts
# outside this charset cannot be served.
ID_FORMAT = /\A[\w-]+\z/ #: Regexp
- # The structured shapes the accessors return: text value and type
+ # The structured shapes the accessors return: text value and types
# 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
@@ -74,9 +74,9 @@ module ProTacts
# 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, :line)
+ Phone = Data.define(:value, :types, :line)
# @rbs skip
- Email = Data.define(:value, :type, :line)
+ Email = Data.define(:value, :types, :line)
# @rbs skip
Note = Data.define(:value, :line)
# @rbs skip
@@ -97,7 +97,7 @@ module ProTacts
:region,
:postal_code,
:country,
- :type,
+ :types,
:line,
)
@@ -277,7 +277,9 @@ module ProTacts
def phones
rows("TEL") { |property, line|
value = text_of(property)
- Phone.new(value:, type: type_of(property), line:) if value
+ # RFC 2426 section 3.3.1: `voice` is the type every number has
+ # by default, so it names none.
+ Phone.new(value:, types: types_of(property, except: %w[voice]), line:) if value
}
end
@@ -286,8 +288,8 @@ module ProTacts
rows("EMAIL") { |property, line|
value = text_of(property)
# RFC 2426 section 3.3.2: `internet` is the format every address
- # has by default and `pref` a preference, so neither names one.
- Email.new(value:, type: type_of(property, except: %w[internet pref]), line:) if value
+ # has by default, so it names none.
+ Email.new(value:, types: types_of(property, except: %w[internet]), line:) if value
}
end
@@ -391,18 +393,20 @@ module ProTacts
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), line:)
+ Address.new(po_box:, extended:, street:, locality:, region:, postal_code:, country:, types: types_of(property), line:)
end
- # RFC 2426 section 3.3.1: the first TYPE not in `except`, which
- # holds the values a property uses for something other than a
- # label. Downcased: the spelling is the card's, and a screen shows
- # one.
- #: (VCard::Parser::Property property, ?except: Array[String]) -> String?
- def type_of(property, except: [])
+ # Every TYPE value (RFC 2426 section 3.3.1) that names a kind of
+ # line. `pref` ranks a line instead, and the client adds it on its
+ # own (docs/macos-contacts.md, "The client rewrites every card it
+ # touches"); `except` holds a property's own values that name no
+ # kind either. Downcased: the spelling is the card's, and a screen
+ # shows one.
+ #: (VCard::Parser::Property property, ?except: Array[String]) -> Array[String]
+ def types_of(property, except: [])
property.parameters
.filter_map { |name, value| value.downcase if name.casecmp?("TYPE") }
- .find { !except.include?(it) }
+ .uniq - ["pref", *except]
end
end
end
diff --git a/sig/pro_tacts/contact.rbs b/sig/pro_tacts/contact.rbs
index 14bf898..717eff1 100644
--- a/sig/pro_tacts/contact.rbs
+++ b/sig/pro_tacts/contact.rbs
@@ -8,18 +8,18 @@ module ProTacts
class Contact
class Phone
attr_reader value: String
- attr_reader type: String?
+ attr_reader types: Array[String]
attr_reader line: VCard::Parser::Line
- def self.new: (value: String, type: String?, line: VCard::Parser::Line) -> instance
+ def self.new: (value: String, types: Array[String], line: VCard::Parser::Line) -> instance
end
class Email
attr_reader value: String
- attr_reader type: String?
+ attr_reader types: Array[String]
attr_reader line: VCard::Parser::Line
- def self.new: (value: String, type: String?, line: VCard::Parser::Line) -> instance
+ def self.new: (value: String, types: Array[String], line: VCard::Parser::Line) -> instance
end
class Note
@@ -51,7 +51,7 @@ module ProTacts
attr_reader region: String?
attr_reader postal_code: String?
attr_reader country: String?
- attr_reader type: String?
+ attr_reader types: Array[String]
attr_reader line: VCard::Parser::Line
def self.new: (
@@ -62,7 +62,7 @@ module ProTacts
region: String?,
postal_code: String?,
country: String?,
- type: String?,
+ types: Array[String],
line: VCard::Parser::Line
) -> instance
end
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index 3bc65ed..8347db5 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -365,6 +365,16 @@ class AdminContactsPagesTest < Minitest::Test
end
end
+ def test_show_labels_a_value_with_every_type_it_carries
+ typed = ADA.sub("TEL;TYPE=mobile:", "TEL;type=WORK;type=FAX;type=VOICE;type=pref:")
+
+ with_contacts({"ada" => typed}) do
+ get "/contacts/ada"
+
+ assert_includes last_response.body, '<dt class="type-label">work, fax</dt>'
+ end
+ end
+
# Empty attributes do not render (docs/DESIGN.md) — a bare card shows
# only the header, not a scaffold of blank rows.
def test_show_hides_attributes_the_contact_has_no_data_for
diff --git a/test/pro_tacts/test_contact.rb b/test/pro_tacts/test_contact.rb
index 5f1dd6b..e44fd49 100644
--- a/test/pro_tacts/test_contact.rb
+++ b/test/pro_tacts/test_contact.rb
@@ -112,9 +112,15 @@ class ContactTest < Minitest::Test
assert_equal "Ada, Countess of Lovelace", contact(escaped).name
end
- def test_reads_every_phone_with_its_type
- assert_equal [["+1-555-0100", "mobile"], ["+1-555-0199", "work"]],
- contact(STRUCTURED).phones.map { [it.value, it.type] }
+ def test_reads_every_phone_with_its_types
+ assert_equal [["+1-555-0100", ["mobile"]], ["+1-555-0199", ["work"]]],
+ contact(STRUCTURED).phones.map { [it.value, it.types] }
+ end
+
+ def test_a_phone_keeps_every_type_but_voice_and_pref
+ apple = STRUCTURED.sub("TEL;TYPE=work:", "TEL;TYPE=work,fax;type=VOICE;type=pref:")
+
+ assert_equal %w[work fax], contact(apple).phones.fetch(1).types
end
# Each phone carries the line it was read from — the address a save
@@ -136,7 +142,7 @@ class ContactTest < Minitest::Test
def test_a_phone_line_that_would_not_read_is_no_phone
broken = STRUCTURED.sub("TEL;TYPE=mobile:+1-555-0100\r\n", "TEL;=;:+1-555-0100\r\n")
- assert_equal [["+1-555-0199", "work"]], contact(broken).phones.map { [it.value, it.type] }
+ assert_equal [["+1-555-0199", ["work"]]], contact(broken).phones.map { [it.value, it.types] }
assert contact(broken).vcard.lines.any? { it.names?("TEL") && it.property.nil? }
end
@@ -144,19 +150,19 @@ class ContactTest < Minitest::Test
emails = contact(STRUCTURED).emails
assert_equal 1, emails.length
assert_equal "ada@example.com", emails.fetch(0).value
- assert_equal "home", emails.fetch(0).type
+ assert_equal ["home"], emails.fetch(0).types
end
- def test_an_emails_type_skips_internet_and_pref
+ def test_an_emails_types_skip_internet_and_pref
apple = STRUCTURED.sub("EMAIL;TYPE=home:", "EMAIL;type=INTERNET;type=pref;type=HOME:")
- assert_equal "home", contact(apple).emails.fetch(0).type
+ assert_equal ["home"], contact(apple).emails.fetch(0).types
end
- def test_an_email_typed_only_internet_and_pref_has_no_type
+ def test_an_email_typed_only_internet_and_pref_has_no_types
apple = STRUCTURED.sub("EMAIL;TYPE=home:", "EMAIL;type=INTERNET;type=pref:")
- assert_nil contact(apple).emails.fetch(0).type
+ assert_empty contact(apple).emails.fetch(0).types
end
def test_every_email_carries_the_line_it_was_read_from
@@ -183,7 +189,13 @@ class ContactTest < Minitest::Test
assert_equal "England", address.region
assert_equal "NW1 1AA", address.postal_code
assert_equal "United Kingdom", address.country
- assert_equal "home", address.type
+ assert_equal ["home"], address.types
+ end
+
+ def test_an_address_keeps_every_type_but_pref
+ apple = STRUCTURED.sub("ADR;TYPE=home:", "ADR;type=HOME;type=pref;type=POSTAL:")
+
+ assert_equal %w[home postal], contact(apple).addresses.fetch(0).types
end
def test_every_address_carries_the_line_it_was_read_from