diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index d9e2eb04..25762258 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -49,11 +49,11 @@ Hermes task landing checklist: ## Current Baseline -- Current tracking checkpoint: 2026-06-07 P3.98 conversation transcript delivery parity, prepared as `feat(conversations): align transcript delivery`. -- Latest implementation checkpoint: this checkpoint, prepared as `feat(conversations): align transcript delivery`. -- Latest documentation/tooling checkpoint: this tracker update records Chatwoot conversation transcript plan gate, rate limit, mailer, and message-scope parity. No route artifacts change. +- Current tracking checkpoint: 2026-06-07 P3.99 conversation destroy async parity, prepared as `feat(conversations): align destroy job`. +- Latest implementation checkpoint: this checkpoint, prepared as `feat(conversations): align destroy job`. +- Latest documentation/tooling checkpoint: this tracker update records Chatwoot conversation destroy `head :ok` and `DeleteObjectJob` parity. No route artifacts change. - Plan landing status: complete for the current known Hermes plans and user-confirmed scope. Future work should update this file directly instead of opening a parallel tracker. -- Worktree status at this implementation checkpoint: account conversation transcript delivery now follows Chatwoot's `ConversationsController#transcript` gates: missing email remains `422`, disabled transcript plans return raw `402`, configured daily email limits return empty `429`, delivered transcripts use the SMTP transcript boundary, increment the account outbound-email counter, and include only public incoming/outgoing chat messages. Automation `send_email_transcript` shares the same enabled/rate-limit behavior and stops at the configured limit. This retains P3.97 conversation typing event parity, P3.96 conversation last-seen read parity, P3.95 conversation mute contact-block parity, P3.94 conversation toggle-priority empty-body parity, P3.93 conversation toggle-status fallback parity, P3.92 automation-rule attachment parity, P3.91 upload/macro attachment parity, P3.90 SearchAPI filter-surface parity, and prior checkpoints. Live API/browser/enterprise smoke still needs the full PostgreSQL/Redis/Meilisearch/GoChat/Vite/Chrome stack. +- Worktree status at this implementation checkpoint: account conversation destroy now returns empty `200 OK` like Chatwoot `head :ok` and, when a WorkerPool is configured, enqueues a low-priority durable `conversation:delete_object` job matching the inspected `DeleteObjectJob` path before the worker performs the actual soft delete and event/search cleanup. No-worker tests keep a synchronous fallback. This retains P3.98 conversation transcript delivery parity, P3.97 conversation typing event parity, P3.96 conversation last-seen read parity, P3.95 conversation mute contact-block parity, P3.94 conversation toggle-priority empty-body parity, P3.93 conversation toggle-status fallback parity, and prior checkpoints. Live API/browser/enterprise smoke still needs the full PostgreSQL/Redis/Meilisearch/GoChat/Vite/Chrome stack. - Next executable implementation checkpoint: continue Phase 2/3 drift audit for the next reused-frontend mismatch, or run B12 live smoke when the full PostgreSQL/Redis/Meilisearch/GoChat/Vite/Chrome stack is available. Re-run Phase 6 placeholder audit after future route/smoke changes. - `go test ./...` passes when run outside the restricted socket sandbox for the latest implementation baseline; the latest docs/tooling checkpoint verified `scripts/parity_frontend_smoke.sh --check` with workspace-local temp/cache dirs after `/tmp` was full. - Route dump succeeds with `967` registered routes after profile MFA route tracking. @@ -156,6 +156,7 @@ This table is the shortest authoritative handoff view. If an older lower section | Priority | Workstream | Current state | Next checkpoint | Commit close rule | | --- | --- | --- | --- | --- | +| 0 | P3.99 conversation destroy async parity | Implemented for reused conversation delete actions: `DELETE /conversations/:conversation_id` now returns empty `200 OK` like Chatwoot and queues a low-priority durable `conversation:delete_object` job when workers are configured. The job performs the existing soft delete, dispatches `conversation.deleted`, and removes the search index entry; no-worker construction keeps synchronous focused-test behavior. | Keep in Review; reopen from B12 conversation delete smoke or fresh reference evidence for enterprise `DeleteObjectJob#process_post_deletion_tasks`, exact Pundit destroy permission drift, or heavy-association purge behavior beyond conversation objects. | Focused ConversationService delete/job tests and ConversationHandler delete response tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. | | 0 | P3.98 conversation transcript delivery parity | Implemented for reused conversation transcript and automation transcript actions: `POST /conversations/:conversation_id/transcript` now checks Chatwoot-style email transcript availability, enforces configured daily email limits, delivers through the SMTP transcript boundary, increments account outbound-email counters, and scopes transcript content to public incoming/outgoing chat messages. Automation `send_email_transcript` no-ops when disabled and stops recipient fan-out at the rate limit before sync delivery or durable enqueue. | Keep in Review; reopen from B12 transcript smoke or fresh reference evidence for exact Redis TTL behavior, cloud-only global default limit handling, mailer template HTML body drift, or provider-specific SMTP availability checks beyond the inspected mailer gate. | Focused ConversationService, ConversationHandler, ActionService, and Account model transcript tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. `accounts.limits` migration is included; no route artifacts change. | | 0 | P3.97 conversation typing event parity | Implemented for reused conversation typing action: `POST /conversations/:conversation_id/toggle_typing_status` now accepts empty bodies and `{}` as Chatwoot no-op `200 OK`, rejects malformed JSON, and emits `conversation.typing_on` / `conversation.typing_off` events with conversation/contact/user context and `is_private` metadata for valid statuses. | Keep in Review; reopen from B12 conversation typing smoke or fresh reference evidence for ActionCable event payload fields beyond the inspected `TypingStatusManager` contract. | Focused ConversationService and ConversationHandler toggle-typing tests passed; full `go test ./...` must pass outside the restricted socket sandbox; `git diff --check` must pass. No route artifacts change. | | 0 | P3.96 conversation last-seen read parity | Implemented for reused conversation read/unread actions: `POST /conversations/:conversation_id/update_last_seen` marks current-user unread notifications for the conversation as read, updates `agent_last_seen_at` plus `assignee_last_seen_at` when the viewer is the assignee and there are unread messages, updates only agent last-seen for non-assignees, and applies Chatwoot's one-hour no-unread write throttle; `unread` backdates both last-seen columns to the last incoming message minus one second. | Keep in Review; reopen from B12 conversation read/unread smoke or fresh reference evidence for unread-count notifier/event side effects not covered by the inspected controller/model contract. | Focused ConversationService and ConversationHandler last-seen/unread tests passed; full `go test ./...` must pass outside the restricted socket sandbox; `git diff --check` must pass. No route artifacts change. | @@ -262,6 +263,7 @@ These rows are the executable development plan from this point forward. A checkp | ID | Owner files | Reference files | Work to land | Exit gate | | --- | --- | --- | --- | --- | +| P3.99 conversation destroy async parity | `internal/handler/api/v1/conversation_handler.go`, `internal/service/conversation_service.go`, `internal/service/conversation_delete_worker.go`, `internal/app/bootstrap.go`, conversation handler/service tests | `reference/chatwoot/app/controllers/api/v1/accounts/conversations_controller.rb#destroy`, `reference/chatwoot/app/jobs/delete_object_job.rb`, reused dashboard `ConversationApi.delete` and `conversations/actions.js#deleteConversation` | Account conversation destroy now matches the inspected Chatwoot controller/job path: the handler returns empty `200 OK`, production service wiring enqueues a low-priority durable delete-object job, and the worker performs the existing soft-delete side effects plus event dispatch/search-index deletion. The service keeps synchronous behavior when no WorkerPool is set so focused tests and non-worker local callers still delete immediately. | Review by `feat(conversations): align destroy job`; focused tests prove handler `200` response shape, not-found behavior, synchronous fallback deletion, durable job enqueue, delayed visibility before worker execution, missing conversations do not enqueue phantom jobs, and worker replay deletion; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. | | P3.98 conversation transcript delivery parity | `internal/handler/api/v1/conversation_handler.go`, `internal/service/conversation_service.go`, `internal/model/account.go`, `internal/model/account_email.go`, `internal/automation/action_service.go`, migration `000037_add_accounts_limits`, conversation handler/service/action/model tests | `reference/chatwoot/app/controllers/api/v1/accounts/conversations_controller.rb#transcript`, `reference/chatwoot/app/models/concerns/account_email_rate_limitable.rb`, `reference/chatwoot/app/mailers/conversation_reply_mailer.rb#conversation_transcript`, `reference/chatwoot/app/models/message.rb#chat`, `reference/chatwoot/app/models/concerns/message_filter_helpers.rb#conversation_transcriptable?`, `reference/chatwoot/app/services/action_service.rb#send_email_transcript`, reused dashboard transcript modal/API/i18n payment-required behavior | Account conversation transcript delivery now matches the inspected Chatwoot controller and mailer contract: missing email returns raw `422`, disabled accounts return raw `402`, configured email limits return empty `429`, success returns empty `200 OK` after delivering through a fakeable SMTP transcript boundary and incrementing account outbound-email count. Transcript bodies include only public incoming/outgoing chat messages, and automation transcript actions respect the same enabled/rate behavior while splitting comma recipients and stopping at the first rate-limit miss. | Review by `feat(conversations): align transcript delivery`; focused tests prove account gate/rate helpers, handler `402/429` shape, service delivery/count/message-scope behavior, and automation disabled/rate/filter behavior; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. | | P3.97 conversation typing event parity | `internal/handler/api/v1/conversation_handler.go`, `internal/service/conversation_service.go`, `internal/channel/event.go`, conversation handler/service tests | `reference/chatwoot/app/controllers/api/v1/accounts/conversations_controller.rb#toggle_typing_status`, `reference/chatwoot/app/services/conversations/typing_status_manager.rb`, `reference/chatwoot/app/javascript/dashboard/api/inbox/conversation.js`, `reference/chatwoot/app/javascript/dashboard/store/modules/conversationTypingStatus.js`, reused dashboard reply-box typing callers | Account conversation `toggle_typing_status` now matches Chatwoot's permissive member action: missing body or missing `typing_status` resolves the conversation and returns empty `200 OK` without dispatching an event, malformed JSON still fails, and `on`/`off` statuses dispatch `conversation.typing_on` / `conversation.typing_off` events with conversation/contact/user context plus `is_private`. | Review by `feat(conversations): align typing events`; focused tests prove service event type/data behavior, blank-status no-op after conversation lookup, handler empty-body and `{}` no-op success, and invalid JSON failure; full `go test ./...` must pass outside the restricted socket sandbox; `git diff --check` must pass. No route artifacts change. | | P3.96 conversation last-seen read parity | `internal/service/conversation_service.go`, `internal/handler/api/v1/conversation_handler.go`, conversation handler/service tests | `reference/chatwoot/app/controllers/api/v1/accounts/conversations_controller.rb#update_last_seen`, `#unread`, `reference/chatwoot/app/models/conversation.rb#unread_messages`, `#assignee_unread_messages`, `reference/chatwoot/app/services/notification/mark_conversation_read_service.rb`, reused dashboard `ConversationApi.markMessageRead/markMessagesUnread` and store `messageReadActions.js` | Account conversation `update_last_seen` now mirrors Chatwoot's read path: it marks unread notifications for the conversation/current user as read, updates assignee last-seen only when the current viewer is the assignee, always updates agent last-seen when unread messages exist, and throttles no-unread writes to one hour. The `unread` endpoint now uses the Chatwoot helper shape by updating both `agent_last_seen_at` and `assignee_last_seen_at` to the last incoming message timestamp minus one second, or clearing both when no incoming message exists. | Review by `feat(conversations): align last seen reads`; focused tests prove notification read side effects, assignee/non-assignee column behavior, throttle behavior, and unread dual-column backdating; full `go test ./...` must pass outside the restricted socket sandbox; `git diff --check` must pass. No route artifacts change. | @@ -384,6 +386,7 @@ This ledger records the committed parity checkpoints that future slices should b | Commit | Scope | Verification summary | Follow-up state | | --- | --- | --- | --- | +| `feat(conversations): align destroy job` | Advances P3.99 with Chatwoot conversation destroy parity. GoChat now returns empty `200 OK` for `DELETE /conversations/:conversation_id`, wires a durable low-priority `conversation:delete_object` job into the WorkerPool, and lets that job perform the existing soft-delete/event/search cleanup while retaining a synchronous fallback without workers. | Focused ConversationService delete/job tests and ConversationHandler delete response tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. | Move P3.99 to Review; continue Phase 2/3 drift audit, Phase 6 placeholder audit, or B12 live smoke. | | `feat(conversations): align transcript delivery` | Advances P3.98 with Chatwoot conversation transcript delivery parity. GoChat now adds the Chatwoot `accounts.limits` field, uses account email transcript availability/rate helpers, maps dashboard transcript disabled/rate-limited responses to raw `402`/empty `429`, delivers transcript emails through the fakeable SMTP boundary, increments outbound-email counters, and filters transcript content to public incoming/outgoing chat messages. Automation `send_email_transcript` now no-ops when disabled and stops recipient fan-out at the configured rate limit. | Focused Account model, ConversationService, ConversationHandler, and ActionService transcript tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. | Move P3.98 to Review; continue Phase 2/3 drift audit, Phase 6 placeholder audit, or B12 live smoke. | | `feat(conversations): align typing events` | Advances P3.97 with Chatwoot conversation `toggle_typing_status` parity. GoChat now treats empty or missing `typing_status` as a no-op `200 OK`, keeps malformed JSON as a validation error, and dispatches Chatwoot-style `conversation.typing_on` / `conversation.typing_off` events with user/contact/conversation context and `is_private` metadata. | Focused ConversationService and ConversationHandler toggle-typing tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. | Move P3.97 to Review; continue Phase 2/3 drift audit, Phase 6 placeholder audit, or B12 live smoke. | | `feat(conversations): align last seen reads` | Advances P3.96 with Chatwoot conversation `update_last_seen` and `unread` parity. GoChat now marks current-user conversation notifications read, applies assignee-aware last-seen updates and the no-unread one-hour throttle, and marks conversations unread by backdating both agent and assignee last-seen columns. | Focused ConversationService and ConversationHandler last-seen/unread tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. | Move P3.96 to Review; continue Phase 2/3 drift audit, Phase 6 placeholder audit, or B12 live smoke. | @@ -2774,3 +2777,4 @@ Verification milestone gates: - 2026-06-07: P3.96 conversation last-seen checkpoint prepared as `feat(conversations): align last seen reads`; audited Chatwoot `ConversationsController#update_last_seen/#unread`, `Conversation#unread_messages/#assignee_unread_messages`, `Notification::MarkConversationReadService`, and reused dashboard message read/unread API/store actions. GoChat account conversation `update_last_seen` now marks current-user unread notifications for the conversation as read, updates agent and assignee last-seen columns according to assignee/unread-message state, and throttles no-unread writes to once per hour; `unread` now backdates or clears both agent and assignee last-seen columns. Focused ConversationService and ConversationHandler tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. - 2026-06-07: P3.97 conversation typing checkpoint prepared as `feat(conversations): align typing events`; audited Chatwoot `ConversationsController#toggle_typing_status`, `Conversations::TypingStatusManager`, and reused dashboard conversation typing API/store/reply-box callers. GoChat account conversation `toggle_typing_status` now accepts empty bodies and missing `typing_status` as no-op `200 OK`, still rejects malformed JSON, and dispatches Chatwoot-style `conversation.typing_on` / `conversation.typing_off` events with current user, contact, conversation, and `is_private` metadata for valid `on`/`off` statuses. Focused ConversationService and ConversationHandler toggle-typing tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. - 2026-06-07: P3.98 conversation transcript checkpoint prepared as `feat(conversations): align transcript delivery`; audited Chatwoot `ConversationsController#transcript`, `AccountEmailRateLimitable`, `ConversationReplyMailer#conversation_transcript`, `Message.chat`, `MessageFilterHelpers#conversation_transcriptable?`, and automation `ActionService#send_email_transcript`. GoChat account conversation transcript now honors email transcript availability, configured email limits, raw `402`/empty `429` response shapes, SMTP transcript delivery, outbound-email count increments, and public incoming/outgoing chat-message scoping; automation `send_email_transcript` no-ops when disabled and stops recipient fan-out at the rate limit before sync delivery or durable enqueue. Focused Account model, ConversationService, ConversationHandler, and ActionService transcript tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. +- 2026-06-07: P3.99 conversation destroy checkpoint prepared as `feat(conversations): align destroy job`; audited Chatwoot `ConversationsController#destroy`, `DeleteObjectJob`, and reused dashboard `ConversationApi.delete` / `deleteConversation` action. GoChat account conversation delete now returns empty `200 OK` instead of local `204`, queues a low-priority durable `conversation:delete_object` job when WorkerPool is configured, and the worker performs the existing soft delete, `conversation.deleted` dispatch, and search-index deletion; no-worker construction keeps synchronous deletion for focused tests/local callers. Focused ConversationService delete/job tests and ConversationHandler delete tests passed; full `go test ./...` passed outside the restricted socket sandbox; `git diff --check` passed. No route artifacts change. diff --git a/internal/app/bootstrap.go b/internal/app/bootstrap.go index 9d1c0c06..a0b63fb4 100644 --- a/internal/app/bootstrap.go +++ b/internal/app/bootstrap.go @@ -503,6 +503,7 @@ func Bootstrap(env string) (*App, error) { accountUserRepo := repository.NewAccountUserRepo(db) agentRepo := repository.NewAgentRepo(db) conversationService := service.NewConversationService(conversationRepo, messageRepo, channelDispatcher, inboxMemberService, accountUserRepo, teamRepo, teamMemberRepo) + conversationService.SetWorkerPool(workerPool) appliedSlaService := service.NewAppliedSlaService(appliedSlaRepo, slaEventRepo, slaPolicyRepo, conversationRepo) conversationService.SetAppliedSlaService(appliedSlaService) service.RegisterSlaProcessingJobs(workerPool, db, appliedSlaService) diff --git a/internal/handler/api/v1/conversation_handler.go b/internal/handler/api/v1/conversation_handler.go index 74486fd9..af50aad2 100644 --- a/internal/handler/api/v1/conversation_handler.go +++ b/internal/handler/api/v1/conversation_handler.go @@ -212,7 +212,7 @@ func (h *ConversationHandler) Delete(c *gin.Context) { Action: "destroy", AuditedChanges: gin.H{"id": conversation.ID, "display_id": conversation.DisplayID}, }) - response.NoContent(c) + c.Status(http.StatusOK) } // @Summary Assign an agent to a conversation diff --git a/internal/handler/api/v1/conversation_handler_crud_test.go b/internal/handler/api/v1/conversation_handler_crud_test.go index 2c78d592..4ca1c192 100644 --- a/internal/handler/api/v1/conversation_handler_crud_test.go +++ b/internal/handler/api/v1/conversation_handler_crud_test.go @@ -589,7 +589,8 @@ func (s *ConversationCrudTestSuite) TestDelete_Success() { req, _ := http.NewRequest("DELETE", s.convURL(s.testConv.ID), nil) s.router.ServeHTTP(w, req) - assert.Equal(s.T(), http.StatusNoContent, w.Code) + assert.Equal(s.T(), http.StatusOK, w.Code) + assert.Empty(s.T(), w.Body.String()) } func (s *ConversationCrudTestSuite) TestDelete_InvalidAccountID() { diff --git a/internal/handler/api/v1/conversation_handler_test.go b/internal/handler/api/v1/conversation_handler_test.go index 2c197f2f..7bee9997 100644 --- a/internal/handler/api/v1/conversation_handler_test.go +++ b/internal/handler/api/v1/conversation_handler_test.go @@ -144,6 +144,7 @@ func (s *ConversationHandlerTestSuite) SetupSuite() { conversations.GET("/:conversation_id/reporting_events", handler.ReportingEvents) conversations.POST("/:conversation_id/toggle_typing", handler.ToggleTyping) conversations.POST("/:conversation_id/update_last_seen", handler.UpdateLastSeen) + conversations.DELETE("/:conversation_id", handler.Delete) } } } @@ -448,6 +449,26 @@ func (s *ConversationHandlerTestSuite) TestTranscript_ConversationNotFound() { assert.Equal(s.T(), http.StatusNotFound, w.Code) } +func (s *ConversationHandlerTestSuite) TestDelete_SuccessReturnsChatwootHeadOK() { + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodDelete, s.accountURL()+"/conversations/"+strconv.FormatUint(uint64(s.testConv.ID), 10), nil) + s.router.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusOK, w.Code) + assert.Empty(s.T(), w.Body.String()) + + var deleted model.Conversation + assert.Error(s.T(), s.db.First(&deleted, s.testConv.ID).Error) +} + +func (s *ConversationHandlerTestSuite) TestDelete_ConversationNotFound() { + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodDelete, s.accountURL()+"/conversations/9999", nil) + s.router.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusNotFound, w.Code) +} + // ========== UpdateCustomAttributes Handler Tests ========== func (s *ConversationHandlerTestSuite) TestUpdateCustomAttributes_Success() { diff --git a/internal/service/conversation_delete_worker.go b/internal/service/conversation_delete_worker.go new file mode 100644 index 00000000..967d8f63 --- /dev/null +++ b/internal/service/conversation_delete_worker.go @@ -0,0 +1,40 @@ +package service + +import ( + "context" + "encoding/json" + "sync" + + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/worker" +) + +const TaskTypeConversationDeleteObject = "conversation:delete_object" + +type conversationDeleteObjectJob struct { + AccountID uint `json:"account_id"` + ConversationID uint `json:"conversation_id"` +} + +var conversationDeleteRegistrations sync.Map + +// RegisterConversationDeleteJobs wires Chatwoot's DeleteObjectJob path for +// conversation destroy actions. Without a worker, ConversationService.Delete +// keeps the synchronous fallback used by focused tests. +func RegisterConversationDeleteJobs(wp *worker.WorkerPool, svc *ConversationService) { + if wp == nil || svc == nil { + return + } + if _, loaded := conversationDeleteRegistrations.LoadOrStore(wp, struct{}{}); loaded { + return + } + wp.Register(TaskTypeConversationDeleteObject, svc.performConversationDeleteObject) +} + +func (s *ConversationService) performConversationDeleteObject(ctx context.Context, job *model.BackgroundJob) error { + var payload conversationDeleteObjectJob + if err := json.Unmarshal(job.Payload, &payload); err != nil { + return err + } + return s.deleteNow(ctx, payload.AccountID, payload.ConversationID) +} diff --git a/internal/service/conversation_service.go b/internal/service/conversation_service.go index daaf15ac..21e1f6be 100644 --- a/internal/service/conversation_service.go +++ b/internal/service/conversation_service.go @@ -13,6 +13,7 @@ import ( "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" "github.com/gochat/gochat/internal/search" + "github.com/gochat/gochat/internal/worker" applogger "github.com/gochat/gochat/pkg/logger" pkgvalidator "github.com/gochat/gochat/pkg/validator" @@ -33,6 +34,7 @@ type ConversationService struct { searchIndexer SearchIndexer appliedSlaSvc *AppliedSlaService transcriptMailer automation.AutomationTranscriptDeliverer + worker *worker.WorkerPool } // NewConversationService creates a new Conversation service. @@ -52,6 +54,11 @@ func (s *ConversationService) SetTranscriptDeliverer(deliverer automation.Automa s.transcriptMailer = deliverer } +func (s *ConversationService) SetWorkerPool(wp *worker.WorkerPool) { + s.worker = wp + RegisterConversationDeleteJobs(wp, s) +} + func (s *ConversationService) DB() *gorm.DB { if s == nil || s.repo == nil { return nil @@ -629,14 +636,29 @@ func (s *ConversationService) Delete(ctx context.Context, accountID, id uint) er if err != nil { return err } + if s.worker != nil { + _, err := s.worker.Enqueue(ctx, TaskTypeConversationDeleteObject, conversationDeleteObjectJob{AccountID: accountID, ConversationID: conversation.ID}, worker.WithQueue("low"), worker.WithMaxAttempts(3), worker.WithIdempotencyKey(fmt.Sprintf("conversation-delete:%d:%d", accountID, conversation.ID))) + return err + } + return s.deleteLoaded(ctx, conversation) +} +func (s *ConversationService) deleteNow(ctx context.Context, accountID, id uint) error { + conversation, err := s.repo.FindByAccountAndID(ctx, accountID, id) + if err != nil { + return err + } + return s.deleteLoaded(ctx, conversation) +} + +func (s *ConversationService) deleteLoaded(ctx context.Context, conversation *model.Conversation) error { if err := s.repo.Delete(ctx, conversation.ID); err != nil { return err } // Dispatch EventConversationDeleted s.dispatchConversationEvent(ctx, channel.EventConversationDeleted, conversation) - s.deleteConversationIndex(ctx, accountID, conversation.ID) + s.deleteConversationIndex(ctx, conversation.AccountID, conversation.ID) return nil } diff --git a/internal/service/conversation_service_test.go b/internal/service/conversation_service_test.go index e7a3f8b4..2a3a8cc2 100644 --- a/internal/service/conversation_service_test.go +++ b/internal/service/conversation_service_test.go @@ -16,6 +16,7 @@ import ( "github.com/gochat/gochat/internal/channel" "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" + "github.com/gochat/gochat/internal/worker" ) // ========== Test Setup ========== @@ -48,6 +49,7 @@ func setupConversationServiceTestDB(t *testing.T) *gorm.DB { &model.SlaPolicy{}, &model.AppliedSLA{}, &model.SlaEvent{}, + &model.BackgroundJob{}, ), "failed to auto-migrate") t.Cleanup(func() { @@ -779,6 +781,60 @@ func TestConversationService_SendTranscript_RateLimited(t *testing.T) { assert.Empty(t, deliverer.requests) } +func TestConversationService_Delete(t *testing.T) { + svc, db := setupConversationService(t) + + account := createConversationServiceTestAccount(t, db) + inbox := createConversationServiceTestInbox(t, db, account.ID) + contact := createConversationServiceTestContact(t, db, account.ID) + conv := createConversationServiceTestConversation(t, db, account.ID, inbox.ID, contact.ID, "open") + + require.NoError(t, svc.Delete(context.Background(), account.ID, conv.ID)) + + var found model.Conversation + assert.Error(t, db.First(&found, conv.ID).Error) + require.NoError(t, db.Unscoped().First(&found, conv.ID).Error) + assert.True(t, found.DeletedAt.Valid) +} + +func TestConversationService_Delete_QueuesDeleteObjectJob(t *testing.T) { + svc, db := setupConversationService(t) + wp := worker.NewWorkerPool(db) + svc.SetWorkerPool(wp) + + account := createConversationServiceTestAccount(t, db) + inbox := createConversationServiceTestInbox(t, db, account.ID) + contact := createConversationServiceTestContact(t, db, account.ID) + conv := createConversationServiceTestConversation(t, db, account.ID, inbox.ID, contact.ID, "open") + + require.NoError(t, svc.Delete(context.Background(), account.ID, conv.ID)) + + var queued model.BackgroundJob + require.NoError(t, db.Where("job_type = ? AND queue = ? AND status = ?", TaskTypeConversationDeleteObject, "low", model.BackgroundJobStatusQueued).First(&queued).Error) + var stillVisible model.Conversation + require.NoError(t, db.First(&stillVisible, conv.ID).Error) + + processed, err := wp.ProcessOne(context.Background()) + require.NoError(t, err) + assert.True(t, processed) + + var deleted model.Conversation + assert.Error(t, db.First(&deleted, conv.ID).Error) +} + +func TestConversationService_DeleteWithWorker_NotFoundDoesNotQueue(t *testing.T) { + svc, db := setupConversationService(t) + wp := worker.NewWorkerPool(db) + svc.SetWorkerPool(wp) + + err := svc.Delete(context.Background(), 1, 9999) + require.ErrorIs(t, err, gorm.ErrRecordNotFound) + + var count int64 + require.NoError(t, db.Model(&model.BackgroundJob{}).Where("job_type = ?", TaskTypeConversationDeleteObject).Count(&count).Error) + assert.Zero(t, count) +} + // ========== UpdateCustomAttributes Tests ========== func TestConversationService_UpdateCustomAttributes(t *testing.T) {