import: name a new group while reviewing a contact
The boxes beside a contact offered the groups this book already had and nothing else, so a group that ought to exist had to wait for the import to finish. Now there is a box to name one. What it writes is a name rather than a group: nothing is in the store until the confirm, and a group created while the walk is still going is one left behind by an import that was abandoned. Import::Land makes it at the landing, where it already resolves the import's own group name, so the same name given to three contacts is one group and a name that matches an existing group joins that one. A name waiting like that reads back as a box of its own, ticked and in the italic the groups dialog gives a name that is not a group yet, so it can be taken off again before the confirm. The staged slot holds two maps now, ids for groups that exist and names for groups that do not -- an id being the only way to name a group that has no name of its own. The memberships are deduplicated on the way in: two answers can mean one group, and a membership written twice is a constraint violation rather than a second membership. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KrREeoPLXx1qsYYq7qkL8h
diff --git a/AGENTS.md b/AGENTS.md
index d5e49d9..f3be664 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -40,9 +40,9 @@ profiles, so leave those to the user.
Importing is not a task at all: a .vcf is uploaded at `/import` in the
app, and reviewed there contact by contact — the card as the file wrote
it beside the card that is landing, with the lines no screen here shows
-struck, and this book's groups as boxes to tick — before the lot is
-landed through the store, into a group made for the import and
-whichever groups each contact was given
+struck, and this book's groups as boxes to tick beside a box for a new
+one — before the lot is landed through the store, into a group made for
+the import and whichever groups each contact was given
(docs/plans/2026-09-21-import-a-vcf.md). The `import:*` tasks, the Swift
reader they drove Contacts.app with, and the plan directories they wrote
are gone.
diff --git a/docs/plans/2026-09-21-import-a-vcf.md b/docs/plans/2026-09-21-import-a-vcf.md
index 7aa5881..8d54c5c 100644
--- a/docs/plans/2026-09-21-import-a-vcf.md
+++ b/docs/plans/2026-09-21-import-a-vcf.md
@@ -109,7 +109,8 @@ So the review is a walk, four screens rather than a submission:
imported" beside it, a strike on its own being a color. On the
right, the contact editor, pre-filled with what was read and
pointed at the import, with this book's groups under its fields as
- boxes to tick. Read the spouse's name off the left card and type it
+ boxes to tick and a box to name one it does not have yet. Read the
+ spouse's name off the left card and type it
into the note on the right, in your own words, and put the contact
where it belongs while you are looking at it.
4. **Confirm**, under a group for the lot, and the cards land as the
@@ -145,7 +146,10 @@ has been edited still has to show what it arrived as, which is why
both. `groups.json` is which groups each of those cards is joining,
keyed by the card's place in the file: a vCard says nothing about this
book's groups, and a line invented to hold the answer would land in the
-contact.
+contact. Two maps in it, because a group this book has and a group this
+import is about to invent are not the same answer — ids for the first,
+which is the only way to name a group that has no name of its own, and
+names for the second, made at the landing.
On disk rather than in the browser: a book with pictures in it is tens
of megabytes, and a form carrying it back and forth is the upload done
@@ -188,12 +192,16 @@ dialog make:
The per-contact boxes ride in the editor's own form, so one Save
writes the card and its groups together and there is no second
- button to wonder about. They offer this book's groups and no new
- name: naming a group per contact would be four hundred chances to
- spell "Booles" two ways, and a contact's own page can make one the
- moment it lands. What each contact is joining is read back on its
- row in the list, because the walk is a screen at a time and the list
- is where ten screens' worth of decisions are seen at once.
+ button to wonder about. Under them is a box for a group this book
+ does not have yet, and what it writes is a name rather than a
+ group: nothing is in the store until the confirm, and a group
+ created while the walk was still going is one left behind by an
+ import that was abandoned. A name waiting like that shows as a box
+ of its own, ticked and in the italic the groups dialog gives a name
+ that is not a group yet, so it can be taken off again. What each
+ contact is joining is read back on its row in the list, because the
+ walk is a screen at a time and the list is where ten screens' worth
+ of decisions are seen at once.
Importing is not idempotent and cannot be. A `.vcf` carries no id this
server minted, so a second upload of the same file is a second set of
diff --git a/lib/pro_tacts/admin/import_groups.rb b/lib/pro_tacts/admin/import_groups.rb
index e3b5d39..7e2a610 100644
--- a/lib/pro_tacts/admin/import_groups.rb
+++ b/lib/pro_tacts/admin/import_groups.rb
@@ -16,26 +16,30 @@ module ProTacts
# (Admin::GroupDialog) — there the card exists and a membership is
# a write of its own, here neither is true until the confirm.
#
- # Boxes over this book's own groups and nothing else. A group this
- # import invents is the one the review screen names for the whole
- # file: naming a new group per contact would be four hundred
- # chances to spell "Booles" two ways, and the contact's own page
- # can make one the moment it lands.
+ # A box per group this book has, then a box per group this import
+ # has been told to make and has not made yet, then somewhere to
+ # name another. A named group is not created here: it is made at
+ # the landing (Import::Land#group_id), because a group created
+ # while the walk is still going is a group left behind by an
+ # import that was abandoned. So it rides as a name until then, and
+ # shows as a ticked box like any other so that it can be unticked.
class ImportGroups < Phlex::HTML
# @rbs @groups: Array[Store::Group]
# @rbs @joined: Array[String]
+ # @rbs @named: Array[String]
# Alphabetical, the groups dialog's own order: this is a list to
# find a name in, and nothing here stays put across a rename.
- #: (groups: Array[Store::Group], joined: Array[String]) -> void
- def initialize(groups:, joined:)
+ # `joined` is the ids ticked so far, `named` the groups this
+ # import is making that do not exist yet.
+ #: (groups: Array[Store::Group], joined: Array[String], named: Array[String]) -> void
+ def initialize(groups:, joined:, named:)
@groups = groups.sort_by { it.label.downcase }
@joined = joined
+ @named = named
end
def view_template
- return if @groups.empty?
-
div(class: "field-stack") do
span(class: "type-label") { "groups" }
@groups.each do |group|
@@ -45,6 +49,23 @@ module ProTacts
render GroupLabel.new(group:)
end
end
+ # Italic and muted, the shape the groups dialog gives a name
+ # that is not a group yet (Admin::GroupDialog): the box
+ # behaves like the ones above it, and the styling is what
+ # says this one is about to be made.
+ @named.each do |name|
+ label do
+ input(type: "checkbox", name: "named[]", value: name, checked: true)
+ span(class: "gl-muted", style: "font-style: italic;") { name }
+ end
+ end
+ # One at a time, and blank by default: a name typed here is
+ # added to the list above on save, where it can be read back
+ # and taken off again.
+ label(class: "field") do
+ span { "New group" }
+ input(type: "text", name: "new", placeholder: "made when the import lands")
+ end
end
end
end
diff --git a/lib/pro_tacts/import/land.rb b/lib/pro_tacts/import/land.rb
index fa8775d..ae5fa62 100644
--- a/lib/pro_tacts/import/land.rb
+++ b/lib/pro_tacts/import/land.rb
@@ -22,6 +22,7 @@ module ProTacts
# @rbs @store: Store
# @rbs @group: String?
# @rbs @joins: Array[Array[String]]
+ # @rbs @named: Array[Array[String]]
# @rbs @group_ids: Hash[String, String]
# The group an import offers to put its arrivals in, named for
@@ -38,25 +39,30 @@ module ProTacts
# the import being the one thing they all have in common.
# `joins` is the rest, a card at a time: the ids of groups this
# book already has, as the walk ticked them contact by contact,
- # in the cards' own order. Either, or neither: nothing here is
- # required, and a card in no group is still in everyone's book
- # (#land).
- #: (Store store, Array[VCard] cards, ?group: String?, ?joins: Array[Array[String]]) -> Array[Contact]
- def self.call(store, cards, group: nil, joins: [])
- new(store, group:, joins:).call(cards)
+ # in the cards' own order. `named` is the same list in names,
+ # for the groups the walk asked for that do not exist yet —
+ # made here and not while the walk was going, an abandoned
+ # import being the case that argues for waiting. Any of them, or
+ # none: a card in no group is still in everyone's book (#land).
+ #: (Store store, Array[VCard] cards, ?group: String?, ?joins: Array[Array[String]], ?named: Array[Array[String]]) -> Array[Contact]
+ def self.call(store, cards, group: nil, joins: [], named: [])
+ new(store, group:, joins:, named:).call(cards)
end
- #: (Store store, ?group: String?, ?joins: Array[Array[String]]) -> void
- def initialize(store, group: nil, joins: [])
+ #: (Store store, ?group: String?, ?joins: Array[Array[String]], ?named: Array[Array[String]]) -> void
+ def initialize(store, group: nil, joins: [], named: [])
@store = store
@group = group
@joins = joins
+ @named = named
@group_ids = {} #: Hash[String, String]
end
#: (Array[VCard] cards) -> Array[Contact]
def call(cards)
- cards.each_with_index.map { |card, index| land(card, @joins.fetch(index, [])) }
+ cards.each_with_index.map { |card, index|
+ land(card, @joins.fetch(index, []), @named.fetch(index, []))
+ }
end
private
@@ -64,20 +70,25 @@ module ProTacts
# A card, then its groups, the order a create writes them in:
# the put makes the contact, and the memberships that follow put
# it somewhere to be found.
- #: (VCard card, Array[String] chosen) -> Contact
- def land(card, chosen)
+ #: (VCard card, Array[String] chosen, Array[String] named) -> Contact
+ def land(card, chosen, named)
id = ChangeId.mint(ChangeId::CONTACT_LENGTH)
# `client: true` for what a client's PUT means here: a card this
# creates joins `sync:*`, or it would be on the server and in
# nobody's book (docs/plans/2026-09-15-client-creates-join-everyone.md).
@store.put(id, identified(card, id), client: true)
+ # Every name is resolved per card rather than up front, for
+ # #group_id's reason; the chosen ids need no resolving, the
+ # walk having read them off this same store. Deduplicated
+ # because two of these can mean one group — a contact's own
+ # new group named what the import's group is named, say —
+ # and a membership written twice is a constraint violation
+ # rather than a second membership.
group = @group
- # The named group is resolved per card rather than up front,
- # for #group_id's reason; the chosen ids need no resolving,
- # the walk having read them off this same store.
- join = group ? [*chosen, group_id(group)] : chosen
- @store.regroup(id, join:, leave: []) unless join.empty?
+ join = [*chosen, *named.map { group_id(it) }]
+ join << group_id(group) if group
+ @store.regroup(id, join: join.uniq, leave: []) unless join.empty?
# Read back rather than kept from the put, because the group
# above moved what the card serves. Nothing can have taken it
diff --git a/lib/pro_tacts/import/staged.rb b/lib/pro_tacts/import/staged.rb
index 033f12c..205f3ce 100644
--- a/lib/pro_tacts/import/staged.rb
+++ b/lib/pro_tacts/import/staged.rb
@@ -39,6 +39,11 @@ module ProTacts
GROUPS = "groups" #: String
SLOTS = [ORIGINAL, LANDING, GROUPS].freeze #: Array[String]
+ # The two halves of the groups slot: the ids of groups this
+ # book has, and the names of groups it does not have yet.
+ CHOSEN = "chosen" #: String
+ NAMED = "named" #: String
+
# How long an import is worth keeping. Long enough to work down
# a book's worth of contacts over an evening, and short enough
# that a window closed on the review screen does not leave that
@@ -61,7 +66,7 @@ module ProTacts
# exists is a file that exists and a write can say which of
# the two it found (#write).
nothing = {} #: Hash[String, Array[String]]
- File.binwrite(directory / file_name(GROUPS), JSON.generate(nothing))
+ File.binwrite(directory / file_name(GROUPS), JSON.generate({CHOSEN => nothing, NAMED => nothing}))
id
end
@@ -96,30 +101,35 @@ module ProTacts
# being no minted id until it lands. Rewritten whole like the
# cards beside it, and for the same reason: one save is one
# state of the whole import.
- #: (String id, Hash[String, Array[String]] groups) -> void
- def self.update_groups(id, groups)
- write(id, GROUPS, JSON.generate(groups))
+ #
+ # Two maps rather than one, because a group this book has and a
+ # group this import is about to invent are not the same answer.
+ # `chosen` holds ids, the only way to name a group that has no
+ # name of its own (db/migrations/005_group_identity.rb).
+ # `named` holds names of groups that do not exist yet: made at
+ # the landing and not before, because a group created while the
+ # walk is still going is a group left behind by an import that
+ # was abandoned (Import::Land#group_id).
+ #: (String id, Hash[String, Array[String]] chosen, Hash[String, Array[String]] named) -> void
+ def self.update_groups(id, chosen, named)
+ write(id, GROUPS, JSON.generate({CHOSEN => chosen, NAMED => named}))
end
- # What #update_groups last wrote, and nothing for an import
- # that is gone. Read back into the shape it was written in
- # rather than trusted: this is a file on disk, and a slot that
- # will not parse is a broken assumption JSON says so about.
- #: (String id) -> Hash[String, Array[String]]
+ # What #update_groups last wrote — the ids and then the names —
+ # and two empty maps for an import that is gone. Read back into
+ # the shape it was written in rather than trusted: this is a
+ # file on disk, and a slot that will not parse is a broken
+ # assumption JSON says so about.
+ #: (String id) -> [Hash[String, Array[String]], Hash[String, Array[String]]]
def self.groups(id)
- chosen = {} #: Hash[String, Array[String]]
raw = read(id, GROUPS)
- return chosen if raw.nil?
-
- parsed = JSON.parse(raw)
- return chosen unless parsed.is_a?(Hash)
-
- parsed.each do |index, ids|
- next unless ids.is_a?(Array)
+ parsed = raw.nil? ? nil : JSON.parse(raw)
+ # #lists makes an empty map of anything that is not one, so
+ # the gone import and the half a file are the same answer
+ # here rather than two spellings of it.
+ return [lists(nil), lists(nil)] unless parsed.is_a?(Hash)
- chosen[index.to_s] = ids.map(&:to_s)
- end
- chosen
+ [lists(parsed[CHOSEN]), lists(parsed[NAMED])]
end
# The import, gone: the last step of a landing, and what keeps
@@ -180,13 +190,28 @@ module ProTacts
File.binwrite(file.to_s, contents)
end
+ # One map of a contact's place in the file to a list of
+ # strings, out of whatever the file actually holds.
+ #: (untyped raw) -> Hash[String, Array[String]]
+ def self.lists(raw)
+ lists = {} #: Hash[String, Array[String]]
+ return lists unless raw.is_a?(Hash)
+
+ raw.each do |index, values|
+ next unless values.is_a?(Array)
+
+ lists[index.to_s] = values.map(&:to_s)
+ end
+ lists
+ end
+
# The cards are a .vcf, and the groups beside them are not.
#: (String slot) -> String
def self.file_name(slot)
slot == GROUPS ? "#{slot}.json" : "#{slot}.vcf"
end
- private_class_method :root, :import_root, :path, :file_name, :write
+ private_class_method :root, :import_root, :path, :file_name, :write, :lists
end
end
end
diff --git a/lib/pro_tacts/web/import.rb b/lib/pro_tacts/web/import.rb
index c8f92ad..f92632f 100644
--- a/lib/pro_tacts/web/import.rb
+++ b/lib/pro_tacts/web/import.rb
@@ -114,7 +114,7 @@ module ProTacts
return expired_screen if staged.nil?
originals, landing = staged
- chosen = Import::Staged.groups(upload)
+ chosen, named = Import::Staged.groups(upload)
# By name, because the row is read rather than submitted: a
# group the walk ticked and someone then deleted is simply not
# among these.
@@ -127,7 +127,7 @@ module ProTacts
# A literal of several elements is an Array until something
# says otherwise, and an inline annotation needs its own line.
dropped = Import::Vcf.read(originals.fetch(index)).dropped
- joined = chosen.fetch(index.to_s, []).filter_map { names[it] }.sort
+ joined = (chosen.fetch(index.to_s, []).filter_map { names[it] } + named.fetch(index.to_s, [])).sort
[import_contact(card, index), Import::Vcf.losses(dropped).length, joined] #: [Contact, Integer, Array[String]]
}
@@ -165,10 +165,7 @@ module ProTacts
action: "/import/#{upload}/#{index}",
back: ["/import/#{upload}", "the import"],
aside: Admin::ImportOriginal.new(card: original, dropped: Import::Vcf.read(original).dropped),
- fields: Admin::ImportGroups.new(
- groups: store.all_groups,
- joined: Import::Staged.groups(upload).fetch(index.to_s, []),
- ),
+ fields: import_groups(upload, index),
)
end
@@ -243,10 +240,17 @@ module ProTacts
# (Admin::ImportGroups): a card the walk has looked at has been
# decided about in both ways at once. Only ids naming a group,
# #apply_groups' own rule, and the boxes are the whole answer —
- # none ticked is none joined, not "leave it as it was".
+ # none ticked is none joined, not "leave it as it was". A name
+ # typed into the new-group box joins the names already standing
+ # for this contact, and none of them is made until the confirm.
+ chosen, named = Import::Staged.groups(upload)
+ ticked = ids_in(r.params["groups"]) & store.all_groups.map(&:id)
+ standing = ids_in(r.params["named"]).map(&:strip).reject(&:empty?)
+ fresh = r.params["new"].to_s.strip
Import::Staged.update_groups(
upload,
- Import::Staged.groups(upload).merge(index.to_s => ids_in(r.params["groups"]) & store.all_groups.map(&:id)),
+ chosen.merge(index.to_s => ticked),
+ named.merge(index.to_s => (fresh.empty? ? standing : standing + [fresh]).uniq),
)
r.redirect "/import/#{upload}", 303
end
@@ -264,14 +268,29 @@ module ProTacts
# confirm, and Store#add_member reads a group with `sole`, so a
# stale id would be a 500 rather than the nothing it means.
known = store.all_groups.map(&:id)
- chosen = Import::Staged.groups(upload)
+ chosen, named = Import::Staged.groups(upload)
joins = landing.each_index.map { chosen.fetch(it.to_s, []) & known }
- landed = Import::Land.call(store, landing, group: group.empty? ? nil : group, joins:)
+ makes = landing.each_index.map { named.fetch(it.to_s, []) }
+ landed = Import::Land.call(store, landing, group: group.empty? ? nil : group, joins:, named: makes)
Import::Staged.close(upload)
import_screen(landed:)
end
+ # The groups half of one contact's screen: what this book has,
+ # what the walk has ticked, and what it has said to make. Read
+ # here rather than in the view, which is handed the answer like
+ # every other view (Admin::ImportGroups).
+ #: (String upload, Integer index) -> Admin::ImportGroups
+ def import_groups(upload, index)
+ chosen, named = Import::Staged.groups(upload)
+ Admin::ImportGroups.new(
+ groups: store.all_groups,
+ joined: chosen.fetch(index.to_s, []),
+ named: named.fetch(index.to_s, []),
+ )
+ end
+
# The import's two readings, or none when it is gone — swept out
# from under a screen left open overnight, or landed already, a
# second confirm finding what the first removed. Ordinary enough
diff --git a/sig/pro_tacts/admin.rbs b/sig/pro_tacts/admin.rbs
index 00f714a..48074fb 100644
--- a/sig/pro_tacts/admin.rbs
+++ b/sig/pro_tacts/admin.rbs
@@ -56,7 +56,7 @@ module ProTacts
end
class ImportGroups
- def self.new: (groups: Array[::ProTacts::Store::Group], joined: Array[String]) -> instance
+ def self.new: (groups: Array[::ProTacts::Store::Group], joined: Array[String], named: Array[String]) -> instance
end
# Rendered by Dashboard, not called from checked code; only
diff --git a/test/pro_tacts/admin/test_import_pages.rb b/test/pro_tacts/admin/test_import_pages.rb
index f494f88..20d5ad6 100644
--- a/test/pro_tacts/admin/test_import_pages.rb
+++ b/test/pro_tacts/admin/test_import_pages.rb
@@ -293,6 +293,36 @@ class AdminImportPagesTest < Minitest::Test
end
end
+ # A group that does not exist yet is named beside the contact and
+ # made at the landing, not before: a group created while the walk
+ # is still going is one left behind by an import that was
+ # abandoned.
+ def test_a_group_named_during_the_walk_waits_for_the_landing
+ with_contacts({}) do |store|
+ id = upload(JANE)
+ get "/import/#{id}/0"
+
+ post "/import/#{id}/0",
+ "etag" => etag, "first" => "Jane", "middle" => "", "last" => "Booles",
+ "nickname" => "", "note" => "", "new" => " Clarks "
+
+ assert_equal 303, last_response.status
+ assert_empty store.all_groups
+
+ # Read back as a box of its own, ticked, so it can be taken off
+ # again before the confirm.
+ get "/import/#{id}/0"
+
+ assert_includes last_response.body,
+ %(<input type="checkbox" name="named[]" value="Clarks" checked>)
+
+ post "/import/#{id}/land", "group" => ""
+ clarks = store.all_groups.find { it.name == "Clarks" }
+
+ assert_equal [store.contacts.fetch(0).id], clarks.members
+ end
+ end
+
# Contact by contact, so the cards do not all land alike. The id
# that names no group is dropped rather than carried into
# Store#add_member, which reads a group with `sole` and would
diff --git a/test/pro_tacts/import/test_land.rb b/test/pro_tacts/import/test_land.rb
index 0a14b88..a365f63 100644
--- a/test/pro_tacts/import/test_land.rb
+++ b/test/pro_tacts/import/test_land.rb
@@ -114,6 +114,31 @@ class ImportLandTest < Minitest::Test
end
end
+ # A group the walk asked for by name is made here and not before:
+ # a group created while the walk was still going is one left behind
+ # by an import that was abandoned.
+ def test_a_group_named_during_the_walk_is_made_at_the_landing
+ with_contacts({}) do |store|
+ landed = Land.call(store, Vcf.cards(JANE + JANE), named: [["Clarks"], ["Clarks"]])
+ clarks = store.all_groups.find { it.name == "Clarks" }
+
+ assert_equal landed.map(&:id).sort, clarks.members.sort
+ assert_equal 1, store.all_groups.count { it.name == "Clarks" }
+ end
+ end
+
+ # Two answers can mean one group, and a membership written twice is
+ # a constraint violation rather than a second membership.
+ def test_a_name_that_is_the_imports_own_group_joins_it_once
+ with_contacts({}) do |store|
+ landed = Land.call(store, Vcf.cards(JANE), group: "Clarks", named: [["Clarks"]])
+ id = landed.fetch(0).id
+
+ assert_equal 1, store.all_groups.count { it.name == "Clarks" }
+ assert_equal [id], store.all_groups.find { it.name == "Clarks" }.members
+ end
+ end
+
# Either, or neither: a chosen group stands on its own when no name
# is typed.
def test_arrivals_join_a_chosen_group_with_no_group_named