Lead every group listing with the sync groups
Assisted-by: GLM-5.3 via pi
change lzpqyzrozrsnknkyoslowovzzswswlyt
commit 94fafd6ae71a1676279531c23e81c68b57193ad2
author Alpha Chen <alpha@kejadlen.dev>
date
parent nzwzmssk
diff --git a/lib/pro_tacts/admin/dashboard.rb b/lib/pro_tacts/admin/dashboard.rb
index 33dfe9c..c709494 100644
--- a/lib/pro_tacts/admin/dashboard.rb
+++ b/lib/pro_tacts/admin/dashboard.rb
@@ -42,7 +42,7 @@ module ProTacts
         @labels = {}
         groups.each { |group| group.members.each { (@labels[it] ||= []) << group.label } }
         q = @query.downcase
-        @groups = @query.empty? ? [] : groups.select { it.label.downcase.include?(q) }
+        @groups = @query.empty? ? [] : groups.select { it.label.downcase.include?(q) }.sort
         @rows = @query.empty? ? recent.first(RECENT_LIMIT) : recent.select { matches?(it, @query) }
       end
 
diff --git a/lib/pro_tacts/admin/group_dialog.rb b/lib/pro_tacts/admin/group_dialog.rb
index 5ad40c4..fdf77a0 100644
--- a/lib/pro_tacts/admin/group_dialog.rb
+++ b/lib/pro_tacts/admin/group_dialog.rb
@@ -40,18 +40,19 @@ module ProTacts
       # @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.
-      # `groups` is every group as a choice — no members read, the
-      # book's whole membership not being this dialog's business
-      # (Store#group_choices) — and `joined` the ids this contact is
-      # 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.
+      # `GroupChoice#<=>`'s order rather than the id order a tag
+      # keeps — a list to find a name in, and nothing here stays put
+      # across a rename. `groups` is every group as a
+      # choice — no members read, the book's whole membership not
+      # being this dialog's business (Store#group_choices) — and
+      # `joined` the ids this contact is 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], ?land: String?) -> void
       def initialize(contact:, groups:, joined:, land: nil)
         @contact = contact
-        @groups = groups.sort_by { it.label.downcase }
+        @groups = groups.sort
         @joined = joined
         @land = land
         @capped = GroupFilter.capped(@groups.map(&:id), joined:)
diff --git a/lib/pro_tacts/admin/groups_index.rb b/lib/pro_tacts/admin/groups_index.rb
index bfa7198..76bd878 100644
--- a/lib/pro_tacts/admin/groups_index.rb
+++ b/lib/pro_tacts/admin/groups_index.rb
@@ -21,16 +21,11 @@ module ProTacts
       #: (groups: Array[Group], login: String, ?notice: String?) -> void
       def initialize(groups:, login:, notice: nil)
         @login = login
-        # The `sync:` groups first — the books a client syncs, which
-        # are the store's own rather than anything a person made — and
-        # the rest alphabetically. Sorted here rather than asked of the
+        # `Group#<=>`'s order, sorted here rather than asked of the
         # store the way ContactsIndex sorts its rows: #all_groups is
         # ordered by id so that a rename never moves a tag, and only
-        # this listing wants the reader's order. The key is the label
-        # the row shows (GroupLabel), so a nameless group sorts by the
-        # id it is shown by, and the id breaks a tie between two groups
-        # labelled the same.
-        @groups = groups.sort_by { [it.sync? ? 0 : 1, it.label.downcase, it.id] }
+        # this listing wants the reader's order.
+        @groups = groups.sort
         @notice = notice
       end
 
diff --git a/lib/pro_tacts/admin/import_groups.rb b/lib/pro_tacts/admin/import_groups.rb
index e03c64d..76e1231 100644
--- a/lib/pro_tacts/admin/import_groups.rb
+++ b/lib/pro_tacts/admin/import_groups.rb
@@ -44,11 +44,12 @@ module ProTacts
       # @rbs @capped: Array[String]
       # @rbs @was: Array[String]
 
-      # Largest first, and alphabetical among groups the same size:
-      # the groups an arrival is likeliest to belong in are the ones
-      # most of the book already does, so they are the rows the cap
-      # leaves standing, and the filter reaches the rest. The groups
-      # dialog stays alphabetical — there a contact's groups are
+      # The `sync:` groups lead (GroupChoice#<=>), then largest first
+      # and alphabetical among groups the same size: the groups an
+      # arrival is likeliest to belong in are the ones most of the
+      # book already does, so they are the rows the cap leaves
+      # standing, and the filter reaches the rest. The groups dialog
+      # keeps the plain listing order — there a contact's groups are
       # being looked over, not guessed at. `groups` is every group as
       # a choice, counted rather than its members read
       # (Store#group_choices); `joined` the ids ticked, `named` the
@@ -72,7 +73,7 @@ module ProTacts
       # what was toggled (Web#apply_groups). A new contact is in none.
       #: (groups: Array[Store::GroupChoice], joined: Array[String], named: Array[String], ?was: Array[String]) -> void
       def initialize(groups:, joined:, named:, was: [])
-        @groups = groups.sort_by { [-it.member_count, it.label.downcase] }
+        @groups = groups.sort_by { [it.sync? ? 0 : 1, -it.member_count, it.label.downcase] }
         @joined = joined
         @named = named
         @was = was
diff --git a/lib/pro_tacts/group.rb b/lib/pro_tacts/group.rb
index c43cf25..43c7104 100644
--- a/lib/pro_tacts/group.rb
+++ b/lib/pro_tacts/group.rb
@@ -42,14 +42,24 @@ module ProTacts
       Contact.new(id:, stored: VCard.new(lines.map { "#{it}\r\n" }.join), birthday: nil, inherited: [])
     end
 
-    # Whether this is one of the `sync:` groups, for the listing that
-    # sets those groups apart from the ones a person made
-    # (Admin::GroupsIndex).
+    # Whether this is one of the `sync:` groups, for the listings
+    # that set those apart from the ones a person made.
     #: () -> bool
     def sync?
       Group.sync_name?(name)
     end
 
+    # The order every listing of groups reads in: the `sync:` groups
+    # — the books a client syncs, the store's own rather than
+    # anything a person made — first, then the rest alphabetically.
+    # GroupChoice spells this again for the pickers; keep the two
+    # alike. Comparable deliberately left out: its `==` would sit in
+    # front of Data's whole-value one, and `sort` asks only for `<=>`.
+    #: (Group other) -> Integer?
+    def <=>(other)
+      [sync? ? 0 : 1, label.downcase, id] <=> [other.sync? ? 0 : 1, other.label.downcase, other.id]
+    end
+
     # The group's etag, for the editor's snapshot guard: a hash of
     # everything the edit screen shows and the save writes, so a
     # group edited since the page loaded refuses the stale save
diff --git a/lib/pro_tacts/store.rb b/lib/pro_tacts/store.rb
index fba4068..e6047cb 100644
--- a/lib/pro_tacts/store.rb
+++ b/lib/pro_tacts/store.rb
@@ -114,6 +114,22 @@ module ProTacts
     # @rbs skip
     GroupChoice = Data.define(:id, :name, :label, :member_count)
 
+    # Reopened rather than defined in a block, Group's own reason.
+    class GroupChoice
+      # Group#sync?'s question, over the lighter row a picker holds.
+      #: () -> bool
+      def sync?
+        Group.sync_name?(name)
+      end
+
+      # Group#<=>'s order spelled again for the pickers that sort
+      # choices; keep the two alike.
+      #: (GroupChoice other) -> Integer?
+      def <=>(other)
+        [sync? ? 0 : 1, label.downcase, id] <=> [other.sync? ? 0 : 1, other.label.downcase, other.id]
+      end
+    end
+
     # SQLite has no ON UPDATE, so the column default stamps a row on
     # insert and this stamps it again on the way past. Same expression as
     # the migration's, deliberately: the database keeps the clock, so two
diff --git a/lib/pro_tacts/web/api.rb b/lib/pro_tacts/web/api.rb
index 3210422..134343d 100644
--- a/lib/pro_tacts/web/api.rb
+++ b/lib/pro_tacts/web/api.rb
@@ -7,11 +7,12 @@ module ProTacts
     # name over HTTP before making it; that import is gone
     # (docs/plans/2026-09-21-import-a-vcf.md) and the route stays as
     # what it always was — the group list, for anything scripted
-    # pointed at this server.
+    # pointed at this server, reading the order every listing does
+    # (`Group#<=>`).
     hash_branch("api") do |r|
       r.get "groups" do
         response["Content-Type"] = "application/json"
-        store.all_groups.map { {id: it.id, name: it.name} }.to_json
+        store.all_groups.sort.map { {id: it.id, name: it.name} }.to_json
       end
     end
   end
diff --git a/test/pro_tacts/admin/test_contacts_pages.rb b/test/pro_tacts/admin/test_contacts_pages.rb
index 967734a..dc70988 100644
--- a/test/pro_tacts/admin/test_contacts_pages.rb
+++ b/test/pro_tacts/admin/test_contacts_pages.rb
@@ -529,6 +529,19 @@ class AdminContactsPagesTest < Minitest::Test
     end
   end
 
+  def test_the_groups_dialog_leads_with_the_sync_groups
+    with_contacts({"ada" => ADA}) do |store|
+      %w[zoetrope Abacus sync:alpha barometer sync:*].each { FixtureData.seed_group(store, name: it) }
+      ids = store.all_groups.to_h { [it.name, it.id] }
+
+      get "/contacts/ada"
+      dialog = last_response.body[%r{<dialog id="edit-groups".*?</dialog>}].to_s
+
+      assert_equal %w[sync:* sync:alpha Abacus barometer zoetrope].map { ids.fetch(it) },
+                   dialog.scan(/name="groups\[\]" value="([^"]+)"/).flatten
+    end
+  end
+
   # The cap (docs/plans/2026-09-22-a-few-groups-at-a-time.md): eight
   # rows stand shown and the ninth is marked, rendered for the filter
   # to reach rather than left out of the page.
diff --git a/test/pro_tacts/admin/test_import_pages.rb b/test/pro_tacts/admin/test_import_pages.rb
index d4a41b7..cb00506 100644
--- a/test/pro_tacts/admin/test_import_pages.rb
+++ b/test/pro_tacts/admin/test_import_pages.rb
@@ -781,6 +781,24 @@ class AdminImportPagesTest < Minitest::Test
     end
   end
 
+  # However few it holds: the listings' order outranks the size
+  # sort here.
+  def test_the_groups_beside_a_card_lead_with_the_sync_groups
+    with_contacts({"jane" => JANE, "sam" => PLAIN}) do |store|
+      big = store.create_group(name: "big")
+      store.add_member(big, "jane")
+      store.add_member(big, "sam")
+      small_book = store.create_group(name: "sync:alpha")
+      store.add_member(small_book, "jane")
+      id = upload(JANE)
+
+      get "/import/#{id}/0"
+
+      assert_equal [small_book, big],
+                   last_response.body.scan(/name="groups\[\]" value="([^"]+)"/).flatten
+    end
+  end
+
   def test_the_groups_beside_a_card_offer_no_such_thing_uncapped
     with_contacts({}) do |store|
       # Seven, the import's own name being the eighth row.
diff --git a/test/pro_tacts/test_api.rb b/test/pro_tacts/test_api.rb
index 5513bce..d2f9d4e 100644
--- a/test/pro_tacts/test_api.rb
+++ b/test/pro_tacts/test_api.rb
@@ -19,16 +19,21 @@ class ApiTest < Minitest::Test
 
   def test_groups_lists_every_group_by_id_and_name
     with_contacts({}) do |store|
-      named = store.create_group(name: "import-20260916T180412Z")
+      %w[import-20260916T180412Z sync:alpha aardvarks].each { store.create_group(name: it) }
       nameless = store.create_group
+      ids = store.all_groups.to_h { [it.name, it.id] }
 
       get "/api/groups"
 
       assert_equal 200, last_response.status
       assert_equal "application/json", last_response["Content-Type"]
       groups = JSON.parse(last_response.body)
-      assert_equal store.all_groups.map { {"id" => it.id, "name" => it.name} }, groups
-      assert_includes groups, {"id" => named, "name" => "import-20260916T180412Z"}
+      # The nameless group's label is its id, so where it falls
+      # among the named is not fixed.
+      listed = groups.map { it.fetch("id") }
+      named = %w[sync:alpha aardvarks import-20260916T180412Z].map { ids.fetch(it) }
+      assert_equal named, listed & named
+      assert_equal "sync:alpha", groups.fetch(0).fetch("name")
       assert_includes groups, {"id" => nameless, "name" => nil}
     end
   end