From c30020d8a8757bcf3dec4a159e9c0a81c85d2074 Mon Sep 17 00:00:00 2001 From: Rogee Date: Sat, 6 Jun 2026 01:43:14 +0800 Subject: [PATCH] feat(agents): align update route semantics --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 12 ++++--- docs/parity/gochat_routes.txt | 3 +- internal/handler/api/v1/agent_handler_test.go | 34 ++++++++++++++----- internal/repository/agent_repo.go | 8 ++--- internal/router/router.go | 3 +- internal/service/agent_service.go | 17 ++++++---- 6 files changed, 51 insertions(+), 26 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 755411c1..ff4c186d 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -16,12 +16,12 @@ Build GoChat as a Go backend that can directly reuse the frontend from `referenc ## Current Baseline -- Current tracking checkpoint: 2026-06-06 after `1b76f8c feat(custom-roles): align update route semantics`, with this implementation checkpoint prepared as `feat(agents): align bulk invite behavior`. -- Latest implementation checkpoint: this checkpoint, prepared as `feat(agents): align bulk invite behavior`. +- 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`. - 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 bulk invite drift is closed for the reused onboarding/settings client: `POST /agents/bulk_create` now skips invalid email entries like Chatwoot's rescued `AgentBuilder` loop, returns empty `200 OK` for empty/no-op arrays, creates valid agents from the same request, and clears the local onboarding step after the bulk flow. The preceding B10.3 CustomRole frontend update drift and Phase 6 contacts pagination/active payload drift 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 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. - `go test ./...` passes. -- Route dump succeeds with `TOTAL: 838` after adding the Chatwoot custom-role `PATCH /api/v1/accounts/:account_id/custom_roles/:id` update route. +- 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`. - Tracked frontend-critical route audit covers 277 Chatwoot routes: 270 exact, 0 method-compatible, 7 parameter-compatible, 0 missing. The 7 parameter-compatible routes are Gin-internal parameter-name differences for nested AgentCapacityPolicy users/inbox limits; the external URL shape is equivalent. - `/api/v1/widget` stubs are burned down and public inbox/contact/conversation/message core flows are backed by real handlers. @@ -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 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. | | `feat(contacts): align active pagination payloads` | Advances Phase 6 dashboard CRM drift by aligning contact list/search/filter/active pagination with Chatwoot `ContactsController::RESULTS_PER_PAGE = 15` and `contacts/active.json.jbuilder`. The reused dashboard active-contacts route now returns `{ meta: { count, current_page }, payload }` using the contact serializer instead of the local `{ contacts, meta: { page, page_size } }` envelope, and selected-key `POST /contacts/:id/destroy_custom_attributes` is covered against Chatwoot's `{ payload }` response with `contact_inboxes`. | `go test ./internal/handler/api/v1 -run 'ContactHandlerCRUD' -count=1`; `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. | @@ -1559,7 +1560,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, and agents bulk invite best-effort behavior 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 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 | @@ -2073,6 +2074,7 @@ Verification milestone gates: ## Progress Log +- 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. - 2026-06-06: Phase 6 contacts active/pagination checkpoint prepared as `feat(contacts): align active pagination payloads`; audited Chatwoot `ContactsController`, `contacts/index.json.jbuilder`, `contacts/search.json.jbuilder`, `contacts/filter.json.jbuilder`, `contacts/active.json.jbuilder`, `contacts/destroy_custom_attributes.json.jbuilder`, and dashboard `contacts.js`. Contact list/search/filter/active now use Chatwoot's fixed `RESULTS_PER_PAGE = 15`; active contacts now return `{ meta: { count, current_page }, payload }` through the contact serializer instead of the local `{ contacts, meta: { page, page_size } }` envelope; selected-key `destroy_custom_attributes` payload coverage confirms the Chatwoot `{ payload }` response and `contact_inboxes` inclusion. Focused handler tests, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue Phase 6 placeholder audit next. diff --git a/docs/parity/gochat_routes.txt b/docs/parity/gochat_routes.txt index 7a2d75ff..02a039ee 100644 --- a/docs/parity/gochat_routes.txt +++ b/docs/parity/gochat_routes.txt @@ -447,6 +447,7 @@ GET /ws PATCH /api/v1/accounts/:account_id/agent_bot_inboxes/:agent_bot_inbox_id/status PATCH /api/v1/accounts/:account_id/agent_capacity_policies/:id PATCH /api/v1/accounts/:account_id/agent_capacity_policies/:id/inbox_limits/:limit_id +PATCH /api/v1/accounts/:account_id/agents/:agent_id PATCH /api/v1/accounts/:account_id/channels/facebook_channel/:fb_id PATCH /api/v1/accounts/:account_id/companies/:company_id PATCH /api/v1/accounts/:account_id/contacts/:contact_id @@ -836,4 +837,4 @@ PUT /public/api/v1/csat_survey/:id PUT /public/api/v1/inboxes/:inbox_id/contacts/:contact_id PUT /public/api/v1/inboxes/:inbox_id/contacts/:contact_id/conversations/:conversation_id/messages/:message_id PUT /widget/direct_uploads/:upload_uuid -TOTAL: 838 +TOTAL: 839 diff --git a/internal/handler/api/v1/agent_handler_test.go b/internal/handler/api/v1/agent_handler_test.go index c0956661..7eba3d60 100644 --- a/internal/handler/api/v1/agent_handler_test.go +++ b/internal/handler/api/v1/agent_handler_test.go @@ -196,9 +196,10 @@ func (s *AgentHandlerTestSuite) TestGetAgentNotFound() { func (s *AgentHandlerTestSuite) TestUpdateAgent() { // Create an agent req := service.CreateAgentRequest{ - Email: "agent4@test.com", - Name: "Agent Four", - Role: "agent", + Email: "agent4@test.com", + Name: "Agent Four", + Role: "agent", + AutoOffline: true, } w, c := s.makeRequest("POST", "/api/v1/accounts/1/agents", req, s.account.ID, s.user.ID) s.handler.Create(c) @@ -209,12 +210,14 @@ func (s *AgentHandlerTestSuite) TestUpdateAgent() { agentID := uint(data["id"].(float64)) // Update the agent - updateReq := service.UpdateAgentRequest{ - Name: "Updated Name", - Role: "administrator", - Availability: "online", + updateReq := map[string]any{ + "agent": map[string]any{ + "name": "Updated Name", + "role": "administrator", + "availability": "online", + }, } - w2, c2 := s.makeRequest("PUT", fmt.Sprintf("/api/v1/accounts/1/agents/%d", agentID), updateReq, s.account.ID, s.user.ID) + w2, c2 := s.makeRequest("PATCH", fmt.Sprintf("/api/v1/accounts/1/agents/%d", agentID), updateReq, s.account.ID, s.user.ID) c2.Params = append(c2.Params, gin.Param{Key: "id", Value: fmt.Sprintf("%d", agentID)}) s.handler.Update(c2) @@ -224,6 +227,21 @@ func (s *AgentHandlerTestSuite) TestUpdateAgent() { assert.Equal(s.T(), "Updated Name", updateData["name"]) assert.Equal(s.T(), "administrator", updateData["role"]) assert.Equal(s.T(), "online", updateData["availability_status"]) + assert.Equal(s.T(), true, updateData["auto_offline"]) + + // Explicit false should still update, matching Rails compact semantics. + disableReq := map[string]any{ + "agent": map[string]any{ + "auto_offline": false, + }, + } + w3, c3 := s.makeRequest("PATCH", fmt.Sprintf("/api/v1/accounts/1/agents/%d", agentID), disableReq, s.account.ID, s.user.ID) + s.handler.Update(c3) + + assert.Equal(s.T(), http.StatusOK, w3.Code) + var disableData map[string]interface{} + json.Unmarshal(w3.Body.Bytes(), &disableData) + assert.Equal(s.T(), false, disableData["auto_offline"]) } func (s *AgentHandlerTestSuite) TestDeleteAgent() { diff --git a/internal/repository/agent_repo.go b/internal/repository/agent_repo.go index f9609002..2d239ba3 100644 --- a/internal/repository/agent_repo.go +++ b/internal/repository/agent_repo.go @@ -188,7 +188,7 @@ func (r *AgentRepo) CreateAgent(ctx context.Context, accountID uint, inviterID u // UpdateAgent updates both the User (name) and AccountUser (role, availability, auto_offline). // Reference: Chatwoot agents_controller.rb#update → agent.update!(name) + current_account_user.update!(role, availability, auto_offline) -func (r *AgentRepo) UpdateAgent(ctx context.Context, userID, accountID uint, name, role, availability string, autoOffline bool, customRoleID *uint, customRoleIDSet bool) (*AgentDetail, error) { +func (r *AgentRepo) UpdateAgent(ctx context.Context, userID, accountID uint, name, role, availability string, autoOffline bool, autoOfflineSet bool, customRoleID *uint, customRoleIDSet bool) (*AgentDetail, error) { // Update user name if provided if name != "" { if err := r.db.WithContext(ctx). @@ -214,9 +214,9 @@ func (r *AgentRepo) UpdateAgent(ctx context.Context, userID, accountID uint, nam updates["custom_role_id"] = *customRoleID } } - // Chatwoot always sends auto_offline (even false) via .compact filter - // so we always update it when the request includes it - updates["auto_offline"] = autoOffline + if autoOfflineSet { + updates["auto_offline"] = autoOffline + } if len(updates) > 0 { if err := r.db.WithContext(ctx). Model(&model.AccountUser{}). diff --git a/internal/router/router.go b/internal/router/router.go index 576f4960..08630127 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -590,7 +590,8 @@ func registerV1Routes(g *gin.RouterGroup, h *Handlers) { accounts.GET("/:account_id/agents", h.Agent.List) // Chatwoot: POST /api/v1/accounts/:account_id/agents (create) accounts.POST("/:account_id/agents", h.Agent.Create) - // Chatwoot: PUT /api/v1/accounts/:account_id/agents/:id (update) + // Chatwoot: PATCH/PUT /api/v1/accounts/:account_id/agents/:id (update) + accounts.PATCH("/:account_id/agents/:agent_id", h.Agent.Update) accounts.PUT("/:account_id/agents/:agent_id", h.Agent.Update) // Chatwoot: DELETE /api/v1/accounts/:account_id/agents/:id (destroy) accounts.DELETE("/:account_id/agents/:agent_id", h.Agent.Delete) diff --git a/internal/service/agent_service.go b/internal/service/agent_service.go index 58e53041..1377e309 100644 --- a/internal/service/agent_service.go +++ b/internal/service/agent_service.go @@ -49,12 +49,13 @@ type CreateAgentRequest struct { // UpdateAgentRequest is the DTO for updating an agent. // Reference: Chatwoot agents_controller.rb#update → agent_params (name on User, role/availability/auto_offline on AccountUser) type UpdateAgentRequest struct { - Name string `json:"name,omitempty" validate:"omitempty,min=1"` - Role string `json:"role,omitempty" validate:"omitempty,oneof=agent administrator"` - Availability string `json:"availability,omitempty" validate:"omitempty,oneof=online offline busy"` - AutoOffline bool `json:"auto_offline"` - CustomRoleID *uint `json:"custom_role_id,omitempty"` - customRoleSet bool + Name string `json:"name,omitempty" validate:"omitempty,min=1"` + Role string `json:"role,omitempty" validate:"omitempty,oneof=agent administrator"` + Availability string `json:"availability,omitempty" validate:"omitempty,oneof=online offline busy"` + AutoOffline bool `json:"auto_offline"` + CustomRoleID *uint `json:"custom_role_id,omitempty"` + autoOfflineSet bool + customRoleSet bool } func (r *UpdateAgentRequest) UnmarshalJSON(data []byte) error { @@ -68,10 +69,12 @@ func (r *UpdateAgentRequest) UnmarshalJSON(data []byte) error { return err } *r = UpdateAgentRequest(decoded) + _, r.autoOfflineSet = raw["auto_offline"] _, r.customRoleSet = raw["custom_role_id"] return nil } +func (r UpdateAgentRequest) AutoOfflineSet() bool { return r.autoOfflineSet } func (r UpdateAgentRequest) CustomRoleIDSet() bool { return r.customRoleSet } // BulkCreateAgentRequest is the DTO for bulk creating agents. @@ -128,7 +131,7 @@ func (s *AgentService) Update(ctx context.Context, userID, accountID uint, req U return nil, fmt.Errorf("validation: %w", err) } - return s.agentRepo.UpdateAgent(ctx, userID, accountID, req.Name, req.Role, req.Availability, req.AutoOffline, req.CustomRoleID, req.CustomRoleIDSet()) + return s.agentRepo.UpdateAgent(ctx, userID, accountID, req.Name, req.Role, req.Availability, req.AutoOffline, req.AutoOfflineSet(), req.CustomRoleID, req.CustomRoleIDSet()) } // Delete removes an agent from an account (deletes AccountUser, optionally deletes User).