import: edit a saved row's groups in the walk
The saved row dropped the groups dialog on the reasoning that membership was the contact page's to change, which made editing it cost the walk its place. The row now carries the same dialog, landed back on itself by `land`. 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 a370b41..8410f01 100644
--- a/lib/pro_tacts/admin/contact_card.rb
+++ b/lib/pro_tacts/admin/contact_card.rb
@@ -20,8 +20,8 @@ module ProTacts
# `groups` is the contact's membership (Store#groups_of), the row
# framing the values under it. `dialog` names whether that row
# carries its "edit groups" button, which needs GroupDialog on the
- # page: the contact's page has it, a walk's screen does not, and
- # membership there is the contact page's to change. `card` names
+ # page: the contact's page and a walk's saved row both render it,
+ # each telling the save where to land. `card` names
# whether this renders the card around its own body or the body
# alone for a card someone else supplies — the walk's read screen
# (ImportSaved), whose trunk RecordCard owns.
diff --git a/lib/pro_tacts/admin/group_dialog.rb b/lib/pro_tacts/admin/group_dialog.rb
index 94d802e..5ad40c4 100644
--- a/lib/pro_tacts/admin/group_dialog.rb
+++ b/lib/pro_tacts/admin/group_dialog.rb
@@ -24,6 +24,12 @@ module ProTacts
# reset a dialog reopens on whatever the last visit left in it — a
# filter typed ten minutes ago, the whole list still unrolled, or
# a name committed and then left there by a Cancel.
+ #
+ # `land` is where a save returns to, other than the contact's own
+ # page: the walk's saved row opens this dialog, and a look back at
+ # a row is not a navigation away from the walk — the row's own
+ # screen is where both a save and a refusal belong
+ # (Web#apply_groups).
class GroupDialog < Phlex::HTML
ID = "edit-groups" #: String
FORM = "edit-groups-form" #: String
@@ -32,6 +38,7 @@ module ProTacts
# @rbs @groups: Array[Store::GroupChoice]
# @rbs @joined: Array[String]
# @rbs @capped: Array[String]
+ # @rbs @land: String?
# Alphabetical rather than the id order a tag keeps: this is a list
# to find a name in, and nothing here stays put across a rename.
@@ -41,11 +48,12 @@ module ProTacts
# already in, which the show screen already holds (Store#groups_of)
# and so does not read a second time. `joined` is also what the
# cap spares: those are the boxes this is opened to untick.
- #: (contact: Contact, groups: Array[Store::GroupChoice], joined: Array[String]) -> void
- def initialize(contact:, groups:, joined:)
+ #: (contact: Contact, groups: Array[Store::GroupChoice], joined: Array[String], ?land: String?) -> void
+ def initialize(contact:, groups:, joined:, land: nil)
@contact = contact
@groups = groups.sort_by { it.label.downcase }
@joined = joined
+ @land = land
@capped = GroupFilter.capped(@groups.map(&:id), joined:)
end
@@ -57,6 +65,7 @@ module ProTacts
render GroupFilter.new(autofocus: true)
form(id: FORM, action: "/contacts/#{@contact.id}/groups", method: "post",
class: "field-stack", x_ref: "options") do
+ input(type: "hidden", name: "land", value: @land) if @land
@groups.each do |group|
joined = @joined.include?(group.id)
label(**GroupFilter.row(group.label, capped: @capped.include?(group.id))) do
diff --git a/lib/pro_tacts/admin/import_saved.rb b/lib/pro_tacts/admin/import_saved.rb
index f282e29..b62ad88 100644
--- a/lib/pro_tacts/admin/import_saved.rb
+++ b/lib/pro_tacts/admin/import_saved.rb
@@ -1,6 +1,7 @@
require "pro_tacts/admin/phlex"
require "pro_tacts/admin/contact_card"
+require "pro_tacts/admin/group_dialog"
require "pro_tacts/admin/layout"
require "pro_tacts/admin/record_card"
@@ -9,27 +10,31 @@ module ProTacts
# GET /import/:id/:n for a row the walk has already saved: the
# contact it became, read beside the card it arrived as
# (docs/plans/2026-09-21-import-a-vcf.md). A look back, not an
- # editor — the write happened at the row's own Save, and what it
- # wrote is changed where every other contact is, not re-decided
- # inside the walk. The details are ContactCard's, the one renderer
- # of a contact's record, without the groups dialog — membership is
- # the contact page's question (dialog: false, #groups_row's own
- # reason).
+ # editor — the write happened at the row's own Save, and later
+ # changes are the ones every other contact takes: the editor's
+ # amend through the row (#edit_card_screen), and membership
+ # through the contact page's own dialog, landed back on the row
+ # (`land`) rather than away from the walk. The details are
+ # ContactCard's, the one renderer of a contact's record.
class ImportSaved < Phlex::HTML
# @rbs @contact: Contact
# @rbs @groups: Array[Store::Group]
+ # @rbs @all_groups: Array[Store::GroupChoice]
# @rbs @aside: Phlex::HTML
# @rbs @sidebar: Phlex::HTML
# @rbs @edit: String
+ # @rbs @land: String
# @rbs @notice: String?
- #: (contact: Contact, groups: Array[Store::Group], aside: Phlex::HTML, sidebar: Phlex::HTML, edit: String, ?notice: String?) -> void
- def initialize(contact:, groups:, aside:, sidebar:, edit:, notice: nil)
+ #: (contact: Contact, groups: Array[Store::Group], all_groups: Array[Store::GroupChoice], aside: Phlex::HTML, sidebar: Phlex::HTML, edit: String, land: String, ?notice: String?) -> void
+ def initialize(contact:, groups:, all_groups:, aside:, sidebar:, edit:, land:, notice: nil)
@contact = contact
@groups = groups
+ @all_groups = all_groups
@aside = aside
@sidebar = sidebar
@edit = edit
+ @land = land
@notice = notice
end
@@ -41,8 +46,14 @@ module ProTacts
aside: @aside,
sidebar: @sidebar,
) do
- render ContactCard.new(contact: @contact, groups: @groups, card: false)
+ render ContactCard.new(contact: @contact, groups: @groups, dialog: true, card: false)
end
+ # The contact page's own dialog over the same card, told to
+ # land back on the row: the boxes to check are the groups this
+ # contact is in, read off the tags above rather than out of
+ # every group's members (ContactsShow's own reason).
+ render GroupDialog.new(contact: @contact, groups: @all_groups,
+ joined: @groups.map(&:id), land: @land)
end
end
end
diff --git a/lib/pro_tacts/web/contacts.rb b/lib/pro_tacts/web/contacts.rb
index 26fc7b0..efa9731 100644
--- a/lib/pro_tacts/web/contacts.rb
+++ b/lib/pro_tacts/web/contacts.rb
@@ -211,6 +211,11 @@ module ProTacts
# save (Admin::GroupFilter::Named) — a tick is what commits a
# name, so there is no single `new` riding the filter to read.
names = ids_in(r.params["named"]).map(&:strip).reject(&:empty?)
+ # The walk's row, when the dialog was opened from one
+ # (Admin::ImportSaved's `land`): where a save returns to and a
+ # refusal renders, so a look back is not a navigation away from
+ # the walk.
+ row = walk_row(r.params["land"].to_s)
begin
store.regroup(id, join: checked - was, leave: was - checked, create: names)
rescue Sequel::UniqueConstraintViolation
@@ -226,9 +231,24 @@ module ProTacts
else
"A group already holds one of those names; nothing was saved."
end
+ if row
+ upload, index = row
+ return card_screen(upload, index, notice:)
+ end
return contact_screen(contact, notice:)
end
- r.redirect "/contacts/#{id}", 303
+ r.redirect(row ? r.params["land"].to_s : "/contacts/#{id}", 303)
+ end
+
+ # A walk row's path (`/import/:upload/:index`), read back into its
+ # parts when the groups dialog's save came from one. Only the
+ # shape this server renders is honored: `land` is a form field,
+ # and the redirect is not its to choose — a path aimed elsewhere
+ # lands on the contact's own page as though none was sent.
+ #: (String land) -> [String, Integer]?
+ def walk_row(land)
+ row = land.match(%r{\A/import/([a-z]+)/(\d+)\z})
+ [row[1], Integer(row[2])] if row
end
# A form's list of ids, and none for a param absent or not a list.
diff --git a/lib/pro_tacts/web/import.rb b/lib/pro_tacts/web/import.rb
index ba42dec..36110ab 100644
--- a/lib/pro_tacts/web/import.rb
+++ b/lib/pro_tacts/web/import.rb
@@ -187,9 +187,11 @@ module ProTacts
return Admin::ImportSaved.call(
contact:,
groups: store.groups_of(contact.id),
+ all_groups: store.group_choices,
aside:,
sidebar:,
edit: "/import/#{upload}/#{index}/edit",
+ land: "/import/#{upload}/#{index}",
notice:,
) if contact
diff --git a/sig/pro_tacts/admin.rbs b/sig/pro_tacts/admin.rbs
index 2a1c793..3252393 100644
--- a/sig/pro_tacts/admin.rbs
+++ b/sig/pro_tacts/admin.rbs
@@ -60,7 +60,7 @@ module ProTacts
end
class ImportSaved
- def self.call: (contact: Contact, groups: Array[Store::Group], aside: untyped, sidebar: untyped, edit: String, ?notice: String?) -> String
+ def self.call: (contact: Contact, groups: Array[Store::Group], all_groups: Array[Store::GroupChoice], aside: untyped, sidebar: untyped, edit: String, land: String, ?notice: String?) -> String
end
# Rendered by Dashboard, not called from checked code; only
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index 4a3cb35..8343b27 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -653,6 +653,18 @@ class AdminContactsPagesTest < Minitest::Test
end
end
+ # `land` is a form field, and the redirect is not its to choose: a
+ # path not shaped like a walk row lands on the contact's own page
+ # as though none was sent (Web#walk_row).
+ def test_a_groups_save_ignores_a_land_that_is_not_a_walk_row
+ with_contacts({"ada" => ADA}) do
+ post "/contacts/ada/groups", "land" => "//evil.example"
+
+ assert_equal 303, last_response.status
+ assert_equal "/contacts/ada", last_response.headers["location"]
+ end
+ end
+
def test_show_labels_untyped_values_with_the_property_name
untyped = ADA.sub("TEL;TYPE=mobile:", "TEL:").sub("EMAIL;TYPE=home:", "EMAIL:")
diff --git a/test/pro_tacts/admin/test_import_pages.rb b/test/pro_tacts/admin/test_import_pages.rb
index d1bff35..790125c 100644
--- a/test/pro_tacts/admin/test_import_pages.rb
+++ b/test/pro_tacts/admin/test_import_pages.rb
@@ -308,11 +308,13 @@ class AdminImportPagesTest < Minitest::Test
assert_includes last_response.body, %(<dl class="detail-grid">)
assert_includes last_response.body, "+1 555 0100"
assert_includes last_response.body, %(data-dropped)
- # No editor, no dialog, no footer: nothing to submit, and
- # nothing to change here.
+ # No editor and no card form to submit; the one write the look
+ # back offers is the groups dialog, the contact page's own,
+ # landed back on the row.
refute_includes last_response.body, %(name="etag")
- refute_includes last_response.body, "edit groups"
- refute_includes last_response.body, %(<footer>)
+ assert_includes last_response.body, "edit groups"
+ assert_includes last_response.body,
+ %(<input type="hidden" name="land" value="/import/#{id}/0">)
# And the row's verdict is the check landed in its circle.
assert_includes last_response.body, %(<path d="m9 12 2 2 4-4">)
# The way into edit mode: the contact page's own edit link, in
@@ -340,7 +342,7 @@ class AdminImportPagesTest < Minitest::Test
assert_includes last_response.body, %(name="etag" value="")
assert_includes last_response.body, "Sam Booles"
# No group boxes: the amend is #apply_edit's form, and membership
- # is the contact page's question.
+ # is the dialog's question, on the row's read screen.
refute_includes last_response.body, %(name="groups[]")
post "/import/#{id}/0",
@@ -360,6 +362,51 @@ class AdminImportPagesTest < Minitest::Test
end
end
+ # Membership is asked on the look back too, with the contact page's
+ # own dialog: the save lands back on the row, and the row answers
+ # with the tags the write left — a look back not costing the walk
+ # its place.
+ def test_a_saved_rows_groups_are_edited_in_the_walk
+ with_contacts({}) do |store|
+ school = store.create_group(name: "school")
+ id = upload(JANE + PLAIN)
+ save(id, 0, first: "Jane", last: "Booles")
+ jane = store.contacts.find { it.name == "Jane Booles" }
+ get "/import/#{id}/0"
+ joined = ticked("groups[]")
+
+ post "/contacts/#{jane.id}/groups",
+ "groups" => [school, *joined], "was" => joined, "land" => "/import/#{id}/0"
+
+ assert_equal 303, last_response.status
+ assert_equal "/import/#{id}/0", last_response.headers["location"]
+ assert_equal [jane.id], store.group(school).members
+
+ get "/import/#{id}/0"
+
+ assert_includes last_response.body, %(href="/groups/#{school}" class="tag">)
+ end
+ end
+
+ # A refused save renders the screen the dialog was opened from, the
+ # notice landing on the row rather than navigating to the contact's
+ # own page.
+ def test_a_refused_groups_save_from_the_walk_lands_on_the_row
+ with_contacts({}) do |store|
+ store.create_group(name: "Clarks")
+ id = upload(JANE + PLAIN)
+ save(id, 0, first: "Jane", last: "Booles")
+ jane = store.contacts.fetch(0)
+ get "/import/#{id}/0"
+
+ post "/contacts/#{jane.id}/groups", "named" => ["Clarks"], "land" => "/import/#{id}/0"
+
+ assert_equal 200, last_response.status
+ assert_includes last_response.body, "Another group is already named Clarks; nothing was saved."
+ assert_includes last_response.body, %(<div class="walk">)
+ end
+ end
+
# A Save pressed twice is not a second contact: the first makes
# the contact, and the second is that row's editor's Save — the
# amend the contact's own page would write — met by the etag guard