feat(agents): align index pagination

This commit is contained in:
2026-06-06 02:47:55 +08:00
parent 0ebaa51cbd
commit d49aafd2c3
4 changed files with 42 additions and 13 deletions
+7 -5
View File
@@ -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 `ec1bb25 feat(agents): align delete scope`, with this implementation checkpoint prepared as `feat(agents): align update validation errors`.
- Latest implementation checkpoint: this checkpoint, prepared as `feat(agents): align update validation errors`.
- Current tracking checkpoint: 2026-06-06 after `0ebaa51 feat(agents): align update validation errors`, with this implementation checkpoint prepared as `feat(agents): align index pagination`.
- Latest implementation checkpoint: this checkpoint, prepared as `feat(agents): align index pagination`.
- 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: Agent update blank-name validation now matches Chatwoot `User#update!` plus `RequestExceptionHandler#render_record_invalid`: explicit blank `agent[name]` returns `422 { message: "Name can't be blank", attributes: ["name"] }` and leaves the user name unchanged. Agent delete already matches Chatwoot's `agents.find(params[:id])` account scope: deleting a user who is not a member of the current account returns `404` and does not remove that user or any other account membership. Duplicate account agent creation already matches Chatwoot's `ActiveRecord::RecordInvalid` response shape: `422` with `{ message, attributes }` instead of a local `409` envelope. Agent creation already matches Chatwoot `AgentBuilder` name fallback: blank or omitted `agent[name]` no longer fails local validation and new users receive the email local-part as `name`. Account conversation/contact attachment list pagination already matches Chatwoot's fixed 100-row page size for `ConversationsController#attachments` and `Contacts::AttachmentsController#index`; reused frontend `per_page` query values no longer shrink shared attachment payloads below the reference page size. Profile account permission drift is closed for reused route guards and dashboard permission helpers: profile account entries now return Chatwoot `AccountUser#permissions` for ordinary roles (`["administrator"]` or `["agent"]`) and continue to return enterprise custom-role permissions plus `custom_role` when `custom_role_id` is present. The preceding platform account-user membership, agents list ordering, 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.
- Worktree status at this implementation checkpoint: Agent index pagination now matches Chatwoot `AgentsController#index`, which renders `Current.account.users.order_by_full_name` without `.page/.per`: reused frontend `page/per_page` query values are ignored and the endpoint returns the full account-agent array ordered by full name. Agent update blank-name validation already matches Chatwoot `User#update!` plus `RequestExceptionHandler#render_record_invalid`: explicit blank `agent[name]` returns `422 { message: "Name can't be blank", attributes: ["name"] }` and leaves the user name unchanged. Agent delete already matches Chatwoot's `agents.find(params[:id])` account scope: deleting a user who is not a member of the current account returns `404` and does not remove that user or any other account membership. Duplicate account agent creation already matches Chatwoot's `ActiveRecord::RecordInvalid` response shape: `422` with `{ message, attributes }` instead of a local `409` envelope. Agent creation already matches Chatwoot `AgentBuilder` name fallback: blank or omitted `agent[name]` no longer fails local validation and new users receive the email local-part as `name`. Account conversation/contact attachment list pagination already matches Chatwoot's fixed 100-row page size for `ConversationsController#attachments` and `Contacts::AttachmentsController#index`; reused frontend `per_page` query values no longer shrink shared attachment payloads below the reference page size. Profile account permission drift is closed for reused route guards and dashboard permission helpers: profile account entries now return Chatwoot `AccountUser#permissions` for ordinary roles (`["administrator"]` or `["agent"]`) and continue to return enterprise custom-role permissions plus `custom_role` when `custom_role_id` is present. The preceding platform account-user membership, agents list ordering, 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: 840` after adding the Chatwoot platform account-user collection destroy route.
- Route parity artifacts now exist under `docs/parity/` and are generated by `cmd/route_parity`.
@@ -87,7 +87,7 @@ This table is the shortest authoritative handoff view. If an older lower section
| Priority | Workstream | Current state | Next checkpoint | Commit close rule |
| --- | --- | --- | --- | --- |
| 1 | Phase 2/3 drift | Tracked route parity is 0 missing for the current critical set; notification list/action serializers, Devise password reset/confirmation payloads, CRM shared attachment payloads plus fixed 100-row attachment pagination, account/settings payloads, assignable-agent payloads, agent create/update/delete defaults/errors/scope, label CRUD payloads, custom filters, and custom attribute definitions now match the inspected Chatwoot frontend contract. | Continue Phase 6 placeholder audit, then open the next named serializer/controller drift slice from reference/smoke evidence. | Regenerate parity artifacts when routes change and add endpoint-family fixture tests. |
| 1 | Phase 2/3 drift | Tracked route parity is 0 missing for the current critical set; notification list/action serializers, Devise password reset/confirmation payloads, CRM shared attachment payloads plus fixed 100-row attachment pagination, account/settings payloads, assignable-agent payloads, agent index full-list behavior, agent create/update/delete defaults/errors/scope, label CRUD payloads, custom filters, and custom attribute definitions now match the inspected Chatwoot frontend contract. | Continue Phase 6 placeholder audit, then open the next named serializer/controller drift slice from reference/smoke evidence. | Regenerate parity artifacts when routes change and add endpoint-family fixture tests. |
| 2 | Phase 6 placeholder audit | Widget/public/webhook critical placeholders are burned down; inbox WhatsApp health/register-webhook and sync-template drift are closed; fresh `rg` audit shows only webhook nil-handler fallbacks still call `chatwootParityStub`; dashboard conversation transcript response drift is closed. | Continue targeted account/contact/conversation/message/inbox drift from reference/smoke evidence. | `rg` placeholder audit is recorded and no reused-frontend blocker is ownerless. |
| 3 | B12 optional live smoke | API/browser/enterprise smoke commands are checked in; live runs need PostgreSQL, Redis, Meilisearch, Vite, and Chrome. | Run full live smoke when environment is available and map failures to the board. | `docs/parity/frontend_smoke_report.md` records pass/fail and linked owners. |
| 4 | B9.3 delayed automation actions | Current reference exposes no delayed automation action params; scheduled-item work is already P5.12; `send_email_to_team` is durable and `add_sla` mutates conversation/applied SLA state. | Keep automation drift closed if future reference/smoke exposes delayed params or unsupported action shapes. | Automation worker/action fixtures verify queued team email replay, retry visibility through worker jobs, and SLA action idempotency. |
@@ -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 index pagination` | Advances P3.2 agents/settings parity by matching Chatwoot `AgentsController#index`, which assigns `@agents = agents` from `Current.account.users.order_by_full_name.includes(...)` without pagination. Account agent lists now ignore reused-frontend `page/per_page` query values, return the full account-agent array, and retain `lower(users.name) ASC` ordering plus account-user role, availability, auto-offline, inviter, account-user ID, and custom-role metadata. | `go test ./internal/handler/api/v1 -run 'TestAgentHandlerSuite/TestList' -count=1`; `go test ./internal/service ./internal/repository -run Agent -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/repository -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 840`. | Continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding. |
| `feat(agents): align update validation errors` | Advances P3.2 agents/settings parity by matching Chatwoot `AgentsController#update`, `User#update!`, and `RequestExceptionHandler#render_record_invalid`. Explicit blank `agent[name]` updates now return `422 { message: "Name can't be blank", attributes: ["name"] }` and preserve the existing user name instead of silently ignoring the invalid value with a `200 OK`. | `go test ./internal/handler/api/v1 -run 'TestAgentHandlerSuite/TestUpdateAgent' -count=1`; `go test ./internal/service -run Agent -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/repository -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 840`. | Continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding. |
| `feat(agents): align delete scope` | Advances P3.2 agents/settings parity by matching Chatwoot `AgentsController#fetch_agent` before `destroy`: deletes are scoped through current account membership and return `404` when the target user is not an agent in the account. The local delete path now looks up the `AccountUser` before deletion so cross-account/non-member users are not accidentally deleted. | `go test ./internal/handler/api/v1 -run 'TestAgentHandlerSuite/TestDeleteAgent' -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/repository -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 840`. | Continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding. |
| `feat(agents): align duplicate create errors` | Advances P3.2 agents/settings parity by matching Chatwoot's `ActiveRecord::RecordInvalid` handling for duplicate account memberships created through `AgentsController#create`. Re-inviting an existing account member now returns `422 { message: "User has already been taken", attributes: ["user_id"] }` instead of the local `409` conflict envelope, while bulk invite continues to skip duplicate failures like the reference controller. | `go test ./internal/handler/api/v1 -run 'TestAgentHandlerSuite/TestCreateAgentDuplicate' -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/repository -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 840`. | Continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding. |
@@ -1568,7 +1569,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; profile account permission arrays now match Chatwoot role/custom-role semantics for dashboard route guards. |
| 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, profile account permission arrays, platform account-user raw/idempotent membership, 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.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, profile account permission arrays, platform account-user raw/idempotent membership, agents `order_by_full_name` ordering with Chatwoot no-pagination index behavior, agents bulk invite best-effort behavior, and agent update PATCH/`auto_offline`/validation 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 |
@@ -2082,6 +2083,7 @@ Verification milestone gates:
## Progress Log
- 2026-06-06: P3.2 agent index-pagination checkpoint prepared as `feat(agents): align index pagination`; audited Chatwoot `AgentsController#index`, `agents`, and `User.order_by_full_name`. Account agent lists now ignore reused-frontend `page/per_page` query values and return the full account-agent array like Chatwoot's unpaginated `@agents = agents`, while preserving lowercased full-name ordering and account-user metadata. Focused list tests, service/repository agent tests, combined handler/service/repository tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding.
- 2026-06-06: P3.2 agent update-validation checkpoint prepared as `feat(agents): align update validation errors`; audited Chatwoot `AgentsController#update`, `User` name presence validation, and `RequestExceptionHandler#render_record_invalid`. Explicit blank `agent[name]` updates now return Chatwoot `422 { message, attributes }`, preserve the existing user name, and continue to allow omitted names plus explicit `auto_offline: false`. Focused update tests, service agent tests, combined handler/service/repository tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding.
- 2026-06-06: P3.2 agent delete-scope checkpoint prepared as `feat(agents): align delete scope`; audited Chatwoot `AgentsController#destroy`, `fetch_agent`, and account-scoped `agents` relation. Agent delete now first resolves the current account membership and returns `404` for non-members, preserving users and memberships in other accounts instead of treating an unscoped delete as success. Focused delete tests, combined handler/service/repository tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding.
- 2026-06-06: P3.2 agent duplicate-create checkpoint prepared as `feat(agents): align duplicate create errors`; audited Chatwoot `AgentsController#create`, `AgentBuilder#create_account_user`, `AccountUser` uniqueness validation, and `RequestExceptionHandler#render_record_invalid`. Duplicate account membership creation through `/agents` now returns Chatwoot `422 { message, attributes }` instead of local `409`, while bulk invite duplicate skipping remains unchanged. Focused duplicate-create tests, combined handler/service/repository tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 agents/account-user drift or the next Phase 6 reference/smoke finding.
+1 -3
View File
@@ -11,7 +11,6 @@ import (
"github.com/gochat/gochat/internal/repository"
"github.com/gochat/gochat/internal/service"
applogger "github.com/gochat/gochat/pkg/logger"
"github.com/gochat/gochat/pkg/pagination"
"github.com/gochat/gochat/pkg/response"
)
@@ -37,8 +36,7 @@ func (h *AgentHandler) List(c *gin.Context) {
return
}
pg := pagination.Parse(c)
agents, total, err := h.svc.List(c.Request.Context(), accountID, pg.Offset, pg.PerPage)
agents, total, err := h.svc.List(c.Request.Context(), accountID, 0, 0)
if err != nil {
applogger.L().Errorf("List agents for account %d: %v", accountID, err)
handleServiceError(c, err)
@@ -137,6 +137,29 @@ func (s *AgentHandlerTestSuite) TestListOrdersByFullName() {
assert.Equal(s.T(), "charlie", data[2]["name"])
}
func (s *AgentHandlerTestSuite) TestListIgnoresPerPageLikeChatwoot() {
for i := 0; i < 30; i++ {
req := service.CreateAgentRequest{
Email: fmt.Sprintf("full-list-%02d@test.com", i),
Name: fmt.Sprintf("Full List %02d", i),
Role: "agent",
}
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?per_page=5", 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, 30)
assert.Equal(s.T(), "Full List 00", data[0]["name"])
assert.Equal(s.T(), "Full List 29", data[29]["name"])
}
func (s *AgentHandlerTestSuite) TestCreateAgent() {
customRoleID := uint(7)
req := service.CreateAgentRequest{
+11 -5
View File
@@ -50,15 +50,21 @@ func (r *AgentRepo) ListByAccount(ctx context.Context, accountID uint, offset, l
return nil, 0, err
}
var accountUsers []model.AccountUser
if err := r.db.WithContext(ctx).
query := r.db.WithContext(ctx).
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 {
Order("users.id ASC")
if offset > 0 {
query = query.Offset(offset)
}
if limit > 0 {
query = query.Limit(limit)
}
var accountUsers []model.AccountUser
if err := query.Find(&accountUsers).Error; err != nil {
return nil, 0, err
}