From 706c706ee421cdde4c0c00935ae7b5fb8734386b Mon Sep 17 00:00:00 2001 From: Rogee Date: Sat, 6 Jun 2026 02:00:15 +0800 Subject: [PATCH] feat(platform): align account user membership --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 12 +++-- docs/parity/gochat_routes.txt | 3 +- .../api/v1/platform_account_user_handler.go | 53 +++++++++---------- internal/handler/api/v1/platform_e2e_test.go | 31 ++++++++++- internal/repository/account_repo.go | 42 ++++++++++++++- internal/router/router.go | 1 + 6 files changed, 105 insertions(+), 37 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 30005be0..be038efd 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 `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`. +- Current tracking checkpoint: 2026-06-06 after `fbb405a feat(agents): align list ordering`, with this implementation checkpoint prepared as `feat(platform): align account user membership`. +- Latest implementation checkpoint: this checkpoint, prepared as `feat(platform): align account user membership`. - 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 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. +- Worktree status at this implementation checkpoint: P3.2 platform account-user membership drift is closed: `POST /platform/api/v1/accounts/:account_id/account_users` now follows Chatwoot `find_or_initialize_by(user_id).update!`, returns raw `AccountUser` JSON with `200 OK`, updates duplicate memberships instead of failing, and `DELETE /account_users/destroy` accepts `user_id` as the Chatwoot collection destroy route while returning empty `200 OK`. The preceding 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: 839` after adding the Chatwoot frontend-used agent `PATCH /api/v1/accounts/:account_id/agents/:agent_id` update route. +- 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`. - 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(platform): align account user membership` | Advances P3.2 platform account-user parity by matching `Platform::Api::V1::AccountUsersController`. Platform account-user index/create now return raw AccountUser JSON instead of the local `{ success, data }` envelope, create uses Chatwoot `find_or_initialize_by(user_id).update!` semantics so duplicate membership requests update role instead of failing, and the Chatwoot collection route `DELETE /platform/api/v1/accounts/:account_id/account_users/destroy?user_id=...` is registered while the legacy `/:user_id` alias remains. | `go test ./internal/handler/api/v1 -run PlatformAccountUser -count=1`; `go run ./cmd/dump_routes > docs/parity/gochat_routes.txt`; `go run ./cmd/route_parity`; `go test ./internal/handler/api/v1 ./internal/repository ./internal/router -count=1`; `go test ./...`; `git diff --check`. Route dump is `TOTAL: 840`; tracked route parity remains `270 exact, 7 parameter-compatible, 0 missing`. | Continue P3.2 permissions/invitations drift or the next Phase 6 reference/smoke finding. | | `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. | @@ -1561,7 +1562,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 `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, 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.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 | @@ -2075,6 +2076,7 @@ Verification milestone gates: ## Progress Log +- 2026-06-06: P3.2 platform account-user checkpoint prepared as `feat(platform): align account user membership`; audited Chatwoot `Platform::Api::V1::AccountUsersController` and platform routes. Platform account-user index/create now return raw AccountUser JSON, create updates existing memberships instead of failing duplicates, the user-permissible extra gate was removed to match the account-scoped reference controller, and collection destroy is available as `DELETE /account_users/destroy` with `user_id`. Focused platform account-user tests, route generation/parity, combined handler/repository/router tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 permissions/invitations drift next. - 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. diff --git a/docs/parity/gochat_routes.txt b/docs/parity/gochat_routes.txt index 02a039ee..15edb7f6 100644 --- a/docs/parity/gochat_routes.txt +++ b/docs/parity/gochat_routes.txt @@ -106,6 +106,7 @@ DELETE /api/v1/widget/labels/:label_id DELETE /auth/sign_out DELETE /platform/api/v1/accounts/:account_id DELETE /platform/api/v1/accounts/:account_id/account_users/:user_id +DELETE /platform/api/v1/accounts/:account_id/account_users/destroy DELETE /platform/api/v1/agent_bots/:id DELETE /platform/api/v1/apps/:id DELETE /platform/api/v1/apps/:id/permissibles/:permissible_id @@ -837,4 +838,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: 839 +TOTAL: 840 diff --git a/internal/handler/api/v1/platform_account_user_handler.go b/internal/handler/api/v1/platform_account_user_handler.go index 74a6eb04..88ffe02e 100644 --- a/internal/handler/api/v1/platform_account_user_handler.go +++ b/internal/handler/api/v1/platform_account_user_handler.go @@ -2,6 +2,7 @@ package v1 import ( "net/http" + "strconv" "github.com/gin-gonic/gin" @@ -60,7 +61,7 @@ func (h *PlatformAccountUserHandler) Index(c *gin.Context) { return } - response.OK(c, accountUsers) + c.JSON(http.StatusOK, accountUsers) } // Create adds a user to an account. @@ -84,39 +85,21 @@ func (h *PlatformAccountUserHandler) Create(c *gin.Context) { } var req struct { - UserID uint `json:"user_id" binding:"required"` - Role string `json:"role,omitempty"` + UserID uint `json:"user_id" form:"user_id" binding:"required"` + Role *string `json:"role,omitempty" form:"role"` } - if err := c.ShouldBindJSON(&req); err != nil { + if err := c.ShouldBind(&req); err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, err.Error()) return } - // Verify PlatformApp has permissible access to this user - userPerm, err := h.permissibleRepo.FindByPlatformAppAndResource(c.Request.Context(), platformAppID, model.PermissibleTypeUser, req.UserID) - if err != nil || userPerm == nil { - response.AbortWithStatusError(c, http.StatusForbidden, response.ErrForbidden, "non permissible resource for user") - return - } - - role := req.Role - if role == "" { - role = "agent" - } - - if err := h.accountRepo.AddUserToAccount(c.Request.Context(), accountID, req.UserID, role); err != nil { + acctUser, err := h.accountRepo.UpsertAccountUser(c.Request.Context(), accountID, req.UserID, req.Role) + if err != nil { response.AbortWithStatusError(c, http.StatusInternalServerError, response.ErrInternal, err.Error()) return } - // Return the created AccountUser - acctUser, err := h.accountRepo.FindAccountUserByUserAndAccount(c.Request.Context(), accountID, req.UserID) - if err != nil { - response.OK(c, gin.H{"account_id": accountID, "user_id": req.UserID, "role": role}) - return - } - - response.Created(c, acctUser) + c.JSON(http.StatusOK, acctUser) } // Destroy removes a user from an account. @@ -130,8 +113,8 @@ func (h *PlatformAccountUserHandler) Destroy(c *gin.Context) { return } - userID, err := parseUintParam(c, "user_id") - if err != nil { + userID, err := platformAccountUserID(c) + if err != nil || userID == 0 { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid user ID") return } @@ -150,5 +133,19 @@ func (h *PlatformAccountUserHandler) Destroy(c *gin.Context) { return } - response.NoContent(c) + c.Status(http.StatusOK) +} + +func platformAccountUserID(c *gin.Context) (uint, error) { + if raw := c.Param("user_id"); raw != "" { + id, err := strconv.ParseUint(raw, 10, 32) + return uint(id), err + } + var req struct { + UserID uint `json:"user_id" form:"user_id"` + } + if err := c.ShouldBind(&req); err != nil { + return 0, err + } + return req.UserID, nil } diff --git a/internal/handler/api/v1/platform_e2e_test.go b/internal/handler/api/v1/platform_e2e_test.go index 0c5497e7..4a953acf 100644 --- a/internal/handler/api/v1/platform_e2e_test.go +++ b/internal/handler/api/v1/platform_e2e_test.go @@ -97,6 +97,7 @@ func setupPlatformTokenTestE2E(t *testing.T) (*gin.Engine, *repository.Permissib // Gin wildcard constraint: nested routes under accounts/:id must reuse :id. platformGroup.GET("/accounts/:id/account_users", platformAccountUser.Index) platformGroup.POST("/accounts/:id/account_users", platformAccountUser.Create) + platformGroup.DELETE("/accounts/:id/account_users/destroy", platformAccountUser.Destroy) platformGroup.DELETE("/accounts/:id/account_users/:user_id", platformAccountUser.Destroy) return engine, permissibleRepo, userRepo, accountRepo @@ -403,7 +404,22 @@ func TestPlatformAccountUserE2E_Create(t *testing.T) { req.Header.Set("Content-Type", "application/json") engine.ServeHTTP(w, req) - assert.Equal(t, http.StatusCreated, w.Code) + require.Equal(t, http.StatusOK, w.Code) + var accountUser map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &accountUser)) + assert.Equal(t, "agent", accountUser["role"]) + assert.NotContains(t, accountUser, "success") + + // Chatwoot find_or_initialize_by updates existing memberships instead of failing duplicates. + acctUserBody = fmt.Sprintf(`{"user_id": %s, "role": "administrator"}`, userID) + w = httptest.NewRecorder() + req, _ = http.NewRequest("POST", "/platform/api/v1/accounts/"+accountID+"/account_users", bytes.NewBufferString(acctUserBody)) + req.Header.Set("Content-Type", "application/json") + engine.ServeHTTP(w, req) + + require.Equal(t, http.StatusOK, w.Code) + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &accountUser)) + assert.Equal(t, "administrator", accountUser["role"]) } func TestPlatformAccountUserE2E_Index(t *testing.T) { @@ -435,12 +451,23 @@ func TestPlatformAccountUserE2E_Index(t *testing.T) { req, _ = http.NewRequest("POST", "/platform/api/v1/accounts/"+accountID+"/account_users", bytes.NewBufferString(acctUserBody)) req.Header.Set("Content-Type", "application/json") engine.ServeHTTP(w, req) - require.Equal(t, http.StatusCreated, w.Code) + require.Equal(t, http.StatusOK, w.Code) // Index account_users w = httptest.NewRecorder() req, _ = http.NewRequest("GET", "/platform/api/v1/accounts/"+accountID+"/account_users", nil) engine.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + var accountUsers []map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &accountUsers)) + require.Len(t, accountUsers, 1) + assert.Equal(t, "agent", accountUsers[0]["role"]) + assert.NotContains(t, accountUsers[0], "success") + + // Chatwoot destroy is a collection route: DELETE /account_users/destroy with user_id param. + w = httptest.NewRecorder() + req, _ = http.NewRequest("DELETE", "/platform/api/v1/accounts/"+accountID+"/account_users/destroy?user_id="+userID, nil) + engine.ServeHTTP(w, req) assert.Equal(t, http.StatusOK, w.Code) } diff --git a/internal/repository/account_repo.go b/internal/repository/account_repo.go index 536daf70..e8547404 100644 --- a/internal/repository/account_repo.go +++ b/internal/repository/account_repo.go @@ -100,6 +100,46 @@ func (r *AccountRepo) AddUserToAccount(ctx context.Context, accountID, userID ui return r.db.WithContext(ctx).Create(&au).Error } +// UpsertAccountUser creates or updates an AccountUser membership. +// Reference: Chatwoot Platform::Api::V1::AccountUsersController#create uses +// account.account_users.find_or_initialize_by(user_id: ...).update!(params). +func (r *AccountRepo) UpsertAccountUser(ctx context.Context, accountID, userID uint, role *string) (*model.AccountUser, error) { + var au model.AccountUser + err := r.db.WithContext(ctx). + Where("account_id = ? AND user_id = ?", accountID, userID). + First(&au).Error + if err != nil && err != gorm.ErrRecordNotFound { + return nil, err + } + + if err == gorm.ErrRecordNotFound { + au = model.AccountUser{ + AccountID: accountID, + UserID: userID, + Role: "agent", + Availability: "offline", + AutoOffline: true, + } + } + if role != nil && *role != "" { + au.Role = *role + } + + if err == gorm.ErrRecordNotFound { + if err := r.db.WithContext(ctx).Create(&au).Error; err != nil { + return nil, err + } + return &au, nil + } + + if role != nil { + if err := r.db.WithContext(ctx).Model(&au).Update("role", au.Role).Error; err != nil { + return nil, err + } + } + return &au, nil +} + // RemoveUserFromAccount deletes an AccountUser association. func (r *AccountRepo) RemoveUserFromAccount(ctx context.Context, accountID, userID uint) error { return r.db.WithContext(ctx).Exec( @@ -171,4 +211,4 @@ func (r *AccountRepo) FindAccountUserByUserAndAccount(ctx context.Context, accou return nil, err } return &au, nil -} \ No newline at end of file +} diff --git a/internal/router/router.go b/internal/router/router.go index 08630127..c90470aa 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -1830,6 +1830,7 @@ func registerPlatformTokenRoutes(g *gin.RouterGroup, h *Handlers) { // Reference: Chatwoot Platform::Api::V1::AccountUsersController (nested under accounts) g.GET("/accounts/:account_id/account_users", h.PlatformAccountUser.Index) g.POST("/accounts/:account_id/account_users", h.PlatformAccountUser.Create) + g.DELETE("/accounts/:account_id/account_users/destroy", h.PlatformAccountUser.Destroy) g.DELETE("/accounts/:account_id/account_users/:user_id", h.PlatformAccountUser.Destroy) }