From f4c56bf1094247a1e0ab6a2b7eb3429934ec68b0 Mon Sep 17 00:00:00 2001 From: Rogee Date: Fri, 5 Jun 2026 09:24:10 +0800 Subject: [PATCH] feat(automation): align rule trigger coverage --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 47 +++-- internal/automation/listener.go | 192 ++++++++++++++++-- internal/automation/listener_test.go | 186 ++++++++++++++++- .../handler/webhook/webhook_lookup_test.go | 53 +++++ internal/service/conversation_service.go | 79 ++++++- internal/service/conversation_service_test.go | 79 +++++++ 6 files changed, 599 insertions(+), 37 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 832d8e61..186b7a22 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -16,9 +16,9 @@ Build GoChat as a Go backend that can directly reuse the frontend from `referenc ## Current Baseline -- Latest implementation checkpoint: `3403770 feat(automation): align automation rule payloads`. -- Latest documentation checkpoint: this planning checkpoint, recorded after B9.1a landed. -- Worktree status at this implementation checkpoint: B9.1a automation-rule CRUD payload and validation parity implemented; next active slice is B9.1b Chatwoot listener trigger coverage, followed by B9.1c execution logs/stop-on-match and B9.2 macro execution parity. +- Latest implementation checkpoint: `feat(automation): align rule trigger coverage`. +- Latest documentation checkpoint: this checkpoint, recorded with the B9.1b implementation. +- Worktree status at this implementation checkpoint: B9.1b Chatwoot listener trigger coverage is implemented; next active slice is B9.1c execution logs/stop-on-match, followed by B9.1d webhook/email transcript retries and B9.2 macro execution parity. - `go test ./...` passes. - Route dump succeeds with `TOTAL: 830` after adding the Chatwoot-compatible applied-SLA index route. - Route parity artifacts now exist under `docs/parity/` and are generated by `cmd/route_parity`. @@ -44,13 +44,12 @@ Next ordered checkpoints: | Order | Slice | Required outcome | Primary verification | | --- | --- | --- | --- | -| 1 | B9.1b | Automation listener trigger coverage matches Chatwoot: conversation created/updated/opened/resolved and message created execute once, skip automation-origin and auto-reply/activity cases, and provider webhooks reach these same dispatcher events. | Automation listener/service tests plus provider dispatch regression where needed. | -| 2 | B9.1c | Automation execution logs, failed-action records, and stop-on-match semantics are explicit and tested. | Rule service/action tests for matched, skipped, failed, and stop-on-match flows. | -| 3 | B9.1d | Webhook and email transcript actions are timeout-bound, retryable, and observable through the worker/action boundary. | Fake HTTP/mailer tests and log/retry metadata assertions. | -| 4 | B9.2 | Macro CRUD and macro execute side effects match Chatwoot frontend expectations. | Macro handler/service tests that reload conversations/messages after execution. | -| 5 | B10 | Audit, CustomRole, and remaining InboxLimit surfaces enforce Chatwoot admin behavior. | Permission matrix tests, audit writer/list tests, inbox/account limit tests. | -| 6 | B11 | Captain/Copilot enterprise screens have real persistence and safe LLM feature gates. | Captain/Copilot handler/service fixtures and disabled-state tests. | -| 7 | B12 | Reused Chatwoot frontend smoke runs repeatably against GoChat. | Checked smoke command plus gap report under `docs/parity/`. | +| 1 | B9.1c | Automation execution logs, failed-action records, and stop-on-match semantics are explicit and tested. | Rule service/action tests for matched, skipped, failed, and stop-on-match flows. | +| 2 | B9.1d | Webhook and email transcript actions are timeout-bound, retryable, and observable through the worker/action boundary. | Fake HTTP/mailer tests and log/retry metadata assertions. | +| 3 | B9.2 | Macro CRUD and macro execute side effects match Chatwoot frontend expectations. | Macro handler/service tests that reload conversations/messages after execution. | +| 4 | B10 | Audit, CustomRole, and remaining InboxLimit surfaces enforce Chatwoot admin behavior. | Permission matrix tests, audit writer/list tests, inbox/account limit tests. | +| 5 | B11 | Captain/Copilot enterprise screens have real persistence and safe LLM feature gates. | Captain/Copilot handler/service fixtures and disabled-state tests. | +| 6 | B12 | Reused Chatwoot frontend smoke runs repeatably against GoChat. | Checked smoke command plus gap report under `docs/parity/`. | ## Execution Snapshot @@ -139,6 +138,7 @@ This ledger records the committed parity checkpoints that future slices should b | `docs: land automation macro execution plan` | Landed the B9 execution contract into this tracker before code changes: Chatwoot automation-rule controller/frontend contracts, current Go handler/service gaps, B9.1a implementation checklist, focused verification commands, and commit/documentation rules. | Documentation-only checkpoint; `git diff --check` passed before commit. | Start B9.1a implementation. | | `feat(automation): align automation rule payloads` | Completed B9.1a automation-rule CRUD contract parity for the reused Chatwoot frontend: list/show/update/clone now return `{ payload }`, create returns the raw rule object, delete returns empty `200 OK`, conditions accept `attribute_key` and Chatwoot operators such as `equal_to`, action params accept frontend arrays, responses serialize `attribute_key`, array `action_params`, Unix `created_on`, and update/delete/clone are account-scoped. | `go test ./internal/handler/api/v1 -run AutomationRule -count=1`; `go test ./internal/automation -run 'AutomationRule\|Condition\|Action' -count=1`; `go test ./internal/handler/api/v1 -count=1`; `go test ./internal/automation -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 830`. | Continue B9.1b trigger coverage, B9.1c execution logs/stop-on-match, and B9.2 macro payload/execute parity. | | `docs: detail automation trigger plan` | Fully landed the next B9 tracking contract: current baseline now points at `3403770`, the control board is ordered from B9.1b through B12, B9 status is `Doing`, and B9.1b explicitly follows the current Chatwoot listener events while excluding non-reference `contact_updated` automation triggers. | Documentation-only checkpoint; `git diff --check` passed before commit. | Start B9.1b implementation from the recorded listener/event contract. | +| `feat(automation): align rule trigger coverage` | Completed B9.1b listener/event coverage: automation rules now honor Chatwoot skip rules for automation-origin events, auto-reply conversations, activity messages, and auto-reply emails; listener extraction works from event conversation ID, `conversation_id`, conversation payloads, and message payloads; conversation update/status/priority paths dispatch `changed_attributes`; initial conversation messages and provider webhook messages carry the message/conversation data needed by automation. | `go test ./internal/automation -run 'AutomationRuleListener\|MatchAndExecute\|Condition' -count=1`; `go test ./internal/service -run 'Conversation.*Automation\|Message.*Automation\|ConversationService' -count=1`; `go test ./internal/handler/webhook -run 'Incoming\|Webhook' -count=1`; `go test ./internal/automation -count=1`; `go test ./internal/service -count=1`; `go test ./internal/handler/webhook -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 830`. | Continue B9.1c execution logs and stop-on-match behavior, then B9.1d retryable webhook/email transcript actions and B9.2 macro parity. | ## Next Slice Contract @@ -380,7 +380,7 @@ Upcoming enterprise task boards: | B8 | B8.2 | Align public CSAT submit/update flow, lock window, message linkage, and one-response-per-message behavior. | `reference/chatwoot/app/controllers/public/api/v1/csat_survey_controller.rb`, CSAT response model/services. | Public handler tests for submit, update within window, update after lock, duplicate response rejection. | Done by `ef3a909` | | B8 | B8.3 | Wire resolve-triggered CSAT survey send with idempotency and channel-aware message creation. | Chatwoot CSAT listener/jobs and inbox CSAT settings. | Listener tests prove one CSAT message per resolved conversation and no send when disabled. | Done by `ef3a909`; WhatsApp/Twilio external template dispatch remains integration follow-up | | B8 | B8.4 | Align CSAT downloads/export payloads used by reports screens. | CSAT report/download controller paths and frontend report API. | CSV/download tests cover filters and frontend column names. | Done by `b36cf07` | -| B9 | B9.1 | Align automation rule CRUD payloads, validation, condition groups, event names, listener skip rules, and event data. | Chatwoot automation rule controllers/models, `AutomationRuleListener`, and dashboard automation builder. | Handler/service/listener tests for CRUD, validation, event mapping, skip rules, changed attributes, and provider-dispatched events. | Doing; B9.1a done by `3403770`, B9.1b is next | +| B9 | B9.1 | Align automation rule CRUD payloads, validation, condition groups, event names, listener skip rules, and event data. | Chatwoot automation rule controllers/models, `AutomationRuleListener`, and dashboard automation builder. | Handler/service/listener tests for CRUD, validation, event mapping, skip rules, changed attributes, and provider-dispatched events. | Review; B9.1a done by `3403770`, B9.1b done by `feat(automation): align rule trigger coverage`; B9.1c/B9.1d remain follow-ups | | B9 | B9.2 | Implement automation action side effects for labels, status, priority, assignee/team, custom attributes, private notes, and snooze. | Chatwoot automation action services/listeners. | Rule listener tests assert real conversation mutations and stop-on-match behavior. | Todo | | B9 | B9.3 | Add durable execution logs, delayed actions, webhook delivery, and email transcript delivery boundaries. | Chatwoot automation jobs and webhook/transcript actions. | Worker/action tests cover retryable external actions and observable failures. | Todo | | B9 | B9.4 | Align macro CRUD/availability and macro execution side effects. | Chatwoot macros controller/model/action execution. | Macro handler/service tests cover frontend payloads and conversation mutations. | Todo | @@ -409,7 +409,7 @@ B9 automation and macro execution breakdown: | Step | Implementation target | Reference source | Required tests | Status | | --- | --- | --- | --- | --- | | B9.1a | Align automation-rule CRUD request/response envelopes, condition/action validation, and rule status toggles. | Chatwoot automation rules controller/model and dashboard automation client. | Handler tests for index/show/create/update/delete/toggle payloads and validation failures. | Done by `feat(automation): align automation rule payloads` | -| B9.1b | Implement real rule trigger coverage for Chatwoot automation events: conversation created/updated/opened/resolved and message created. Provider webhook persistence must dispatch into those same conversation/message events. `contact_updated` is not a current Chatwoot automation trigger and stays out unless the local reference/frontend proves otherwise. | `reference/chatwoot/app/listeners/automation_rule_listener.rb`, `reference/chatwoot/app/listeners/base_listener.rb`, existing `channel.Dispatcher` events. | Listener tests prove matching rules execute once per event with account/inbox filters, changed attributes, provider-event reachability, and Chatwoot skip rules. | Todo | +| B9.1b | Implement real rule trigger coverage for Chatwoot automation events: conversation created/updated/opened/resolved and message created. Provider webhook persistence must dispatch into those same conversation/message events. `contact_updated` is not a current Chatwoot automation trigger and stays out unless the local reference/frontend proves otherwise. | `reference/chatwoot/app/listeners/automation_rule_listener.rb`, `reference/chatwoot/app/listeners/base_listener.rb`, existing `channel.Dispatcher` events. | Listener tests prove matching rules execute once per event with account/inbox filters, changed attributes, provider-event reachability, and Chatwoot skip rules. | Done by `feat(automation): align rule trigger coverage` | | B9.1c | Persist execution logs and stop-on-match behavior, including failed action records. | Chatwoot automation execution/log models and services. | Service tests cover success, skipped, failed, retryable, and stop-on-match flows. | Todo | | B9.1d | Make webhook and email transcript actions timeout-bound, retryable, and observable. | Chatwoot action services/jobs and Phase 5 worker plan. | Tests use fake HTTP/mailer boundaries and assert retry/log metadata. | Todo | | B9.2a | Align macro CRUD payloads, visibility rules, and validation. | Chatwoot macros controller/model and dashboard macro client. | Handler tests for raw macro payloads, ownership, account scoping, and delete status. | Todo | @@ -489,6 +489,28 @@ env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./... git diff --check ``` +B9.1b current checkpoint: + +- `AutomationRuleListener` now follows the current Chatwoot reference listener event set: `conversation_created`, `conversation_updated`, `conversation_opened`, `conversation_resolved`, and `message_created`; `contact_updated` remains excluded because the local reference listener does not subscribe to it. +- Listener skip rules now match Chatwoot: events performed by automation are ignored, auto-reply conversation-created/opened events are ignored, and message-created ignores activity messages plus email auto-replies. +- Listener context extraction now supports `event.ConversationID`, `event.Data["conversation_id"]`, `event.Data["conversation"]`, and `event.Data["message"]`, with account fallback from conversation/message payloads. +- Conversation update/status/priority paths now dispatch Chatwoot-style `changed_attributes` so `attribute_changed` automation conditions can match status, priority, and SLA policy changes. +- Conversation creation with an initial message now dispatches `message.created` with `ConversationID`, `ContactID`, message payload, and conversation payload; provider webhook persistence is covered by an automation regression proving incoming provider messages trigger `message_created` rules once. +- B9 remains `Doing` because B9.1c execution log/stop-on-match semantics, B9.1d retryable webhook/email transcript actions, and B9.2 macro parity are still active. + +B9.1b verification: + +```bash +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/automation -run 'AutomationRuleListener|MatchAndExecute|Condition' -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/service -run 'Conversation.*Automation|Message.*Automation|ConversationService' -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/handler/webhook -run 'Incoming|Webhook' -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/automation -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/service -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./internal/handler/webhook -count=1 +env GOCACHE=/tmp/gochat-gocache GOMODCACHE=/tmp/gochat-gomodcache go test ./... +git diff --check +``` + B10 admin and limits breakdown: | Step | Implementation target | Reference source | Required tests | Status | @@ -1134,3 +1156,4 @@ Verification milestone gates: - 2026-06-05: B9 planning checkpoint committed as `docs: land automation macro execution plan`; B9.1a now has an explicit Chatwoot automation-rule controller/frontend contract, current Go gap table, implementation checklist, and focused exit commands before automation/macro code changes begin. - 2026-06-05: B9.1a automation-rule CRUD checkpoint committed as `feat(automation): align automation rule payloads`; automation-rule list/show/update/clone now return Chatwoot `{ payload }`, create returns the raw rule object, delete returns empty `200 OK`, frontend `attribute_key`/`equal_to` conditions and array `action_params` are accepted, responses serialize Chatwoot field names and Unix `created_on`, and get/update/delete/clone are account-scoped. Focused automation handler/service tests, handler and automation package tests, full `go test ./...`, and `git diff --check` passed. Next slice is B9.1b trigger coverage and action side-effect parity. - 2026-06-05: B9 planning tracker was fully landed for the next handoff: current baseline points at `3403770 feat(automation): align automation rule payloads`, the ordered control board now starts at B9.1b and runs through B12, B9 is marked `Doing`, and B9.1b records the exact Chatwoot listener contract. The current reference listener only subscribes to conversation created/updated/opened/resolved and message created, so `contact_updated` is explicitly not an automation trigger unless a later local reference/frontend audit proves otherwise. +- 2026-06-05: B9.1b automation trigger checkpoint committed as `feat(automation): align rule trigger coverage`; automation listener now covers all current Chatwoot reference events, skips automation-origin/auto-reply/activity auto-reply-email cases, extracts account/conversation context from message and conversation event payloads, conversation update/status/priority paths dispatch `changed_attributes`, initial conversation messages dispatch full message-created context, and provider webhook persistence has a regression proving message-created automation reachability. Focused automation/service/webhook tests, package tests, full `go test ./...`, and `git diff --check` passed. Next slice is B9.1c execution log/stop-on-match parity. diff --git a/internal/automation/listener.go b/internal/automation/listener.go index b2602c22..5a3c3e49 100644 --- a/internal/automation/listener.go +++ b/internal/automation/listener.go @@ -2,10 +2,12 @@ package automation import ( "context" + "encoding/json" "fmt" "strings" "github.com/gochat/gochat/internal/channel" + "github.com/gochat/gochat/internal/model" applogger "github.com/gochat/gochat/pkg/logger" ) @@ -27,7 +29,7 @@ type AutomationRuleListener struct { // NewAutomationRuleListener creates a new AutomationRuleListener. func NewAutomationRuleListener(db DBProvider) *AutomationRuleListener { return &AutomationRuleListener{ - db: db, + db: db, ruleService: NewAutomationRuleService(db), } } @@ -51,27 +53,189 @@ func (l *AutomationRuleListener) OnEvent(ctx context.Context, event *channel.Cha applogger.L().Infof("automation listener processing event %s (mapped to %s) for account %d", event.Type, eventName, event.AccountID) + if shouldSkipAutomationEvent(event, eventName) { + applogger.L().Debugf("automation listener skipping event %s by Chatwoot skip rules", event.Type) + return nil + } + // Get conversation ID from event - conversationID := event.ConversationID - if conversationID == 0 { - // Try to extract from event data - if convID, ok := extractUintFromData(event.Data, "conversation_id"); ok { - conversationID = convID - } else { - applogger.L().Debugf("automation listener: no conversation_id in event %s", event.Type) - return nil - } + conversationID, ok := extractConversationIDFromEvent(event) + if !ok || conversationID == 0 { + applogger.L().Debugf("automation listener: no conversation_id in event %s", event.Type) + return nil + } + + accountID := event.AccountID + if accountID == 0 { + accountID = extractAccountIDFromEvent(event) + } + if accountID == 0 { + applogger.L().Debugf("automation listener: no account_id in event %s", event.Type) + return nil } // Run match and execute — this loads active rules for the account+event, // runs condition filters, and executes actions for matching rules. - return l.ruleService.MatchAndExecute(ctx, event.AccountID, eventName, conversationID, event.Data) + return l.ruleService.MatchAndExecute(ctx, accountID, eventName, conversationID, event.Data) +} + +func shouldSkipAutomationEvent(event *channel.ChannelEvent, eventName string) bool { + if performedByAutomation(event) { + return true + } + if eventName == "message_created" { + return ignoreMessageCreatedEvent(event) + } + if eventName == "conversation_created" || eventName == "conversation_opened" { + return ignoreAutoReplyConversationEvent(event) + } + return false +} + +func performedByAutomation(event *channel.ChannelEvent) bool { + if event == nil || event.Data == nil { + return false + } + performedBy, ok := event.Data["performed_by"] + if !ok || performedBy == nil { + return false + } + switch v := performedBy.(type) { + case AutomationRule, *AutomationRule: + return true + case string: + return v == "automation_rule" || v == "AutomationRule" + default: + return false + } +} + +func ignoreAutoReplyConversationEvent(event *channel.ChannelEvent) bool { + conversation, ok := extractConversationFromData(event.Data) + if !ok || conversation == nil { + return false + } + attrs := listenerJSONMap(conversation.AdditionalAttributes) + return truthy(attrs["auto_reply"]) +} + +func ignoreMessageCreatedEvent(event *channel.ChannelEvent) bool { + message, ok := extractMessageFromData(event.Data) + if !ok || message == nil { + return false + } + if strings.EqualFold(message.MessageType, "activity") { + return true + } + attrs := listenerJSONMap(message.ContentAttributes) + if email, ok := attrs["email"].(map[string]any); ok && truthy(email["auto_reply"]) { + return true + } + return false +} + +func extractConversationIDFromEvent(event *channel.ChannelEvent) (uint, bool) { + if event == nil { + return 0, false + } + if event.ConversationID != 0 { + return event.ConversationID, true + } + if convID, ok := extractUintFromData(event.Data, "conversation_id"); ok { + return convID, true + } + if conversation, ok := extractConversationFromData(event.Data); ok && conversation != nil && conversation.ID != 0 { + return conversation.ID, true + } + if message, ok := extractMessageFromData(event.Data); ok && message != nil && message.ConversationID != 0 { + return message.ConversationID, true + } + return 0, false +} + +func extractAccountIDFromEvent(event *channel.ChannelEvent) uint { + if event == nil { + return 0 + } + if accountID, ok := extractUintFromData(event.Data, "account_id"); ok { + return accountID + } + if conversation, ok := extractConversationFromData(event.Data); ok && conversation != nil { + return conversation.AccountID + } + if message, ok := extractMessageFromData(event.Data); ok && message != nil { + return message.AccountID + } + return 0 +} + +func extractConversationFromData(data map[string]interface{}) (*model.Conversation, bool) { + if data == nil { + return nil, false + } + raw, ok := data["conversation"] + if !ok || raw == nil { + return nil, false + } + switch v := raw.(type) { + case *model.Conversation: + return v, true + case model.Conversation: + return &v, true + default: + return nil, false + } +} + +func extractMessageFromData(data map[string]interface{}) (*model.Message, bool) { + if data == nil { + return nil, false + } + raw, ok := data["message"] + if !ok || raw == nil { + return nil, false + } + switch v := raw.(type) { + case *model.Message: + return v, true + case model.Message: + return &v, true + default: + return nil, false + } +} + +func listenerJSONMap(raw []byte) map[string]interface{} { + if len(raw) == 0 { + return map[string]interface{}{} + } + var out map[string]interface{} + if err := json.Unmarshal(raw, &out); err != nil { + return map[string]interface{}{} + } + return out +} + +func truthy(value interface{}) bool { + switch v := value.(type) { + case bool: + return v + case string: + return strings.EqualFold(v, "true") || v == "1" + case float64: + return v != 0 + case int: + return v != 0 + default: + return false + } } // mapEventToAutomationEventName maps channel.EventType to Chatwoot automation event_name strings. // Reference: Chatwoot AutomationRuleListener subscribes to: -// conversation_updated, conversation_created, conversation_opened, -// conversation_resolved, message_created +// +// conversation_updated, conversation_created, conversation_opened, +// conversation_resolved, message_created func mapEventToAutomationEventName(eventType channel.EventType) string { switch eventType { case channel.EventConversationCreated: @@ -139,4 +303,4 @@ func isValidAutomationEventName(name string) bool { // Ensure the event name is normalized (lowercase, underscored) func normalizeEventName(name string) string { return strings.ToLower(strings.ReplaceAll(name, ".", "_")) -} \ No newline at end of file +} diff --git a/internal/automation/listener_test.go b/internal/automation/listener_test.go index 0514c9ba..bf7af2f1 100644 --- a/internal/automation/listener_test.go +++ b/internal/automation/listener_test.go @@ -6,8 +6,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gorm.io/datatypes" "github.com/gochat/gochat/internal/channel" + "github.com/gochat/gochat/internal/model" ) // =========================== @@ -246,6 +248,188 @@ func TestAutomationRuleListener_OnEvent_ExtractsConversationIDFromData(t *testin assert.Equal(t, accountID, exec.AccountID) } +func TestAutomationRuleListener_OnEvent_DispatchesAllChatwootReferenceEvents(t *testing.T) { + dbProvider := setupAutomationTestDBProvider(t) + db := dbProvider.DB() + + accountID, _ := seedTestAccount(db, t) + inboxID := seedTestInbox(db, t, accountID) + contactID := seedTestContact(db, t, accountID) + conversationID := seedTestConversation(db, t, accountID, inboxID, contactID) + + ruleSvc := NewAutomationRuleService(dbProvider) + listener := NewAutomationRuleListener(dbProvider) + + tests := []struct { + name string + eventName string + event *channel.ChannelEvent + }{ + { + name: "conversation_updated", + eventName: "conversation_updated", + event: &channel.ChannelEvent{Type: channel.EventConversationUpdated, AccountID: accountID, InboxID: inboxID, ConversationID: conversationID, ContactID: contactID, Data: map[string]interface{}{}}, + }, + { + name: "conversation_opened", + eventName: "conversation_opened", + event: &channel.ChannelEvent{Type: channel.EventConversationOpened, AccountID: accountID, InboxID: inboxID, ConversationID: conversationID, ContactID: contactID, Data: map[string]interface{}{}}, + }, + { + name: "conversation_resolved", + eventName: "conversation_resolved", + event: &channel.ChannelEvent{Type: channel.EventConversationResolved, AccountID: accountID, InboxID: inboxID, ConversationID: conversationID, ContactID: contactID, Data: map[string]interface{}{}}, + }, + { + name: "message_created_extracts_conversation_and_account_from_message", + eventName: "message_created", + event: &channel.ChannelEvent{Type: channel.EventMessageCreated, InboxID: inboxID, Data: map[string]interface{}{ + "message": &model.Message{AccountID: accountID, InboxID: inboxID, ConversationID: conversationID, MessageType: "incoming", ContentType: "text"}, + }}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rule := &AutomationRule{AccountID: accountID, EventName: tt.eventName, Name: tt.name, Conditions: Conditions{}, Actions: Actions{}, Active: true} + require.NoError(t, ruleSvc.Create(context.Background(), rule)) + + require.NoError(t, listener.OnEvent(context.Background(), tt.event)) + + var count int64 + require.NoError(t, db.Model(&AutomationExecution{}).Where("rule_id = ? AND conversation_id = ?", rule.ID, conversationID).Count(&count).Error) + assert.Equal(t, int64(1), count) + }) + } +} + +func TestAutomationRuleListener_OnEvent_UsesChangedAttributesForConditions(t *testing.T) { + dbProvider := setupAutomationTestDBProvider(t) + db := dbProvider.DB() + + accountID, _ := seedTestAccount(db, t) + inboxID := seedTestInbox(db, t, accountID) + contactID := seedTestContact(db, t, accountID) + conversationID := seedTestConversationWithDetails(db, t, accountID, inboxID, contactID, "resolved", "low", "web", 0) + + rule := &AutomationRule{ + AccountID: accountID, + EventName: "conversation_resolved", + Name: "Status changed to resolved", + Conditions: Conditions{{ + Attribute: "status", + FilterOperator: "attribute_changed", + Values: []string{"resolved"}, + }}, + Actions: Actions{}, + Active: true, + } + require.NoError(t, NewAutomationRuleService(dbProvider).Create(context.Background(), rule)) + + listener := NewAutomationRuleListener(dbProvider) + event := &channel.ChannelEvent{ + Type: channel.EventConversationResolved, + AccountID: accountID, + InboxID: inboxID, + ConversationID: conversationID, + Data: map[string]interface{}{ + "changed_attributes": map[string]interface{}{ + "status": map[string]interface{}{"from": "open", "to": "resolved"}, + }, + }, + } + + require.NoError(t, listener.OnEvent(context.Background(), event)) + + var count int64 + require.NoError(t, db.Model(&AutomationExecution{}).Where("rule_id = ?", rule.ID).Count(&count).Error) + assert.Equal(t, int64(1), count) +} + +func TestAutomationRuleListener_OnEvent_SkipsPerformedByAutomation(t *testing.T) { + dbProvider := setupAutomationTestDBProvider(t) + db := dbProvider.DB() + + accountID, _ := seedTestAccount(db, t) + inboxID := seedTestInbox(db, t, accountID) + contactID := seedTestContact(db, t, accountID) + conversationID := seedTestConversation(db, t, accountID, inboxID, contactID) + + rule := &AutomationRule{AccountID: accountID, EventName: "conversation_created", Name: "skip performed by automation", Conditions: Conditions{}, Actions: Actions{}, Active: true} + require.NoError(t, NewAutomationRuleService(dbProvider).Create(context.Background(), rule)) + + event := &channel.ChannelEvent{ + Type: channel.EventConversationCreated, + AccountID: accountID, + InboxID: inboxID, + ConversationID: conversationID, + Data: map[string]interface{}{"performed_by": &AutomationRule{Base: model.Base{ID: rule.ID}}}, + } + + require.NoError(t, NewAutomationRuleListener(dbProvider).OnEvent(context.Background(), event)) + + var count int64 + require.NoError(t, db.Model(&AutomationExecution{}).Where("rule_id = ?", rule.ID).Count(&count).Error) + assert.Equal(t, int64(0), count) +} + +func TestAutomationRuleListener_OnEvent_SkipsAutoReplyConversationCreateAndOpen(t *testing.T) { + dbProvider := setupAutomationTestDBProvider(t) + db := dbProvider.DB() + + accountID, _ := seedTestAccount(db, t) + inboxID := seedTestInbox(db, t, accountID) + contactID := seedTestContact(db, t, accountID) + conversationID := seedTestConversation(db, t, accountID, inboxID, contactID) + conversation := &model.Conversation{} + require.NoError(t, db.First(conversation, conversationID).Error) + conversation.AdditionalAttributes = datatypes.JSON([]byte(`{"auto_reply":true}`)) + + ruleSvc := NewAutomationRuleService(dbProvider) + createdRule := &AutomationRule{AccountID: accountID, EventName: "conversation_created", Name: "skip auto reply created", Conditions: Conditions{}, Actions: Actions{}, Active: true} + openedRule := &AutomationRule{AccountID: accountID, EventName: "conversation_opened", Name: "skip auto reply opened", Conditions: Conditions{}, Actions: Actions{}, Active: true} + require.NoError(t, ruleSvc.Create(context.Background(), createdRule)) + require.NoError(t, ruleSvc.Create(context.Background(), openedRule)) + + listener := NewAutomationRuleListener(dbProvider) + for _, eventType := range []channel.EventType{channel.EventConversationCreated, channel.EventConversationOpened} { + event := &channel.ChannelEvent{Type: eventType, AccountID: accountID, InboxID: inboxID, ConversationID: conversationID, Data: map[string]interface{}{"conversation": conversation}} + require.NoError(t, listener.OnEvent(context.Background(), event)) + } + + var count int64 + require.NoError(t, db.Model(&AutomationExecution{}).Where("rule_id IN ?", []uint{createdRule.ID, openedRule.ID}).Count(&count).Error) + assert.Equal(t, int64(0), count) +} + +func TestAutomationRuleListener_OnEvent_SkipsActivityAndAutoReplyEmailMessages(t *testing.T) { + dbProvider := setupAutomationTestDBProvider(t) + db := dbProvider.DB() + + accountID, _ := seedTestAccount(db, t) + inboxID := seedTestInbox(db, t, accountID) + contactID := seedTestContact(db, t, accountID) + conversationID := seedTestConversation(db, t, accountID, inboxID, contactID) + + rule := &AutomationRule{AccountID: accountID, EventName: "message_created", Name: "skip ignored messages", Conditions: Conditions{}, Actions: Actions{}, Active: true} + require.NoError(t, NewAutomationRuleService(dbProvider).Create(context.Background(), rule)) + + listener := NewAutomationRuleListener(dbProvider) + messages := []*model.Message{ + {AccountID: accountID, InboxID: inboxID, ConversationID: conversationID, MessageType: "activity", ContentType: "text"}, + {AccountID: accountID, InboxID: inboxID, ConversationID: conversationID, MessageType: "incoming", ContentType: "text", ContentAttributes: datatypes.JSON([]byte(`{"email":{"auto_reply":true}}`))}, + } + + for _, message := range messages { + event := &channel.ChannelEvent{Type: channel.EventMessageCreated, AccountID: accountID, InboxID: inboxID, Data: map[string]interface{}{"message": message}} + require.NoError(t, listener.OnEvent(context.Background(), event)) + } + + var count int64 + require.NoError(t, db.Model(&AutomationExecution{}).Where("rule_id = ?", rule.ID).Count(&count).Error) + assert.Equal(t, int64(0), count) +} + // =========================== // RegisterAutomationRuleListener — correctly registers on channel.Dispatcher // =========================== @@ -324,4 +508,4 @@ func TestMapEventToAutomationEventName(t *testing.T) { assert.Equal(t, tt.expected, result) }) } -} \ No newline at end of file +} diff --git a/internal/handler/webhook/webhook_lookup_test.go b/internal/handler/webhook/webhook_lookup_test.go index 72cd61db..25233101 100644 --- a/internal/handler/webhook/webhook_lookup_test.go +++ b/internal/handler/webhook/webhook_lookup_test.go @@ -17,6 +17,7 @@ import ( "time" "github.com/gin-gonic/gin" + "github.com/gochat/gochat/internal/automation" "github.com/gochat/gochat/internal/channel" linechannel "github.com/gochat/gochat/internal/channel/line" channelprovider "github.com/gochat/gochat/internal/channel/provider" @@ -33,6 +34,12 @@ type recordingListener struct { events []*channel.ChannelEvent } +type webhookAutomationDBProvider struct { + db *gorm.DB +} + +func (p webhookAutomationDBProvider) DB() *gorm.DB { return p.db } + func (l *recordingListener) Name() string { return "recording-listener" } func (l *recordingListener) OnEvent(ctx context.Context, event *channel.ChannelEvent) error { @@ -55,6 +62,8 @@ func newWebhookLookupTestDB(t *testing.T) *gorm.DB { &model.Conversation{}, &model.Message{}, &model.DeliveryStatus{}, + &automation.AutomationRule{}, + &automation.AutomationExecution{}, &channelmodel.ChannelTelegram{}, &channelmodel.ChannelLINE{}, &channelmodel.ChannelTwilioSMS{}, @@ -68,6 +77,50 @@ func newWebhookLookupTestDB(t *testing.T) *gorm.DB { return db } +func TestIncomingPersisterTriggersAutomationRuleFromMessageCreated(t *testing.T) { + db := newWebhookLookupTestDB(t) + inbox := seedWebhookInbox(t, db, "telegram") + dispatcher := channel.NewDispatcher() + automation.RegisterAutomationRuleListener(dispatcher, webhookAutomationDBProvider{db: db}) + persister := NewIncomingPersister(db, dispatcher) + + rule := &automation.AutomationRule{ + AccountID: inbox.AccountID, + EventName: "message_created", + Name: "provider message created", + Conditions: automation.Conditions{}, + Actions: automation.Actions{}, + Active: true, + } + if err := automation.NewAutomationRuleService(webhookAutomationDBProvider{db: db}).Create(t.Context(), rule); err != nil { + t.Fatalf("create automation rule: %v", err) + } + + msg := &channel.IncomingMessage{ + ChannelType: channel.ChannelTelegram, + SourceID: "tg-automation-1", + SenderID: "tg-automation-user", + SenderName: "Automation User", + SenderType: channel.SenderContact, + Content: "trigger automation", + ContentType: channel.ContentText, + InboxID: inbox.ID, + AccountID: inbox.AccountID, + } + result, err := persister.PersistIncoming(t.Context(), &inbox, msg) + if err != nil { + t.Fatalf("persist incoming: %v", err) + } + + var exec automation.AutomationExecution + if err := db.Where("rule_id = ? AND conversation_id = ?", rule.ID, result.Conversation.ID).First(&exec).Error; err != nil { + t.Fatalf("expected automation execution from provider message_created: %v", err) + } + if exec.Status != automation.ExecutionStatusSuccess { + t.Fatalf("expected success execution, got %s", exec.Status) + } +} + func TestIncomingPersisterUpdatesMessageStatus(t *testing.T) { db := newWebhookLookupTestDB(t) inbox := seedWebhookInbox(t, db, "telegram") diff --git a/internal/service/conversation_service.go b/internal/service/conversation_service.go index 15e7b601..d7a7b96f 100644 --- a/internal/service/conversation_service.go +++ b/internal/service/conversation_service.go @@ -72,12 +72,19 @@ func (s *ConversationService) indexMessage(ctx context.Context, message *model.M // dispatchConversationEvent is a helper to build and dispatch a conversation event. func (s *ConversationService) dispatchConversationEvent(ctx context.Context, eventType channel.EventType, conversation *model.Conversation) { + s.dispatchConversationEventWithData(ctx, eventType, conversation, nil) +} + +func (s *ConversationService) dispatchConversationEventWithData(ctx context.Context, eventType channel.EventType, conversation *model.Conversation, data map[string]interface{}) { event := channel.NewChannelEvent(eventType, channel.ChannelType(conversation.ChannelType), conversation.AccountID, conversation.InboxID) event.ConversationID = conversation.ID event.ContactID = conversation.ContactID if conversation.AssigneeID != nil { event.UserID = *conversation.AssigneeID } + for key, value := range data { + event.Data[key] = value + } event.Data["conversation"] = conversation applogger.L().Infof("dispatching event %s for conversation %d", eventType, conversation.ID) if err := s.dispatcher.Dispatch(ctx, event); err != nil { @@ -85,6 +92,27 @@ func (s *ConversationService) dispatchConversationEvent(ctx context.Context, eve } } +func changedAttributes(changes map[string][2]interface{}) map[string]interface{} { + out := make(map[string]interface{}, len(changes)) + for attr, values := range changes { + if fmt.Sprintf("%v", values[0]) == fmt.Sprintf("%v", values[1]) { + continue + } + out[attr] = map[string]interface{}{ + "from": values[0], + "to": values[1], + } + } + return out +} + +func eventDataWithChanges(changes map[string]interface{}) map[string]interface{} { + if len(changes) == 0 { + return nil + } + return map[string]interface{}{"changed_attributes": changes} +} + // ListByAccount retrieves all conversations for an account. func (s *ConversationService) ListByAccount(ctx context.Context, accountID uint, offset, limit int) ([]model.Conversation, int64, error) { return s.repo.FindByAccount(ctx, accountID, offset, limit) @@ -213,7 +241,14 @@ func (s *ConversationService) Create(ctx context.Context, accountID uint, req Cr // Non-critical: conversation was created, message creation failure is logged but not fatal } else { s.indexMessage(ctx, initialMsg) - s.dispatcher.Dispatch(ctx, channel.NewChannelEvent(channel.EventMessageCreated, channel.ChannelAPI, accountID, req.InboxID)) + event := channel.NewChannelEvent(channel.EventMessageCreated, channel.ChannelAPI, accountID, req.InboxID) + event.ConversationID = conversation.ID + event.ContactID = conversation.ContactID + event.Data["message"] = initialMsg + event.Data["conversation"] = conversation + if err := s.dispatcher.Dispatch(ctx, event); err != nil { + applogger.L().Errorf("failed to dispatch event %s for message %d: %v", channel.EventMessageCreated, initialMsg.ID, err) + } } } @@ -239,6 +274,11 @@ func (s *ConversationService) Update(ctx context.Context, accountID, id uint, re } oldStatus := conversation.Status + oldPriority := conversation.Priority + var oldSlaPolicyID interface{} + if conversation.SlaPolicyID != nil { + oldSlaPolicyID = *conversation.SlaPolicyID + } if req.Status != "" { conversation.Status = req.Status @@ -267,8 +307,19 @@ func (s *ConversationService) Update(ctx context.Context, accountID, id uint, re return nil, err } + var newSlaPolicyID interface{} + if conversation.SlaPolicyID != nil { + newSlaPolicyID = *conversation.SlaPolicyID + } + changes := changedAttributes(map[string][2]interface{}{ + "status": {oldStatus, conversation.Status}, + "priority": {oldPriority, conversation.Priority}, + "sla_policy_id": {oldSlaPolicyID, newSlaPolicyID}, + }) + changeData := eventDataWithChanges(changes) + // Dispatch EventConversationUpdated - s.dispatchConversationEvent(ctx, channel.EventConversationUpdated, conversation) + s.dispatchConversationEventWithData(ctx, channel.EventConversationUpdated, conversation, changeData) s.indexConversation(ctx, conversation) // If status changed, dispatch appropriate status event @@ -276,9 +327,9 @@ func (s *ConversationService) Update(ctx context.Context, accountID, id uint, re newStatus := model.ConversationStatus(req.Status) switch newStatus { case model.ConversationStatusResolved: - s.dispatchConversationEvent(ctx, channel.EventConversationResolved, conversation) + s.dispatchConversationEventWithData(ctx, channel.EventConversationResolved, conversation, changeData) case model.ConversationStatusOpen: - s.dispatchConversationEvent(ctx, channel.EventConversationOpened, conversation) + s.dispatchConversationEventWithData(ctx, channel.EventConversationOpened, conversation, changeData) } } @@ -417,7 +468,8 @@ func (s *ConversationService) ToggleStatus(ctx context.Context, accountID, id ui } conversation.Status = string(model.ConversationStatusOpen) // Fire bot handoff event (Chatwoot dispatches conversation.bot_handoff!) - s.dispatchConversationEvent(ctx, channel.EventConversationOpened, conversation) + changes := changedAttributes(map[string][2]interface{}{"status": {string(oldStatus), conversation.Status}}) + s.dispatchConversationEventWithData(ctx, channel.EventConversationOpened, conversation, eventDataWithChanges(changes)) s.indexConversation(ctx, conversation) return conversation, nil } @@ -468,14 +520,16 @@ func (s *ConversationService) ToggleStatus(ctx context.Context, accountID, id ui } // Dispatch appropriate status event based on new status + changes := changedAttributes(map[string][2]interface{}{"status": {string(oldStatus), conversation.Status}}) + changeData := eventDataWithChanges(changes) switch newStatus { case model.ConversationStatusResolved: - s.dispatchConversationEvent(ctx, channel.EventConversationResolved, conversation) + s.dispatchConversationEventWithData(ctx, channel.EventConversationResolved, conversation, changeData) case model.ConversationStatusOpen: - s.dispatchConversationEvent(ctx, channel.EventConversationOpened, conversation) + s.dispatchConversationEventWithData(ctx, channel.EventConversationOpened, conversation, changeData) default: // For pending/snoozed, dispatch generic updated event - s.dispatchConversationEvent(ctx, channel.EventConversationUpdated, conversation) + s.dispatchConversationEventWithData(ctx, channel.EventConversationUpdated, conversation, changeData) } s.indexConversation(ctx, conversation) @@ -810,14 +864,19 @@ func (s *ConversationService) UpdatePriority(ctx context.Context, accountID, id if err != nil { return nil, err } + oldPriority := conversation.Priority conversation.Priority = string(newPriority) if err := s.repo.Update(ctx, conversation); err != nil { return nil, err } - // Dispatch EventConversationPriorityUpdated - s.dispatchConversationEvent(ctx, channel.EventConversationPriorityUpdated, conversation) + changes := changedAttributes(map[string][2]interface{}{"priority": {oldPriority, conversation.Priority}}) + changeData := eventDataWithChanges(changes) + + // Dispatch Chatwoot automation-visible update plus local priority event. + s.dispatchConversationEventWithData(ctx, channel.EventConversationUpdated, conversation, changeData) + s.dispatchConversationEventWithData(ctx, channel.EventConversationPriorityUpdated, conversation, changeData) s.indexConversation(ctx, conversation) return conversation, nil diff --git a/internal/service/conversation_service_test.go b/internal/service/conversation_service_test.go index 273d7759..45f184d8 100644 --- a/internal/service/conversation_service_test.go +++ b/internal/service/conversation_service_test.go @@ -105,6 +105,17 @@ func setupConversationService(t *testing.T) (*ConversationService, *gorm.DB) { return svc, db } +type captureConversationEventsListener struct { + events []*channel.ChannelEvent +} + +func (l *captureConversationEventsListener) Name() string { return "capture_conversation_events" } + +func (l *captureConversationEventsListener) OnEvent(ctx context.Context, event *channel.ChannelEvent) error { + l.events = append(l.events, event) + return nil +} + func createConversationServiceTestSlaPolicy(t *testing.T, db *gorm.DB, accountID uint) *model.SlaPolicy { t.Helper() policy := &model.SlaPolicy{ @@ -187,6 +198,74 @@ func TestConversationService_Update_RejectsSlaPolicyReplacement(t *testing.T) { assert.Contains(t, err.Error(), "different sla") } +func TestConversationService_Update_DispatchesChangedAttributesForAutomation(t *testing.T) { + svc, db := setupConversationService(t) + capture := &captureConversationEventsListener{} + svc.dispatcher.Register(capture) + + account := createConversationServiceTestAccount(t, db) + inbox := createConversationServiceTestInbox(t, db, account.ID) + contact := createConversationServiceTestContact(t, db, account.ID) + conversation := createConversationServiceTestConversation(t, db, account.ID, inbox.ID, contact.ID, "open") + + _, err := svc.Update(context.Background(), account.ID, conversation.ID, UpdateConversationRequest{Status: "resolved", Priority: "high"}) + require.NoError(t, err) + + var updatedEvent, resolvedEvent *channel.ChannelEvent + for _, event := range capture.events { + switch event.Type { + case channel.EventConversationUpdated: + updatedEvent = event + case channel.EventConversationResolved: + resolvedEvent = event + } + } + require.NotNil(t, updatedEvent) + require.NotNil(t, resolvedEvent) + + for _, event := range []*channel.ChannelEvent{updatedEvent, resolvedEvent} { + changes, ok := event.Data["changed_attributes"].(map[string]interface{}) + require.True(t, ok) + statusChange, ok := changes["status"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "open", statusChange["from"]) + assert.Equal(t, "resolved", statusChange["to"]) + priorityChange, ok := changes["priority"].(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "none", priorityChange["from"]) + assert.Equal(t, "high", priorityChange["to"]) + } +} + +func TestConversationService_Create_DispatchesInitialMessageWithConversationData(t *testing.T) { + svc, db := setupConversationService(t) + capture := &captureConversationEventsListener{} + svc.dispatcher.Register(capture) + + account := createConversationServiceTestAccount(t, db) + inbox := createConversationServiceTestInbox(t, db, account.ID) + contact := createConversationServiceTestContact(t, db, account.ID) + + conversation, err := svc.Create(context.Background(), account.ID, CreateConversationRequest{InboxID: inbox.ID, ContactID: contact.ID, MessageContent: "hello"}) + require.NoError(t, err) + + var messageEvent *channel.ChannelEvent + for _, event := range capture.events { + if event.Type == channel.EventMessageCreated { + messageEvent = event + break + } + } + require.NotNil(t, messageEvent) + assert.Equal(t, conversation.ID, messageEvent.ConversationID) + assert.Equal(t, contact.ID, messageEvent.ContactID) + message, ok := messageEvent.Data["message"].(*model.Message) + require.True(t, ok) + assert.Equal(t, conversation.ID, message.ConversationID) + assert.Equal(t, account.ID, message.AccountID) + assert.Equal(t, inbox.ID, message.InboxID) +} + // ========== GetMeta Tests ========== func TestConversationService_GetMeta(t *testing.T) {