From fbb405a7a02ed14a94f171d96d583c21a89bfd9c Mon Sep 17 00:00:00 2001 From: Rogee Date: Sat, 6 Jun 2026 01:51:31 +0800 Subject: [PATCH] feat(agents): align list ordering --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 10 ++++---- internal/handler/api/v1/agent_handler_test.go | 24 +++++++++++++++++++ internal/repository/agent_repo.go | 22 ++++++++++------- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index ff4c186d..30005be0 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -16,10 +16,10 @@ Build GoChat as a Go backend that can directly reuse the frontend from `referenc ## Current Baseline -- Current tracking checkpoint: 2026-06-06 after `bd1520f feat(agents): align bulk invite behavior`, with this implementation checkpoint prepared as `feat(agents): align update route semantics`. -- Latest implementation checkpoint: this checkpoint, prepared as `feat(agents): align update route semantics`. +- Current tracking checkpoint: 2026-06-06 after `c30020d feat(agents): align update route semantics`, with this implementation checkpoint prepared as `feat(agents): align list ordering`. +- Latest implementation checkpoint: this checkpoint, prepared as `feat(agents): align list ordering`. - Latest documentation-only checkpoint: `8b378c7 docs: refresh parity tracker baseline`; this document is now the active follow-up plan and supersedes `.hermes/plans/*`. -- Worktree status at this implementation checkpoint: P3.2 agents update drift is closed for the reused settings client: `PATCH /agents/:id` is registered alongside the existing `PUT` alias used by Rails resources, and update preserves `account_users.auto_offline` when omitted while still applying explicit `auto_offline: false` like Chatwoot strong params plus `compact`. The preceding agents bulk invite, B10.3 CustomRole frontend update, and Phase 6 contacts pagination/active payload drifts are closed. Conversation transcript and custom-attribute response drifts are already closed. The fresh placeholder audit found remaining `chatwootParityStub` routes only in webhook handler-not-injected fallbacks, not in the reused dashboard account/contact/conversation/message/inbox critical path. P3.6 custom filters/custom attribute definitions and label CRUD are already in Review. B11.1a-B11.3e Captain/Copilot slices remain in Review; B12.1/B12.2/B12.3 smoke harnesses remain in Review; P5 durable job work through P5.13b is in Review. Next active implementation slice is the next named Phase 2/3 or Phase 6 drift from fresh reference/smoke evidence. +- Worktree status at this implementation checkpoint: P3.2 agents index ordering drift is closed for the reused settings and report filters: `GET /agents` now follows Chatwoot `Current.account.users.order_by_full_name` by ordering agents with `lower(users.name) ASC` before pagination. The preceding agents update-route/`auto_offline`, agents bulk invite, B10.3 CustomRole frontend update, and Phase 6 contacts pagination/active payload drifts are closed. Conversation transcript and custom-attribute response drifts are already closed. The fresh placeholder audit found remaining `chatwootParityStub` routes only in webhook handler-not-injected fallbacks, not in the reused dashboard account/contact/conversation/message/inbox critical path. P3.6 custom filters/custom attribute definitions and label CRUD are already in Review. B11.1a-B11.3e Captain/Copilot slices remain in Review; B12.1/B12.2/B12.3 smoke harnesses remain in Review; P5 durable job work through P5.13b is in Review. Next active implementation slice is the next named Phase 2/3 or Phase 6 drift from fresh reference/smoke evidence. - `go test ./...` passes. - Route dump succeeds with `TOTAL: 839` after adding the Chatwoot frontend-used agent `PATCH /api/v1/accounts/:account_id/agents/:agent_id` update route. - Route parity artifacts now exist under `docs/parity/` and are generated by `cmd/route_parity`. @@ -143,6 +143,7 @@ This ledger records the committed parity checkpoints that future slices should b | Commit | Scope | Verification summary | Follow-up state | | --- | --- | --- | --- | +| `feat(agents): align list ordering` | Advances P3.2 agents/settings parity by matching Chatwoot `AgentsController#index`, which renders `Current.account.users.order_by_full_name`. Account agent lists now order by `lower(users.name) ASC` before pagination while preserving account-user role, availability, auto-offline, inviter, account-user ID, and custom-role metadata. | `go test ./internal/handler/api/v1 -run AgentHandler -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 839`. | Continue P3.2 account users/invitations/permissions drift or the next Phase 6 reference/smoke finding. | | `feat(agents): align update route semantics` | Advances P3.2 agents/settings parity by aligning update with Chatwoot `AgentsController#update` and the reused dashboard `ApiClient.update` path. Agent update now exposes `PATCH /api/v1/accounts/:account_id/agents/:agent_id` alongside the existing `PUT` alias, accepts nested `{ agent: ... }` payloads, preserves `auto_offline` when the field is omitted, and still applies explicit `auto_offline: false` like Rails strong params with `compact`. | `go test ./internal/handler/api/v1 -run AgentHandler -count=1`; `go test ./internal/service -run Agent -count=1`; `go run ./cmd/dump_routes > docs/parity/gochat_routes.txt`; `go run ./cmd/route_parity`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./...`; `git diff --check`. Route dump is `TOTAL: 839`; tracked route parity remains `270 exact, 7 parameter-compatible, 0 missing`. | Continue P3.2 account users/invitations/permissions drift or the next Phase 6 reference/smoke finding. | | `feat(agents): align bulk invite behavior` | Advances P3.2 agents/onboarding parity by aligning `POST /api/v1/accounts/:account_id/agents/bulk_create` with Chatwoot `AgentsController#bulk_create`. Bulk invite now treats the email list as a best-effort loop: invalid entries are skipped instead of failing the whole request, duplicate failures remain ignored, empty arrays return empty `200 OK`, valid agents in the same request are still created, and the local onboarding step is cleared after the bulk flow. | `go test ./internal/handler/api/v1 -run AgentHandler -count=1`; `go test ./internal/service -run Agent -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./...`; `git diff --check`. | Continue P3.2 account users/invitations/permissions drift or the next Phase 6 reference/smoke finding. | | `feat(custom-roles): align update route semantics` | Advances B10.3 CustomRole frontend parity by registering Chatwoot's Rails resource `PATCH /api/v1/accounts/:account_id/custom_roles/:id` update route used by the reused `ApiClient.update` path, while keeping the existing `PUT` alias. Custom role updates now distinguish omitted `description` from an explicit empty string, so the enterprise settings form can clear descriptions like `CustomRolesController#update` with strong params. | `go test ./internal/handler/api/v1 -run CustomRole -count=1`; `go test ./internal/service -run CustomRole -count=1`; `go run ./cmd/dump_routes > docs/parity/gochat_routes.txt`; `go run ./cmd/route_parity`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./...`; `git diff --check`. | Continue the next named Phase 2/3 or Phase 6 drift from reference/smoke evidence. | @@ -1560,7 +1561,7 @@ Frontend-critical API groups to audit first: | ID | Area | Scope | Status | | --- | --- | --- | --- | | P3.1 | Auth/session/profile | Login, logout, current user, profile, availability, notification settings, Devise password reset, and confirmation. | Done | -| P3.2 | Accounts/users/teams | Account settings, users, agents, teams, team members, invitations, roles, permissions. | Doing; account show/update/create/cache-key/active-at plus agents/teams/team-member, assignable-agent settings payloads, agents bulk invite best-effort behavior, and agent update PATCH/`auto_offline` presence semantics now have focused Chatwoot-style coverage. | +| P3.2 | Accounts/users/teams | Account settings, users, agents, teams, team members, invitations, roles, permissions. | Doing; account show/update/create/cache-key/active-at plus agents/teams/team-member, assignable-agent settings payloads, agents `order_by_full_name` index ordering, agents bulk invite best-effort behavior, and agent update PATCH/`auto_offline` presence semantics now have focused Chatwoot-style coverage. | | P3.3 | Inboxes/channels | Inbox CRUD, assignable agents, avatars, channel config, business hours, widget config. | Review; assignable-agent payload drift closed, broader inbox serializer drift remains evidence-driven. | | P3.4 | Conversations/messages | List filters, status changes, assignment, labels, private notes, attachments, drafts, typing/read events. | Review | | P3.5 | Contacts/companies | CRUD, merge, labels, notes, custom attributes, import/export, conversations relation, and shared attachments. | Doing | @@ -2074,6 +2075,7 @@ Verification milestone gates: ## Progress Log +- 2026-06-06: P3.2 agents index-order checkpoint prepared as `feat(agents): align list ordering`; audited Chatwoot `AgentsController#index` and `User.order_by_full_name`. Account agent lists now join users and order by `lower(users.name) ASC` before pagination, while preserving account-user metadata in the Chatwoot agent serializer. Focused handler tests, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 account users/invitations drift next. - 2026-06-06: P3.2 agents update-route checkpoint prepared as `feat(agents): align update route semantics`; audited Chatwoot `AgentsController#update`, dashboard `agents.js`, and shared `ApiClient.update`. Agent update now exposes frontend-used `PATCH /api/v1/accounts/:account_id/agents/:agent_id` alongside the existing `PUT` alias, preserves existing `auto_offline` when omitted from the request, and still applies explicit `auto_offline: false` like Rails strong params with `compact`. Focused handler/service tests, route generation/parity, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 account users/invitations drift next. - 2026-06-06: P3.2 agents bulk invite checkpoint prepared as `feat(agents): align bulk invite behavior`; audited Chatwoot `AgentsController#bulk_create`, dashboard `agents.js`, and the reused onboarding/settings bulk invite path. Bulk invite now skips invalid email entries instead of rejecting the full request, keeps duplicate failures best-effort, creates valid agents from mixed arrays, returns empty `200 OK` for empty/no-op arrays, and clears the local onboarding step after the bulk flow. Focused handler/service tests, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 account users/invitations drift next. - 2026-06-06: B10.3 CustomRole update-route checkpoint prepared as `feat(custom-roles): align update route semantics`; audited Chatwoot `CustomRolesController`, custom role Jbuilder views, and dashboard `customRole.js`/`ApiClient.update`. CustomRole update now exposes `PATCH /api/v1/accounts/:account_id/custom_roles/:id` alongside the existing `PUT` alias, route dump is `TOTAL: 838`, tracked route parity remains `270 exact, 7 parameter-compatible, 0 missing`, and explicit empty `description` values now clear the role description while omitted descriptions are preserved. Focused handler/service tests, route generation/parity, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue Phase 2/3 drift audit next. diff --git a/internal/handler/api/v1/agent_handler_test.go b/internal/handler/api/v1/agent_handler_test.go index 7eba3d60..b39094a5 100644 --- a/internal/handler/api/v1/agent_handler_test.go +++ b/internal/handler/api/v1/agent_handler_test.go @@ -113,6 +113,30 @@ func (s *AgentHandlerTestSuite) TestListEmpty() { assert.Equal(s.T(), 0, len(data)) } +func (s *AgentHandlerTestSuite) TestListOrdersByFullName() { + agents := []service.CreateAgentRequest{ + {Email: "charlie@test.com", Name: "charlie", Role: "agent"}, + {Email: "alpha@test.com", Name: "Alpha", Role: "agent"}, + {Email: "bravo@test.com", Name: "bravo", Role: "agent"}, + } + for _, req := range agents { + w, c := s.makeRequest("POST", "/api/v1/accounts/1/agents", req, s.account.ID, s.user.ID) + s.handler.Create(c) + assert.Equal(s.T(), http.StatusOK, w.Code) + } + + w, c := s.makeRequest("GET", "/api/v1/accounts/1/agents", nil, s.account.ID, s.user.ID) + s.handler.List(c) + + assert.Equal(s.T(), http.StatusOK, w.Code) + var data []map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &data) + s.Require().Len(data, 3) + assert.Equal(s.T(), "Alpha", data[0]["name"]) + assert.Equal(s.T(), "bravo", data[1]["name"]) + assert.Equal(s.T(), "charlie", data[2]["name"]) +} + func (s *AgentHandlerTestSuite) TestCreateAgent() { customRoleID := uint(7) req := service.CreateAgentRequest{ diff --git a/internal/repository/agent_repo.go b/internal/repository/agent_repo.go index 2d239ba3..e10e9420 100644 --- a/internal/repository/agent_repo.go +++ b/internal/repository/agent_repo.go @@ -45,15 +45,18 @@ func (r *AgentRepo) ListByAccount(ctx context.Context, accountID uint, offset, l var count int64 if err := r.db.WithContext(ctx). Model(&model.AccountUser{}). - Where("account_id = ?", accountID). + Where("account_users.account_id = ?", accountID). Count(&count).Error; err != nil { return nil, 0, err } var accountUsers []model.AccountUser if err := r.db.WithContext(ctx). - Where("account_id = ?", accountID). - Order("id ASC"). + Model(&model.AccountUser{}). + Joins("JOIN users ON users.id = account_users.user_id"). + Where("account_users.account_id = ?", accountID). + Order("lower(users.name) ASC"). + Order("users.id ASC"). Offset(offset).Limit(limit). Find(&accountUsers).Error; err != nil { return nil, 0, err @@ -65,10 +68,8 @@ func (r *AgentRepo) ListByAccount(ctx context.Context, accountID uint, offset, l // Collect user IDs userIDs := make([]uint, len(accountUsers)) - auMap := make(map[uint]model.AccountUser) // userID → AccountUser for i, au := range accountUsers { userIDs[i] = au.UserID - auMap[au.UserID] = au } // Fetch users @@ -79,10 +80,15 @@ func (r *AgentRepo) ListByAccount(ctx context.Context, accountID uint, offset, l return nil, 0, err } - // Build AgentDetail list - details := make([]AgentDetail, 0, len(users)) + userMap := make(map[uint]model.User, len(users)) for _, u := range users { - au, ok := auMap[u.ID] + userMap[u.ID] = u + } + + // Build AgentDetail list in Chatwoot's order_by_full_name order. + details := make([]AgentDetail, 0, len(accountUsers)) + for _, au := range accountUsers { + u, ok := userMap[au.UserID] if !ok { continue }