admin: a lent line's group is a tag that opens it
A badge gave a name, and two groups can share one,
so each lent line now brings its whole group from the store,
and its tag opens that group, as the groups row's did before.

Assisted-by: Claude Opus 5 via Claude Code
change zqvnvtwownzqtyntzuxvtoqrolmozzuk
commit d8374e3a677e609e133ad033b0209d7748a822ef
author Alpha Chen <alpha@kejadlen.dev>
date
parent ukwlwqvm
diff --git a/lib/pro_tacts/admin/contacts_show.rb b/lib/pro_tacts/admin/contacts_show.rb
index a870901..c6afb61 100644
--- a/lib/pro_tacts/admin/contacts_show.rb
+++ b/lib/pro_tacts/admin/contacts_show.rb
@@ -149,22 +149,25 @@ module ProTacts
 
       # Membership as its own row, above the card's own: what a contact
       # belongs to frames the values under it, and several of those
-      # values are the group's rather than the contact's. Gloss' Tag
-      # rather than the Badge the inherited rows carry — a badge marks
-      # the row it sits on, and these are the row's whole value
-      # (docs/DESIGN.md, "Relationships are navigable"). Every tag opens
-      # the group it names, the other half of the member tags on a
-      # group's own card (Admin::GroupsShow).
+      # values are the group's rather than the contact's. Every tag opens
+      # the group it names (docs/DESIGN.md, "Relationships are
+      # navigable"), the other half of the member tags on a group's own
+      # card (Admin::GroupsShow).
       #: () -> void
       def groups_row
         dt(class: "type-label") { "groups" }
         dd(class: "type-body-sm") do
           div(class: "tag-set") do
-            @groups.each { |group| a(href: "/groups/#{group.id}", class: "tag") { render GroupLabel.new(group:) } }
+            @groups.each { group_tag(it) }
           end
         end
       end
 
+      #: (Store::Group group) -> void
+      def group_tag(group)
+        a(href: "/groups/#{group.id}", class: "tag") { render GroupLabel.new(group:) }
+      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.
       # In the value cell rather than beside the type label: the type
@@ -178,24 +181,13 @@ module ProTacts
         dt(class: "type-label") { type }
         dd(class: "type-body-sm") do
           render_value(value)
-          # Gloss' Badge: a catalog mark annotating the row it sits on,
-          # pinned to that 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. No tone: the accent is held
-          # constant across a view, and inherited is not a status.
-          span(class: "badge") { group_mark(group) } if group
+          # 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
         end
       end
 
-      # A lent row knows its group only by label (Contact#group_of), and
-      # a nameless group's label is its id — so the id is looked up among
-      # the contact's own groups to mark it the way GroupLabel does.
-      #: (String label) -> void
-      def group_mark(label)
-        nameless = @groups.find { it.name.nil? && it.id == label }
-        nameless ? render(GroupLabel.new(group: nameless)) : plain(label)
-      end
-
       #: (String | Array[String] value) -> void
       def render_value(value)
         if value.is_a?(Array)
diff --git a/lib/pro_tacts/contact.rb b/lib/pro_tacts/contact.rb
index ecc62b6..30de471 100644
--- a/lib/pro_tacts/contact.rb
+++ b/lib/pro_tacts/contact.rb
@@ -55,7 +55,7 @@ module ProTacts
     # @rbs @inherited: Array[Inherited]
     # @rbs @vcard: VCard
     # @rbs @etag: String
-    # @rbs @groups_by_line: Hash[String, String]
+    # @rbs @groups_by_line: Hash[String, Store::Group]
 
     # Ids end up in paths and arrive from client-supplied hrefs, so an id
     # outside this charset cannot be served.
@@ -82,12 +82,10 @@ module ProTacts
     # @rbs skip
     Photo = Data.define(:mime_type, :bytes)
 
-    # One line a group lends this contact, and what to call the group
-    # that lends it: its name, or its id where it has no name
-    # (Store#inherited_rows). A label rather than the group itself,
-    # because the label is the whole of what a card shows about it
-    # today and nothing here can navigate to a record that has no
-    # screen yet (docs/DESIGN.md, "Relationships are navigable").
+    # One line a group lends this contact, and the group that lends it,
+    # whole (Store#inheritance). Not its name, because two groups may
+    # share one, and a screen marking the line opens the one group that
+    # lent it (docs/DESIGN.md, "Relationships are navigable").
     # @rbs skip
     Inherited = Data.define(:group, :line)
     # @rbs skip
@@ -311,7 +309,7 @@ module ProTacts
       }
     end
 
-    # The name of the group a composed line came from, or nil for a
+    # 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.
     #
@@ -323,7 +321,7 @@ module ProTacts
     # 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) -> String?
+    #: (VCard::Parser::Line line) -> Store::Group?
     def group_of(line)
       groups_by_line[line.verbatim.chomp]
     end
@@ -333,12 +331,12 @@ module ProTacts
     # 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, String]
+    #: () -> Hash[String, Store::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, String]
+        [it.line.chomp, it.group] #: [String, Store::Group]
       }
     end
 
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 97349e8..24fdc46 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -72,12 +72,11 @@ module ProTacts
     # One line a group lends one card, with the row it is lent from —
     # the write path's reading of what #inherited_of answers as the
     # model's. A member's edit has to reach the group_properties row
-    # the line came from, and the label a screen marks the row with
-    # cannot name it: two groups may share a name, and a nameless one
-    # is labelled by its id. The signature is in
-    # sig/pro_tacts/store.rbs with Store's own.
+    # the line came from, which takes the position as well as the
+    # group. The signature is in sig/pro_tacts/store.rbs with Store's
+    # own.
     # @rbs skip
-    Lent = Data.define(:group_id, :position, :label, :line)
+    Lent = Data.define(:group_id, :position, :line)
 
     # What one classified line asks of the group it came from: the row
     # to write, and the line that replaces it — or nil to remove the
@@ -1231,8 +1230,8 @@ module ProTacts
       row && birthday_from(row)
     end
 
-    # The content lines a contact inherits, each beside the name of the
-    # group lending it — every property of every group it belongs to,
+    # The content lines a contact inherits, each beside the group
+    # lending it — every property of every group it belongs to,
     # nothing of a group that holds nothing, and [] for a contact in no
     # group at all. Ordered by group id and then position, so the
     # composed card is the same bytes every read.
@@ -1256,51 +1255,55 @@ module ProTacts
     # caller, which is what it means.
     #: () -> Hash[String, Array[Contact::Inherited]]
     def inherited_by_id
-      inherited_rows.all
+      rows = inherited_rows.all
+      lenders = groups_by_id(rows.map { it.fetch(:group_id).to_s })
+      rows
         .group_by { it.fetch(:card_id).to_s }
-        .transform_values { |rows| inheritance(rows.map { lent_from(it) }) }
+        .transform_values { |mine| inheritance(mine.map { lent_from(it) }, lenders) }
     end
 
-    # What a model asks of a group's rows: the label to mark a row
-    # with and the line itself. Which group_properties row lent it is
+    # What a model asks of a group's rows: the group lending each,
+    # whole, and the line itself. Which group_properties row lent it is
     # the write path's business and stops here (Contact#group_of).
-    #: (Array[Lent] lent) -> Array[Contact::Inherited]
-    def inheritance(lent)
-      lent.map { Contact::Inherited.new(group: it.label, line: it.line) }
+    #: (Array[Lent] lent, ?Hash[String, Group] lenders) -> Array[Contact::Inherited]
+    def inheritance(lent, lenders = groups_by_id(lent.map(&:group_id)))
+      lent.map { Contact::Inherited.new(group: lenders.fetch(it.group_id), line: it.line) }
+    end
+
+    # The groups these ids name, whole and keyed by id; no reads at all
+    # for a contact in no group, the common case.
+    #: (Array[String] ids) -> Hash[String, Group]
+    def groups_by_id(ids)
+      return {} if ids.empty?
+
+      load_groups(groups.select(*GROUP_COLUMNS, group_label.as(:label)).where(id: ids.uniq).all).to_h {
+        [it.id, it] #: [String, Group]
+      }
     end
 
     # What to call a group on a screen: its name, or its id where it
     # has none (db/migrations/005_group_identity.rb). In SQL rather
-    # than in Ruby so that both reads of it — a tag and an inherited
-    # row's mark — are the one answer.
+    # than in Ruby so that every read of a group — a tag, a heading —
+    # is the one answer.
     #: () -> untyped
     def group_label
       Sequel.function(:coalesce, Sequel[:groups][:name], Sequel[:groups][:id])
     end
 
     # The join both inherited reads walk: a membership to the property
-    # it inherits and to the group's own row for its name, qualified
-    # and ordered so the composition neither depends on what SQLite
-    # feels like returning nor trips over the group_id the tables
-    # share. Ordered by the group's id rather than its name, because a
-    # rename must not move a member's lines and change every etag in
-    # the group.
-    #
-    # A group with no name is lent under its id instead, coalesced here
-    # rather than at the surfaces: a mark on an inherited row names the
-    # group it came from, and a group with no name still has to be
-    # named as some one group among several
-    # (db/migrations/005_group_identity.rb).
+    # it inherits, qualified and ordered so the composition neither
+    # depends on what SQLite feels like returning nor trips over the
+    # group_id the tables share. Ordered by the group's id rather than
+    # its name, because a rename must not move a member's lines and
+    # change every etag in the group.
     #: () -> Sequel::Dataset
     def inherited_rows
       group_members
         .join(:group_properties, group_id: :group_id)
-        .join(:groups, id: Sequel[:group_members][:group_id])
         .select(
           Sequel[:group_members][:card_id],
           Sequel[:group_properties][:group_id],
           Sequel[:group_properties][:position],
-          group_label.as(:group_name),
           Sequel[:group_properties][:line],
         )
         .order(
@@ -1315,7 +1318,6 @@ module ProTacts
       Lent.new(
         group_id: row.fetch(:group_id).to_s,
         position: row.fetch(:position).to_i,
-        label: row.fetch(:group_name).to_s,
         line: row.fetch(:line).to_s,
       )
     end
diff --git a/public/admin.css b/public/admin.css
index fb17ead..67f2dd9 100644
--- a/public/admin.css
+++ b/public/admin.css
@@ -539,7 +539,7 @@ img.avatar {
   margin: 0;
 }
 
-/* The group a row is lent by, marked at the right edge of the value
+/* The group a row is lent by, tagged at the right edge of the value
    column on the row's own first line (ContactsShow#row). Under the
    value it read as a mark between two rows — the grid's row gap is
    wider than a line, but not by enough to say which row a line below
@@ -548,34 +548,29 @@ img.avatar {
 
    A nested grid rather than a float or a flex row, because the value
    is several block lines for an address and one for everything else:
-   the lines stack down column 1 as they always did, and the mark
+   the lines stack down column 1 as they always did, and the tag
    takes column 2 of the first row alone. No wrapper element to make
    them one flex item, and no source-order inversion — the value is
-   still read before the group that lends it. */
-.detail-grid dd:has(> .badge),
+   still read before the group that lends it.
+
+   Baseline-aligned: a grid item blockifies, so the tag's inline-flex
+   becomes a flex container, which takes its baseline from its first
+   item — the tag's text — and so shares the value's. A group card's
+   "edit members" takes the same place, and is plain text with a
+   baseline of its own. */
+.detail-grid dd:has(> .tag),
 .detail-grid dd:has(> a.type-label) {
   display: grid;
   grid-template-columns: minmax(0, 1fr) auto;
-  align-items: start;
+  align-items: baseline;
   column-gap: var(--gl-space-s);
 }
-/* Not baseline-aligned, though a mark beside a value is exactly what
-   baseline is for: a grid item blockifies, so Badge's inline-flex
-   becomes a flex container, and a flex container takes its baseline
-   from its first item — which is Badge's leading square, a box with
-   no text in it. The synthesized baseline lands a few pixels off the
-   value's.
-
-   So both boxes are made one value-line tall and top-aligned instead:
-   the value's line box is --gl-step--1/1.5 (Gloss' .type-body-sm) and
-   the badge centers its own content (align-items: center, its own
-   rule), so equal heights put the two texts on the same center. */
-.detail-grid dd > .badge {
+.detail-grid dd > .tag,
+.detail-grid dd > a.type-label {
   grid-column: 2;
   grid-row: 1;
-  min-height: calc(var(--gl-step--1) * 1.5);
 }
-.detail-grid dd:has(> .badge) > div,
+.detail-grid dd:has(> .tag) > div,
 .detail-grid dd:has(> a.type-label) > div {
   grid-column: 1;
 }
@@ -589,17 +584,6 @@ img.avatar {
   color: var(--gl-color-text-secondary);
 }
 
-/* A row's action — a group card's "edit members" — takes the badge's
-   place at the row's right edge. Unlike the badge it is plain text,
-   so it has a real baseline to share with the tags beside it. */
-.detail-grid dd:has(> a.type-label) {
-  align-items: baseline;
-}
-.detail-grid dd > a.type-label {
-  grid-column: 2;
-  grid-row: 1;
-}
-
 /* The stored card in its own collapsed card under the record — the
    column's gap spaces the two, so no margin here. <details> is native,
    keeping the admin UI script-free; the summary rides the label
diff --git a/sig/pro_tacts/contact.rbs b/sig/pro_tacts/contact.rbs
index 3bbdaf3..14bf898 100644
--- a/sig/pro_tacts/contact.rbs
+++ b/sig/pro_tacts/contact.rbs
@@ -30,10 +30,10 @@ module ProTacts
     end
 
     class Inherited
-      attr_reader group: String
+      attr_reader group: Store::Group
       attr_reader line: String
 
-      def self.new: (group: String, line: String) -> instance
+      def self.new: (group: Store::Group, line: String) -> instance
     end
 
     class Photo
diff --git a/sig/pro_tacts/store.rbs b/sig/pro_tacts/store.rbs
index 2d61a9f..9687cf2 100644
--- a/sig/pro_tacts/store.rbs
+++ b/sig/pro_tacts/store.rbs
@@ -31,10 +31,9 @@ module ProTacts
     class Lent
       attr_reader group_id: String
       attr_reader position: Integer
-      attr_reader label: String
       attr_reader line: String
 
-      def self.new: (group_id: String, position: Integer, label: String, line: String) -> instance
+      def self.new: (group_id: String, position: Integer, line: String) -> instance
     end
 
     class Group
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index 4d67830..3bc65ed 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -279,16 +279,16 @@ class AdminContactsPagesTest < Minitest::Test
     end
   end
 
-  # A row a group lends is marked with the group's name, and the
+  # 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
   # marked.
   def test_show_marks_the_rows_a_group_lends
     with_contacts({"ada" => ADA}) do |store|
-      FixtureData.seed_group(store, name: "Booles", members: ["ada"], lines: HOUSEHOLD)
+      id = FixtureData.seed_group(store, name: "Booles", members: ["ada"], lines: HOUSEHOLD)
 
       get "/contacts/ada"
-      marks = last_response.body.scan('<span class="badge">Booles</span>')
+      marks = last_response.body.scan(%(<a href="/groups/#{id}" class="tag">Booles</a></dd>))
 
       assert_equal 2, marks.size
       assert_includes last_response.body, "7 Calculus Close"
@@ -308,7 +308,8 @@ class AdminContactsPagesTest < Minitest::Test
       nameless = FixtureData.seed_group(store, members: ["ada"], lines: [])
 
       get "/contacts/ada"
-      tags = last_response.body.scan(%r{<a href="/groups/([^"]*)" class="tag">(.*?)</a>})
+      tag_set = last_response.body[%r{<div class="tag-set">.*?</div>}].to_s
+      tags = tag_set.scan(%r{<a href="/groups/([^"]*)" class="tag">(.*?)</a>})
 
       assert_equal({named => "Booles", nameless => %(<span data-nameless>#{nameless}</span>)}.sort, tags)
       # The grid leads with what the contact belongs to, so the values
@@ -328,26 +329,26 @@ class AdminContactsPagesTest < Minitest::Test
   end
 
   # A group with no name marks its rows with its id, which is what the
-  # mark is for: an empty badge would say a row came from somewhere and
+  # mark is for: an empty tag would say a row came from somewhere and
   # then not say where.
   def test_show_marks_a_nameless_groups_rows_with_its_id
     with_contacts({"ada" => ADA}) do |store|
       id = FixtureData.seed_group(store, members: ["ada"], lines: HOUSEHOLD)
 
       get "/contacts/ada"
-      marks = last_response.body.scan(%r{<span class="badge">(.*?)</span></dd>})
+      marks = last_response.body.scan(%r{<a href="/groups/#{id}" class="tag">((?:(?!</a>).)*)</a></dd>})
 
       assert_equal [[%(<span data-nameless>#{id}</span>)]] * 2, marks
     end
   end
 
-  # A contact in no group is marked nowhere: the badge is provenance,
+  # A contact in no group is marked nowhere: the tag is provenance,
   # not decoration, so a card with nothing lent renders none.
   def test_show_marks_nothing_on_a_contact_in_no_group
     with_contacts({"ada" => ADA}) do
       get "/contacts/ada"
 
-      refute_includes last_response.body, "badge"
+      refute_includes last_response.body, 'class="tag"'
     end
   end
 
diff --git a/test/pro_tacts/test_contact.rb b/test/pro_tacts/test_contact.rb
index ea7c10c..0cccafd 100644
--- a/test/pro_tacts/test_contact.rb
+++ b/test/pro_tacts/test_contact.rb
@@ -25,9 +25,9 @@ class ContactTest < Minitest::Test
   # card ever carries a BDAY the model recomposes — and inheritance
   # defaults to none, the shape of every contact in no group.
   # Inherited lines are given as bare strings and lent by GROUP, the
-  # name a composed row is marked with; a test that needs a second
+  # group a composed row is marked with; a test that needs a second
   # lender passes its own.
-  GROUP = "Booles" #: String
+  GROUP = ProTacts::Store::Group.new(id: "nous", name: "Booles", label: "Booles", lines: [], members: ["aiden"]) #: ProTacts::Store::Group
 
   def contact(bytes = CARD, id: "aiden", birthday: nil, inherited: [], group: GROUP)
     lent = inherited.map { ProTacts::Contact::Inherited.new(group:, line: it) }
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index 3e47a86..761ef41 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -1104,30 +1104,16 @@ class StoreTest < Minitest::Test
     end
   end
 
-  # Every inherited line carries the name of the group lending it, on
-  # the single read and on the listing read that composes a whole
-  # collection — a screen marks a row as the household's from either.
-  def test_an_inherited_line_carries_its_groups_name
+  # Every inherited line carries the group lending it, whole, on the
+  # single read and on the listing read that composes a whole
+  # collection — a screen tags a row as the household's from either.
+  def test_an_inherited_line_carries_its_group
     with_store({"aiden" => AIDEN}) do |store|
-      FixtureData.seed_group(store, name: "Household", members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+      id = FixtureData.seed_group(store, name: "Household", members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
 
       listed = store.contacts.find { it.id == "aiden" }
       [store.contact("aiden"), listed].each do |contact|
-        assert_equal "Household", contact.group_of(contact.addresses.fetch(0).line)
-      end
-    end
-  end
-
-  # A group with no name lends its lines under its id, on both reads —
-  # a mark that says which group a row came from, where the name would
-  # otherwise leave the row marked with nothing at all.
-  def test_a_nameless_groups_lines_are_marked_with_its_id
-    with_store({"aiden" => AIDEN}) do |store|
-      id = FixtureData.seed_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
-
-      listed = store.contacts.find { it.id == "aiden" }
-      [store.contact("aiden"), listed].each do |contact|
-        assert_equal id, contact.group_of(contact.addresses.fetch(0).line)
+        assert_equal store.group(id), contact.group_of(contact.addresses.fetch(0).line)
       end
     end
   end
@@ -1735,7 +1721,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 "Booles", contact.group_of(contact.addresses.fetch(0).line)
+        assert_equal "nous", contact.group_of(contact.addresses.fetch(0).line)&.id
 
         database(store)[:cards].where(id: "aiden").delete
         assert_empty database(store)[:group_members].all