new group: its own Create route, and the members screen in one branch
Read without its comments, POST /groups chose where to land by
whether a members param was present, which a Create with every box
unticked never sends: it made an empty nameless group and landed on
its editor. The Create now posts back to /contacts/:id/groups/new,
the GET's own path as the editors' POSTs are, and refuses an empty
pick. POST /groups is main's again.

GroupsMembers held the record twice (@from and @group) and asked
which it was in three places; one case in initialize now sets what
the view reads, and was[] renders from @was, what the page loaded
with, instead of a member-and-group test.

Store#create_group's comment named a Web method that no longer
exists.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186Ru2zvdWEwdwGhQ5V9z5X
change
commit f978f41e35ca6a4ffede4c29bdb335cafb0f51f8
author Claude <noreply@anthropic.com>
date
parent 671290e7
diff --git a/lib/pro_tacts/admin/groups_members.rb b/lib/pro_tacts/admin/groups_members.rb
index 4922410..987ac45 100644
--- a/lib/pro_tacts/admin/groups_members.rb
+++ b/lib/pro_tacts/admin/groups_members.rb
@@ -31,54 +31,67 @@ module ProTacts
     #
     # The same screen picks a group's first members before the group
     # exists: a contact card's "new" (Admin::ContactCard) opens it
-    # with that contact ticked, and its Create is the create (POST
-    # /groups), so a group someone backs out of is never made. With no
-    # group there is nothing toggled against, so no `was[]`, and the
-    # way back is the card it was opened from.
+    # with that contact ticked, and its Create is the create
+    # (Web#apply_new_group), so a group someone backs out of is never
+    # made.
     class GroupsMembers < Phlex::HTML
-      # @rbs @from: Group | Contact
-      # @rbs @group: Group?
       # @rbs @login: String
+      # @rbs @notice: String?
       # @rbs @contacts: Array[ListedContact]
-      # @rbs @members: Array[String]
+      # @rbs @title: String
+      # @rbs @action: String
+      # @rbs @back: [String, (String | Phlex::HTML)]
+      # @rbs @submit: String
+      # @rbs @ticked: Array[String]
+      # @rbs @was: Array[String]
       # @rbs @capped: Array[String]
 
       # The form's id, for the footer's Save outside it.
       FORM = "members-form" #: String
       private_constant :FORM
 
-      # `from` is the record the screen was opened from, and the one
-      # thing that tells its two cases apart, so no call can name both
-      # or neither: a group's page edits that group's members, and a
-      # contact's card starts a group with that contact in it.
-      #: (from: Group | Contact, contacts: Array[ListedContact], login: String) -> void
-      def initialize(from:, contacts:, login:)
-        @from = from
-        @group = from if from.is_a?(Group)
+      # `from` is the record the screen was opened from: a group's
+      # page edits that group's members, and a contact's card starts a
+      # group with that contact in it. `was` is what the page loaded
+      # with, which a group not yet made has none of.
+      #: (from: Group | Contact, contacts: Array[ListedContact], login: String, ?notice: String?) -> void
+      def initialize(from:, contacts:, login:, notice: nil)
         @login = login
+        @notice = notice
         # The order the show screen's list reads (Format.sort_key),
         # which is the order the book is browsed in everywhere else.
         @contacts = contacts.sort_by { [Format.sort_key(it), it.id] }
-        @members = from.is_a?(Group) ? from.members : [from.id]
-        # The members are never capped (GroupFilter.capped).
-        @capped = GroupFilter.capped(@contacts.map(&:id), joined: @members)
+        case from
+        in Group
+          @title = "Members of #{from.label}"
+          @action = "/groups/#{from.id}/members"
+          @back = ["/groups/#{from.id}", GroupLabel.new(group: from)]
+          @submit = "Save"
+          @ticked = from.members
+          @was = from.members
+        in Contact
+          @title = "New group"
+          @action = "/contacts/#{from.id}/groups/new"
+          @back = ["/contacts/#{from.id}", from.name || from.id]
+          # The verb the two create dialogs end on (GroupsIndex,
+          # ContactDialog).
+          @submit = "Create"
+          @ticked = [from.id]
+          @was = []
+        end
+        # The ticked are never capped (GroupFilter.capped).
+        @capped = GroupFilter.capped(@contacts.map(&:id), joined: @ticked)
       end
 
       def view_template
-        group = @group
-        title = group ? "Members of #{group.label}" : "New group"
-        render Layout.new(title:, login: @login) do
-          form(action: group ? "/groups/#{group.id}/members" : "/groups", method: "post",
-               class: "member-picker", id: FORM, x_data: GroupFilter::STATE) do
-            # Create where the save makes the group, the verb the two
-            # create dialogs end on (GroupsIndex, ContactDialog).
-            render RecordCard.new(back:, submit: group ? "Save" : "Create", form: FORM) do
+        render Layout.new(title: @title, login: @login, notice: @notice) do
+          form(action: @action, method: "post", class: "member-picker", id: FORM, x_data: GroupFilter::STATE) do
+            render RecordCard.new(back: @back, submit: @submit, form: FORM) do
               render GroupFilter.new(autofocus: true, in_form: true, placeholder: "Filter contacts")
             end
             div(class: "member-list") do
               ul(class: "card", x_ref: "options") do
                 @contacts.each do |contact|
-                  member = @members.include?(contact.id)
                   # The row's label is the name the lists show
                   # (Format.name_label), nickname and all, because
                   # that is the name a person filters by; the row
@@ -87,11 +100,11 @@ module ProTacts
                   li(**GroupFilter.row(Format.name_label(contact),
                                        capped: @capped.include?(contact.id))) do
                     label do
-                      input(type: "checkbox", name: "members[]", value: contact.id, checked: member)
+                      input(type: "checkbox", name: "members[]", value: contact.id, checked: @ticked.include?(contact.id))
                       render Avatar.new(contact:, size: "lg")
                       div(style: "flex: 1; min-width: 0; font-weight: 550;") { Format.name_label(contact) }
                     end
-                    input(type: "hidden", name: "was[]", value: contact.id) if member && @group
+                    input(type: "hidden", name: "was[]", value: contact.id) if @was.include?(contact.id)
                   end
                 end
               end
@@ -101,17 +114,6 @@ module ProTacts
           end
         end
       end
-
-      private
-
-      # The way back to the record the screen was opened from.
-      #: () -> [String, (String | Phlex::HTML)]
-      def back
-        case (from = @from)
-        in Group then ["/groups/#{from.id}", GroupLabel.new(group: from)]
-        in Contact then ["/contacts/#{from.id}", from.name || from.id]
-        end
-      end
     end
   end
 end
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index 8505d5b..205ba62 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -634,9 +634,9 @@ module ProTacts
     # attempt draws again.
     #
     # `members` join in the same transaction, for a group made from a
-    # contact's card (Web's add): a group that exists only once its
-    # first member is in it, rather than one left empty and nameless
-    # when the join fails.
+    # contact's card (Web#apply_new_group): a group that exists only
+    # once its first member is in it, rather than one left empty and
+    # nameless when the join fails.
     #: (?name: String?, ?members: Array[String]) -> String
     def create_group(name: nil, members: [])
       GROUP_ID_ATTEMPTS.times do
diff --git a/lib/pro_tacts/web/contacts.rb b/lib/pro_tacts/web/contacts.rb
index 1651cd8..81125bd 100644
--- a/lib/pro_tacts/web/contacts.rb
+++ b/lib/pro_tacts/web/contacts.rb
@@ -99,15 +99,16 @@ module ProTacts
         end
 
         # The groups row's "new" (Admin::ContactCard): the members
-        # screen of a group not yet made, this contact ticked; its Create
-        # is the create (Admin::GroupsMembers).
+        # screen of a group not yet made, this contact ticked, whose
+        # Create posts back here. Above the edit's POST, as the dialog's
+        # save is.
         r.get "groups", "new" do
           contact = store.contact(id)
+          new_group_screen(contact) if contact
+        end
 
-          if contact
-            response["Content-Type"] = "text/html; charset=utf-8"
-            Admin::GroupsMembers.call(from: contact, contacts: store.listed_contacts, login: @login)
-          end
+        r.post "groups", "new" do
+          apply_new_group(r, id)
         end
 
         r.post do
@@ -266,6 +267,28 @@ module ProTacts
       r.redirect(row ? "/import/#{row.fetch(0)}/#{row.fetch(1)}" : "/contacts/#{id}", 303)
     end
 
+    # The new group's Create: a nameless group of the ticked, landed
+    # on, it being something already. None ticked makes none, the
+    # store keeping no group that never had a member
+    # (Store#create_group). Only ids that name a card
+    # (#apply_members' reason).
+    #: (untyped r, String id) -> String?
+    def apply_new_group(r, id)
+      contact = store.contact(id)
+      return if contact.nil?
+
+      members = ids_in("members") & store.contact_ids
+      return new_group_screen(contact, notice: "A group needs a member; nothing was made.") if members.empty?
+
+      r.redirect "/groups/#{store.create_group(members:)}", 303
+    end
+
+    #: (Contact contact, ?notice: String) -> String
+    def new_group_screen(contact, notice: nil)
+      response["Content-Type"] = "text/html; charset=utf-8"
+      Admin::GroupsMembers.call(from: contact, contacts: store.listed_contacts, login: @login, notice:)
+    end
+
     #: (Contact contact, ?notice: String) -> String
     def contact_screen(contact, notice: nil)
       response["Content-Type"] = "text/html; charset=utf-8"
diff --git a/lib/pro_tacts/web/groups.rb b/lib/pro_tacts/web/groups.rb
index 5c3f015..ec00357 100644
--- a/lib/pro_tacts/web/groups.rb
+++ b/lib/pro_tacts/web/groups.rb
@@ -28,17 +28,10 @@ module ProTacts
           Admin::GroupsIndex.call(groups: store.all_groups, login: @login)
         end
 
-        # A create from the groups page names the group and lands on
-        # its editor; one from the members screen of a group not yet
-        # made (Admin::GroupsMembers, opened from a card's "new")
-        # carries its first members and lands on the group itself,
-        # being something already. Only ids that name a card
-        # (#apply_group_edit's reason).
         r.post do
           name = typecast_params.str("name", "").strip
-          members = ids_in("members") & store.contact_ids
-          id = store.create_group(name: name.empty? ? nil : name, members:)
-          r.redirect(r.params.key?("members") ? "/groups/#{id}" : "/groups/#{id}/edit", 303)
+          id = store.create_group(name: name.empty? ? nil : name)
+          r.redirect "/groups/#{id}/edit", 303
         rescue Sequel::UniqueConstraintViolation
           # A taken name (db/migrations/008_group_names.rb).
           response["Content-Type"] = "text/html; charset=utf-8"
diff --git a/sig/pro_tacts/admin.rbs b/sig/pro_tacts/admin.rbs
index 9c1cb13..6f43e1b 100644
--- a/sig/pro_tacts/admin.rbs
+++ b/sig/pro_tacts/admin.rbs
@@ -49,7 +49,7 @@ module ProTacts
     end
 
     class GroupsMembers
-      def self.call: (from: Group | Contact, contacts: Array[ListedContact], login: String) -> String
+      def self.call: (from: Group | Contact, contacts: Array[ListedContact], login: String, ?notice: String?) -> String
     end
 
     class DeviceSetup
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index fa5ad3c..3915856 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -921,7 +921,7 @@ class AdminContactsPagesTest < Minitest::Test
 
       assert_equal 200, last_response.status
       assert_empty store.all_groups
-      assert_includes last_response.body, %(<form action="/groups" method="post")
+      assert_includes last_response.body, %(<form action="/contacts/ada/groups/new" method="post")
       assert_includes last_response.body, %(<input type="checkbox" name="members[]" value="ada" checked>)
       assert_includes last_response.body, %(<a href="/contacts/ada" class="btn">Cancel</a>)
       assert_includes last_response.body, %(data-variant="primary">Create</button>)
@@ -933,7 +933,7 @@ class AdminContactsPagesTest < Minitest::Test
   # ticked contacts and lands on it.
   def test_the_new_groups_save_makes_a_nameless_group_of_the_ticked
     with_contacts({"ada" => ADA}) do |store|
-      post "/groups", members: %w[ada nobody]
+      post "/contacts/ada/groups/new", members: %w[ada nobody]
 
       groups = store.all_groups
       group = groups.fetch(0)
@@ -946,11 +946,25 @@ class AdminContactsPagesTest < Minitest::Test
     end
   end
 
+  # A Create with nobody ticked makes nothing and says so.
+  def test_the_new_groups_save_with_none_ticked_makes_none
+    with_contacts({"ada" => ADA}) do |store|
+      post "/contacts/ada/groups/new", members: %w[nobody]
+
+      assert_equal 200, last_response.status
+      assert_includes last_response.body, "A group needs a member; nothing was made."
+      assert_empty store.all_groups
+    end
+  end
+
   def test_a_new_group_for_an_unknown_contact_is_a_404
-    with_contacts({"ada" => ADA}) do
+    with_contacts({"ada" => ADA}) do |store|
       get "/contacts/nobody/groups/new"
+      assert_equal 404, last_response.status
 
+      post "/contacts/nobody/groups/new", members: %w[ada]
       assert_equal 404, last_response.status
+      assert_empty store.all_groups
     end
   end