diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 0bce6d74..77008fc1 100644 --- a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md +++ b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md @@ -16,10 +16,10 @@ Build GoChat as a Go backend that can directly reuse the frontend from `referenc ## Current Baseline -- Current tracking checkpoint: 2026-06-06 after `2ab7b58 feat(accounts): align account payloads`, with this documentation checkpoint prepared as `docs: refresh parity tracker baseline`. -- Latest implementation checkpoint: `2ab7b58 feat(accounts): align account payloads`. -- Latest documentation-only checkpoint: this checkpoint, prepared as `docs: refresh parity tracker baseline`; this document is now the active follow-up plan and supersedes `.hermes/plans/*`. -- Worktree status at this documentation checkpoint: clean after `2ab7b58`. B11.1a aligns Captain assistant CRUD/tools/inbox bindings; B11.1b aligns Captain scenarios and custom tools; B11.1c aligns Captain documents, assistant responses, bulk actions, and custom-tool test payloads; B11.2 aligns Copilot thread/message create/list/get/delete payloads, account/user scoping, and no-LLM fallback persistence; B11.3a aligns Captain preferences show/update payloads and account-level model/feature storage; B11.3b aligns Captain playground request/response payloads, account scoping, and no-LLM fallback; B11.3c adds the fakeable Captain document sync backend gate with disabled, failed, and fake-success states; B11.3d aligns Captain task request/response payloads, no-provider disabled states, follow-up context, suggestion persistence, and Copilot message tool-call key validation; B11.3e aligns Captain stream DTOs/disabled SSE fallbacks and Copilot push-event payload shapes; B12.1 adds the reusable GoChat server/seed entrypoint plus a Meilisearch-first reused Chatwoot frontend smoke harness and report; B12.2a adds API smoke assertions for auth/profile, inbox, conversation/messages, contact/company, widget config/message, and public CSAT; B12.2b adds a zero-dependency Chrome DevTools browser smoke that loads the reused Chatwoot login and dashboard entrypoints through Vite and checks browser auth/dashboard API requests; B12.3a adds enterprise API smoke assertions for SLA reports/download, CSAT reports/download, automation/macros, audit/custom roles, capacity, Captain, and Copilot; B12.3b adds reused-frontend enterprise browser route navigation for SLA, CSAT, automation, macros, audit logs, custom roles, capacity, Captain, and Copilot request coverage; P5 durable job work through P5.13b is in Review; B9.3 verifies the current reference exposes no delayed automation action params and completes remaining automation action side effects; Phase 2/3 drift now aligns notifications, auth reset/confirmation, CRM shared attachments, account payloads, agents, teams, team members, and assignable-agent payload envelopes. Next active implementation slice is P3.6 label CRUD payload parity, followed by continued Phase 6 placeholder audit from fresh reference/smoke evidence. +- Current tracking checkpoint: 2026-06-06 after `8b378c7 docs: refresh parity tracker baseline`, with this implementation checkpoint prepared as `feat(labels): align chatwoot label payloads`. +- Latest implementation checkpoint: this checkpoint, prepared as `feat(labels): align chatwoot label payloads`. +- Latest documentation-only checkpoint: `8b378c7 docs: refresh parity tracker baseline`; this document is now the active follow-up plan and supersedes `.hermes/plans/*`. +- Worktree status at this implementation checkpoint: P3.6 label CRUD now aligns Chatwoot account `/labels` list/create/show/update/delete payloads, `title/description/color/show_on_sidebar` persistence, nested frontend request bodies, legacy `/tags` compatibility, account-scoped lookup, empty `200 OK` delete, and label association cleanup/rename side effects. B11.1a aligns Captain assistant CRUD/tools/inbox bindings; B11.1b aligns Captain scenarios and custom tools; B11.1c aligns Captain documents, assistant responses, bulk actions, and custom-tool test payloads; B11.2 aligns Copilot thread/message create/list/get/delete payloads, account/user scoping, and no-LLM fallback persistence; B11.3a aligns Captain preferences show/update payloads and account-level model/feature storage; B11.3b aligns Captain playground request/response payloads, account scoping, and no-LLM fallback; B11.3c adds the fakeable Captain document sync backend gate with disabled, failed, and fake-success states; B11.3d aligns Captain task request/response payloads, no-provider disabled states, follow-up context, suggestion persistence, and Copilot message tool-call key validation; B11.3e aligns Captain stream DTOs/disabled SSE fallbacks and Copilot push-event payload shapes; 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 continued Phase 6 placeholder audit from fresh reference/smoke evidence, then the next P3.6 custom-attributes/custom-filters drift slice. - `go test ./...` passes. - Route dump succeeds with `TOTAL: 837` after adding Chatwoot auth reset/confirmation routes and `GET /api/v1/accounts/:account_id/contacts/:contact_id/attachments`. - Route parity artifacts now exist under `docs/parity/` and are generated by `cmd/route_parity`. @@ -87,7 +87,7 @@ This table is the shortest authoritative handoff view. If an older lower section | Priority | Workstream | Current state | Next checkpoint | Commit close rule | | --- | --- | --- | --- | --- | -| 1 | Phase 2/3 drift | Tracked route parity is 0 missing for the current critical set; notification list/action serializers, Devise password reset/confirmation payloads, CRM shared attachment payloads, account show/update/create/cache-key/active-at payloads, settings agents/teams payloads, and assignable-agent payloads now match Chatwoot's frontend contract. Label CRUD drift is now captured as P3.6. | Land P3.6 label CRUD payload parity, then expand route/serializer fixtures when smoke or reference inspection exposes more drift. | Regenerate parity artifacts when routes change and add endpoint-family fixture tests. | +| 1 | Phase 2/3 drift | Tracked route parity is 0 missing for the current critical set; notification list/action serializers, Devise password reset/confirmation payloads, CRM shared attachment payloads, account/settings payloads, assignable-agent payloads, and label CRUD payloads now match Chatwoot's frontend contract. | Continue Phase 6 placeholder audit, then open the next P3.6 custom-attributes/custom-filters drift slice from reference/smoke evidence. | Regenerate parity artifacts when routes change and add endpoint-family fixture tests. | | 2 | Phase 6 placeholder audit | Widget/public/webhook critical placeholders are burned down; inbox WhatsApp health/register-webhook and sync-template drift are closed; account/contact/conversation/message/inbox audit remains broad. | Continue fresh placeholder audit and assign every frontend-reachable stub to a tracked owner. | `rg` placeholder audit is recorded and no reused-frontend blocker is ownerless. | | 3 | B12 optional live smoke | API/browser/enterprise smoke commands are checked in; live runs need PostgreSQL, Redis, Meilisearch, Vite, and Chrome. | Run full live smoke when environment is available and map failures to the board. | `docs/parity/frontend_smoke_report.md` records pass/fail and linked owners. | | 4 | B9.3 delayed automation actions | Current reference exposes no delayed automation action params; scheduled-item work is already P5.12; `send_email_to_team` is durable and `add_sla` mutates conversation/applied SLA state. | Keep automation drift closed if future reference/smoke exposes delayed params or unsupported action shapes. | Automation worker/action fixtures verify queued team email replay, retry visibility through worker jobs, and SLA action idempotency. | @@ -104,7 +104,7 @@ These rows are the executable development plan from this point forward. A checkp | P5.11c Copilot and conversation response jobs | `internal/service/copilot_service.go`, `internal/service/copilot_response_worker.go`, `internal/service/captain_conversation_service.go`, `internal/service/message_service.go`, `internal/app/bootstrap.go` | `enterprise/app/jobs/captain/copilot/response_job.rb`, `enterprise/app/jobs/captain/conversation/response_builder_job.rb`, `enterprise/app/services/captain/copilot/chat_service.rb`, `enterprise/app/services/enterprise/message_templates/hook_execution_service.rb`, `enterprise/app/models/copilot_message.rb` | Queue assistant replies after Copilot user messages and Captain pending-conversation triggers. Persist assistant messages, enqueue Captain conversation replies/handoff messages, open handoff conversations, and keep fakeable provider disabled/failure states observable through durable retry. | Review by `feat(captain): queue copilot response jobs`; focused tests cover Copilot enqueue/persist/fallback/retry, Captain conversation enqueue/handoff/retry/non-pending skip, and service/worker/app package replay. | | P5.13a analytics placeholder burn-down | `internal/service/analytics_service.go`, `internal/service/analytics_query_helpers.go`, live/report handlers | Chatwoot `live_reports_controller.rb`, `reports_controller.rb`, `BotMetricsBuilder`, `InboxLabelMatrixBuilder`, `FirstResponseTimeDistributionBuilder`, `OutgoingMessagesCountBuilder` | Replace frontend-visible zero/empty placeholder responses for live conversations, grouped live conversations, bot summary/metrics, conversation summary, inbox-label matrix, first-response distribution, and outgoing-message counts with persisted conversation/message/reporting-event queries. | Review by `feat(reports): derive analytics aggregates`; focused service/handler tests prove non-zero values from persisted rows and `rg` finds no placeholder TODOs in these methods. | | P5.13b scheduled/cached analytics | `internal/service/analytics_service.go`, `internal/service/analytics_query_helpers.go`, `internal/service/reporting_rollup_worker.go`, `internal/service/reporting_rollup_service.go`, report handlers/services, worker bootstrap | Chatwoot report controllers/services used by dashboard analytics, `Reports::DataSource`, reporting rollup/backfill jobs | Wire scheduled or lazy cached rollup freshness/idempotency and route `GET /reports` to metric timeseries instead of the summary handler. Define freshness rules for expensive rollups. | Review by `feat(reports): add analytics timeseries rollups`; report fixtures prove timeseries values are derived from persisted conversations/messages/reporting events, rollups refresh idempotently, durable `reporting:rollup_day` jobs replay, and hidden placeholder report JSON does not reappear. | -| P3.6 label CRUD payload parity | `internal/handler/api/v1/label_handler.go`, `internal/service/tag_service.go`, `internal/model/tag.go`, `internal/repository/tag_repo.go`, migrations, label handler/service tests | `reference/chatwoot/app/controllers/api/v1/accounts/labels_controller.rb`, `app/views/api/v1/accounts/labels/*.json.jbuilder`, `app/models/label.rb`, `db/schema.rb`, dashboard `api/labels.js`, `store/modules/labels.js` | Align account `/labels` CRUD with Chatwoot: list returns `{ payload: [...] }`; create/show/update return raw `{ id, title, description, color, show_on_sidebar }`; delete returns empty `200 OK` and removes/renames associated conversation/contact labels when title changes or is destroyed. Add persisted `description` and `show_on_sidebar`, default color `#1f93ff`, title lowercasing/uniqueness, nested `{ label: ... }` bodies with legacy `name` compatibility, account-scoped lookup, and keep `/tags` aliases only as local compatibility surfaces. | Review by `feat(labels): align chatwoot label payloads`; focused handler/service/model tests cover frontend `response.data.payload` and raw mutation bodies, title/name compatibility, defaults, update/delete association side effects, and no local `{ success, data }` envelope. Run `go test ./internal/handler/api/v1 -run LabelHandler -count=1`, `go test ./internal/service -run 'TagService|LabelService' -count=1`, combined handler/service/router tests, full `go test ./...`, and `git diff --check`. | +| P3.6 label CRUD payload parity | `internal/handler/api/v1/label_handler.go`, `internal/service/tag_service.go`, `internal/model/tag.go`, `internal/repository/tag_repo.go`, migrations, label handler/service tests | `reference/chatwoot/app/controllers/api/v1/accounts/labels_controller.rb`, `app/views/api/v1/accounts/labels/*.json.jbuilder`, `app/models/label.rb`, `db/schema.rb`, dashboard `api/labels.js`, `store/modules/labels.js` | Account `/labels` CRUD now returns Chatwoot `{ payload: [...] }` lists, raw mutation/show payloads, empty `200 OK` delete, `title/description/color/show_on_sidebar` persistence, title lowercasing/uniqueness, account scope, nested `{ label: ... }` request bodies, legacy `name` compatibility, default color, and local association cleanup/rename side effects. | Review by `feat(labels): align chatwoot label payloads`; focused handler/service/repository tests, combined handler/service/router tests, `go test ./cmd/migrate -count=1`, full `go test ./...`, and `git diff --check` passed. | | Phase 2/3 drift audit | `cmd/route_parity`, `docs/parity/*`, serializer tests | `reference/chatwoot/config/routes.rb`, controller Jbuilder views, reused frontend API clients | Convert any smoke/reference mismatch into a named route, controller, or serializer slice. Static route extraction remains acceptable until Ruby/Bundler is available. | Regenerated route parity shows 0 missing tracked frontend routes; new serializer fixtures cover the drift. | | Phase 6 placeholder burn-down | Account/contact/conversation/message/inbox handlers and services | Matching reference controllers/Jbuilder views plus reused frontend screens | Re-run placeholder audit and assign every frontend-reachable stub to a specific owner. Burn down the highest-impact stubs before broad feature expansion. | `rg` placeholder audit is recorded here; no reused-frontend critical path is ownerless. | | B12 live smoke | `scripts/parity_frontend_smoke.sh`, `docs/parity/frontend_smoke_report.md`, `cmd/gochat` | Reused `reference/chatwoot` Vite frontend, dashboard route/API clients | Run optional live API/browser/enterprise smoke with PostgreSQL, Redis, Meilisearch, GoChat, Vite, and Chrome. Convert failures into named rows above. | Smoke report records command, environment, pass/fail, artifacts, and linked follow-up owners. | @@ -140,6 +140,7 @@ This ledger records the committed parity checkpoints that future slices should b | Commit | Scope | Verification summary | Follow-up state | | --- | --- | --- | --- | +| `feat(labels): align chatwoot label payloads` | Advances P3.6 label CRUD parity by aligning account `/labels` with Chatwoot `LabelsController`, label Jbuilder views, `Label` model/schema, and dashboard labels API/store. List now returns `{ payload }`; create/show/update return raw label payloads with `id`, `title`, `description`, `color`, and `show_on_sidebar`; create/update accept nested `{ label }` bodies and legacy raw `name`; delete returns empty `200 OK`; title normalization, default color, account scope, persisted description/sidebar flags, and local conversation/contact association cleanup are covered. | `go test ./internal/handler/api/v1 -run LabelHandler -count=1`; `go test ./internal/service -run 'TagService|LabelService' -count=1`; `go test ./internal/repository -run TagRepo -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./cmd/migrate -count=1`; `go test ./...`; `git diff --check`. | P3.6 label CRUD moves to Review; continue Phase 6 placeholder audit, then custom attributes/custom filters drift. | | `docs: refresh parity tracker baseline` | Documentation-only checkpoint after `2ab7b58 feat(accounts): align account payloads`. Refreshes the Current Baseline to the committed account payload work, records the clean handoff state, and lands the next executable P3.6 label CRUD parity contract from direct Chatwoot reference/frontend inspection. | `git diff --check`. No Go code changed. | Start `feat(labels): align chatwoot label payloads`, then continue Phase 6 placeholder audit. | | `feat(accounts): align account payloads` | Advances Phase 2/3 account settings parity by aligning core reused-frontend account endpoints with Chatwoot `AccountsController`, `_account.json.jbuilder`, `show/update/create` views, dashboard `account.js`, and the account Vuex store. Account show/update/settings now return raw account objects instead of local `{ success, data }` envelopes, with `settings`, `custom_attributes`, `features`, `cache_keys`, `support_email`, `created_at`, locale/name/domain/status fields. Authenticated account creation accepts Chatwoot `account_name` and returns `{ data: { account_id } }` for the add-account modal. `update_active_at` now returns empty `200 OK`, and `cache_keys` now returns Chatwoot `{ cache_keys: { label, inbox, team } }` with zero-epoch defaults. | `go test ./internal/handler/api/v1 -run AccountHandler -count=1`; `go test ./internal/service -run AccountService -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./...`; `git diff --check`. | P3.2 remains Doing for account users/invitations/permissions drift; continue Phase 6 placeholder audit from fresh reference/smoke evidence. | | `feat(settings): align assignable agents payloads` | Advances P3.2/P3.3 settings and inbox assignment parity by aligning `GET /api/v1/accounts/:account_id/assignable_agents` and `GET /api/v1/accounts/:account_id/inboxes/:inbox_id/assignable_agents` with Chatwoot `AssignableAgentsController#index`, `inboxes#assignable_agents`, and `_agent.json.jbuilder`. Assignable-agent endpoints now return `{ payload: [...] }` instead of the local `{ success, data }` envelope and serialize agent fields required by the reused conversation assignment UI: `account_id`, `availability_status`, `auto_offline`, `confirmed`, `provider`, `available_name`, `thumbnail`, role, and enterprise `custom_role_id`. AccountUser role, availability, auto-offline, and custom-role data are loaded from the account membership instead of the global user role. | `go test ./internal/handler/api/v1 -run AssignableAgent -count=1`; `go test ./internal/service -run AssignableAgent -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./...`; `git diff --check`. | P3.2/P3.3 remain Doing for deeper account users/invitations/permissions and inbox serializer drift; continue Phase 6 placeholder audit from fresh reference/smoke evidence. | @@ -260,11 +261,11 @@ This ledger records the committed parity checkpoints that future slices should b ## Next Slice Contract -Completed implementation slice: `2ab7b58 feat(accounts): align account payloads` now closes the core reused-frontend account show/update/create/cache-key/active-at payload drift. +Completed implementation slice: P3.6 label CRUD now closes the reused-frontend account label list/create/show/update/delete payload drift. -Completed documentation slice: this checkpoint refreshes the tracker baseline after the latest implementation commits and adds the P3.6 label CRUD payload contract from direct `reference/chatwoot` inspection. +Completed documentation slice: this checkpoint records the label CRUD implementation, verification commands, and next handoff target in the active tracker. -Next implementation slice: land P3.6 label CRUD payload parity, then continue Phase 6 placeholder burn-down. Optional B12 live smoke remains environment-dependent and should convert failures into named slice owners. +Next implementation slice: continue Phase 6 placeholder burn-down from a fresh audit, then open the next P3.6 custom-attributes/custom-filters drift slice from reference or smoke evidence. Optional B12 live smoke remains environment-dependent and should convert failures into named slice owners. | Step | Required result | Reference source | Verification | | --- | --- | --- | --- | @@ -296,7 +297,8 @@ Next implementation slice: land P3.6 label CRUD payload parity, then continue Ph | N26 | Keep B12.1 smoke harness as the current frontend verification baseline. | `reference/chatwoot` frontend boot scripts and GoChat dev/test boot flow. | Done by `test(parity): add frontend smoke harness`; next step is B12.2 browser/API assertions against the checked command/report. | | N27 | Keep Hermes source plans mapped but inactive. | `.hermes/plans/2025-05-24-global-search-meilisearch.md`, `.hermes/plans/2026-05-24-automation-macro-csat.md`. | New work must update this tracker directly; Hermes files are read-only source notes unless the user asks otherwise. | | N28 | Update this tracker after every implementation checkpoint. | This document. | `git diff --check`; `go test ./...` for Go changes. | -| N29 | Land P3.6 label CRUD payload parity before resuming broad placeholder burn-down. | Chatwoot labels controller/views/model/schema and dashboard labels API/store. | Owner row above defines code files, payloads, persistence, association side effects, and exit commands for `feat(labels): align chatwoot label payloads`. | +| N29 | Keep P3.6 label CRUD as current label baseline. | Chatwoot labels controller/views/model/schema and dashboard labels API/store. | Done by `feat(labels): align chatwoot label payloads`; raw/list payloads, persistence, account scope, and association cleanup are covered. | +| N30 | Resume Phase 6 placeholder burn-down from fresh evidence. | `rg` placeholder audit, reused frontend API calls, matching reference controllers/Jbuilder views. | Next implementation slice should record every frontend-reachable stub owner and burn down the highest-impact remaining placeholder. | Current B2 profile checkpoint: @@ -1549,7 +1551,7 @@ Frontend-critical API groups to audit first: | P3.3 | Inboxes/channels | Inbox CRUD, assignable agents, avatars, channel config, business hours, widget config. | Review; assignable-agent payload drift closed, broader inbox serializer drift remains evidence-driven. | | P3.4 | Conversations/messages | List filters, status changes, assignment, labels, private notes, attachments, drafts, typing/read events. | Review | | P3.5 | Contacts/companies | CRUD, merge, labels, notes, custom attributes, import/export, conversations relation, and shared attachments. | Doing | -| P3.6 | Labels/custom attributes/custom filters | Create/update/list behavior and exact response shapes. | Doing; label CRUD drift is captured as the next named checkpoint with Chatwoot `title/description/color/show_on_sidebar` payloads and raw/list response contracts. | +| P3.6 | Labels/custom attributes/custom filters | Create/update/list behavior and exact response shapes. | Doing; label CRUD is in Review with Chatwoot `title/description/color/show_on_sidebar` payloads and raw/list response contracts, while custom attributes/custom filters remain evidence-driven follow-ups. | | P3.7 | Notifications/reports/help center/campaigns | Frontend-visible payloads and pagination/error envelopes. | Doing; notifications and reports now have focused Chatwoot payload fixtures, help center/campaigns remain broader audit scope. | | P3.8 | Widget/public APIs | Widget init, campaigns, config, contact, conversations, messages, direct uploads, public inbox flow, public CSAT. | Doing | | P3.9 | Search payloads | Global search and entity search documents backed by Meilisearch. | Review | @@ -2059,6 +2061,7 @@ Verification milestone gates: ## Progress Log +- 2026-06-06: P3.6 label CRUD checkpoint prepared as `feat(labels): align chatwoot label payloads`; audited Chatwoot `LabelsController`, label Jbuilder views, `Label` model/schema, and dashboard labels API/store. Account `/labels` list now returns `{ payload }`, create/show/update return raw label payloads, create/update accept nested `{ label }` bodies plus legacy raw `name`, title values are normalized/lowercased, color defaults to `#1f93ff`, `description` and `show_on_sidebar` are persisted, account-scoped get/update/delete prevents cross-account access, and delete returns empty `200 OK` while removing conversation/contact label associations and legacy conversation label text. Focused label handler/service/repository tests, combined handler/service/router tests, migration tests, full `go test ./...`, and `git diff --check` passed; continue Phase 6 placeholder audit next. - 2026-06-06: Documentation checkpoint prepared as `docs: refresh parity tracker baseline`; current baseline now points at `2ab7b58 feat(accounts): align account payloads`, the clean handoff state is recorded, and P3.6 label CRUD parity is fully landed as the next executable slice. Direct reference audit covered `LabelsController`, label Jbuilder views, `Label` model/schema, and dashboard labels API/store; current Go gaps are local `{ success, data }` envelopes, `name` instead of `title`, missing `description/show_on_sidebar/default color`, `201/204` status drift, and missing label title update/delete association side effects. Verification for this docs-only checkpoint: `git diff --check`; next implementation commit is `feat(labels): align chatwoot label payloads`. - 2026-06-05: Phase 2/3 account settings checkpoint prepared as `feat(accounts): align account payloads`; audited Chatwoot `AccountsController`, `_account.json.jbuilder`, account create/show/update views, dashboard `account.js`, account Vuex store, and account settings components. Account show/update/settings now return raw Chatwoot account objects with settings, features, custom attributes, cache keys, locale/name/domain/status/support fields instead of local success/data envelopes. Account creation now accepts Chatwoot `account_name` and returns `{ data: { account_id } }` for the add-account modal. `update_active_at` returns empty `200 OK`, and `cache_keys` returns `{ cache_keys: { label, inbox, team } }` with zero-epoch defaults. Focused account handler/service tests, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue account users/invitations/permissions drift and Phase 6 placeholder audit next. - 2026-06-05: Phase 2/3 settings/inbox assignment checkpoint prepared as `feat(settings): align assignable agents payloads`; audited Chatwoot `AssignableAgentsController#index`, inbox `assignable_agents` view, `_agent.json.jbuilder`, and reused dashboard assignable-agent API/store consumers. Assignable-agent endpoints now return Chatwoot `{ payload: [...] }` instead of local success/data envelopes, serialize the same agent fields used by conversation assignment UI, and source role, availability, auto-offline, and custom-role data from the account membership. Focused handler/service tests, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue account users/invitations/permissions drift and Phase 6 placeholder audit next. diff --git a/internal/handler/api/v1/helpers.go b/internal/handler/api/v1/helpers.go index 621ed936..5eba1cd7 100644 --- a/internal/handler/api/v1/helpers.go +++ b/internal/handler/api/v1/helpers.go @@ -11,6 +11,9 @@ import ( ) func bindJSONWrappedOrRaw(c *gin.Context, wrapperKey string, target any) error { + if c.Request.Body == nil { + return fmt.Errorf("empty request body") + } body, err := io.ReadAll(c.Request.Body) if err != nil { return err diff --git a/internal/handler/api/v1/label_handler.go b/internal/handler/api/v1/label_handler.go index 13d897c4..10a986ff 100644 --- a/internal/handler/api/v1/label_handler.go +++ b/internal/handler/api/v1/label_handler.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/gin-gonic/gin" + "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/service" applogger "github.com/gochat/gochat/pkg/logger" "github.com/gochat/gochat/pkg/response" @@ -31,15 +32,11 @@ func (h *LabelHandler) CreateTag(c *gin.Context) { return } - // Chatwoot: params.require(:label) → {"label": {...}} - var wrapper struct { - Label service.CreateTagRequest `json:"label"` - } - if err := c.ShouldBindJSON(&wrapper); err != nil { + var req service.CreateTagRequest + if err := bindJSONWrappedOrRaw(c, "label", &req); err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, err.Error()) return } - req := wrapper.Label tag, err := h.tagSvc.Create(c.Request.Context(), accountID, &req) if err != nil { @@ -48,26 +45,31 @@ func (h *LabelHandler) CreateTag(c *gin.Context) { return } - response.Created(c, tag) + c.JSON(http.StatusOK, serializeLabel(tag)) } // GetTag retrieves a tag by ID. // GET /api/v1/accounts/:account_id/tags/:tag_id func (h *LabelHandler) GetTag(c *gin.Context) { + accountID, err := parseUintParam(c, "account_id") + if err != nil { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account_id") + return + } tagID, err := parseUintParam(c, "tag_id") if err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid tag_id") return } - tag, err := h.tagSvc.GetByID(c.Request.Context(), tagID) + tag, err := h.tagSvc.GetByIDAndAccountID(c.Request.Context(), accountID, tagID) if err != nil { applogger.L().Errorf("Get tag: %v", err) handleServiceError(c, err) return } - response.OK(c, tag) + c.JSON(http.StatusOK, serializeLabel(tag)) } // ListTags returns all tags in an account. @@ -86,27 +88,34 @@ func (h *LabelHandler) ListTags(c *gin.Context) { return } - response.OK(c, tags) + c.JSON(http.StatusOK, gin.H{"payload": serializeLabels(tags)}) } // UpdateTag modifies an existing tag. // PUT /api/v1/accounts/:account_id/tags/:tag_id func (h *LabelHandler) UpdateTag(c *gin.Context) { + accountID, err := parseUintParam(c, "account_id") + if err != nil { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account_id") + return + } tagID, err := parseUintParam(c, "tag_id") if err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid tag_id") return } - // Chatwoot: params.require(:label) → {"label": {...}} - var wrapper struct { - Label service.UpdateTagRequest `json:"label"` + if _, err := h.tagSvc.GetByIDAndAccountID(c.Request.Context(), accountID, tagID); err != nil { + applogger.L().Errorf("Get tag for update: %v", err) + handleServiceError(c, err) + return } - if err := c.ShouldBindJSON(&wrapper); err != nil { + + var req service.UpdateTagRequest + if err := bindJSONWrappedOrRaw(c, "label", &req); err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, err.Error()) return } - req := wrapper.Label tag, err := h.tagSvc.Update(c.Request.Context(), tagID, &req) if err != nil { @@ -115,25 +124,61 @@ func (h *LabelHandler) UpdateTag(c *gin.Context) { return } - response.OK(c, tag) + c.JSON(http.StatusOK, serializeLabel(tag)) } // DeleteTag soft-deletes a tag. // DELETE /api/v1/accounts/:account_id/tags/:tag_id func (h *LabelHandler) DeleteTag(c *gin.Context) { + accountID, err := parseUintParam(c, "account_id") + if err != nil { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account_id") + return + } tagID, err := parseUintParam(c, "tag_id") if err != nil { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid tag_id") return } + if _, err := h.tagSvc.GetByIDAndAccountID(c.Request.Context(), accountID, tagID); err != nil { + applogger.L().Errorf("Get tag for delete: %v", err) + handleServiceError(c, err) + return + } + if err := h.tagSvc.Delete(c.Request.Context(), tagID); err != nil { applogger.L().Errorf("Delete tag: %v", err) handleServiceError(c, err) return } - response.NoContent(c) + c.Status(http.StatusOK) +} + +func serializeLabels(tags []model.Tag) []gin.H { + payload := make([]gin.H, 0, len(tags)) + for i := range tags { + payload = append(payload, serializeLabel(&tags[i])) + } + return payload +} + +func serializeLabel(tag *model.Tag) gin.H { + return gin.H{ + "id": tag.ID, + "title": tag.Name, + "description": tag.Description, + "color": tag.Color, + "show_on_sidebar": labelShowOnSidebar(tag), + } +} + +func labelShowOnSidebar(tag *model.Tag) bool { + if tag.ShowOnSidebar == nil { + return false + } + return *tag.ShowOnSidebar } // ========== Conversation-Label Association ========== @@ -327,9 +372,9 @@ func (h *LabelHandler) GetConversationsByTag(c *gin.Context) { } response.OK(c, gin.H{ - "labels": labels, - "count": count, - "page": page, + "labels": labels, + "count": count, + "page": page, "per_page": perPage, }) -} \ No newline at end of file +} diff --git a/internal/handler/api/v1/label_handler_test.go b/internal/handler/api/v1/label_handler_test.go index ec23f3fb..1364a67f 100644 --- a/internal/handler/api/v1/label_handler_test.go +++ b/internal/handler/api/v1/label_handler_test.go @@ -33,7 +33,7 @@ func (s *LabelHandlerTestSuite) SetupSuite() { Logger: logger.Default.LogMode(logger.Silent), }) s.Require().NoError(err) - s.Require().NoError(db.AutoMigrate(&model.Account{}, &model.User{}, &model.Tag{}, &model.ConversationLabel{}, &model.Conversation{}, &model.Inbox{}, &model.Contact{})) + s.Require().NoError(db.AutoMigrate(&model.Account{}, &model.User{}, &model.Tag{}, &model.ConversationLabel{}, &model.ContactLabel{}, &model.Conversation{}, &model.Inbox{}, &model.Contact{})) s.db = db convLabelRepo := repository.NewConversationLabelRepo(db) @@ -57,15 +57,31 @@ func TestLabelHandlerSuite(t *testing.T) { suite.Run(t, new(LabelHandlerTestSuite)) } +func labelBoolPtr(v bool) *bool { return &v } + +func (s *LabelHandlerTestSuite) SetupTest() { + s.Require().NoError(s.db.Exec("DELETE FROM conversation_labels").Error) + s.Require().NoError(s.db.Exec("DELETE FROM contact_labels").Error) + s.Require().NoError(s.db.Unscoped().Where("account_id = ?", s.account.ID).Delete(&model.Tag{}).Error) +} + func (s *LabelHandlerTestSuite) TestListTags_Success() { r := gin.New() r.GET("/api/v1/accounts/:account_id/tags", s.handler.ListTags) + s.Require().NoError(s.db.Create(&model.Tag{AccountID: s.account.ID, Name: "billing", Color: "#ff0000", Description: "Billing issues", ShowOnSidebar: labelBoolPtr(true)}).Error) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", fmt.Sprintf("/api/v1/accounts/%d/tags", s.account.ID), nil) r.ServeHTTP(w, req) assert.Equal(s.T(), http.StatusOK, w.Code) + var resp map[string][]map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &resp)) + s.Require().Len(resp["payload"], 1) + assert.Equal(s.T(), "billing", resp["payload"][0]["title"]) + assert.Equal(s.T(), "Billing issues", resp["payload"][0]["description"]) + assert.Equal(s.T(), "#ff0000", resp["payload"][0]["color"]) + assert.Equal(s.T(), true, resp["payload"][0]["show_on_sidebar"]) } func (s *LabelHandlerTestSuite) TestCreateTag_BadRequest_EmptyBody() { @@ -81,10 +97,31 @@ func (s *LabelHandlerTestSuite) TestCreateTag_BadRequest_EmptyBody() { } func (s *LabelHandlerTestSuite) TestCreateTag_Success() { + r := gin.New() + r.POST("/api/v1/accounts/:account_id/labels", s.handler.CreateTag) + + body := map[string]interface{}{"label": map[string]interface{}{"title": "Priority", "description": "Hot queue", "color": "#FF0000", "show_on_sidebar": false}} + b, _ := json.Marshal(body) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", fmt.Sprintf("/api/v1/accounts/%d/labels", s.account.ID), bytes.NewBuffer(b)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusOK, w.Code) + var resp map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(s.T(), "priority", resp["title"]) + assert.Equal(s.T(), "Hot queue", resp["description"]) + assert.Equal(s.T(), "#FF0000", resp["color"]) + assert.Equal(s.T(), false, resp["show_on_sidebar"]) +} + +func (s *LabelHandlerTestSuite) TestCreateTag_RawNameCompatibility() { r := gin.New() r.POST("/api/v1/accounts/:account_id/tags", s.handler.CreateTag) - body := map[string]interface{}{"name": "priority", "color": "#FF0000"} + body := map[string]interface{}{"name": "legacy", "color": ""} b, _ := json.Marshal(body) w := httptest.NewRecorder() @@ -92,7 +129,59 @@ func (s *LabelHandlerTestSuite) TestCreateTag_Success() { req.Header.Set("Content-Type", "application/json") r.ServeHTTP(w, req) - assert.Equal(s.T(), http.StatusCreated, w.Code) + assert.Equal(s.T(), http.StatusOK, w.Code) + var resp map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(s.T(), "legacy", resp["title"]) + assert.Equal(s.T(), "#1f93ff", resp["color"]) + assert.Equal(s.T(), true, resp["show_on_sidebar"]) +} + +func (s *LabelHandlerTestSuite) TestUpdateTag_ChatwootPayloadAndAccountScope() { + r := gin.New() + r.PUT("/api/v1/accounts/:account_id/labels/:tag_id", s.handler.UpdateTag) + tag := &model.Tag{AccountID: s.account.ID, Name: "old", Color: "#000000", ShowOnSidebar: labelBoolPtr(true)} + s.Require().NoError(s.db.Create(tag).Error) + + desc := "New description" + color := "#00ff00" + show := false + body := map[string]interface{}{"label": map[string]interface{}{"title": "New", "description": desc, "color": color, "show_on_sidebar": show}} + b, _ := json.Marshal(body) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("PUT", fmt.Sprintf("/api/v1/accounts/%d/labels/%d", s.account.ID, tag.ID), bytes.NewBuffer(b)) + req.Header.Set("Content-Type", "application/json") + r.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusOK, w.Code) + var resp map[string]interface{} + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(s.T(), "new", resp["title"]) + assert.Equal(s.T(), desc, resp["description"]) + assert.Equal(s.T(), color, resp["color"]) + assert.Equal(s.T(), show, resp["show_on_sidebar"]) +} + +func (s *LabelHandlerTestSuite) TestDeleteTag_ReturnsOKAndRemovesAssociations() { + r := gin.New() + r.DELETE("/api/v1/accounts/:account_id/labels/:tag_id", s.handler.DeleteTag) + tag := &model.Tag{AccountID: s.account.ID, Name: "old", Color: "#000000", ShowOnSidebar: labelBoolPtr(true)} + s.Require().NoError(s.db.Create(tag).Error) + s.Require().NoError(s.db.Create(&model.ConversationLabel{AccountID: s.account.ID, ConversationID: 10, TagID: tag.ID}).Error) + s.Require().NoError(s.db.Create(&model.ContactLabel{AccountID: s.account.ID, ContactID: 20, TagID: tag.ID}).Error) + + w := httptest.NewRecorder() + req, _ := http.NewRequest("DELETE", fmt.Sprintf("/api/v1/accounts/%d/labels/%d", s.account.ID, tag.ID), nil) + r.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusOK, w.Code) + var convCount int64 + s.Require().NoError(s.db.Model(&model.ConversationLabel{}).Where("tag_id = ?", tag.ID).Count(&convCount).Error) + assert.Equal(s.T(), int64(0), convCount) + var contactCount int64 + s.Require().NoError(s.db.Model(&model.ContactLabel{}).Where("tag_id = ?", tag.ID).Count(&contactCount).Error) + assert.Equal(s.T(), int64(0), contactCount) } func (s *LabelHandlerTestSuite) TestGetTag_BadRequest_InvalidID() { @@ -196,4 +285,4 @@ func (s *LabelHandlerTestSuite) TestBatchRemoveLabel_BadRequest_InvalidAccountID r.ServeHTTP(w, req) assert.Equal(s.T(), http.StatusBadRequest, w.Code) -} \ No newline at end of file +} diff --git a/internal/model/tag.go b/internal/model/tag.go index 6ecea95b..df3a657b 100644 --- a/internal/model/tag.go +++ b/internal/model/tag.go @@ -10,15 +10,17 @@ import ( // Reference: Chatwoot Tag + Taggings merged + P2B M4 spec // Each tag belongs to an account and has a unique name within that account. type Tag struct { - ID uint `gorm:"primaryKey;autoIncrement" json:"id"` - AccountID uint `gorm:"not null;uniqueIndex:idx_tag_account_name" json:"account_id"` - Name string `gorm:"size:255;not null;uniqueIndex:idx_tag_account_name" json:"name"` - Color string `gorm:"size:50" json:"color,omitempty"` - CreatedAt time.Time `gorm:"autoCreateTime" json:"created_at"` - UpdatedAt time.Time `gorm:"autoUpdateTime" json:"updated_at"` - DeletedAt gorm.DeletedAt `gorm:"index" json:"deleted_at,omitempty"` + ID uint `gorm:"primaryKey;autoIncrement" json:"id"` + AccountID uint `gorm:"not null;uniqueIndex:idx_tag_account_name" json:"account_id"` + Name string `gorm:"size:255;not null;uniqueIndex:idx_tag_account_name" json:"name"` + Color string `gorm:"size:50;not null;default:#1f93ff" json:"color,omitempty"` + Description string `gorm:"type:text" json:"description,omitempty"` + ShowOnSidebar *bool `json:"show_on_sidebar"` + CreatedAt time.Time `gorm:"autoCreateTime" json:"created_at"` + UpdatedAt time.Time `gorm:"autoUpdateTime" json:"updated_at"` + DeletedAt gorm.DeletedAt `gorm:"index" json:"deleted_at,omitempty"` Account Account `gorm:"foreignKey:AccountID" json:"account,omitempty"` } -func (Tag) TableName() string { return "tags" } \ No newline at end of file +func (Tag) TableName() string { return "tags" } diff --git a/internal/repository/tag_repo.go b/internal/repository/tag_repo.go index c4cd3617..fbdf9a59 100644 --- a/internal/repository/tag_repo.go +++ b/internal/repository/tag_repo.go @@ -2,6 +2,7 @@ package repository import ( "context" + "strings" "github.com/gochat/gochat/internal/model" "gorm.io/gorm" @@ -30,6 +31,15 @@ func (r *TagRepo) GetByID(ctx context.Context, id uint) (*model.Tag, error) { return &tag, nil } +// GetByIDAndAccountID retrieves a tag scoped to an account. +func (r *TagRepo) GetByIDAndAccountID(ctx context.Context, accountID, id uint) (*model.Tag, error) { + var tag model.Tag + if err := r.db.WithContext(ctx).Where("account_id = ?", accountID).First(&tag, id).Error; err != nil { + return nil, err + } + return &tag, nil +} + // Update saves changes to an existing tag. func (r *TagRepo) Update(ctx context.Context, tag *model.Tag) error { return r.db.WithContext(ctx).Save(tag).Error @@ -40,6 +50,29 @@ func (r *TagRepo) Delete(ctx context.Context, id uint) error { return r.db.WithContext(ctx).Delete(&model.Tag{}, id).Error } +// DeleteWithAssociations removes a tag and the local association rows that point to it. +func (r *TagRepo) DeleteWithAssociations(ctx context.Context, tag *model.Tag) error { + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + if err := tx.Where("account_id = ? AND tag_id = ?", tag.AccountID, tag.ID).Delete(&model.ConversationLabel{}).Error; err != nil { + return err + } + if err := tx.Where("account_id = ? AND tag_id = ?", tag.AccountID, tag.ID).Delete(&model.ContactLabel{}).Error; err != nil { + return err + } + if err := r.updateConversationLabelText(ctx, tx, tag.AccountID, tag.Name, ""); err != nil { + return err + } + return tx.Delete(&model.Tag{}, tag.ID).Error + }) +} + +// RenameConversationLabelText keeps the legacy conversations.labels field aligned with label title changes. +func (r *TagRepo) RenameConversationLabelText(ctx context.Context, accountID uint, oldName, newName string) error { + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + return r.updateConversationLabelText(ctx, tx, accountID, oldName, newName) + }) +} + // FindByAccountID returns all tags for an account, ordered by name. func (r *TagRepo) FindByAccountID(ctx context.Context, accountID uint) ([]model.Tag, error) { var tags []model.Tag @@ -75,6 +108,61 @@ func (r *TagRepo) FindByNameAndAccountID(ctx context.Context, accountID uint, na return &tag, nil } +func (r *TagRepo) updateConversationLabelText(ctx context.Context, tx *gorm.DB, accountID uint, oldName, newName string) error { + oldName = strings.TrimSpace(oldName) + newName = strings.TrimSpace(newName) + if oldName == "" || oldName == newName { + return nil + } + + var conversations []model.Conversation + if err := tx.WithContext(ctx). + Where("account_id = ? AND labels IS NOT NULL AND labels != ''", accountID). + Find(&conversations).Error; err != nil { + return err + } + for _, conversation := range conversations { + updated, changed := replaceLabelName(conversation.Labels, oldName, newName) + if !changed { + continue + } + if err := tx.Model(&model.Conversation{}).Where("id = ?", conversation.ID).Update("labels", updated).Error; err != nil { + return err + } + } + return nil +} + +func replaceLabelName(labels, oldName, newName string) (string, bool) { + parts := strings.Split(labels, ",") + seen := map[string]struct{}{} + changed := false + result := make([]string, 0, len(parts)) + for _, part := range parts { + label := strings.TrimSpace(part) + if label == "" { + continue + } + if strings.EqualFold(label, oldName) { + changed = true + label = newName + } + if label == "" { + continue + } + key := strings.ToLower(label) + if _, ok := seen[key]; ok { + continue + } + seen[key] = struct{}{} + result = append(result, label) + } + if !changed { + return labels, false + } + return strings.Join(result, ","), true +} + // FindByIDs retrieves multiple tags by their IDs. func (r *TagRepo) FindByIDs(ctx context.Context, ids []uint) ([]model.Tag, error) { var tags []model.Tag @@ -82,4 +170,4 @@ func (r *TagRepo) FindByIDs(ctx context.Context, ids []uint) ([]model.Tag, error return nil, err } return tags, nil -} \ No newline at end of file +} diff --git a/internal/service/tag_service.go b/internal/service/tag_service.go index af75bc67..1eebf760 100644 --- a/internal/service/tag_service.go +++ b/internal/service/tag_service.go @@ -3,6 +3,7 @@ package service import ( "context" "errors" + "strings" "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" @@ -20,28 +21,52 @@ func NewTagService(repo *repository.TagRepo) *TagService { // CreateTagRequest is the DTO for creating a tag. type CreateTagRequest struct { - Name string `json:"name" validate:"required,min=1,max=255"` - Color string `json:"color"` + Name string `json:"name" validate:"omitempty,min=1,max=255"` + Title string `json:"title" validate:"omitempty,min=1,max=255"` + Description string `json:"description"` + Color string `json:"color"` + ShowOnSidebar *bool `json:"show_on_sidebar"` } // UpdateTagRequest is the DTO for updating a tag. type UpdateTagRequest struct { - Name string `json:"name" validate:"omitempty,min=1,max=255"` - Color string `json:"color"` + Name string `json:"name" validate:"omitempty,min=1,max=255"` + Title string `json:"title" validate:"omitempty,min=1,max=255"` + Description *string `json:"description"` + Color *string `json:"color"` + ShowOnSidebar *bool `json:"show_on_sidebar"` } // Create creates a new tag for the given account. func (s *TagService) Create(ctx context.Context, accountID uint, req *CreateTagRequest) (*model.Tag, error) { + name := normalizeLabelTitle(req.Title) + if name == "" { + name = normalizeLabelTitle(req.Name) + } + if name == "" { + return nil, errors.New("label title is required") + } + color := strings.TrimSpace(req.Color) + if color == "" { + color = "#1f93ff" + } + showOnSidebar := true + if req.ShowOnSidebar != nil { + showOnSidebar = *req.ShowOnSidebar + } + // Check for duplicate name within account - existing, err := s.repo.FindByNameAndAccountID(ctx, accountID, req.Name) + existing, err := s.repo.FindByNameAndAccountID(ctx, accountID, name) if err == nil && existing != nil { return nil, errors.New("tag name already exists in this account") } tag := &model.Tag{ - AccountID: accountID, - Name: req.Name, - Color: req.Color, + AccountID: accountID, + Name: name, + Description: req.Description, + Color: color, + ShowOnSidebar: &showOnSidebar, } if err := s.repo.Create(ctx, tag); err != nil { applogger.L().Errorf("Create tag: %v", err) @@ -59,6 +84,15 @@ func (s *TagService) GetByID(ctx context.Context, id uint) (*model.Tag, error) { return tag, nil } +// GetByIDAndAccountID retrieves a tag by ID scoped to an account. +func (s *TagService) GetByIDAndAccountID(ctx context.Context, accountID, id uint) (*model.Tag, error) { + tag, err := s.repo.GetByIDAndAccountID(ctx, accountID, id) + if err != nil { + return nil, err + } + return tag, nil +} + // Update modifies an existing tag. func (s *TagService) Update(ctx context.Context, id uint, req *UpdateTagRequest) (*model.Tag, error) { tag, err := s.repo.GetByID(ctx, id) @@ -66,34 +100,62 @@ func (s *TagService) Update(ctx context.Context, id uint, req *UpdateTagRequest) return nil, errors.New("tag not found") } - if req.Name != "" { + oldName := tag.Name + name := normalizeLabelTitle(req.Title) + if name == "" { + name = normalizeLabelTitle(req.Name) + } + if name != "" { // Check for duplicate name within account (excluding current tag) - existing, err := s.repo.FindByNameAndAccountID(ctx, tag.AccountID, req.Name) + existing, err := s.repo.FindByNameAndAccountID(ctx, tag.AccountID, name) if err == nil && existing != nil && existing.ID != id { return nil, errors.New("tag name already exists in this account") } - tag.Name = req.Name + tag.Name = name } - if req.Color != "" { - tag.Color = req.Color + if req.Color != nil { + color := strings.TrimSpace(*req.Color) + if color == "" { + color = "#1f93ff" + } + tag.Color = color + } + if req.Description != nil { + tag.Description = *req.Description + } + if req.ShowOnSidebar != nil { + tag.ShowOnSidebar = req.ShowOnSidebar } if err := s.repo.Update(ctx, tag); err != nil { applogger.L().Errorf("Update tag: %v", err) return nil, err } + if oldName != tag.Name { + if err := s.repo.RenameConversationLabelText(ctx, tag.AccountID, oldName, tag.Name); err != nil { + return nil, err + } + } return tag, nil } // Delete removes a tag by ID. func (s *TagService) Delete(ctx context.Context, id uint) error { - if err := s.repo.Delete(ctx, id); err != nil { + tag, err := s.repo.GetByID(ctx, id) + if err != nil { + return err + } + if err := s.repo.DeleteWithAssociations(ctx, tag); err != nil { applogger.L().Errorf("Delete tag: %v", err) return err } return nil } +func normalizeLabelTitle(title string) string { + return strings.ToLower(strings.TrimSpace(title)) +} + // List returns all tags for an account. func (s *TagService) List(ctx context.Context, accountID uint) ([]model.Tag, error) { tags, err := s.repo.FindByAccountID(ctx, accountID) @@ -113,4 +175,4 @@ func (s *TagService) ListPaginated(ctx context.Context, accountID uint, page, pe return nil, 0, err } return tags, count, nil -} \ No newline at end of file +} diff --git a/internal/service/tag_service_test.go b/internal/service/tag_service_test.go new file mode 100644 index 00000000..34239b1b --- /dev/null +++ b/internal/service/tag_service_test.go @@ -0,0 +1,89 @@ +package service + +import ( + "context" + "testing" + + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/repository" + "github.com/stretchr/testify/require" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +func newTagServiceTestDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&model.Account{}, &model.Tag{}, &model.Conversation{}, &model.ConversationLabel{}, &model.ContactLabel{})) + return db +} + +func tagBoolPtr(v bool) *bool { return &v } + +func TestTagService_CreateNormalizesChatwootLabelFields(t *testing.T) { + db := newTagServiceTestDB(t) + svc := NewTagService(repository.NewTagRepo(db)) + show := false + + tag, err := svc.Create(context.Background(), 1, &CreateTagRequest{ + Title: "Priority", + Description: "Hot queue", + Color: "", + ShowOnSidebar: &show, + }) + + require.NoError(t, err) + require.Equal(t, "priority", tag.Name) + require.Equal(t, "Hot queue", tag.Description) + require.Equal(t, "#1f93ff", tag.Color) + require.NotNil(t, tag.ShowOnSidebar) + require.False(t, *tag.ShowOnSidebar) +} + +func TestTagService_UpdateRenamesLegacyConversationLabelText(t *testing.T) { + db := newTagServiceTestDB(t) + svc := NewTagService(repository.NewTagRepo(db)) + tag := &model.Tag{AccountID: 1, Name: "old", Color: "#000000", ShowOnSidebar: tagBoolPtr(true)} + require.NoError(t, db.Create(tag).Error) + conversation := &model.Conversation{AccountID: 1, InboxID: 1, ContactID: 1, ChannelType: "Channel::WebWidget", Channel: "web_widget", Labels: "old,keep"} + require.NoError(t, db.Create(conversation).Error) + desc := "Renamed" + color := "#123456" + + updated, err := svc.Update(context.Background(), tag.ID, &UpdateTagRequest{ + Title: "New", + Description: &desc, + Color: &color, + }) + + require.NoError(t, err) + require.Equal(t, "new", updated.Name) + require.Equal(t, desc, updated.Description) + require.Equal(t, color, updated.Color) + require.NoError(t, db.First(conversation, conversation.ID).Error) + require.Equal(t, "new,keep", conversation.Labels) +} + +func TestTagService_DeleteRemovesAssociationsAndLegacyConversationText(t *testing.T) { + db := newTagServiceTestDB(t) + svc := NewTagService(repository.NewTagRepo(db)) + tag := &model.Tag{AccountID: 1, Name: "old", Color: "#000000", ShowOnSidebar: tagBoolPtr(true)} + require.NoError(t, db.Create(tag).Error) + conversation := &model.Conversation{AccountID: 1, InboxID: 1, ContactID: 1, ChannelType: "Channel::WebWidget", Channel: "web_widget", Labels: "old,keep"} + require.NoError(t, db.Create(conversation).Error) + require.NoError(t, db.Create(&model.ConversationLabel{AccountID: 1, ConversationID: conversation.ID, TagID: tag.ID}).Error) + require.NoError(t, db.Create(&model.ContactLabel{AccountID: 1, ContactID: 10, TagID: tag.ID}).Error) + + require.NoError(t, svc.Delete(context.Background(), tag.ID)) + + var convLabelCount int64 + require.NoError(t, db.Model(&model.ConversationLabel{}).Where("tag_id = ?", tag.ID).Count(&convLabelCount).Error) + require.Equal(t, int64(0), convLabelCount) + var contactLabelCount int64 + require.NoError(t, db.Model(&model.ContactLabel{}).Where("tag_id = ?", tag.ID).Count(&contactLabelCount).Error) + require.Equal(t, int64(0), contactLabelCount) + require.NoError(t, db.First(conversation, conversation.ID).Error) + require.Equal(t, "keep", conversation.Labels) +} diff --git a/migrations/000028_align_tags_with_chatwoot_labels.down.sql b/migrations/000028_align_tags_with_chatwoot_labels.down.sql new file mode 100644 index 00000000..3117b879 --- /dev/null +++ b/migrations/000028_align_tags_with_chatwoot_labels.down.sql @@ -0,0 +1,7 @@ +DROP INDEX IF EXISTS idx_tags_account_id; + +ALTER TABLE tags + ALTER COLUMN color DROP NOT NULL, + ALTER COLUMN color DROP DEFAULT, + DROP COLUMN IF EXISTS show_on_sidebar, + DROP COLUMN IF EXISTS description; diff --git a/migrations/000028_align_tags_with_chatwoot_labels.up.sql b/migrations/000028_align_tags_with_chatwoot_labels.up.sql new file mode 100644 index 00000000..18eb5f13 --- /dev/null +++ b/migrations/000028_align_tags_with_chatwoot_labels.up.sql @@ -0,0 +1,26 @@ +CREATE TABLE IF NOT EXISTS tags ( + id SERIAL PRIMARY KEY, + account_id INTEGER NOT NULL, + name VARCHAR(255) NOT NULL, + color VARCHAR(50) NOT NULL DEFAULT '#1f93ff', + description TEXT, + show_on_sidebar BOOLEAN, + created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(), + updated_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(), + deleted_at TIMESTAMP WITH TIME ZONE +); + +ALTER TABLE tags + ADD COLUMN IF NOT EXISTS description TEXT, + ADD COLUMN IF NOT EXISTS show_on_sidebar BOOLEAN; + +ALTER TABLE tags + ALTER COLUMN color SET DEFAULT '#1f93ff'; + +UPDATE tags SET color = '#1f93ff' WHERE color IS NULL OR color = ''; + +ALTER TABLE tags + ALTER COLUMN color SET NOT NULL; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_tag_account_name ON tags(account_id, name) WHERE deleted_at IS NULL; +CREATE INDEX IF NOT EXISTS idx_tags_account_id ON tags(account_id) WHERE deleted_at IS NULL;