contact: carry row provenance by position, not bytes
A member whose own card spells a lent line exactly — an address
already on the card when its holder joined — read as the group's over
both copies; the walk now spends the stored card's copies first, so
only the composed copy names the group.
Assisted-by: GLM-5.3 via pi
diff --git a/lib/pro_tacts/admin/contact_card.rb b/lib/pro_tacts/admin/contact_card.rb
index 62f3f64..ec32def 100644
--- a/lib/pro_tacts/admin/contact_card.rb
+++ b/lib/pro_tacts/admin/contact_card.rb
@@ -81,21 +81,22 @@ module ProTacts
# A missing TYPE parameter still gets a key: the fallback names
# the kind of value, so no row renders unlabeled in the grid.
- # Every row that reads from a line hands it over, because which
- # group lends a row is a question about the line it came from.
- # The birthday hands over none: it is the model beside the card,
- # and a group holds only addresses and notes anyway (see
- # db/migrations/004_groups.rb).
+ # Every row hands over the group that lends it, nil where the
+ # card's own — provenance carried on the row
+ # (Contact::Phone#lent_by and kin), not re-derived from the
+ # composed bytes. The birthday hands over none: it is the model
+ # beside the card, and a group holds only addresses and notes
+ # anyway (see db/migrations/004_groups.rb).
def rows
groups_row if @dialog || @groups.any?
@contact.phones.each do |phone|
- row(Format.type_label(phone.label, phone.types, "phone"), phone.value, phone.line)
+ row(Format.type_label(phone.label, phone.types, "phone"), phone.value, phone.lent_by)
end
@contact.emails.each do |email|
- row(Format.type_label(nil, email.types, "email"), email.value, email.line)
+ row(Format.type_label(nil, email.types, "email"), email.value, email.lent_by)
end
@contact.addresses.each do |address|
- row(Format.type_label(nil, address.types, "address"), Format.address_lines(address), address.line)
+ row(Format.type_label(nil, address.types, "address"), Format.address_lines(address), address.lent_by)
end
row("birthday", @birthday) if @birthday
note_rows
@@ -155,22 +156,21 @@ module ProTacts
end
# The type in one column and the value in the other, with the
- # group that lends the line named under the value when one does.
+ # group that lends the row named under the value when one does.
# In the value cell rather than beside the type label: the type
# column holds one thing for every row in the card
# (docs/DESIGN.md, "Alignment is the layout"), and a mark on
# the value is what says this address is the household's rather
# than this contact's.
- #: (String type, String | Array[String] value, ?VCard::Parser::Line? line) -> void
- def row(type, value, line = nil)
- group = line && @contact.group_of(line)
+ #: (String type, String | Array[String] value, ?Group? lent_by) -> void
+ def row(type, value, lent_by = nil)
dt(class: "type-label") { type }
dd(class: "type-body-sm") do
render_value(value)
# The groups row's own tag, pinned to this row's first line
# at the right edge of the value column (admin.css) — under
# the value it could be read as marking the row below.
- group_tag(group) if group
+ group_tag(lent_by) if lent_by
end
end
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
index aa1f157..0ea0cbe 100644
--- a/lib/pro_tacts/contact.rb
+++ b/lib/pro_tacts/contact.rb
@@ -47,7 +47,7 @@ module ProTacts
# @rbs @inherited: Array[Inherited]
# @rbs @vcard: VCard
# @rbs @etag: String
- # @rbs @groups_by_line: Hash[String, Group]
+ # @rbs @lent_at: Hash[Integer, Group]
# Ids end up in paths and arrive from client-supplied hrefs, so an id
# outside this charset cannot be served.
@@ -60,16 +60,17 @@ module ProTacts
# where the card left the position blank or stopped short of it).
# A phone's `label` is the `X-ABLabel` sharing its property group,
# a different fact from its types
- # (docs/plans/2026-09-18-phone-labels.md, "Reading"), and every
+ # (docs/plans/2026-09-18-phone-labels.md, "Reading"), every
# shape's `line` is the parsed line it was read from, the address
# a save names that row by
- # (docs/plans/2026-09-05-web-card-editor.md).
+ # (docs/plans/2026-09-05-web-card-editor.md), and `lent_by` the
+ # group lending the row, nil where the card's own (#lent_at).
# Data classes, whose members the inline syntax cannot read; the
# signatures live in sig/pro_tacts/contact.rbs.
# @rbs skip
- Phone = Data.define(:value, :label, :types, :line)
+ Phone = Data.define(:value, :label, :types, :line, :lent_by)
# @rbs skip
- Email = Data.define(:value, :types, :line)
+ Email = Data.define(:value, :types, :line, :lent_by)
# @rbs skip
Note = Data.define(:value, :line)
# @rbs skip
@@ -94,6 +95,7 @@ module ProTacts
:country,
:types,
:line,
+ :lent_by,
)
# An ADR's editable components, in the order the value spells them
@@ -327,7 +329,7 @@ module ProTacts
#: () -> Array[Phone]
def phones
labels = labels_by_group
- rows("TEL") { |property, line|
+ rows("TEL") { |property, line, lent_by|
value = text_of(property)
group = property.group
# RFC 2426 section 3.3.1: `voice` is the type every number has
@@ -337,23 +339,24 @@ module ProTacts
label: group && labels[group],
types: property.types - %w[voice],
line:,
+ lent_by:,
) if value
}
end
#: () -> Array[Email]
def emails
- rows("EMAIL") { |property, line|
+ rows("EMAIL") { |property, line, lent_by|
value = text_of(property)
# RFC 2426 section 3.3.2: `internet` is the format every address
# has by default, so it names none.
- Email.new(value:, types: property.types - %w[internet], line:) if value
+ Email.new(value:, types: property.types - %w[internet], line:, lent_by:) if value
}
end
#: () -> Array[Address]
def addresses
- rows("ADR") { |property, line| address_of(property, line:) }
+ rows("ADR") { |property, line, lent_by| address_of(property, line:, lent_by:) }
end
# The card's NOTEs (RFC 2426 section 3.6.2), in text form. On the
@@ -373,35 +376,42 @@ module ProTacts
}
end
- # The group a composed line came from, or nil for a
- # line the contact's own card carries — the provenance a screen
- # marks an inherited row with.
- #
- # Matched by the line's bytes, which is the same blindness the
- # editor's digests have (VCard::Parser::Line#digest): a stored
- # line whose bytes are a group's line reads as the group's here.
- # No write makes that state — EditedContact takes the
- # group's copy back out of a submission — but a member whose own
- # card spells the line exactly as its group does still has one, and
- # naming the group over both copies says something true about the
- # value even where it is wrong about the byte.
- #: (VCard::Parser::Line line) -> Group?
- def group_of(line)
- groups_by_line[line.verbatim.chomp]
- end
-
private
- # Every inherited line's group, keyed by the line's own bytes with
- # the terminator off — a group's line is stored without one and
- # composes with the card's, so neither side is compared as it lies.
- #: () -> Hash[String, Group]
- def groups_by_line
- return @groups_by_line if defined?(@groups_by_line)
-
- @groups_by_line = @inherited.to_h {
- [it.line.chomp, it.group] #: [String, Group]
- }
+ # Which of the composed card's lines the groups lend, keyed by
+ # where each sits in #vcard's parse: #vcard appends what the
+ # groups lend after the stored card's own lines, so a walk that
+ # spends the stored card's copies first leaves every lent line to
+ # pair with the inherited row composition placed there. A member
+ # whose own card spells a line exactly as its group lends — an
+ # address already on the card when its holder joined — reads as
+ # the member's own first and the group's second, where a match on
+ # bytes named the group over both copies and the member's own
+ # address read as the group's. Keyed by position rather than by
+ # the Line itself because a Line is a value: the two copies a card
+ # holds of one line hash alike, and only where each sits tells
+ # them apart. A NOTE is never matched: a lent one composes as a
+ # section of the member's single NOTE, so a screen marking one
+ # reads the inheritance directly.
+ #: () -> Hash[Integer, Group]
+ def lent_at
+ return @lent_at if defined?(@lent_at)
+
+ remaining = @stored.lines.map { it.verbatim.chomp }.tally
+ rows = @inherited.reject { note_row?(it.line) } #: Array[Contact::Inherited]
+ @lent_at = {} #: Hash[Integer, Group]
+ vcard.lines.each_with_index do |served, index|
+ next if served.names?("NOTE")
+
+ bytes = served.verbatim.chomp
+ count = remaining[bytes] || 0
+ if count.positive?
+ remaining[bytes] = count - 1
+ elsif (row = rows.shift)
+ @lent_at[index] = row.group
+ end
+ end
+ @lent_at
end
# Whether a lent line is a NOTE, read off its bytes the way
@@ -461,25 +471,19 @@ module ProTacts
# The rows a repeatable property reads as: each line naming it
# folded by the block, which answers nil for a row with nothing
- # to show.
- #: [T] (String name) { (VCard::Parser::Property, VCard::Parser::Line) -> T? } -> Array[T]
+ # to show — and told which group lends the line, the provenance a
+ # screen marks a row with, carried by position rather than read
+ # back off the bytes (#lent_at's reason).
+ #: [T] (String name) { (VCard::Parser::Property, VCard::Parser::Line, Group?) -> T? } -> Array[T]
def rows(name)
- of_line(name).filter_map { |line|
+ vcard.lines.each.with_index.filter_map { |line, index|
+ next unless line.names?(name)
+
property = line.property
- yield(property, line) unless property.nil?
+ yield(property, line, lent_at[index]) unless property.nil?
}
end
- # The lines naming `name`, parsed beside their bytes — the walk the
- # provenance-carrying accessors read from, so a row can carry the
- # line it was read from as its address. A line that would not
- # read is a row no form can address and no accessor reads; it
- # stays enumerable in VCard#lines, served from its bytes.
- #: (String name) -> Array[VCard::Parser::Line]
- def of_line(name)
- vcard.lines.select { it.names?(name) }
- end
-
# 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.
@@ -502,13 +506,13 @@ module ProTacts
# ADR's components, with a value that is blank throughout reading
# as no address at all. Carries the line it was read from, the
# other provenance-carrying shapes' own address for a save.
- #: (VCard::Parser::Property property, line: VCard::Parser::Line) -> Address?
- def address_of(property, line:)
+ #: (VCard::Parser::Property property, line: VCard::Parser::Line, lent_by: Group?) -> Address?
+ def address_of(property, line:, lent_by:)
components = components_of(property)
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:, types: property.types, line:)
+ Address.new(po_box:, extended:, street:, locality:, region:, postal_code:, country:, types: property.types, line:, lent_by:)
end
end
diff --git a/sig/pro_tacts/contact.rbs b/sig/pro_tacts/contact.rbs
index e8c8edd..635caa0 100644
--- a/sig/pro_tacts/contact.rbs
+++ b/sig/pro_tacts/contact.rbs
@@ -11,16 +11,18 @@ module ProTacts
attr_reader label: String?
attr_reader types: Array[String]
attr_reader line: VCard::Parser::Line
+ attr_reader lent_by: Group?
- def self.new: (value: String, label: String?, types: Array[String], line: VCard::Parser::Line) -> instance
+ def self.new: (value: String, label: String?, types: Array[String], line: VCard::Parser::Line, lent_by: Group?) -> instance
end
class Email
attr_reader value: String
attr_reader types: Array[String]
attr_reader line: VCard::Parser::Line
+ attr_reader lent_by: Group?
- def self.new: (value: String, types: Array[String], line: VCard::Parser::Line) -> instance
+ def self.new: (value: String, types: Array[String], line: VCard::Parser::Line, lent_by: Group?) -> instance
end
class Note
@@ -55,6 +57,7 @@ module ProTacts
attr_reader country: String?
attr_reader types: Array[String]
attr_reader line: VCard::Parser::Line
+ attr_reader lent_by: Group?
def self.new: (
po_box: String?,
@@ -65,7 +68,8 @@ module ProTacts
postal_code: String?,
country: String?,
types: Array[String],
- line: VCard::Parser::Line
+ line: VCard::Parser::Line,
+ lent_by: Group?
) -> instance
end
end
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index d07e624..e4b0578 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -503,6 +503,26 @@ class AdminContactsPagesTest < Minitest::Test
end
end
+ # A member whose own card already carries the address its group
+ # lends — the address was on the card before the join — shows both
+ # rows, and only the composed copy is marked as the group's: the
+ # member's own reads as the member's own (the row's lent_by).
+ def test_show_marks_only_the_lent_copy_of_an_address_the_member_also_holds
+ with_contacts({"ada" => ADA}) do |store|
+ own = ADA.sub("ADR;TYPE=home:;;12 Analytical Way;London;England;NW1 1AA;United Kingdom", HOUSEHOLD.fetch(0))
+ store.put("ada", ProTacts::VCard.new(own))
+ id = FixtureData.seed_group(store, name: "Booles", members: ["ada"], lines: [HOUSEHOLD.fetch(0)])
+
+ get "/contacts/ada"
+ rows = last_response.body.scan(%r{<dt class="type-label">home</dt><dd class="type-body-sm">.*?</dd>}m)
+ .select { it.include?("7 Calculus Close") }
+
+ assert_equal 2, rows.size
+ refute_includes rows.fetch(0), %(class="tag")
+ assert_includes rows.fetch(1), %(<a href="/groups/#{id}" class="tag">Booles</a>)
+ end
+ end
+
# A row a group lends is marked with a tag opening the group, and the
# contact's own rows are not: Ada carries an address and a note of
# her own beside the household's two, and only the household's are
@@ -1437,7 +1457,7 @@ class AdminContactsPagesTest < Minitest::Test
with_contacts({"ada" => ADA}) do |store|
FixtureData.seed_group(store, name: "Booles", members: ["ada"], lines: HOUSEHOLD)
contact = store.contact("ada")
- lent = contact.addresses.find { contact.group_of(it.line) }
+ lent = contact.addresses.find { it.lent_by }
post "/contacts/ada", first: "Ada", last: "Lovelace", note: "Countess of Lovelace.",
etag: contact.etag,
diff --git a/test/pro_tacts/test_contact.rb b/test/pro_tacts/test_contact.rb
index 8c177c9..963b40f 100644
--- a/test/pro_tacts/test_contact.rb
+++ b/test/pro_tacts/test_contact.rb
@@ -390,8 +390,24 @@ class ContactTest < Minitest::Test
addresses = contact.addresses
assert_equal 2, addresses.size
- assert_nil contact.group_of(addresses.fetch(0).line)
- assert_equal GROUP, contact.group_of(addresses.fetch(1).line)
+ assert_nil addresses.fetch(0).lent_by
+ assert_equal GROUP, addresses.fetch(1).lent_by
+ end
+
+ # The member's own spelling of a line its group also lends — an
+ # address already on the card when its holder joined: the stored
+ # copy reads as the member's own and only the composed one names
+ # the group, where a match on bytes named the group over both and
+ # the member's own address read as the group's.
+ def test_a_line_the_stored_card_spells_as_its_group_does_reads_as_its_own_first
+ lent = "ADR;TYPE=home:;;7 Calculus Close;London;England;NW1 1AB;United Kingdom"
+ same = STRUCTURED.sub("ADR;TYPE=home:;;12 Analytical Way;London;England;NW1 1AA;United Kingdom", lent)
+ contact = contact(same, inherited: [lent])
+ addresses = contact.addresses
+
+ assert_equal 2, addresses.size
+ assert_nil addresses.fetch(0).lent_by
+ assert_equal GROUP, addresses.fetch(1).lent_by
end
# A lent NOTE composes into the member's own as one value — the
@@ -465,15 +481,16 @@ class ContactTest < Minitest::Test
end
# The joined line matches nothing by bytes — neither the member's
- # own nor what a group lends — so a screen asking after provenance
- # reads the stored card and the inheritance, not the composed value
- # (Admin::ContactCard's note rows).
+ # own nor what a group lends — so a screen marking a note's
+ # provenance reads the stored card and the inheritance, not the
+ # composed value (Admin::ContactCard's note rows; a Note carries no
+ # lent_by for the asking).
def test_the_composed_note_names_no_group
- notes = contact(STRUCTURED, inherited: ["NOTE:Gate code 1854."]).notes
+ contact = contact(STRUCTURED, inherited: ["NOTE:Gate code 1854."])
+ notes = contact.notes
assert_equal 1, notes.size
- assert_nil contact.group_of(notes.fetch(0).line)
- assert_equal ["Countess, mathematician."], contact(STRUCTURED, inherited: ["NOTE:Gate code 1854."]).own.notes.map { it.value }
+ assert_equal ["Countess, mathematician."], contact.own.notes.map { it.value }
end
def test_unescapes_notes
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index d00825d..cbfccfb 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -1284,7 +1284,7 @@ class StoreTest < Minitest::Test
listed = store.contacts.find { it.id == "aiden" }
[store.contact("aiden"), listed].each do |contact|
- assert_equal store.group(id), contact.group_of(contact.addresses.fetch(0).line)
+ assert_equal store.group(id), contact.addresses.fetch(0).lent_by
end
end
end
@@ -2336,7 +2336,7 @@ class StoreTest < Minitest::Test
ProTacts::Store.connect(path) do |store|
contact = store.contact("aiden")
assert_includes contact.vcard.to_s, HOUSEHOLD_ADDRESS
- assert_equal "nous", contact.group_of(contact.addresses.fetch(0).line)&.id
+ assert_equal "nous", contact.addresses.fetch(0).lent_by&.id
database(store)[:cards].where(id: "aiden").delete
assert_empty database(store)[:group_members].all