Keep a book's group from being renamed
Deleting sync:* was refused, but renaming it was not: clearing its
name in the group editor moved every card out of every book and told
every client to drop them. A group whose name is a sync: name now keeps
it. Store#rename_group and #edit_group refuse the rename before their
transactions, the editor renders the field read-only, and #name_book
renames a login's group through the unguarded #rename, the one way a
book's group changes name. A rename into a sync: name still makes a
book.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013VNdx5qiRSWADBoVfDRcrd
diff --git a/lib/pro_tacts/admin/groups_edit.rb b/lib/pro_tacts/admin/groups_edit.rb
index 5261463..50e1342 100644
--- a/lib/pro_tacts/admin/groups_edit.rb
+++ b/lib/pro_tacts/admin/groups_edit.rb
@@ -57,11 +57,14 @@ module ProTacts
form(action: "/groups/#{@group.id}", method: "post", class: "field-stack", id: FORM) do
input(type: "hidden", name: "version", value: @group.version)
# The id as the blank's placeholder, because a group
- # with no name is shown as its id everywhere else.
+ # with no name is shown as its id everywhere else. A
+ # book's group keeps its name (Store#rename_group), so
+ # its field is read-only — still submitted, so the
+ # save carries the name it found.
label(class: "field") do
span { "Name" }
input(type: "text", name: "name", value: @group.name,
- placeholder: @group.id, autofocus: true)
+ placeholder: @group.id, autofocus: !@group.sync?, readonly: @group.sync?)
end
@reading.addresses.each { |address| address_row(address) }
added_address_row
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index e6047cb..09cbcc1 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -157,6 +157,9 @@ module ProTacts
# A book named `*`, which would be everyone's (#name_book).
class EveryonesBookName < ArgumentError; end
+ # A rename of a group whose name is a book's (#rename_group).
+ class SyncGroupRename < ArgumentError; end
+
# Sequel's migrations, run on open. They ship with the code rather
# than with a deployment, so the path is relative to this file.
# `__dir__` is nil only for code with no file behind it, which a
@@ -377,7 +380,7 @@ module ProTacts
# Sets the name a login's book goes by, or with nil or a blank goes
# back to the login. The login's `sync:` group, if it has one, is
# renamed in the same transaction, so the book keeps its cards and
- # #rename_group logs every member for the clients syncing it. A name
+ # #rename logs every member for the clients syncing it. A name
# another login's book goes by raises on the unique index, and `*`
# is refused before the transaction, #put's reason for building a
# Contact outside one.
@@ -391,7 +394,7 @@ module ProTacts
group = groups.where(name: own_sync_name(login)).select_map(:id).first
books.where(login:).delete
books.insert(login:, name:) unless name.nil?
- rename_group(group.to_s, name: own_sync_name(login)) unless group.nil?
+ rename(group.to_s, own_sync_name(login)) unless group.nil?
end
end
@@ -664,27 +667,17 @@ module ProTacts
# A group's name, or none — NULL being the one spelling of
# nameless (db/migrations/005_group_identity.rb), so a blank is
- # stored as that rather than refused by the schema. A name is on no
- # card, so no member's bytes move, but a `sync:` name is what puts a
- # card in a book: a rename into, out of, or between them moves every
- # member between books and logs each one (#fan_out's `moved`).
- # Logged in the group's own log only where the name moved, the log
- # recording what happened rather than that a save was asked
- # (docs/plans/2026-09-23-group-change-log.md, "One entry per
- # primitive").
+ # stored as that rather than refused by the schema. A group whose
+ # name is a book's keeps it: renaming `sync:*` would tell every
+ # client to drop every card, and a login's `sync:` group is named by
+ # #name_book, which a rename here would leave naming a group that is
+ # not there (docs/plans/2026-09-16-book-names.md). Refused before
+ # the transaction, #name_book's reason. A rename into a `sync:` name
+ # is how a book is made, and stays.
#: (String id, name: String?) -> void
def rename_group(id, name:)
- name = nil if name.to_s.strip.empty?
- @database.transaction do
- was = groups.where(id:).sole.fetch(:name)&.to_s
- if was != name && (Group.sync_name?(was) || Group.sync_name?(name))
- members = member_ids([id])
- fan_out(members, moved: members) { groups.where(id:).update(name:) }
- else
- groups.where(id:).update(name:)
- end
- record_group_change(id, action: GroupAction::RENAME, detail: {"was" => was, "name" => name}) if was != name
- end
+ refuse_sync_rename(id, name)
+ rename(id, name)
end
# Replaces what a group lends, wholesale, at positions from zero.
@@ -726,10 +719,11 @@ module ProTacts
# about to stop or had yet to start serving.
#: (String id, name: String?, lines: Array[String], members: Array[String]) -> void
def edit_group(id, name:, lines:, members:)
+ refuse_sync_rename(id, name)
@database.transaction do
current = member_ids([id])
(current - members).each { remove_member(id, it) }
- rename_group(id, name:)
+ rename(id, name)
set_group_lines(id, lines)
(members - current).each { add_member(id, it) }
end
@@ -964,6 +958,48 @@ module ProTacts
group_changes.insert(group_id:, action:, detail: detail.to_json)
end
+ # #rename_group's refusal, for it and for #edit_group, which renames
+ # inside a transaction of its own and so has to ask before opening
+ # it.
+ #: (String id, String? name) -> void
+ def refuse_sync_rename(id, name)
+ was = groups.where(id:).sole.fetch(:name)&.to_s
+ return if was == group_name(name) || !Group.sync_name?(was)
+
+ raise SyncGroupRename, "#{was} is a book's group, named by the book"
+ end
+
+ # The rename itself, with no refusal: #name_book's way of renaming
+ # the one group #rename_group will not. A name is on no card, so no
+ # member's bytes move, but a `sync:` name is what puts a card in a
+ # book: a rename into, out of, or between them moves every member
+ # between books and logs each one (#fan_out's `moved`). Logged in
+ # the group's own log only where the name moved, the log recording
+ # what happened rather than that a save was asked
+ # (docs/plans/2026-09-23-group-change-log.md, "One entry per
+ # primitive").
+ #: (String id, String? name) -> void
+ def rename(id, name)
+ name = group_name(name)
+ @database.transaction do
+ was = groups.where(id:).sole.fetch(:name)&.to_s
+ if was != name && (Group.sync_name?(was) || Group.sync_name?(name))
+ members = member_ids([id])
+ fan_out(members, moved: members) { groups.where(id:).update(name:) }
+ else
+ groups.where(id:).update(name:)
+ end
+ record_group_change(id, action: GroupAction::RENAME, detail: {"was" => was, "name" => name}) if was != name
+ end
+ end
+
+ # A name as a group stores it: a blank is NULL, nameless's one
+ # spelling (db/migrations/005_group_identity.rb).
+ #: (String? name) -> String?
+ def group_name(name)
+ name unless name.to_s.strip.empty?
+ end
+
# The block's group writes, bracketed by a read of every member
# they could move: what each served before and what each serves
# after, and a `group` entry wherever the two differ. Why an entry
diff --git a/lib/pro_tacts/web/groups.rb b/lib/pro_tacts/web/groups.rb
index 625ff5d..8fd9249 100644
--- a/lib/pro_tacts/web/groups.rb
+++ b/lib/pro_tacts/web/groups.rb
@@ -104,6 +104,10 @@ module ProTacts
# A taken name (db/migrations/008_group_names.rb). The save is
# one transaction, so nothing of it landed.
return group_edit_screen(group, notice: "Another group is already named #{r.params['name']}; nothing was saved.")
+ rescue Store::SyncGroupRename
+ # A book's group keeps its name (Store#rename_group); the
+ # editor renders it read-only, so this is a doctored save.
+ return group_edit_screen(group, notice: "The #{group.name} group names a book and keeps its name; nothing was saved.")
end
r.redirect "/groups/#{id}", 303
end
diff --git a/test/pro_tacts/admin/test_groups_pages.rb b/test/pro_tacts/admin/test_groups_pages.rb
index 4b10f9e..a508a7d 100644
--- a/test/pro_tacts/admin/test_groups_pages.rb
+++ b/test/pro_tacts/admin/test_groups_pages.rb
@@ -287,6 +287,31 @@ class AdminGroupsPagesTest < Minitest::Test
end
end
+ # Everyone's book keeps its name: renaming it would empty every
+ # device's book (Store#rename_group).
+ def test_a_rename_of_a_books_group_is_refused_and_changes_nothing
+ with_contacts(BOOLES) do |store|
+ id = FixtureData.seed_group(store, name: ProTacts::Group::EVERYONE, members: %w[george])
+
+ post "/groups/#{id}", version: store.group(id).version, name: "Everyone", members: ["", "george", "mary"]
+
+ assert_equal 200, last_response.status
+ assert_includes last_response.body, "The sync:* group names a book and keeps its name; nothing was saved."
+ assert_equal ProTacts::Group::EVERYONE, store.group(id).name
+ assert_equal %w[george], store.group(id).members
+ end
+ end
+
+ def test_a_books_group_renders_its_name_read_only
+ with_contacts(BOOLES) do |store|
+ id = FixtureData.seed_group(store, name: ProTacts::Group::EVERYONE, members: %w[george])
+
+ get "/groups/#{id}/edit"
+
+ assert_includes last_response.body, "readonly"
+ end
+ end
+
# No list at all is a POST that says nothing about membership; a
# doctored id names no card and is dropped rather than a 500.
def test_membership_moves_only_for_a_list_of_real_cards
diff --git a/test/pro_tacts/test_store.rb b/test/pro_tacts/test_store.rb
index ee8cc50..fab9270 100644
--- a/test/pro_tacts/test_store.rb
+++ b/test/pro_tacts/test_store.rb
@@ -1761,18 +1761,51 @@ class StoreTest < Minitest::Test
end
end
- def test_a_rename_into_between_or_out_of_sync_logs_every_member
+ # Into a book by a rename, and between books by #name_book, the one
+ # way a book's group is renamed.
+ def test_a_rename_into_or_between_books_logs_every_member
with_store({"aiden" => AIDEN, "znorth" => ZED}) do |store|
id = FixtureData.seed_group(store, name: "Household", members: %w[aiden znorth])
- store.rename_group(id, name: "sync:Alpha Chen")
- store.rename_group(id, name: "sync:Alpha Chen")
- store.rename_group(id, name: "sync:Zoë Chen")
- store.rename_group(id, name: "Household")
+ store.rename_group(id, name: "sync:alpha@example.com")
+ store.rename_group(id, name: "sync:alpha@example.com")
+ store.name_book("alpha@example.com", "Alpha Chen")
%w[aiden znorth].each do
- assert_equal %w[group group group put], store.changes_of(it).map(&:action)
+ assert_equal %w[group group put], store.changes_of(it).map(&:action)
+ end
+ end
+ end
+
+ # A book's group keeps its name: out of `sync:` or between two such
+ # names, and everyone's book above all, which would tell every
+ # client to drop every card.
+ def test_a_books_group_refuses_a_rename
+ with_store({"aiden" => AIDEN}) do |store|
+ everyone = FixtureData.seed_group(store, name: ProTacts::Group::EVERYONE, members: %w[aiden])
+ alpha = store.create_group(name: "sync:Alpha Chen")
+
+ assert_raises(ProTacts::Store::SyncGroupRename) { store.rename_group(everyone, name: "Everyone") }
+ assert_raises(ProTacts::Store::SyncGroupRename) { store.rename_group(everyone, name: " ") }
+ assert_raises(ProTacts::Store::SyncGroupRename) { store.rename_group(alpha, name: "sync:Zoë Chen") }
+ assert_raises(ProTacts::Store::SyncGroupRename) do
+ store.edit_group(everyone, name: "Everyone", lines: [], members: %w[aiden])
end
+
+ assert_equal ProTacts::Group::EVERYONE, store.group(everyone).name
+ assert_equal "sync:Alpha Chen", store.group(alpha).name
+ assert_equal %w[put], store.changes_of("aiden").map(&:action)
+ end
+ end
+
+ # Keeping the name it has is no rename, which every group edit does.
+ def test_a_books_group_keeps_its_name_through_an_edit
+ with_store({"aiden" => AIDEN}) do |store|
+ everyone = FixtureData.seed_group(store, name: ProTacts::Group::EVERYONE, members: %w[aiden])
+
+ store.edit_group(everyone, name: ProTacts::Group::EVERYONE, lines: [], members: [])
+
+ assert_empty store.group(everyone).members
end
end