From 76604353f8a4ad105e3b833c31643156b59ef6ca Mon Sep 17 00:00:00 2001 From: Rogee Date: Sat, 6 Jun 2026 04:59:46 +0800 Subject: [PATCH] feat(inboxes): align agent bot member actions --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 10 +- .../inbox_agentbot_avatar_campaigns_test.go | 86 +++++++++++++++ internal/handler/api/v1/inbox_handler.go | 46 +++++--- internal/service/inbox_service.go | 100 +++++++++++------- internal/service/inbox_service_test.go | 43 ++++---- 5 files changed, 211 insertions(+), 74 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 1de3b463..38e11210 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -37,10 +37,10 @@ Hermes plan landing map: ## Current Baseline -- Current tracking checkpoint: 2026-06-06 after this implementation checkpoint, prepared as `feat(messages): align retry status parity`. -- Latest implementation checkpoint: this checkpoint, prepared as `feat(messages): align retry status parity`. +- Current tracking checkpoint: 2026-06-06 after this implementation checkpoint, prepared as `feat(inboxes): align agent bot member actions`. +- Latest implementation checkpoint: this checkpoint, prepared as `feat(inboxes): align agent bot member actions`. - Latest documentation/tooling checkpoint: this checkpoint, prepared as `docs: expand help center route parity tracking`; this document is now the active follow-up plan and supersedes `.hermes/plans/*`. -- Worktree status at this implementation checkpoint: P6 message retry parity now matches Chatwoot `MessagesController#retry`: retry sets the message status back to `sent`, clears provider failure data from `content_attributes`, and queues the existing durable `SendReplyJob` path when a worker pool is configured. P6 conversation custom-attribute response parity already matches Chatwoot `ConversationsController#custom_attributes`, `custom_attributes.json.jbuilder`, and the reused dashboard conversation store action. Route parity tracking includes the Chatwoot help-center portal/category/article route family from `reference/chatwoot/config/routes.rb:385-404`, so the audited critical set covers portal CRUD/member actions, nested categories, nested articles, and article bulk actions. P3.7c help center article parity matches the reused dashboard `helpCenter/articles.js` client and Chatwoot `ArticlesController`/`Articles::BulkActionsController`/Jbuilder surface. Campaign create/update already mirrors Chatwoot `Campaign#ensure_correct_campaign_attributes`; account campaign CRUD already matches the reused dashboard `campaigns` store and Chatwoot Jbuilder contract. Agent/settings/profile/notification/contact/custom-field/report enterprise drifts listed below remain at their prior checkpoint states. 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 another Phase 6 placeholder burn-down item or a new Phase 2/3 drift slice from fresh reference/smoke evidence. +- Worktree status at this implementation checkpoint: P6.5 inbox agent-bot member actions now match Chatwoot `InboxesController#agent_bot`/`#set_agent_bot` and the reused dashboard `agentBots` store path: `set_agent_bot` accepts the frontend `{ agent_bot: id|null }` body, returns empty `200 OK`, disconnect destroys the `agent_bot_inboxes` row, reassign updates the inbox's single binding, and `agent_bot` returns the Chatwoot `{ agent_bot: ... }` partial shape with `{}` when disconnected. P6 message retry parity already matches Chatwoot `MessagesController#retry`: retry sets the message status back to `sent`, clears provider failure data from `content_attributes`, and queues the existing durable `SendReplyJob` path when a worker pool is configured. P6 conversation custom-attribute response parity already matches Chatwoot `ConversationsController#custom_attributes`, `custom_attributes.json.jbuilder`, and the reused dashboard conversation store action. Route parity tracking includes the Chatwoot help-center portal/category/article route family from `reference/chatwoot/config/routes.rb:385-404`, so the audited critical set covers portal CRUD/member actions, nested categories, nested articles, and article bulk actions. P3.7c help center article parity matches the reused dashboard `helpCenter/articles.js` client and Chatwoot `ArticlesController`/`Articles::BulkActionsController`/Jbuilder surface. Campaign create/update already mirrors Chatwoot `Campaign#ensure_correct_campaign_attributes`; account campaign CRUD already matches the reused dashboard `campaigns` store and Chatwoot Jbuilder contract. Agent/settings/profile/notification/contact/custom-field/report enterprise drifts listed below remain at their prior checkpoint states. 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 another Phase 6 placeholder burn-down item or a new Phase 2/3 drift slice from fresh reference/smoke evidence. - `go test ./...` passes. - Route dump succeeds with `TOTAL: 860` after adding no-trailing-slash article index/create aliases, frontend-used article PATCH update route, and frontend article bulk-action routes. - Route parity artifacts now exist under `docs/parity/` and are generated by `cmd/route_parity`. @@ -169,6 +169,7 @@ This ledger records the committed parity checkpoints that future slices should b | Commit | Scope | Verification summary | Follow-up state | | --- | --- | --- | --- | +| `feat(inboxes): align agent bot member actions` | Advances P6.5 inbox API parity by aligning `GET /inboxes/:id/agent_bot` and `POST /inboxes/:id/set_agent_bot` with Chatwoot `InboxesController#agent_bot`/`#set_agent_bot`, `inboxes/agent_bot.json.jbuilder`, `_agent_bot.json.jbuilder`, and the reused dashboard `agentBots` store/API path. The setter accepts Chatwoot `{ agent_bot: id|null }` bodies, keeps legacy `{ agent_bot_id }` compatibility, treats omitted/null params as disconnect, returns empty `200 OK`, destroys disconnected bindings instead of leaving inactive local rows, and updates the inbox's single binding on reassignment. The getter now returns `{ agent_bot: {} }` when disconnected and serializes assigned bots with Chatwoot fields such as `thumbnail`, `bot_type`, `bot_config`, and `outgoing_url`. | `go test ./internal/service -run 'InboxService_SetAgentBot' -count=1`; `go test ./internal/handler/api/v1 -run 'InboxHandler_Set.*AgentBot\|InboxHandler_GetAgentBot\|InboxSetAgentBot' -count=1`; `go test ./internal/handler/api/v1 ./internal/service -run 'Inbox.*AgentBot\|SetAgentBot' -count=1`; `go test ./...`; `git diff --check`. No route changes. | P6.5 remains Doing for additional inbox/channel drift; continue Phase 6 placeholder audit or the next evidence-backed inbox member action gap. | | `feat(messages): align retry status parity` | Advances P6 message API parity by matching Chatwoot `MessagesController#retry`, `messages/retry.json.jbuilder`, and the reused dashboard `MessageApi.retry` path. Message retry now resets failed messages to `sent`, clears `content_attributes` to `{}`, returns the Chatwoot message serializer, and queues the existing durable `message:send_reply` path when a WorkerPool is configured instead of leaving local `retrying` state/provider failure details visible. | `go test ./internal/service -run MessageService_Retry -count=1`; `go test ./internal/handler/api/v1 -run 'MessageHandlerTestSuite/TestRetry' -count=1`; `go test ./internal/handler/api/v1 ./internal/service -run 'Message.*Retry' -count=1`; `go test ./...`; `git diff --check`. | P6.4 message retry drift moves to Review; continue Phase 6 placeholder burn-down or fresh evidence-backed Phase 2/3 drift. | | `feat(conversations): align custom attribute response` | Advances P6 conversation response parity by matching Chatwoot `ConversationsController#custom_attributes`, `custom_attributes.json.jbuilder`, and the reused dashboard `ConversationApi.updateCustomAttributes` store action. The account conversation custom-attributes route now returns only `{ custom_attributes: ... }`, excludes local conversation/payload/meta wrappers, and converts empty/null JSON to `{}` for frontend merge stability. The placeholder audit was rerun and still shows only webhook handler-not-injected fallbacks calling `chatwootParityStub`. | `go test ./internal/handler/api/v1 -run 'ConversationHandler.*UpdateCustomAttributes\|TestConversationHandler' -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -run 'UpdateCustomAttributes\|ConversationHandler\|ConversationService\|Router' -count=1`; `rg -n "chatwootParityStub\|TODO.*placeholder\|placeholder\|stub" internal/handler internal/service internal/router -g'*.go'`; `go test ./...`; `git diff --check`. | P6 conversation custom-attribute response drift moves to Review; continue Phase 6 placeholder burn-down from the remaining webhook nil-handler fallbacks or fresh reference/smoke evidence. | | `docs: expand help center route parity tracking` | Audit/tooling checkpoint that expands `cmd/route_parity` to track the full Chatwoot help-center portal/category/article route family from `reference/chatwoot/config/routes.rb:385-404`, including portal member actions, nested category CRUD/reorder, nested article CRUD/reorder, and article bulk actions. Regenerated `docs/parity/route_parity.md` now reports `298 exact, 0 method-compatible, 7 parameter-compatible, 0 missing out of 305 tracked critical routes`; `docs/parity/gochat_routes.txt` remains `TOTAL: 860`. | `go run ./cmd/route_parity`; `go test ./cmd/route_parity -count=1`; `git diff --check`. | Continue Phase 6 placeholder burn-down or the next evidence-backed Phase 2/3 drift slice; keep route artifacts regenerated whenever the tracked set changes. | @@ -2036,7 +2037,7 @@ Tracking table: | P6.2 | Contact APIs | `docs/ROUTE_GAP_ANALYSIS.md`, contact handlers/services | Finish merge, import/export/data-import, notes serializer depth, and Meilisearch-backed CRM search. | Doing | | P6.3 | Conversation APIs | `docs/ROUTE_GAP_ANALYSIS.md`, conversation handlers/services | Frontend-critical filters, assignment, status, snooze, merge, bulk actions, transcript responses, and custom-attribute responses are implemented or covered by focused drift slices; continue only from fresh reference/smoke evidence. | Review | | P6.4 | Message APIs | `docs/ROUTE_GAP_ANALYSIS.md`, message handlers/services | Create/list/delete/private notes/attachments/source attribution have focused coverage; retry now matches Chatwoot status reset/content-attribute clearing and durable send-reply enqueue behavior. Continue only from fresh reference/smoke evidence. | Review | -| P6.5 | Inbox APIs | `docs/ROUTE_GAP_ANALYSIS.md`, inbox handlers/services | Implement CRUD, assignable agents, avatar, campaigns, channel settings, reset secret. WhatsApp Cloud health/register-webhook/sync-template member actions now match the reference concern. | Doing | +| P6.5 | Inbox APIs | `docs/ROUTE_GAP_ANALYSIS.md`, inbox handlers/services | Implement CRUD, assignable agents, avatar, campaigns, channel settings, reset secret. WhatsApp Cloud health/register-webhook/sync-template member actions now match the reference concern; agent-bot get/set member actions now accept Chatwoot `{ agent_bot }` payloads, return `head :ok`, destroy disconnected bindings, and serialize `{ agent_bot }` through the Chatwoot partial shape. | Doing | | P6.6 | Widget/public APIs | `docs/ROUTE_GAP_ANALYSIS.md`, widget/channel provider code, `chatwootParityStub` routes | Widget/public frontend-critical route behavior is handler-backed, including public inbox flow, direct uploads/attachments, and public CSAT survey submission. | Done | | P6.7 | Webhook ingress | `internal/router/router.go`, `internal/handler/webhook/*`, channel providers | Replace generic placeholder with provider-specific verified ingestion and dispatch. | Done | @@ -2123,6 +2124,7 @@ Verification milestone gates: ## Progress Log +- 2026-06-06: P6.5 inbox agent-bot checkpoint prepared as `feat(inboxes): align agent bot member actions`; audited Chatwoot `InboxesController#agent_bot`, `InboxesController#set_agent_bot`, `inboxes/agent_bot.json.jbuilder`, `_agent_bot.json.jbuilder`, and the reused dashboard `agentBots` store/API. `POST /inboxes/:id/set_agent_bot` now accepts the frontend `{ agent_bot: id|null }` body, keeps legacy `{ agent_bot_id }` compatibility, treats omitted/null bodies as disconnect, returns empty `200 OK`, destroys disconnected bindings like Chatwoot `@inbox.agent_bot_inbox.destroy!`, and updates the inbox's single binding when reassigning bots. `GET /inboxes/:id/agent_bot` now returns `{ agent_bot: {} }` when disconnected and Chatwoot partial fields such as `thumbnail`, `bot_type`, `bot_config`, and `outgoing_url` when assigned. Focused inbox service/handler tests, combined handler/service tests, full `go test ./...`, and `git diff --check` passed; continue Phase 6 placeholder burn-down or the next evidence-backed inbox drift. - 2026-06-06: P6 message retry checkpoint prepared as `feat(messages): align retry status parity`; audited Chatwoot `MessagesController#retry`, `messages/retry.json.jbuilder`, and reused dashboard `api/inbox/message.js`. Account message retry now resets status to `sent`, clears `content_attributes` to `{}`, returns the normal Chatwoot message serializer, and queues the existing durable send-reply job path when a WorkerPool is configured instead of exposing local `retrying` state/provider failure details. Focused retry service/handler tests, combined handler/service retry tests, full `go test ./...`, and `git diff --check` passed; continue Phase 6 placeholder burn-down or a fresh evidence-backed Phase 2/3 drift slice. - 2026-06-06: P6 conversation custom-attribute response checkpoint prepared as `feat(conversations): align custom attribute response`; audited Chatwoot `ConversationsController#custom_attributes`, `custom_attributes.json.jbuilder`, reused dashboard `api/inbox/conversation.js`, and the conversation store action that reads `response.data.custom_attributes`. Account conversation custom-attribute updates now return only `{ custom_attributes: ... }`, exclude local conversation/payload/meta wrappers, and serialize empty/null custom attributes as `{}` for frontend merge stability. Focused handler tests, combined handler/service/router tests, placeholder audit, full `go test ./...`, and `git diff --check` passed; continue Phase 6 placeholder burn-down or a fresh evidence-backed Phase 2/3 drift slice. - 2026-06-06: Audit/tooling checkpoint prepared as `docs: expand help center route parity tracking`; expanded `cmd/route_parity` to cover the full Chatwoot help-center portal/category/article route family from `reference/chatwoot/config/routes.rb:385-404`, including portal CRUD/member actions, nested category CRUD/reorder, nested article CRUD/reorder, and article bulk actions. Regenerated route parity reports `298 exact, 0 method-compatible, 7 parameter-compatible, 0 missing out of 305 tracked critical routes`; route dump remains `TOTAL: 860`. Verification for this checkpoint: `go run ./cmd/route_parity`, `go test ./cmd/route_parity -count=1`, and `git diff --check`; continue Phase 6 placeholder burn-down or a fresh evidence-backed Phase 2/3 drift slice. diff --git a/internal/handler/api/v1/inbox_agentbot_avatar_campaigns_test.go b/internal/handler/api/v1/inbox_agentbot_avatar_campaigns_test.go index 75884ec5..87c0f04b 100644 --- a/internal/handler/api/v1/inbox_agentbot_avatar_campaigns_test.go +++ b/internal/handler/api/v1/inbox_agentbot_avatar_campaigns_test.go @@ -1,13 +1,23 @@ package v1 import ( + "bytes" + "encoding/json" "net/http" "net/http/httptest" + "strconv" + "strings" "testing" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/repository" "github.com/gochat/gochat/internal/service" ) @@ -15,12 +25,39 @@ func setupInboxAgentBotRouter(handler *InboxHandler) *gin.Engine { gin.SetMode(gin.TestMode) r := gin.New() r.Use(gin.Recovery()) + r.POST("/api/v1/accounts/:account_id/inboxes/:inbox_id/set_agent_bot", handler.SetAgentBot) r.GET("/api/v1/accounts/:account_id/inboxes/:inbox_id/agent_bot", handler.GetAgentBot) r.DELETE("/api/v1/accounts/:account_id/inboxes/:inbox_id/avatar", handler.DeleteAvatar) r.GET("/api/v1/accounts/:account_id/inboxes/:inbox_id/campaigns", handler.ListCampaigns) return r } +func setupInboxAgentBotDB(t *testing.T) (*gin.Engine, *gorm.DB, *model.Account, *model.Inbox, *model.AgentBot) { + t.Helper() + dbName := "file:" + strings.ReplaceAll(t.Name(), "/", "_") + "?mode=memory&cache=shared" + db, err := gorm.Open(sqlite.Open(dbName), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + t.Cleanup(func() { + sqlDB, dbErr := db.DB() + if dbErr == nil { + sqlDB.Close() + } + }) + require.NoError(t, db.AutoMigrate(&model.Account{}, &model.Inbox{}, &model.AgentBot{}, &model.AgentBotInbox{})) + account := &model.Account{Name: "Agent bot org", Locale: "en", Active: true} + require.NoError(t, db.Create(account).Error) + inbox := &model.Inbox{AccountID: account.ID, Name: "Support", ChannelType: "web_widget", ChannelID: 1} + require.NoError(t, db.Create(inbox).Error) + bot := &model.AgentBot{AccountID: &account.ID, Name: "Triage bot", Description: "Routes chats", AvatarURL: "https://example.test/bot.png", OutgoingURL: "https://example.test/hook", BotType: "webhook", Config: json.RawMessage(`{"handoff":true}`), AccessToken: "access-token", Secret: "secret"} + require.NoError(t, db.Create(bot).Error) + svc := service.NewInboxService(repository.NewInboxRepo(db), repository.NewAgentBotInboxRepo(db), repository.NewAgentBotRepo(db), nil, nil, nil, nil) + return setupInboxAgentBotRouter(NewInboxHandler(svc)), db, account, inbox, bot +} + +func inboxAgentBotTestID(id uint) string { + return strconv.FormatUint(uint64(id), 10) +} + // --- GetAgentBot tests --- func TestInboxHandler_GetAgentBot_InvalidAccountID(t *testing.T) { @@ -57,6 +94,55 @@ func TestInboxHandler_GetAgentBot_ValidIDs_ZeroService(t *testing.T) { assert.True(t, w.Code == http.StatusUnprocessableEntity || w.Code == http.StatusOK) } +func TestInboxHandler_SetAndGetAgentBot_ChatwootPayload(t *testing.T) { + router, _, account, inbox, bot := setupInboxAgentBotDB(t) + + body, _ := json.Marshal(map[string]uint{"agent_bot": bot.ID}) + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodPost, "/api/v1/accounts/"+inboxAgentBotTestID(account.ID)+"/inboxes/"+inboxAgentBotTestID(inbox.ID)+"/set_agent_bot", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + assert.Empty(t, w.Body.String(), "Chatwoot set_agent_bot responds head :ok") + + w = httptest.NewRecorder() + req, _ = http.NewRequest(http.MethodGet, "/api/v1/accounts/"+inboxAgentBotTestID(account.ID)+"/inboxes/"+inboxAgentBotTestID(inbox.ID)+"/agent_bot", nil) + router.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + + var resp map[string]map[string]interface{} + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp)) + agentBot := resp["agent_bot"] + assert.Equal(t, float64(bot.ID), agentBot["id"]) + assert.Equal(t, bot.Name, agentBot["name"]) + assert.Equal(t, bot.AvatarURL, agentBot["thumbnail"]) + assert.Equal(t, bot.OutgoingURL, agentBot["outgoing_url"]) + assert.Equal(t, bot.BotType, agentBot["bot_type"]) + assert.NotContains(t, agentBot, "avatar_url") +} + +func TestInboxHandler_SetAgentBot_NullDisconnectsWithHeadOK(t *testing.T) { + router, db, account, inbox, bot := setupInboxAgentBotDB(t) + require.NoError(t, db.Create(&model.AgentBotInbox{AccountID: &account.ID, InboxID: inbox.ID, AgentBotID: bot.ID, Status: model.AgentBotInboxActive}).Error) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodPost, "/api/v1/accounts/"+inboxAgentBotTestID(account.ID)+"/inboxes/"+inboxAgentBotTestID(inbox.ID)+"/set_agent_bot", bytes.NewReader([]byte(`{"agent_bot":null}`))) + req.Header.Set("Content-Type", "application/json") + router.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + assert.Empty(t, w.Body.String()) + + var count int64 + require.NoError(t, db.Model(&model.AgentBotInbox{}).Where("inbox_id = ?", inbox.ID).Count(&count).Error) + assert.Equal(t, int64(0), count) + + w = httptest.NewRecorder() + req, _ = http.NewRequest(http.MethodGet, "/api/v1/accounts/"+inboxAgentBotTestID(account.ID)+"/inboxes/"+inboxAgentBotTestID(inbox.ID)+"/agent_bot", nil) + router.ServeHTTP(w, req) + require.Equal(t, http.StatusOK, w.Code) + assert.JSONEq(t, `{"agent_bot":{}}`, w.Body.String()) +} + // --- DeleteAvatar tests --- func TestInboxHandler_DeleteAvatar_InvalidAccountID(t *testing.T) { diff --git a/internal/handler/api/v1/inbox_handler.go b/internal/handler/api/v1/inbox_handler.go index 3a4a00a7..59467116 100644 --- a/internal/handler/api/v1/inbox_handler.go +++ b/internal/handler/api/v1/inbox_handler.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/gin-gonic/gin" + "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" "github.com/gochat/gochat/internal/service" "github.com/gochat/gochat/pkg/response" @@ -576,29 +577,29 @@ func (h *InboxHandler) SetAgentBot(c *gin.Context) { return } - var req service.SetAgentBotRequest - if err := c.ShouldBindJSON(&req); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body: " + err.Error()}) - return + req := service.SetAgentBotRequest{} + if c.Request.Body != nil && c.Request.ContentLength != 0 { + if err := c.ShouldBindJSON(&req); err != nil { + if c.Request.ContentLength < 0 && errors.Is(err, io.EOF) { + // Chunked empty body behaves like omitted params in Chatwoot and disconnects the bot. + } else { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body: " + err.Error()}) + return + } + } } if !h.svc.Ready() { response.AbortWithStatusError(c, http.StatusUnprocessableEntity, response.ErrInternal, "failed to set agent bot") return } - binding, svcErr := h.svc.SetAgentBot(c.Request.Context(), accountID, inboxID, req) + _, svcErr := h.svc.SetAgentBot(c.Request.Context(), accountID, inboxID, req) if svcErr != nil { handleServiceError(c, svcErr) return } - if binding == nil { - // Agent bot was removed - c.JSON(http.StatusOK, gin.H{"message": "agent bot removed from inbox"}) - return - } - - c.JSON(http.StatusOK, binding) + c.Status(http.StatusOK) } // Health checks the health status of an inbox's channel connection. @@ -738,11 +739,28 @@ func (h *InboxHandler) GetAgentBot(c *gin.Context) { } if agentBot == nil { - c.JSON(http.StatusOK, gin.H{"agent_bot": nil}) + c.JSON(http.StatusOK, gin.H{"agent_bot": gin.H{}}) return } - c.JSON(http.StatusOK, gin.H{"agent_bot": agentBot}) + c.JSON(http.StatusOK, gin.H{"agent_bot": serializeAgentBot(agentBot)}) +} + +func serializeAgentBot(bot *model.AgentBot) gin.H { + if bot == nil { + return gin.H{} + } + return gin.H{ + "id": bot.ID, + "name": bot.Name, + "description": bot.Description, + "thumbnail": bot.AvatarURL, + "outgoing_url": bot.OutgoingURL, + "bot_type": bot.BotType, + "bot_config": bot.Config, + "account_id": bot.AccountID, + "access_token": bot.AccessToken, + } } // DeleteAvatar removes the avatar URL from an inbox. diff --git a/internal/service/inbox_service.go b/internal/service/inbox_service.go index c94c76e6..0ef4f28c 100644 --- a/internal/service/inbox_service.go +++ b/internal/service/inbox_service.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "os" + "strconv" "strings" "github.com/gochat/gochat/internal/campaign" @@ -1516,12 +1517,45 @@ func (s *InboxService) CreateFacebookInbox(ctx context.Context, accountID uint, // SetAgentBotRequest represents the request body for setting an agent bot on an inbox. // Reference: Chatwoot InboxesController#set_agent_bot type SetAgentBotRequest struct { - AgentBotID uint `json:"agent_bot_id" validate:"required"` // 0 means "remove" current bot + AgentBotID *uint `json:"-"` // nil means "remove" current bot. +} + +// UnmarshalJSON accepts Chatwoot's frontend key (`agent_bot`) and the old local +// compatibility key (`agent_bot_id`). A null or missing value disconnects the bot. +func (r *SetAgentBotRequest) UnmarshalJSON(data []byte) error { + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + value, ok := raw["agent_bot"] + if !ok { + value, ok = raw["agent_bot_id"] + } + if !ok || string(value) == "null" || string(value) == `""` { + r.AgentBotID = nil + return nil + } + var id uint + if err := json.Unmarshal(value, &id); err == nil { + r.AgentBotID = &id + return nil + } + var idString string + if err := json.Unmarshal(value, &idString); err != nil { + return err + } + parsed, err := strconv.ParseUint(idString, 10, 64) + if err != nil { + return err + } + id = uint(parsed) + r.AgentBotID = &id + return nil } // SetAgentBot assigns or removes an agent bot from an inbox. -// If agent_bot_id is 0, any existing active bot assignment is deactivated. -// If agent_bot_id > 0, a new AgentBotInbox binding is created (or existing one updated). +// If agent_bot is absent/null, the existing bot assignment is destroyed. +// If agent_bot is present, the inbox's single AgentBotInbox binding is created or updated. // Reference: Chatwoot InboxesController#set_agent_bot func (s *InboxService) SetAgentBot(ctx context.Context, accountID, inboxID uint, req SetAgentBotRequest) (*model.AgentBotInbox, error) { // Verify inbox belongs to account @@ -1530,67 +1564,61 @@ func (s *InboxService) SetAgentBot(ctx context.Context, accountID, inboxID uint, return nil, fmt.Errorf("inbox not found: %w", err) } - // If agent_bot_id is 0, deactivate any existing active bindings for this inbox - if req.AgentBotID == 0 { - activeBindings, err := s.agentBotInboxRepo.FindActiveByInboxID(ctx, inbox.ID) - if err != nil { - return nil, fmt.Errorf("failed to find active bot bindings: %w", err) - } - for _, binding := range activeBindings { - binding.Status = model.AgentBotInboxInactive - if err := s.agentBotInboxRepo.Update(ctx, &binding); err != nil { - applogger.L().Warnf("Failed to deactivate agent_bot_inbox id=%d: %v", binding.ID, err) + existingBindings, err := s.agentBotInboxRepo.FindByInboxID(ctx, inbox.ID) + if err != nil { + return nil, fmt.Errorf("failed to find bot bindings: %w", err) + } + + // Chatwoot destroys the existing has_one binding when no agent_bot param is present. + if req.AgentBotID == nil { + for _, binding := range existingBindings { + if err := s.agentBotInboxRepo.Delete(ctx, binding.ID); err != nil { + applogger.L().Warnf("Failed to delete agent_bot_inbox id=%d: %v", binding.ID, err) } } applogger.L().Infof("Removed agent bot from inbox (account_id=%d, inbox_id=%d)", accountID, inboxID) return nil, nil } + agentBotID := *req.AgentBotID // Verify the agent bot exists and is accessible to this account - bot, err := s.agentBotRepo.FindByID(ctx, req.AgentBotID) + bot, err := s.agentBotRepo.FindByID(ctx, agentBotID) if err != nil { return nil, fmt.Errorf("agent bot not found: %w", err) } if bot.AccountID != nil && *bot.AccountID != accountID { - return nil, fmt.Errorf("agent bot %d not accessible to account %d", req.AgentBotID, accountID) + return nil, fmt.Errorf("agent bot %d not accessible to account %d", agentBotID, accountID) } - // Check if there's an existing binding for this inbox+bot combo - existing, err := s.agentBotInboxRepo.FindByAgentBotAndInbox(ctx, req.AgentBotID, inbox.ID) - if err == nil && existing != nil { - // Update existing binding to active - existing.Status = model.AgentBotInboxActive - if err := s.agentBotInboxRepo.Update(ctx, existing); err != nil { + if len(existingBindings) > 0 { + binding := existingBindings[0] + binding.AgentBotID = agentBotID + binding.AccountID = &accountID + binding.Status = model.AgentBotInboxActive + if err := s.agentBotInboxRepo.Update(ctx, &binding); err != nil { return nil, fmt.Errorf("failed to update agent_bot_inbox: %w", err) } - applogger.L().Infof("Re-activated agent bot %d on inbox %d", req.AgentBotID, inboxID) - return existing, nil - } - - // Deactivate any previous active bot on this inbox first - activeBindings, err := s.agentBotInboxRepo.FindActiveByInboxID(ctx, inbox.ID) - if err != nil { - return nil, fmt.Errorf("failed to find active bot bindings: %w", err) - } - for _, binding := range activeBindings { - binding.Status = model.AgentBotInboxInactive - if err := s.agentBotInboxRepo.Update(ctx, &binding); err != nil { - applogger.L().Warnf("Failed to deactivate previous agent_bot_inbox id=%d: %v", binding.ID, err) + for _, stale := range existingBindings[1:] { + if err := s.agentBotInboxRepo.Delete(ctx, stale.ID); err != nil { + applogger.L().Warnf("Failed to delete stale agent_bot_inbox id=%d: %v", stale.ID, err) + } } + applogger.L().Infof("Set agent bot %d on inbox %d (account_id=%d)", agentBotID, inboxID, accountID) + return &binding, nil } // Create new binding binding := &model.AgentBotInbox{ AccountID: &accountID, InboxID: inbox.ID, - AgentBotID: req.AgentBotID, + AgentBotID: agentBotID, Status: model.AgentBotInboxActive, } if err := s.agentBotInboxRepo.Create(ctx, binding); err != nil { return nil, fmt.Errorf("failed to create agent_bot_inbox: %w", err) } - applogger.L().Infof("Set agent bot %d on inbox %d (account_id=%d)", req.AgentBotID, inboxID, accountID) + applogger.L().Infof("Set agent bot %d on inbox %d (account_id=%d)", agentBotID, inboxID, accountID) return binding, nil } diff --git a/internal/service/inbox_service_test.go b/internal/service/inbox_service_test.go index f986e080..debc1af1 100644 --- a/internal/service/inbox_service_test.go +++ b/internal/service/inbox_service_test.go @@ -102,6 +102,8 @@ func createInboxTestPrereqs(t *testing.T, db *gorm.DB, channelType string) (*mod return account, inbox } +func ptrUint(v uint) *uint { return &v } + func createWhatsAppInboxTestPrereqs(t *testing.T, db *gorm.DB, provider string) (*model.Account, *model.Inbox, *channelmodel.ChannelWhatsApp) { t.Helper() account, inbox := createInboxTestPrereqs(t, db, "whatsapp") @@ -193,7 +195,7 @@ func TestInboxService_SetAgentBot_AssignBot(t *testing.T) { bot := createTestAgentBot(t, db, account.ID, "assign") binding, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: bot.ID, + AgentBotID: ptrUint(bot.ID), }) require.NoError(t, err) assert.NotNil(t, binding) @@ -209,24 +211,26 @@ func TestInboxService_SetAgentBot_RemoveBot(t *testing.T) { // First assign a bot binding, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: bot.ID, + AgentBotID: ptrUint(bot.ID), }) require.NoError(t, err) assert.NotNil(t, binding) - // Now remove it (agent_bot_id = 0) - result, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: 0, - }) + // Now remove it (missing/null agent_bot) + result, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{}) require.NoError(t, err) assert.Nil(t, result, "removing bot should return nil binding") + + bindings, err := repository.NewAgentBotInboxRepo(db).FindByInboxID(context.Background(), inbox.ID) + require.NoError(t, err) + assert.Empty(t, bindings, "Chatwoot destroys the agent_bot_inbox row on disconnect") } func TestInboxService_SetAgentBot_InboxNotFound(t *testing.T) { svc, _ := setupInboxServiceTest(t) binding, err := svc.SetAgentBot(context.Background(), 9999, 9999, SetAgentBotRequest{ - AgentBotID: 1, + AgentBotID: ptrUint(1), }) assert.Error(t, err) assert.Nil(t, binding) @@ -238,7 +242,7 @@ func TestInboxService_SetAgentBot_BotNotFound(t *testing.T) { account, inbox := createInboxTestPrereqs(t, db, "web_widget") binding, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: 9999, + AgentBotID: ptrUint(9999), }) assert.Error(t, err) assert.Nil(t, binding) @@ -253,40 +257,39 @@ func TestInboxService_SetAgentBot_ReassignBot(t *testing.T) { // Assign first bot binding1, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: bot1.ID, + AgentBotID: ptrUint(bot1.ID), }) require.NoError(t, err) assert.Equal(t, bot1.ID, binding1.AgentBotID) - // Reassign to second bot — should deactivate bot1 and activate bot2 + // Reassign to second bot — Chatwoot updates the inbox's single has_one binding. binding2, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: bot2.ID, + AgentBotID: ptrUint(bot2.ID), }) require.NoError(t, err) assert.Equal(t, bot2.ID, binding2.AgentBotID) assert.Equal(t, model.AgentBotInboxActive, binding2.Status) + assert.Equal(t, binding1.ID, binding2.ID) } -func TestInboxService_SetAgentBot_ReactivateExistingBinding(t *testing.T) { +func TestInboxService_SetAgentBot_RecreatesBindingAfterDisconnect(t *testing.T) { svc, db := setupInboxServiceTest(t) account, inbox := createInboxTestPrereqs(t, db, "web_widget") - bot := createTestAgentBot(t, db, account.ID, "reactivate") + bot := createTestAgentBot(t, db, account.ID, "recreate") // Assign bot _, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: bot.ID, + AgentBotID: ptrUint(bot.ID), }) require.NoError(t, err) - // Remove bot (deactivate) - _, err = svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: 0, - }) + // Remove bot (destroy binding) + _, err = svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{}) require.NoError(t, err) - // Re-assign same bot — should reactivate the existing binding + // Re-assign same bot — should create a fresh binding. binding2, err := svc.SetAgentBot(context.Background(), account.ID, inbox.ID, SetAgentBotRequest{ - AgentBotID: bot.ID, + AgentBotID: ptrUint(bot.ID), }) require.NoError(t, err) assert.NotNil(t, binding2)