admin: edit a contact's groups from its card
Edit groups opens a checklist with the contact's groups ticked; the save moves only what you toggled, so no other tab's change gets nicked. Assisted-by: Claude Opus 5 via Claude Code
diff --git a/Steepfile b/Steepfile
index f062c0e..d1652cd 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 device_setup format
- group_label groups_edit groups_index groups_show icons layout phlex upcoming_birthdays
+ group_dialog group_label groups_edit groups_index groups_show icons layout phlex upcoming_birthdays
].map { "lib/pro_tacts/admin/#{it}.rb" }, inline: true)
signature "sig"
diff --git a/lib/pro_tacts/admin/contacts_show.rb b/lib/pro_tacts/admin/contacts_show.rb
index 8ac09c4..2167e6e 100644
--- a/lib/pro_tacts/admin/contacts_show.rb
+++ b/lib/pro_tacts/admin/contacts_show.rb
@@ -2,6 +2,7 @@ require "pro_tacts/admin/phlex"
require "pro_tacts/admin/avatar"
require "pro_tacts/admin/format"
+require "pro_tacts/admin/group_dialog"
require "pro_tacts/admin/group_label"
require "pro_tacts/admin/layout"
@@ -33,11 +34,12 @@ module ProTacts
# beside it for the same reason and required for the stronger
# one: a card with no entries is a card whose history was lost,
# and an empty default would render that as an ordinary quiet
- # record.
- #: (Contact contact, Array[Store::Group] groups, Array[Store::Change] changes) -> void
- def initialize(contact:, groups:, changes:)
+ # record. Every group is the groups dialog's (GroupDialog).
+ #: (Contact contact, Array[Store::Group] groups, Array[Store::Group] all_groups, Array[Store::Change] changes) -> void
+ def initialize(contact:, groups:, all_groups:, changes:)
@contact = contact
@groups = groups
+ @all_groups = all_groups
@changes = changes
@birthday = Format.birthday(contact)
end
@@ -112,6 +114,8 @@ module ProTacts
end
end
end
+ # No row, so outside the record's grid.
+ render GroupDialog.new(contact: @contact, groups: @all_groups) if @all_groups.any?
end
end
@@ -120,7 +124,7 @@ module ProTacts
#: () -> bool
def has_data?
@contact.phones.any? || @contact.emails.any? || @contact.addresses.any? ||
- !@birthday.nil? || @contact.notes.any? || @groups.any?
+ !@birthday.nil? || @contact.notes.any? || @all_groups.any?
end
# A missing TYPE parameter still gets a key: the fallback names
@@ -131,7 +135,7 @@ module ProTacts
# and a group holds only addresses and notes anyway (see
# db/migrations/004_groups.rb).
def rows
- groups_row if @groups.any?
+ groups_row if @all_groups.any?
@contact.phones.each do |phone|
row(Format.type_label(phone.types, "phone"), phone.value, phone.line)
end
@@ -152,14 +156,20 @@ module ProTacts
# 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).
+ # card (Admin::GroupsShow). Rendered for a contact in no group too
+ # while any group exists, because the row holds the way to join
+ # one — the group card's members row, for the same reason, with
+ # its "edit members" in the same place.
#: () -> void
def groups_row
dt(class: "type-label") { "groups" }
dd(class: "type-body-sm") do
- div(class: "tag-set") do
- @groups.each { group_tag(it) }
+ if @groups.any?
+ div(class: "tag-set") do
+ @groups.each { group_tag(it) }
+ end
end
+ button(type: "button", data_size: "sm", popovertarget: GroupDialog::ID) { "edit groups" }
end
end
diff --git a/lib/pro_tacts/admin/group_dialog.rb b/lib/pro_tacts/admin/group_dialog.rb
new file mode 100644
index 0000000..afbc746
--- /dev/null
+++ b/lib/pro_tacts/admin/group_dialog.rb
@@ -0,0 +1,61 @@
+require "pro_tacts/admin/phlex"
+
+require "pro_tacts/admin/group_label"
+
+module ProTacts
+ module Admin
+ # The groups dialog on a contact's card, opened by the groups row's
+ # "edit groups": ContactDialog's popover, over every group as a
+ # checkbox, the contact's own checked. The save sends what the page
+ # loaded with beside what it sends back (`was[]`), so the route can
+ # move only what was toggled (Web#apply_groups).
+ #
+ # Alpine only filters, matching each row's lowercased label in
+ # data-label so no group's name is spliced into script; a row it
+ # hides still submits its box. The filter sits outside the form,
+ # because Enter in a form's text box would save mid-filter.
+ class GroupDialog < Phlex::HTML
+ ID = "edit-groups" #: String
+ FORM = "edit-groups-form" #: String
+
+ STATE = "{ filter: '', " \
+ "shows(label) { return label.includes(this.filter.trim().toLowerCase()) }, " \
+ "get none() { return ![...this.$refs.options.querySelectorAll('[data-label]')]" \
+ ".some(row => this.shows(row.dataset.label)) } }" #: 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.
+ #: (contact: Contact, groups: Array[Store::Group]) -> void
+ def initialize(contact:, groups:)
+ @contact = contact
+ @groups = groups.sort_by { it.label.downcase }
+ end
+
+ def view_template
+ dialog(id: ID, popover: "auto", x_data: STATE) do
+ header { "Groups" }
+ div(class: "field-stack") do
+ input(type: "search", placeholder: "Filter groups", aria_label: "Filter groups",
+ autofocus: true, x_model: "filter")
+ form(id: FORM, action: "/contacts/#{@contact.id}/groups", method: "post",
+ class: "field-stack", x_ref: "options") do
+ @groups.each do |group|
+ joined = group.members.include?(@contact.id)
+ label(data: {label: group.label.downcase}, ":hidden": "!shows($el.dataset.label)") do
+ input(type: "checkbox", name: "groups[]", value: group.id, checked: joined)
+ render GroupLabel.new(group:)
+ end
+ input(type: "hidden", name: "was[]", value: group.id) if joined
+ end
+ end
+ template(x_if: "none") { p(class: "gl-muted") { "No groups match." } }
+ end
+ footer do
+ button(type: "button", popovertarget: ID, popovertargetaction: "hide") { "Cancel" }
+ button(type: "submit", form: FORM, data: {variant: "primary"}) { "Save" }
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 46f5743..a4ecad4 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -670,6 +670,16 @@ module ProTacts
end
end
+ # A card's side of #edit_group: one transaction, leavers first and
+ # joiners last for that method's reason.
+ #: (String card_id, join: Array[String], leave: Array[String]) -> void
+ def regroup(card_id, join:, leave:)
+ @database.transaction do
+ leave.each { remove_member(it, card_id) }
+ join.each { add_member(it, card_id) }
+ end
+ end
+
# The change log from a sequence number on, oldest first: the window
# a sync-collection report answers from (RFC 6578 section 3.2) — the
# entries after the client's token are the changes it has not seen.
diff --git a/lib/pro_tacts/web/contacts.rb b/lib/pro_tacts/web/contacts.rb
index 06e1d42..918e8b4 100644
--- a/lib/pro_tacts/web/contacts.rb
+++ b/lib/pro_tacts/web/contacts.rb
@@ -63,6 +63,13 @@ module ProTacts
end
end
+ # Membership from the contact's side, the groups dialog's save
+ # (Admin::GroupDialog). Above the edit's POST, whose bare verb
+ # block would match this path too.
+ r.post "groups" do
+ apply_groups(r, id)
+ end
+
r.post do
apply_edit(r, id)
end
@@ -88,11 +95,7 @@ module ProTacts
r.get do
contact = store.contact(id)
- if contact
- response["Content-Type"] = "text/html; charset=utf-8"
- Admin::ContactsShow.call(contact:, groups: store.groups_of(id),
- changes: store.changes_of(id))
- end
+ contact_screen(contact) if contact
end
end
end
@@ -171,6 +174,40 @@ module ProTacts
r.redirect "/contacts/#{id}", 303
end
+ # The dialog's save as what it toggled rather than the set it shows:
+ # `was` is the boxes checked when the page loaded, so a group joined
+ # or left elsewhere since stays as it is, and a stale page has
+ # nothing to revert and no snapshot to refuse. Only ids naming a
+ # group: Store#add_member reads the group with `sole`, so a doctored
+ # one would be a 500 rather than the bad input it is.
+ #: (untyped r, String id) -> String?
+ def apply_groups(r, id)
+ return if store.contact(id).nil?
+
+ known = store.all_groups.map(&:id)
+ checked = ids_in(r.params["groups"]) & known
+ was = ids_in(r.params["was"]) & known
+ store.regroup(id, join: checked - was, leave: was - checked)
+ r.redirect "/contacts/#{id}", 303
+ end
+
+ # A form's list of ids, and none for a param absent or not a list.
+ #: (untyped param) -> Array[String]
+ def ids_in(param)
+ param.is_a?(Array) ? param.map(&:to_s) : []
+ end
+
+ #: (Contact contact) -> String
+ def contact_screen(contact)
+ response["Content-Type"] = "text/html; charset=utf-8"
+ Admin::ContactsShow.call(
+ contact:,
+ groups: store.groups_of(contact.id),
+ all_groups: store.all_groups,
+ changes: store.changes_of(contact.id),
+ )
+ end
+
#: (Contact contact, ?notice: String) -> String
def edit_screen(contact, notice: nil)
response["Content-Type"] = "text/html; charset=utf-8"
diff --git a/public/admin.css b/public/admin.css
index 251f92b..4e453a5 100644
--- a/public/admin.css
+++ b/public/admin.css
@@ -582,21 +582,25 @@ img.avatar {
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. */
+ baseline of its own; a contact card's "edit groups" button takes
+ it too, a flex container like the tag. */
.detail-grid dd:has(> .tag),
-.detail-grid dd:has(> a.type-label) {
+.detail-grid dd:has(> a.type-label),
+.detail-grid dd:has(> button) {
display: grid;
grid-template-columns: minmax(0, 1fr) auto;
align-items: baseline;
column-gap: var(--gl-space-s);
}
.detail-grid dd > .tag,
-.detail-grid dd > a.type-label {
+.detail-grid dd > a.type-label,
+.detail-grid dd > button {
grid-column: 2;
grid-row: 1;
}
.detail-grid dd:has(> .tag) > div,
-.detail-grid dd:has(> a.type-label) > div {
+.detail-grid dd:has(> a.type-label) > div,
+.detail-grid dd:has(> button) > div {
grid-column: 1;
}
/* A nameless group shown by its id (Admin::GroupLabel): set in mono,
@@ -624,6 +628,21 @@ dialog > footer {
padding-block: var(--gl-space-2xs);
}
+/* The groups dialog (Admin::GroupDialog) is a list of one-line rows,
+ not the settings form Gloss spaces its stacks for, so it closes up
+ the way the record's form does (.card-body form.field-stack): the
+ rows to the scale's smallest step, the filter to the list at the
+ record's row gap, and the filter box shedding its block padding. */
+dialog .field-stack {
+ gap: var(--gl-space-2xs);
+}
+dialog form.field-stack {
+ gap: var(--gl-space-3xs);
+}
+dialog > .field-stack > input[type="search"] {
+ padding-block: var(--gl-space-3xs);
+}
+
/* 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/admin.rbs b/sig/pro_tacts/admin.rbs
index b3c6836..8c283f5 100644
--- a/sig/pro_tacts/admin.rbs
+++ b/sig/pro_tacts/admin.rbs
@@ -7,7 +7,7 @@
module ProTacts
module Admin
class ContactsShow
- def self.call: (contact: Contact, groups: Array[Store::Group], changes: Array[Store::Change]) -> String
+ def self.call: (contact: Contact, groups: Array[Store::Group], all_groups: Array[Store::Group], changes: Array[Store::Change]) -> String
end
class ContactsEdit
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index 3e8bfe3..233acae 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -319,9 +319,10 @@ class AdminContactsPagesTest < Minitest::Test
end
end
- # No membership, no row: an empty tag set would say a contact
- # belongs to nothing in the space where belonging is shown.
- def test_show_omits_the_groups_row_for_a_contact_in_none
+ # No membership and nothing to join, no row: an empty tag set would
+ # say a contact belongs to nothing in the space where belonging is
+ # shown.
+ def test_show_omits_the_groups_row_with_no_group_to_show_or_join
with_contacts({"ada" => ADA}) do
get "/contacts/ada"
@@ -353,6 +354,90 @@ class AdminContactsPagesTest < Minitest::Test
end
end
+ # The row holds the way to join a group, so a contact in none still
+ # gets it while any group exists.
+ def test_show_offers_a_contact_in_no_group_its_groups_to_edit
+ with_contacts({"ada" => ADA}) do |store|
+ FixtureData.seed_group(store, name: "Booles")
+
+ get "/contacts/ada"
+
+ assert_includes last_response.body, %(<dt class="type-label">groups</dt>)
+ assert_includes last_response.body, %(popovertarget="edit-groups")
+ end
+ end
+
+ def test_the_groups_dialog_checks_the_groups_a_contact_is_in
+ with_contacts({"ada" => ADA}) do |store|
+ joined = FixtureData.seed_group(store, name: "Booles", members: ["ada"])
+ other = FixtureData.seed_group(store, name: "Babbages")
+
+ get "/contacts/ada"
+ dialog = last_response.body[%r{<dialog id="edit-groups".*?</dialog>}].to_s
+
+ assert_includes dialog, %(name="groups[]" value="#{joined}" checked>)
+ assert_includes dialog, %(name="groups[]" value="#{other}">)
+ end
+ end
+
+ # Joining is the membership and what comes of it: the card serves
+ # what the group lends, and the change log tells every client so.
+ def test_checking_a_group_adds_the_contact_to_it
+ with_contacts({"ada" => ADA}) do |store|
+ id = FixtureData.seed_group(store, name: "Booles", lines: HOUSEHOLD)
+
+ post "/contacts/ada/groups", groups: [id]
+
+ assert_equal 303, last_response.status
+ assert_equal "/contacts/ada", last_response["Location"]
+ assert_equal ["ada"], store.group(id).members
+ assert_includes store.contact("ada").vcard.to_s, "7 Calculus Close"
+ assert_equal "group", store.changes_of("ada").first.action
+ end
+ end
+
+ # Leaving twice is leaving once: the second finds nothing to remove
+ # and lands on the same page.
+ def test_unchecking_a_group_removes_the_contact_from_it
+ with_contacts({"ada" => ADA}) do |store|
+ id = FixtureData.seed_group(store, name: "Booles", members: ["ada"])
+
+ 2.times do
+ post "/contacts/ada/groups", was: [id]
+
+ assert_equal 303, last_response.status
+ assert_equal "/contacts/ada", last_response["Location"]
+ end
+ assert_empty store.group(id).members
+ end
+ end
+
+ # A save moves only the boxes its page toggled: a group joined in
+ # another tab since stays joined, and an id naming no group is
+ # dropped rather than reaching a foreign key.
+ def test_a_save_moves_only_the_groups_it_toggled
+ with_contacts({"ada" => ADA}) do |store|
+ booles = FixtureData.seed_group(store, name: "Booles", members: ["ada"])
+ babbages = FixtureData.seed_group(store, name: "Babbages")
+ elsewhere = FixtureData.seed_group(store, name: "Clarks")
+ store.add_member(elsewhere, "ada")
+
+ post "/contacts/ada/groups", groups: [babbages, "zzzz"], was: [booles]
+
+ assert_equal [babbages, elsewhere].sort, store.groups_of("ada").map(&:id).sort
+ end
+ end
+
+ def test_a_save_for_an_unknown_contact_is_a_404
+ with_contacts({"ada" => ADA}) do |store|
+ id = FixtureData.seed_group(store, name: "Booles")
+
+ post "/contacts/nobody/groups", groups: [id]
+
+ assert_equal 404, last_response.status
+ end
+ end
+
# A value with no TYPE parameter still gets a key — the fallback
# names the kind of value, so every row in the grid is labeled.
def test_show_labels_untyped_values_with_the_property_name
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index 042ce58..21027ad 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -1313,6 +1313,18 @@ class StoreTest < Minitest::Test
end
end
+ def test_a_regroup_leaves_and_joins
+ with_store({"aiden" => AIDEN}) do |store|
+ left = FixtureData.seed_group(store, members: ["aiden"], lines: [HOUSEHOLD_ADDRESS])
+ joined = FixtureData.seed_group(store, members: [])
+
+ store.regroup("aiden", join: [joined], leave: [left])
+
+ assert_equal [joined], store.groups_of("aiden").map(&:id)
+ assert_equal AIDEN, store.contact("aiden").vcard.to_s
+ end
+ end
+
# Nothing a client downloads moved, so nothing is logged: a group
# that lends nothing composes nothing into a new member, and a name
# is on no card.