import: put each contact in its own groups
Before: the groups an import joined were one choice for the whole file, ticked on the review screen. A book exported from one place is not one kind of person, and "these three are from the school list" had to wait until everything had landed. After: this book's groups are boxes beside each contact's own card, inside the editor's form, so one Save writes the card and its groups together. The review screen keeps the group it names for the lot -- what landed together is still found together -- and each row reads back what that contact is joining, because the walk is a screen at a time and the list is where ten screens are seen at once. The boxes offer groups that exist 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. How: a third staged slot, groups.json, keyed by a card's place in the file the way every screen of the walk names one -- a vCard says nothing about this book's groups, and a line invented to hold the answer would land in the contact. Import::Land takes `joins`, one list per card in the cards' order. The ids are filtered against the store where they are written and again where they are used, so a group deleted between the tick and the confirm is nothing rather than a 500 out of Store#add_member's `sole`. 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 fa4d3aa..d5e49d9 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -40,8 +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 — before the lot is landed through the store, into a group made
-for the import and whichever of this book's groups the review ticked
+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
(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/Steepfile b/Steepfile
index fce51b7..c56d4cf 100644
--- a/Steepfile
+++ b/Steepfile
@@ -22,7 +22,7 @@ target :lib do
# the files stay checked.
ignore(*%w[
avatar contact_dialog contacts_edit contacts_index contacts_show dashboard device_setup format
- group_dialog group_label groups_edit groups_index groups_show icons import_original import_review
+ group_dialog group_label groups_edit groups_index groups_show icons import_groups import_original import_review
import_upload layout list_item phlex upcoming_birthdays
].map { "lib/pro_tacts/admin/#{it}.rb" }, inline: true)
signature "sig"
diff --git a/docs/plans/2026-09-21-import-a-vcf.md b/docs/plans/2026-09-21-import-a-vcf.md
index 093d1ce..7aa5881 100644
--- a/docs/plans/2026-09-21-import-a-vcf.md
+++ b/docs/plans/2026-09-21-import-a-vcf.md
@@ -108,17 +108,22 @@ So the review is a walk, four screens rather than a submission:
not coming in struck in Gloss's danger color and labelled "not
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. Read the spouse's name off the left card and
- type it into the note on the right, in your own words.
-4. **Confirm**, under whichever groups the arrivals should join, and
- the cards land as the walk left them.
+ 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
+ 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
+ walk left them.
The right-hand card is `Admin::ContactsEdit` itself, not a copy of it.
A second editor for imports would be a place where the two could
disagree — a field an import writes that an edit cannot undo — so the
-editor grew three arguments instead (`action`, `back`, `aside`) and the
-ordinary edit passes none of them. The birthday is split out of the
-card and into the model here exactly as `Store#put` splits it, so the
+editor grew four arguments instead (`action`, `back`, `aside`,
+`fields`) and the ordinary edit passes none of them. `fields` is the
+groups: they belong to the contact rather than to its card, and inside
+the editor's own form they save with everything else. The birthday is
+split out of the card and into the model here exactly as `Store#put`
+splits it, so the
same row renders over a staged card as over a stored contact, and put
back as a BDAY line on save; a BDAY spelling the model does not read
stays in the card untouched, that method's own rule. The one birthday
@@ -132,12 +137,15 @@ it was typed.
## The import waits on the server
An import is a directory under `data/imports`, keyed by a minted id
-that every screen of the walk carries in its path. Two slots in it,
-because the review screen shows two things at once: `original.vcf`, the
-uploaded file, written once and never again, and `landing.vcf`, the
-cards as they will land — pared when the import opens and rewritten
-whole each time the editor saves one of them. A card that has been
-edited still has to show what it arrived as, which is why both.
+that every screen of the walk carries in its path. Three slots in it.
+`original.vcf` is the uploaded file, written once and never again, and
+`landing.vcf` the cards as they will land — pared when the import opens
+and rewritten whole each time the editor saves one of them. A card that
+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.
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
@@ -166,21 +174,26 @@ dialog make:
2. `Store#put` with `client: true`, so a card this creates joins
everyone's book the way a client's create does
(`2026-09-15-client-creates-join-everyone.md`).
-3. `Store#regroup` into whichever groups the review screen settled on.
- There are two ways to name one there and they are both joins. The
- text box makes a group, named `import-<timestamp>` by default — what
- landed together can be found together, and undone together — and is
- emptiable, a name this server already has joining that group rather
- than colliding with it. Beside it every group this book already has,
- as a box to tick, because an import is as often "these are the people
- from the school list" as it is a batch that only needs finding again.
- Neither is required: a card in no group is still in everyone's book.
-
- Ticked for the whole file rather than per contact. The walk's pair of
- cards is for fixing what one contact carries, and a group is not
- something the file says anything about; once the arrivals have landed
- the group they landed in is how to find them, and a group's own
- editor is where its members are moved (`Admin::GroupsEdit`).
+3. `Store#regroup` into whichever groups the walk settled on. Two
+ questions, asked in the two places they belong. The review screen
+ names one group for the lot, `import-<timestamp>` by default — what
+ landed together can be found together, and undone together — and it
+ is emptiable, a name this server already has joining that group
+ rather than colliding with it. Beside each contact's own card, every
+ group this book already has as a box to tick: an import is as often
+ "these three are from the school list" as it is a batch that only
+ needs finding again, and which people those are is exactly what the
+ walk is for. Neither is required, and a card in no group is still in
+ everyone's book.
+
+ 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.
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/contacts_edit.rb b/lib/pro_tacts/admin/contacts_edit.rb
index 699318b..91c4c7c 100644
--- a/lib/pro_tacts/admin/contacts_edit.rb
+++ b/lib/pro_tacts/admin/contacts_edit.rb
@@ -79,21 +79,26 @@ module ProTacts
# @rbs @action: String
# @rbs @back: [String, String]
# @rbs @aside: Phlex::HTML?
+ # @rbs @fields: Phlex::HTML?
- # `action`, `back` and `aside` are the import's: the same editor,
- # over a card that is not stored yet, saving into the import
- # rather than into a contact and rendered beside the card as it
- # was exported (Admin::ImportOriginal). One editor rather than a
+ # `action`, `back`, `aside` and `fields` are the import's: the
+ # same editor, over a card that is not stored yet, saving into
+ # the import rather than into a contact, rendered beside the
+ # card as it was exported (Admin::ImportOriginal) and carrying
+ # what the walk decides about a contact that a card cannot hold
+ # — which groups it joins (Admin::ImportGroups), inside this
+ # form so that one Save writes the lot. One editor rather than a
# second one for imports, because a field the two disagreed
# about is a field an import writes and an edit cannot undo.
# Defaulted to the contact's own, so the ordinary edit says
# nothing about any of it.
- #: (contact: Contact, ?notice: String?, ?action: String?, ?back: [String, String]?, ?aside: Phlex::HTML?) -> void
- def initialize(contact:, notice: nil, action: nil, back: nil, aside: nil)
+ #: (contact: Contact, ?notice: String?, ?action: String?, ?back: [String, String]?, ?aside: Phlex::HTML?, ?fields: Phlex::HTML?) -> void
+ def initialize(contact:, notice: nil, action: nil, back: nil, aside: nil, fields: nil)
@contact = contact
@action = action || "/contacts/#{contact.id}"
@back = back || ["/contacts/#{contact.id}", contact.name || contact.id]
@aside = aside
+ @fields = fields
# The rows are the contact's own, never what a group lends it
# (Contact#own); the details page is where an inherited row is
# read (Admin::ContactsShow). The etag below is still the
@@ -200,6 +205,10 @@ module ProTacts
textarea(name: "note", rows: 4,
placeholder: note ? "removed on save" : nil) { note.to_s }
end
+ # Last, and only where something passed them: what
+ # is not a property of the card reads after
+ # everything that is.
+ render @fields if @fields
end
end
# The card's action bar, a dialog footer's shape (admin.css),
diff --git a/lib/pro_tacts/admin/import_groups.rb b/lib/pro_tacts/admin/import_groups.rb
new file mode 100644
index 0000000..e3b5d39
--- /dev/null
+++ b/lib/pro_tacts/admin/import_groups.rb
@@ -0,0 +1,52 @@
+require "pro_tacts/admin/phlex"
+
+require "pro_tacts/admin/group_label"
+
+module ProTacts
+ module Admin
+ # The groups one arriving contact will join, rendered into the
+ # editor's own form on the import's review screen
+ # (docs/plans/2026-09-21-import-a-vcf.md).
+ #
+ # Inside that form rather than beside it, because a staged
+ # contact's groups are one more thing the walk is deciding about
+ # it: one Save writes the card and the groups together, and there
+ # is no second button to wonder about. That is the difference from
+ # a stored contact, whose groups are a dialog on the details page
+ # (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.
+ class ImportGroups < Phlex::HTML
+ # @rbs @groups: Array[Store::Group]
+ # @rbs @joined: 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:)
+ @groups = groups.sort_by { it.label.downcase }
+ @joined = joined
+ end
+
+ def view_template
+ return if @groups.empty?
+
+ div(class: "field-stack") do
+ span(class: "type-label") { "groups" }
+ @groups.each do |group|
+ label do
+ input(type: "checkbox", name: "groups[]", value: group.id,
+ checked: @joined.include?(group.id))
+ render GroupLabel.new(group:)
+ end
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/pro_tacts/admin/import_review.rb b/lib/pro_tacts/admin/import_review.rb
index 0dbc825..23da0ba 100644
--- a/lib/pro_tacts/admin/import_review.rb
+++ b/lib/pro_tacts/admin/import_review.rb
@@ -1,7 +1,6 @@
require "pro_tacts/admin/phlex"
require "pro_tacts/admin/format"
-require "pro_tacts/admin/group_label"
require "pro_tacts/admin/layout"
require "pro_tacts/admin/list_item"
@@ -26,25 +25,22 @@ module ProTacts
# going to copy across by hand.
class ImportReview < Phlex::HTML
# @rbs @upload: String
- # @rbs @rows: Array[[::ProTacts::Contact, Integer]]
+ # @rbs @rows: Array[[::ProTacts::Contact, Integer, Array[String]]]
# @rbs @unknown: Array[::ProTacts::Import::Vcf::Unknown]
# @rbs @group: String
- # @rbs @groups: Array[::ProTacts::Store::Group]
# @rbs @notice: String?
- # `rows` is the contact each card will land as, paired with how
- # many of its original's lines are not coming with it. `upload`
- # is the staged import every link and the confirm carry.
- # `group` is the name the import offers to make; `groups` is
- # every group this book already has, alphabetical because this
- # is a list to find a name in (the groups dialog's own order).
- #: (upload: String, rows: Array[[::ProTacts::Contact, Integer]], unknown: Array[::ProTacts::Import::Vcf::Unknown], group: String, groups: Array[::ProTacts::Store::Group], ?notice: String?) -> void
- def initialize(upload:, rows:, unknown:, group:, groups:, notice: nil)
+ # `rows` is the contact each card will land as, how many of its
+ # original's lines are not coming with it, and the groups the
+ # walk has put it in so far. `upload` is the staged import every
+ # link and the confirm carry, and `group` the name of the group
+ # this import offers to make for the lot of them.
+ #: (upload: String, rows: Array[[::ProTacts::Contact, Integer, Array[String]]], unknown: Array[::ProTacts::Import::Vcf::Unknown], group: String, ?notice: String?) -> void
+ def initialize(upload:, rows:, unknown:, group:, notice: nil)
@upload = upload
@rows = rows
@unknown = unknown
@group = group
- @groups = groups.sort_by { it.label.downcase }
@notice = notice
end
@@ -69,48 +65,24 @@ module ProTacts
h1(class: "type-h2", style: "margin: 0;") { count(@rows.length, "contact") }
losses
form(action: "/import/#{@upload}/land", method: "post", class: "field-stack") do
- # A group made for this import, named for the moment by
- # default so one import can be found — or undone — apart
- # from the next. Editable, and emptiable; a name this
- # book already uses joins that group rather than making
- # a second one by the same name (Import::Land#group_id).
+ # A group made for this import and taking in every
+ # contact, named for the moment by default so one import
+ # can be found — or undone — apart from the next.
+ # Editable, and emptiable; a name this book already uses
+ # joins that group rather than making a second one by the
+ # same name (Import::Land#group_id). Which of this book's
+ # own groups a contact joins is that contact's question,
+ # asked beside its card (Admin::ImportGroups).
label(class: "field") do
- span(class: "type-label") { "new group" }
+ span(class: "type-label") { "group for the lot" }
input(type: "text", name: "group", value: @group, placeholder: "no group")
end
- existing_groups
button(type: "submit", data: {variant: "primary"}) { "import #{count(@rows.length, "contact")}" }
end
end
end
end
- # The groups this book already has, every one of them a box the
- # arrivals can join: an import is as often "these are the people
- # from the school list" as it is a batch that only needs finding
- # again, and a name typed into the box above is a name you have
- # to know. Ticked alongside that name rather than instead of it
- # — both are joins, and neither is required.
- #
- # Unchecked to begin with, every time. The import's own group is
- # the default this screen argues for; putting four hundred
- # arrivals somewhere else is a thing to say, not a box to leave
- # as it was found.
- #: () -> void
- def existing_groups
- return if @groups.empty?
-
- div(class: "field-stack") do
- span(class: "type-label") { "add to" }
- @groups.each do |group|
- label do
- input(type: "checkbox", name: "groups[]", value: group.id)
- render GroupLabel.new(group:)
- end
- end
- end
- end
-
# What the file is losing, by property rather than by line: a
# reader deciding whether to open any of the rows below wants to
# know it is `X-SOCIALPROFILE` and `PRODID` going, not that
@@ -152,12 +124,17 @@ module ProTacts
h2(class: "type-label") { "contacts" }
end
ul(class: "card") do
- @rows.each_with_index do |(contact, dropped), index|
+ @rows.each_with_index do |(contact, dropped, joined), index|
render ListItem.new(
href: "/import/#{@upload}/#{index}",
trailing: (count(dropped, "line") + " left behind" if dropped.positive?),
) do
div { Format.name_label(contact) }
+ # The groups this contact is joining, read back under
+ # its name: the walk is a screen at a time, and the
+ # list is where you see what the last ten screens
+ # settled on without opening them again.
+ div(class: "type-label") { joined.join(", ") } if joined.any?
end
end
end
diff --git a/lib/pro_tacts/import/land.rb b/lib/pro_tacts/import/land.rb
index 090aad8..fa8775d 100644
--- a/lib/pro_tacts/import/land.rb
+++ b/lib/pro_tacts/import/land.rb
@@ -21,7 +21,7 @@ module ProTacts
class Land
# @rbs @store: Store
# @rbs @group: String?
- # @rbs @join: Array[String]
+ # @rbs @joins: Array[Array[String]]
# @rbs @group_ids: Hash[String, String]
# The group an import offers to put its arrivals in, named for
@@ -34,37 +34,38 @@ module ProTacts
end
# `group` is a group named rather than chosen — this server's
- # own by that name, or a new one — and `join` the ids of groups
- # it already has. Both, because the review screen offers both: a
- # group named for the moment, so that what landed together can
- # be found together, and the groups these people actually belong
- # in. 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?, ?join: Array[String]) -> Array[Contact]
- def self.call(store, cards, group: nil, join: [])
- new(store, group:, join:).call(cards)
+ # own by that name, or a new one — and it takes in every card,
+ # 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)
end
- #: (Store store, ?group: String?, ?join: Array[String]) -> void
- def initialize(store, group: nil, join: [])
+ #: (Store store, ?group: String?, ?joins: Array[Array[String]]) -> void
+ def initialize(store, group: nil, joins: [])
@store = store
@group = group
- @join = join
+ @joins = joins
@group_ids = {} #: Hash[String, String]
end
#: (Array[VCard] cards) -> Array[Contact]
def call(cards)
- cards.map { land(it) }
+ cards.each_with_index.map { |card, index| land(card, @joins.fetch(index, [])) }
end
private
- # A card, then its group, the order a create writes them in: the
- # put makes the contact, and the membership that follows puts it
- # somewhere to be found.
- #: (VCard card) -> Contact
- def land(card)
+ # 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)
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
@@ -74,8 +75,8 @@ module ProTacts
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 review screen having read them off this same store.
- join = group ? [*@join, group_id(group)] : @join
+ # the walk having read them off this same store.
+ join = group ? [*chosen, group_id(group)] : chosen
@store.regroup(id, join:, leave: []) unless join.empty?
# Read back rather than kept from the put, because the group
diff --git a/lib/pro_tacts/import/staged.rb b/lib/pro_tacts/import/staged.rb
index 62e047e..978ef39 100644
--- a/lib/pro_tacts/import/staged.rb
+++ b/lib/pro_tacts/import/staged.rb
@@ -1,4 +1,5 @@
require "fileutils"
+require "json"
require "pathname"
require "pro_tacts"
@@ -11,13 +12,15 @@ module ProTacts
# over the contacts in a file, a screen at a time, and what the
# walk is reading and writing has to outlive each request.
#
- # Two slots per import, because the review screen shows two things
- # at once. ORIGINAL is the uploaded file, written once and never
- # again: it is what the left-hand card renders, and a card that has
- # been edited still has to show what it arrived as. LANDING is the
- # cards as they will land — pared to what this book reads when the
- # import opens (Vcf.read), and rewritten whole each time the editor
- # saves one of them.
+ # Three slots per import. ORIGINAL is the uploaded file, written
+ # once and never again: it is what the left-hand card renders, and
+ # a card that has been edited still has to show what it arrived
+ # as. LANDING is the cards as they will land — pared to what this
+ # book reads when the import opens (Vcf.read), and rewritten whole
+ # each time the editor saves one of them. GROUPS is which groups
+ # each of those cards is joining, a decision the cards themselves
+ # cannot carry: a vCard says nothing about this book's groups, and
+ # a line invented to hold the answer would land in the contact.
#
# 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
@@ -33,7 +36,8 @@ module ProTacts
ORIGINAL = "original" #: String
LANDING = "landing" #: String
- SLOTS = [ORIGINAL, LANDING].freeze #: Array[String]
+ GROUPS = "groups" #: String
+ SLOTS = [ORIGINAL, LANDING, GROUPS].freeze #: Array[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
@@ -53,6 +57,10 @@ module ProTacts
FileUtils.mkdir_p(directory)
File.binwrite(directory / file_name(ORIGINAL), original)
File.binwrite(directory / file_name(LANDING), landing)
+ # Empty rather than absent, so every slot of an import that
+ # exists is a file that exists and a write can say which of
+ # the two it found (#write).
+ File.binwrite(directory / file_name(GROUPS), JSON.generate({}))
id
end
@@ -79,10 +87,32 @@ module ProTacts
# names one.
#: (String id, String landing) -> void
def self.update(id, landing)
- file = path(id, LANDING) or raise ArgumentError, "#{id} is not an import id"
- raise ArgumentError, "#{id} is not an import in progress" unless file.file?
+ write(id, LANDING, landing)
+ end
+
+ # Which groups each contact joins, by its place in the file —
+ # the name every screen of the walk calls a contact by, there
+ # 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))
+ 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]]
+ def self.groups(id)
+ raw = read(id, GROUPS)
+ return {} if raw.nil?
- File.binwrite(file.to_s, landing)
+ parsed = JSON.parse(raw)
+ return {} unless parsed.is_a?(Hash)
+
+ parsed.to_h { |index, ids| [index.to_s, (ids.is_a?(Array) ? ids : []).map(&:to_s)] }
end
# The import, gone: the last step of a landing, and what keeps
@@ -132,12 +162,24 @@ module ProTacts
directory / file_name(slot)
end
+ # One slot, rewritten. Both refusals name which of the two
+ # things went wrong: an id this never minted, or an import that
+ # is no longer here.
+ #: (String id, String slot, String contents) -> void
+ def self.write(id, slot, contents)
+ file = path(id, slot) or raise ArgumentError, "#{id} is not an import id"
+ raise ArgumentError, "#{id} is not an import in progress" unless file.file?
+
+ File.binwrite(file.to_s, contents)
+ end
+
+ # The cards are a .vcf, and the groups beside them are not.
#: (String slot) -> String
def self.file_name(slot)
- "#{slot}.vcf"
+ slot == GROUPS ? "#{slot}.json" : "#{slot}.vcf"
end
- private_class_method :root, :import_root, :path, :file_name
+ private_class_method :root, :import_root, :path, :file_name, :write
end
end
end
diff --git a/lib/pro_tacts/web/import.rb b/lib/pro_tacts/web/import.rb
index decf37a..62d10d3 100644
--- a/lib/pro_tacts/web/import.rb
+++ b/lib/pro_tacts/web/import.rb
@@ -1,5 +1,6 @@
require "pro_tacts/admin/card_form"
require "pro_tacts/admin/contacts_edit"
+require "pro_tacts/admin/import_groups"
require "pro_tacts/admin/import_original"
require "pro_tacts/admin/import_review"
require "pro_tacts/admin/import_upload"
@@ -113,11 +114,17 @@ module ProTacts
return expired_screen if staged.nil?
originals, landing = staged
+ chosen = 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.
+ names = store.all_groups.to_h { |group| [group.id, group.label] }
rows = landing.each_with_index.map { |card, index|
- # A two-element literal is an Array until something says
- # otherwise, and an inline annotation needs its own line.
+ # 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
- [import_contact(card, index), Import::Vcf.losses(dropped).length] #: [Contact, Integer]
+ joined = chosen.fetch(index.to_s, []).filter_map { names[it] }.sort
+ [import_contact(card, index), Import::Vcf.losses(dropped).length, joined] #: [Contact, Integer, Array[String]]
}
response["Content-Type"] = "text/html; charset=utf-8"
@@ -126,7 +133,6 @@ module ProTacts
rows:,
unknown: Import::Vcf.unknown(originals),
group: Import::Land.default_group,
- groups: store.all_groups,
notice:,
)
end
@@ -155,6 +161,10 @@ 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, []),
+ ),
)
end
@@ -225,6 +235,15 @@ module ProTacts
landing[index] = edited
Import::Staged.update(upload, joined(landing))
+ # The groups ride in the same form and save with it
+ # (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".
+ Import::Staged.update_groups(
+ upload,
+ Import::Staged.groups(upload).merge(index.to_s => ids_in(r.params["groups"]) & store.all_groups.map(&:id)),
+ )
r.redirect "/import/#{upload}", 303
end
@@ -236,11 +255,14 @@ module ProTacts
_originals, landing = staged
group = r.params["group"].to_s.strip
- # Only ids naming a group, #apply_groups' own rule: Store#add_member
- # reads the group with `sole`, so a doctored one would be a 500
- # rather than the bad input it is.
- join = ids_in(r.params["groups"]) & store.all_groups.map(&:id)
- landed = Import::Land.call(store, landing, group: group.empty? ? nil : group, join:)
+ # Filtered again here, and not only where the walk wrote them
+ # (#save_card): a group can be deleted between the tick and the
+ # 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)
+ joins = landing.each_index.map { chosen.fetch(it.to_s, []) & known }
+ landed = Import::Land.call(store, landing, group: group.empty? ? nil : group, joins:)
Import::Staged.close(upload)
import_screen(landed:)
diff --git a/sig/pro_tacts/admin.rbs b/sig/pro_tacts/admin.rbs
index 721570f..00f714a 100644
--- a/sig/pro_tacts/admin.rbs
+++ b/sig/pro_tacts/admin.rbs
@@ -14,7 +14,7 @@ module ProTacts
# ships no signatures and the views are ignored (see the
# Steepfile).
class ContactsEdit
- def self.call: (contact: Contact, ?notice: String?, ?action: String?, ?back: [String, String]?, ?aside: untyped) -> String
+ def self.call: (contact: Contact, ?notice: String?, ?action: String?, ?back: [String, String]?, ?aside: untyped, ?fields: untyped) -> String
end
class Dashboard
@@ -46,15 +46,19 @@ module ProTacts
end
class ImportReview
- def self.call: (upload: String, rows: Array[[Contact, Integer]], unknown: Array[::ProTacts::Import::Vcf::Unknown], group: String, groups: Array[::ProTacts::Store::Group], ?notice: String?) -> String
+ def self.call: (upload: String, rows: Array[[Contact, Integer, Array[String]]], unknown: Array[::ProTacts::Import::Vcf::Unknown], group: String, ?notice: String?) -> String
end
- # Rendered into ContactsEdit rather than called, so it is the one
- # view here whose `new` is what checked code reaches for.
+ # Rendered into ContactsEdit rather than called, so these are the
+ # views here whose `new` is what checked code reaches for.
class ImportOriginal
def self.new: (card: ::ProTacts::VCard, dropped: Array[::ProTacts::VCard::Parser::Line]) -> instance
end
+ class ImportGroups
+ def self.new: (groups: Array[::ProTacts::Store::Group], joined: Array[String]) -> instance
+ end
+
# Rendered by Dashboard, not called from checked code; only
# its LIMIT is read in web.rb.
class UpcomingBirthdays
diff --git a/test/pro_tacts/admin/test_import_pages.rb b/test/pro_tacts/admin/test_import_pages.rb
index f306d54..f494f88 100644
--- a/test/pro_tacts/admin/test_import_pages.rb
+++ b/test/pro_tacts/admin/test_import_pages.rb
@@ -261,37 +261,83 @@ class AdminImportPagesTest < Minitest::Test
end
end
- # The groups this book already has, offered beside the name the
- # import would make: an import is as often people who belong in a
- # group that exists as it is a batch that only needs finding again.
- def test_the_review_offers_the_groups_this_book_already_has
+ # The whole file's group is named on the review screen; which of
+ # this book's own groups a contact joins is asked beside that
+ # contact's card, and saved by the same Save as its fields.
+ def test_a_contact_is_put_in_its_groups_beside_its_own_card
with_contacts({}) do |store|
school = store.create_group(name: "school")
+ id = upload(JANE + PLAIN)
- upload(JANE)
- follow_redirect!
+ get "/import/#{id}/0"
- assert_includes last_response.body, %(<input type="text" name="group" value="import-)
assert_includes last_response.body, %(<input type="checkbox" name="groups[]" value="#{school}">school)
+
+ post "/import/#{id}/0",
+ "etag" => etag, "first" => "Jane", "middle" => "", "last" => "Booles",
+ "nickname" => "", "note" => "", "groups" => [school]
+
+ assert_equal 303, last_response.status
+ assert_empty store.changes
+
+ # Read back on the row, so the walk can be seen without opening
+ # every contact again, and in the box when the card is reopened.
+ follow_redirect!
+
+ assert_includes last_response.body, %(<div class="type-label">school</div>)
+
+ get "/import/#{id}/0"
+
+ assert_includes last_response.body,
+ %(<input type="checkbox" name="groups[]" value="#{school}" checked>school)
end
end
- # Ticked and typed are both joins. The id that names no group is
- # dropped rather than carried into Store#add_member, which reads a
- # group with `sole` and would answer bad input with a 500.
- def test_confirming_joins_the_groups_that_were_ticked
+ # 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
+ # answer bad input with a 500.
+ def test_confirming_joins_each_contact_to_its_own_groups
with_contacts({}) do |store|
school = store.create_group(name: "school")
- id = upload(JANE)
+ id = upload(JANE + PLAIN)
- post "/import/#{id}/land", "group" => "import-20260921T031655Z", "groups" => [school, "zzzz"]
+ get "/import/#{id}/0"
+ post "/import/#{id}/0",
+ "etag" => etag, "first" => "Jane", "middle" => "", "last" => "Booles",
+ "nickname" => "", "note" => "", "groups" => [school, "zzzz"]
+
+ post "/import/#{id}/land", "group" => "import-20260921T031655Z"
assert_equal 200, last_response.status
- landed = store.contacts.fetch(0)
+ jane = store.contacts.find { it.name == "Jane Booles" }
+ sam = store.contacts.find { it.name == "Sam Booles" }
- assert_equal [landed.id], store.group(school).members
+ assert_equal [jane.id], store.group(school).members
assert_equal ["import-20260921T031655Z", "school", ProTacts::Store::EVERYONE],
- store.all_groups.select { it.members.include?(landed.id) }.map(&:name).sort
+ store.all_groups.select { it.members.include?(jane.id) }.map(&:name).sort
+ assert_equal ["import-20260921T031655Z", ProTacts::Store::EVERYONE],
+ store.all_groups.select { it.members.include?(sam.id) }.map(&:name).sort
+ end
+ end
+
+ # A group deleted between the tick and the confirm is nothing, not
+ # a 500: the ids are filtered again where they are used.
+ def test_a_group_that_went_away_before_the_confirm_is_dropped
+ with_contacts({}) do |store|
+ school = store.create_group(name: "school")
+ id = upload(JANE)
+
+ get "/import/#{id}/0"
+ post "/import/#{id}/0",
+ "etag" => etag, "first" => "Jane", "middle" => "", "last" => "Booles",
+ "nickname" => "", "note" => "", "groups" => [school]
+ store.delete_group(school)
+
+ post "/import/#{id}/land", "group" => ""
+
+ assert_equal 200, last_response.status
+ assert_equal [ProTacts::Store::EVERYONE], store.all_groups.map(&:name)
end
end
diff --git a/test/pro_tacts/import/test_land.rb b/test/pro_tacts/import/test_land.rb
index 4d1c8a5..0a14b88 100644
--- a/test/pro_tacts/import/test_land.rb
+++ b/test/pro_tacts/import/test_land.rb
@@ -81,14 +81,14 @@ class ImportLandTest < Minitest::Test
end
end
- # The groups the review screen ticked, joined alongside the name it
- # typed: an import is as often "these are the people from the school
- # list" as it is a batch that only needs finding again.
- def test_arrivals_join_the_groups_the_review_chose
+ # The groups the walk ticked, joined alongside the name the review
+ # screen typed: the import's own group takes in everything, and the
+ # rest is a contact at a time.
+ def test_an_arrival_joins_the_groups_chosen_for_it
with_contacts({}) do |store|
school = store.create_group(name: "school")
- landed = Land.call(store, Vcf.cards(JANE), group: "import-20260921T031655Z", join: [school])
+ landed = Land.call(store, Vcf.cards(JANE), group: "import-20260921T031655Z", joins: [[school]])
id = landed.fetch(0).id
assert_equal [id], store.group(school).members
@@ -97,13 +97,30 @@ class ImportLandTest < Minitest::Test
end
end
- # Either, or neither: the chosen groups stand on their own when no
- # name is typed.
+ # A contact at a time means the cards do not all land alike: the
+ # choices are read by the card's own place in the file.
+ def test_each_arrival_joins_its_own_groups
+ with_contacts({}) do |store|
+ school = store.create_group(name: "school")
+ work = store.create_group(name: "work")
+
+ landed = Land.call(store, Vcf.cards(JANE + JANE + JANE), joins: [[school], [], [work, school]])
+ first, second, third = landed.map(&:id)
+
+ assert_equal [first, third].sort, store.group(school).members.sort
+ assert_equal [third], store.group(work).members
+ assert_equal [ProTacts::Store::EVERYONE],
+ store.all_groups.select { it.members.include?(second) }.map(&:name)
+ 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
with_contacts({}) do |store|
school = store.create_group(name: "school")
- landed = Land.call(store, Vcf.cards(JANE), join: [school])
+ landed = Land.call(store, Vcf.cards(JANE), joins: [[school]])
assert_equal landed.map(&:id), store.group(school).members
assert_equal %w[school], store.all_groups.map(&:name).reject { it == ProTacts::Store::EVERYONE }