admin: create a group from a contact's groups dialog
A filter naming no group can be ticked to create it on save;
left unticked, "boo" on the way to Booles is a group nobody made.
The groups row shows before any group, so the first one starts here too;
a taken sync: name refuses the save, and none of it goes through.
Assisted-by: Claude Opus 5 via Claude Code
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
diff --git a/lib/pro_tacts/admin/contacts_show.rb b/lib/pro_tacts/admin/contacts_show.rb
index 2167e6e..773ddb1 100644
--- a/lib/pro_tacts/admin/contacts_show.rb
+++ b/lib/pro_tacts/admin/contacts_show.rb
@@ -35,17 +35,18 @@ module ProTacts
# 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. 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, Array[Store::Group] groups, Array[Store::Group] all_groups, Array[Store::Change] changes, ?notice: String?) -> void
+ def initialize(contact:, groups:, all_groups:, changes:, notice: nil)
@contact = contact
@groups = groups
@all_groups = all_groups
@changes = changes
+ @notice = notice
@birthday = Format.birthday(contact)
end
def view_template
- render Layout.new(title: @contact.name || @contact.id) do
+ render Layout.new(title: @contact.name || @contact.id, notice: @notice) do
# The back-link line and the record card are one block, the
# line its caption row: spaced like the dashboard's
# section-head over its card (see .record in admin.css), not
@@ -75,11 +76,8 @@ module ProTacts
# the avatar is the row's visual anchor, not a caption.
render Avatar.new(contact: @contact, size: "xl") if @contact.photo
end
- # Only rendered when there's something to show: an empty
- # <dl> would still take up the gap card-body puts between
- # its children, leaving the header off-center in a card
- # with nothing else in it.
- dl(class: "detail-grid") { rows } if has_data?
+ # Never empty: the groups row is always there.
+ dl(class: "detail-grid") { rows }
end
end
end
@@ -115,18 +113,12 @@ module ProTacts
end
end
# No row, so outside the record's grid.
- render GroupDialog.new(contact: @contact, groups: @all_groups) if @all_groups.any?
+ render GroupDialog.new(contact: @contact, groups: @all_groups)
end
end
private
- #: () -> bool
- def has_data?
- @contact.phones.any? || @contact.emails.any? || @contact.addresses.any? ||
- !@birthday.nil? || @contact.notes.any? || @all_groups.any?
- end
-
# A missing TYPE parameter still gets a key: the fallback names
# the kind of value, so no row renders unlabeled in the grid.
# Every row that reads from a line hands it over, because which
@@ -135,7 +127,7 @@ module ProTacts
# and a group holds only addresses and notes anyway (see
# db/migrations/004_groups.rb).
def rows
- groups_row if @all_groups.any?
+ groups_row
@contact.phones.each do |phone|
row(Format.type_label(phone.types, "phone"), phone.value, phone.line)
end
@@ -156,10 +148,10 @@ 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). 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.
+ # card (Admin::GroupsShow). Rendered for a contact in no group too,
+ # even before any group exists, because the row holds the way to
+ # join or start 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" }
diff --git a/lib/pro_tacts/admin/group_dialog.rb b/lib/pro_tacts/admin/group_dialog.rb
index afbc746..97e8869 100644
--- a/lib/pro_tacts/admin/group_dialog.rb
+++ b/lib/pro_tacts/admin/group_dialog.rb
@@ -10,18 +10,26 @@ module ProTacts
# 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
+ # Alpine 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.
+ #
+ # A filter naming no group exactly also offers itself as a new
+ # group (`new`), created and joined by the save. Unchecked until
+ # ticked: a filter is usually half a name typed to find a group,
+ # and a save must not create "boo" on the way to Booles.
class GroupDialog < Phlex::HTML
ID = "edit-groups" #: String
FORM = "edit-groups-form" #: String
STATE = "{ filter: '', " \
+ "get labels() { return [...this.$refs.options.querySelectorAll('[data-label]')]" \
+ ".map(row => row.dataset.label) }, " \
"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
+ "get none() { return !this.labels.some(label => this.shows(label)) }, " \
+ "get creatable() { const name = this.filter.trim().toLowerCase(); " \
+ "return name !== '' && !this.labels.includes(name) } }" #: 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.
@@ -35,7 +43,7 @@ module ProTacts
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",
+ input(type: "search", placeholder: "Filter or add groups", aria_label: "Filter or add groups",
autofocus: true, x_model: "filter")
form(id: FORM, action: "/contacts/#{@contact.id}/groups", method: "post",
class: "field-stack", x_ref: "options") do
@@ -47,8 +55,16 @@ module ProTacts
end
input(type: "hidden", name: "was[]", value: group.id) if joined
end
+ template(x_if: "creatable") do
+ label do
+ input(type: "checkbox", name: "new", ":value": "filter.trim()")
+ span(class: "gl-muted", style: "font-style: italic;", x_text: "filter.trim()")
+ end
+ end
+ end
+ template(x_if: "none && !creatable") do
+ p(class: "gl-muted") { @groups.empty? ? "No groups yet." : "No groups match." }
end
- template(x_if: "none") { p(class: "gl-muted") { "No groups match." } }
end
footer do
button(type: "button", popovertarget: ID, popovertargetaction: "hide") { "Cancel" }
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 91df6eb..6591eee 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -671,10 +671,13 @@ module ProTacts
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:)
+ # joiners last for that method's reason. A name creates a group to
+ # join inside the same transaction, so a taken `sync:` name
+ # (#create_group) moves nothing.
+ #: (String card_id, join: Array[String], leave: Array[String], ?create: String?) -> void
+ def regroup(card_id, join:, leave:, create: nil)
@database.transaction do
+ join += [create_group(name: create)] if create
leave.each { remove_member(it, card_id) }
join.each { add_member(it, card_id) }
end
diff --git a/lib/pro_tacts/web/contacts.rb b/lib/pro_tacts/web/contacts.rb
index 918e8b4..f3913a6 100644
--- a/lib/pro_tacts/web/contacts.rb
+++ b/lib/pro_tacts/web/contacts.rb
@@ -179,15 +179,25 @@ module ProTacts
# 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.
+ # one would be a 500 rather than the bad input it is. A `new` name
+ # is a group the filter box offered to create, joined in the same
+ # save; a blank one is none.
#: (untyped r, String id) -> String?
def apply_groups(r, id)
- return if store.contact(id).nil?
+ contact = store.contact(id)
+ return if contact.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)
+ name = r.params["new"].to_s.strip
+ begin
+ store.regroup(id, join: checked - was, leave: was - checked, create: (name unless name.empty?))
+ rescue Sequel::UniqueConstraintViolation
+ # A taken `sync:` name (db/migrations/007_sync_names.rb), refused
+ # with the rest of the save in regroup's transaction.
+ return contact_screen(contact, notice: "Another group is already named #{name}; nothing was saved.")
+ end
r.redirect "/contacts/#{id}", 303
end
@@ -197,14 +207,15 @@ module ProTacts
param.is_a?(Array) ? param.map(&:to_s) : []
end
- #: (Contact contact) -> String
- def contact_screen(contact)
+ #: (Contact contact, ?notice: String) -> String
+ def contact_screen(contact, notice: nil)
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),
+ notice:,
)
end
diff --git a/sig/pro_tacts/admin.rbs b/sig/pro_tacts/admin.rbs
index 8c283f5..32a62b7 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], all_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], ?notice: String?) -> 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 c56eaa7..81d4f71 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -319,17 +319,6 @@ class AdminContactsPagesTest < Minitest::Test
end
end
- # 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"
-
- refute_includes last_response.body, "groups</dt>"
- end
- end
-
# A group with no name marks its rows with its id, which is what the
# mark is for: an empty tag would say a row came from somewhere and
# then not say where.
@@ -354,16 +343,24 @@ 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")
-
+ # The row holds the way to join a group or start one, so it renders
+ # before any group exists.
+ def test_show_offers_the_groups_dialog_before_any_group_exists
+ with_contacts({"ada" => ADA}) do
get "/contacts/ada"
assert_includes last_response.body, %(<dt class="type-label">groups</dt>)
assert_includes last_response.body, %(popovertarget="edit-groups")
+ assert_includes last_response.body, %(<dialog id="edit-groups")
+ end
+ end
+
+ def test_the_groups_dialog_offers_the_filter_as_an_unchecked_new_group
+ with_contacts({"ada" => ADA}) do
+ get "/contacts/ada"
+ dialog = last_response.body[%r{<dialog id="edit-groups".*?</dialog>}].to_s
+
+ assert_includes dialog, %(<input type="checkbox" name="new" :value="filter.trim()">)
end
end
@@ -428,6 +425,42 @@ class AdminContactsPagesTest < Minitest::Test
end
end
+ def test_a_new_group_is_created_and_joined_with_the_rest_of_the_save
+ with_contacts({"ada" => ADA}) do |store|
+ booles = FixtureData.seed_group(store, name: "Booles")
+
+ post "/contacts/ada/groups", groups: [booles], new: " Clarks "
+
+ assert_equal 303, last_response.status
+ assert_equal ["Booles", "Clarks"], store.groups_of("ada").map(&:label).sort
+ end
+ end
+
+ def test_a_blank_new_group_creates_nothing
+ with_contacts({"ada" => ADA}) do |store|
+ post "/contacts/ada/groups", new: " "
+
+ assert_equal 303, last_response.status
+ assert_empty store.all_groups
+ end
+ end
+
+ # A `sync:` name is the one kind that must be unique
+ # (db/migrations/007_sync_names.rb), and the refusal is the whole save.
+ def test_a_new_group_with_a_taken_sync_name_saves_nothing
+ with_contacts({"ada" => ADA}) do |store|
+ booles = FixtureData.seed_group(store, name: "Booles")
+ FixtureData.seed_group(store, name: "sync:alpha")
+
+ post "/contacts/ada/groups", groups: [booles], new: "sync:alpha"
+
+ assert_equal 200, last_response.status
+ assert_includes last_response.body, "Another group is already named sync:alpha; nothing was saved."
+ assert_empty store.groups_of("ada")
+ assert_equal 2, store.all_groups.length
+ 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")
@@ -635,16 +668,17 @@ class AdminContactsPagesTest < Minitest::Test
# A card that will not parse is served from its bytes regardless —
# the raw section is the one thing such a card has to show, so it
- # renders with no grid under the header, not instead of the page.
- # The record card, specifically: the change log rides the same grid
- # in a card of its own, and every card has a history.
- def test_show_of_a_bare_contact_renders_no_grid
+ # renders with only the groups row under the header, not instead of
+ # the page. The record card, specifically: the change log rides the
+ # same grid in a card of its own, and every card has a history.
+ def test_show_of_a_bare_contact_renders_only_the_groups_row
bare = "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Bare Contact\r\nUID:bare\r\nEND:VCARD\r\n"
with_contacts({"bare" => bare}) do
get "/contacts/bare"
+ grid = last_response.body[%r{<dl class="detail-grid">.*?</dl>}].to_s
- refute_includes last_response.body, '<dl class="detail-grid">'
+ assert_equal ["groups"], grid.scan(%r{<dt class="type-label">([^<]*)</dt>}).flatten
assert_includes last_response.body, "<details"
end
end
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index c58e9a7..ea4f719 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -1325,6 +1325,14 @@ class StoreTest < Minitest::Test
end
end
+ def test_a_regroup_creates_the_group_it_names_and_joins_it
+ with_store({"aiden" => AIDEN}) do |store|
+ store.regroup("aiden", join: [], leave: [], create: "Clarks")
+
+ assert_equal ["Clarks"], store.groups_of("aiden").map(&:label)
+ 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.