From 3c30596bade43d606d8060f4ba03b2757cdd4565 Mon Sep 17 00:00:00 2001 From: Rogee Date: Sat, 6 Jun 2026 02:19:10 +0800 Subject: [PATCH] feat(attachments): align list pagination --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 10 +++-- internal/handler/api/v1/contact_handler.go | 5 +-- .../api/v1/contact_handler_crud_test.go | 41 +++++++++++++++++++ .../handler/api/v1/conversation_handler.go | 6 ++- .../api/v1/conversation_handler_test.go | 36 ++++++++++++++++ internal/handler/api/v1/helpers.go | 8 ++++ 6 files changed, 97 insertions(+), 9 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 8aea7343..d14d281e 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 `706c706 feat(platform): align account user membership`, with this implementation checkpoint prepared as `feat(profile): align account permissions`. -- Latest implementation checkpoint: this checkpoint, prepared as `feat(profile): align account permissions`. +- Current tracking checkpoint: 2026-06-06 after `0fb4e6c feat(profile): align account permissions`, with this implementation checkpoint prepared as `feat(attachments): align list pagination`. +- Latest implementation checkpoint: this checkpoint, prepared as `feat(attachments): align list pagination`. - 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: Profile account permission drift is closed for reused route guards and dashboard permission helpers: profile account entries now return Chatwoot `AccountUser#permissions` for ordinary roles (`["administrator"]` or `["agent"]`) and continue to return enterprise custom-role permissions plus `custom_role` when `custom_role_id` is present. The preceding platform account-user membership, agents list ordering, update-route/`auto_offline`, agents bulk invite, B10.3 CustomRole frontend update, and Phase 6 contacts pagination/active payload drifts are closed. Conversation transcript and custom-attribute response drifts are already closed. The fresh placeholder audit found remaining `chatwootParityStub` routes only in webhook handler-not-injected fallbacks, not in the reused dashboard account/contact/conversation/message/inbox critical path. P3.6 custom filters/custom attribute definitions and label CRUD are already in Review. B11.1a-B11.3e Captain/Copilot slices remain in Review; B12.1/B12.2/B12.3 smoke harnesses remain in Review; P5 durable job work through P5.13b is in Review. Next active implementation slice is the next named Phase 2/3 or Phase 6 drift from fresh reference/smoke evidence. +- Worktree status at this implementation checkpoint: Account conversation/contact attachment list pagination now matches Chatwoot's fixed 100-row page size for `ConversationsController#attachments` and `Contacts::AttachmentsController#index`; reused frontend `per_page` query values no longer shrink shared attachment payloads below the reference page size. Profile account permission drift is closed for reused route guards and dashboard permission helpers: profile account entries now return Chatwoot `AccountUser#permissions` for ordinary roles (`["administrator"]` or `["agent"]`) and continue to return enterprise custom-role permissions plus `custom_role` when `custom_role_id` is present. The preceding platform account-user membership, agents list ordering, update-route/`auto_offline`, agents bulk invite, B10.3 CustomRole frontend update, and Phase 6 contacts pagination/active payload drifts are closed. Conversation transcript and custom-attribute response drifts are already closed. The fresh placeholder audit found remaining `chatwootParityStub` routes only in webhook handler-not-injected fallbacks, not in the reused dashboard account/contact/conversation/message/inbox critical path. P3.6 custom filters/custom attribute definitions and label CRUD are already in Review. B11.1a-B11.3e Captain/Copilot slices remain in Review; B12.1/B12.2/B12.3 smoke harnesses remain in Review; P5 durable job work through P5.13b is in Review. Next active implementation slice is the next named Phase 2/3 or Phase 6 drift from fresh reference/smoke evidence. - `go test ./...` passes. - Route dump succeeds with `TOTAL: 840` after adding the Chatwoot platform account-user collection destroy route. - Route parity artifacts now exist under `docs/parity/` and are generated by `cmd/route_parity`. @@ -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/settings payloads, assignable-agent payloads, label CRUD payloads, custom filters, and custom attribute definitions now match the inspected Chatwoot frontend contract. | Continue Phase 6 placeholder audit, then open the next named serializer/controller drift slice from reference/smoke evidence. | 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 plus fixed 100-row attachment pagination, account/settings payloads, assignable-agent payloads, label CRUD payloads, custom filters, and custom attribute definitions now match the inspected Chatwoot frontend contract. | Continue Phase 6 placeholder audit, then open the next named serializer/controller 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; fresh `rg` audit shows only webhook nil-handler fallbacks still call `chatwootParityStub`; dashboard conversation transcript response drift is closed. | Continue targeted account/contact/conversation/message/inbox drift from reference/smoke evidence. | `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. | @@ -143,6 +143,7 @@ This ledger records the committed parity checkpoints that future slices should b | Commit | Scope | Verification summary | Follow-up state | | --- | --- | --- | --- | +| `feat(attachments): align list pagination` | Advances Phase 6 dashboard/CRM attachment drift by matching Chatwoot `ConversationsController#attachments`, `Contacts::AttachmentsController#index`, and the shared attachment Jbuilder payloads. Conversation and contact attachment lists now page with the reference fixed 100-row limit from `ATTACHMENT_RESULTS_PER_PAGE`/`RESULTS_PER_PAGE` and ignore reused-frontend `per_page` query values, while retaining newest-first attachment ordering and `{ meta: { total_count }, payload }` responses. | `go test ./internal/handler/api/v1 -run 'Test(ConversationHandlerTestSuite|ContactHandlerCRUDTestSuite)/TestListAttachments' -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/repository -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 840`. | Continue the next named Phase 2/3 or Phase 6 drift from reference/smoke evidence. | | `feat(profile): align account permissions` | Advances P3.1/P3.2 profile and permission parity by matching Chatwoot `api/v1/models/_user.json.jbuilder`, `AccountUser#permissions`, and the enterprise custom-role prepend used by dashboard route guards. Profile account entries now expose ordinary role permissions as `["administrator"]` or `["agent"]`, and custom-role memberships expose the role's permission keys plus `custom_role` while retaining nested `custom_role` metadata. | `go test ./internal/handler/api/v1 -run ProfileHandler -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/repository -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 840`. | Continue P3.2 permissions/invitations drift or the next Phase 6 reference/smoke finding. | | `feat(platform): align account user membership` | Advances P3.2 platform account-user parity by matching `Platform::Api::V1::AccountUsersController`. Platform account-user index/create now return raw AccountUser JSON instead of the local `{ success, data }` envelope, create uses Chatwoot `find_or_initialize_by(user_id).update!` semantics so duplicate membership requests update role instead of failing, and the Chatwoot collection route `DELETE /platform/api/v1/accounts/:account_id/account_users/destroy?user_id=...` is registered while the legacy `/:user_id` alias remains. | `go test ./internal/handler/api/v1 -run PlatformAccountUser -count=1`; `go run ./cmd/dump_routes > docs/parity/gochat_routes.txt`; `go run ./cmd/route_parity`; `go test ./internal/handler/api/v1 ./internal/repository ./internal/router -count=1`; `go test ./...`; `git diff --check`. Route dump is `TOTAL: 840`; tracked route parity remains `270 exact, 7 parameter-compatible, 0 missing`. | Continue P3.2 permissions/invitations drift or the next Phase 6 reference/smoke finding. | | `feat(agents): align list ordering` | Advances P3.2 agents/settings parity by matching Chatwoot `AgentsController#index`, which renders `Current.account.users.order_by_full_name`. Account agent lists now order by `lower(users.name) ASC` before pagination while preserving account-user role, availability, auto-offline, inviter, account-user ID, and custom-role metadata. | `go test ./internal/handler/api/v1 -run AgentHandler -count=1`; `go test ./internal/handler/api/v1 ./internal/service ./internal/router -count=1`; `go test ./...`; `git diff --check`. No route changes; route dump remains `TOTAL: 839`. | Continue P3.2 account users/invitations/permissions drift or the next Phase 6 reference/smoke finding. | @@ -2077,6 +2078,7 @@ Verification milestone gates: ## Progress Log +- 2026-06-06: Phase 6 attachment pagination checkpoint prepared as `feat(attachments): align list pagination`; audited Chatwoot `ConversationsController#attachments`, `Contacts::AttachmentsController#index`, and the shared attachment Jbuilder partials. Conversation and contact shared attachment lists now use Chatwoot's fixed 100-row page size and ignore `per_page` query values, preserving `{ meta: { total_count }, payload }`, newest-first ordering, sender serialization, and contact attachment `conversation_id`. Focused attachment suite tests, combined handler/service/repository tests, full `go test ./...`, and `git diff --check` passed; continue the next Phase 2/3 or Phase 6 drift from reference/smoke evidence. - 2026-06-06: P3.1/P3.2 profile permissions checkpoint prepared as `feat(profile): align account permissions`; audited Chatwoot `_user.json.jbuilder`, `AccountUser#permissions`, enterprise `Enterprise::AccountUser`, and dashboard `permissionsHelper.js`. Profile account entries now expose `["administrator"]`/`["agent"]` for ordinary roles and custom-role permission keys plus `custom_role` for custom-role memberships, preserving nested custom-role metadata for the reused frontend. Focused profile tests, combined handler/service/repository tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 permissions/invitations drift next. - 2026-06-06: P3.2 platform account-user checkpoint prepared as `feat(platform): align account user membership`; audited Chatwoot `Platform::Api::V1::AccountUsersController` and platform routes. Platform account-user index/create now return raw AccountUser JSON, create updates existing memberships instead of failing duplicates, the user-permissible extra gate was removed to match the account-scoped reference controller, and collection destroy is available as `DELETE /account_users/destroy` with `user_id`. Focused platform account-user tests, route generation/parity, combined handler/repository/router tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 permissions/invitations drift next. - 2026-06-06: P3.2 agents index-order checkpoint prepared as `feat(agents): align list ordering`; audited Chatwoot `AgentsController#index` and `User.order_by_full_name`. Account agent lists now join users and order by `lower(users.name) ASC` before pagination, while preserving account-user metadata in the Chatwoot agent serializer. Focused handler tests, combined handler/service/router tests, full `go test ./...`, and `git diff --check` passed; continue P3.2 account users/invitations drift next. diff --git a/internal/handler/api/v1/contact_handler.go b/internal/handler/api/v1/contact_handler.go index 7b669c23..939563a9 100644 --- a/internal/handler/api/v1/contact_handler.go +++ b/internal/handler/api/v1/contact_handler.go @@ -11,7 +11,6 @@ import ( "github.com/gochat/gochat/internal/repository" "github.com/gochat/gochat/internal/search" "github.com/gochat/gochat/internal/service" - "github.com/gochat/gochat/pkg/pagination" "github.com/gochat/gochat/pkg/response" ) @@ -847,8 +846,8 @@ func (h *ContactHandler) ListAttachments(c *gin.Context) { return } - p := pagination.Parse(c) - attachments, total, svcErr := h.svc.ListAttachments(c.Request.Context(), accountID, contactID, p.Offset, p.PerPage) + _, offset := fixedPageOffset(c, chatwootAttachmentResultsPerPage) + attachments, total, svcErr := h.svc.ListAttachments(c.Request.Context(), accountID, contactID, offset, chatwootAttachmentResultsPerPage) if svcErr != nil { c.JSON(http.StatusUnprocessableEntity, gin.H{"error": "failed to list contact attachments"}) return diff --git a/internal/handler/api/v1/contact_handler_crud_test.go b/internal/handler/api/v1/contact_handler_crud_test.go index 9a0c1207..5b6378ba 100644 --- a/internal/handler/api/v1/contact_handler_crud_test.go +++ b/internal/handler/api/v1/contact_handler_crud_test.go @@ -644,6 +644,47 @@ func (s *ContactHandlerCRUDTestSuite) TestListAttachmentsReturnsChatwootPayload( s.Contains(item, "sender") } +func (s *ContactHandlerCRUDTestSuite) TestListAttachmentsUsesChatwootFixedPageSize() { + contact := &model.Contact{AccountID: s.account.ID, Name: "Attachment Page Contact", Email: "attachment-page@example.com"} + s.Require().NoError(s.db.Create(contact).Error) + inbox := &model.Inbox{AccountID: s.account.ID, Name: "Shared Files Fixed Page", ChannelType: "web_widget"} + s.Require().NoError(s.db.Create(inbox).Error) + displayID := uint(77) + conversation := &model.Conversation{AccountID: s.account.ID, InboxID: inbox.ID, ContactID: contact.ID, DisplayID: &displayID, Status: "open", ChannelType: "web_widget", Channel: "web_widget"} + s.Require().NoError(s.db.Create(conversation).Error) + message := &model.Message{AccountID: s.account.ID, InboxID: inbox.ID, ConversationID: conversation.ID, SenderID: &contact.ID, SenderType: "contact", Content: "files", MessageType: "incoming", ContentType: "text", Status: "sent"} + s.Require().NoError(s.db.Create(message).Error) + + baseTime := time.Now().Add(-time.Hour) + var newestID uint + for i := 0; i < 30; i++ { + attachment := &model.Attachment{ + Base: model.Base{CreatedAt: baseTime.Add(time.Duration(i) * time.Minute), UpdatedAt: baseTime.Add(time.Duration(i) * time.Minute)}, + AccountID: s.account.ID, + MessageID: message.ID, + FileType: "file", + FileURL: fmt.Sprintf("https://cdn.example.com/contact-file-%02d.txt", i), + FileName: fmt.Sprintf("contact-file-%02d.txt", i), + } + s.Require().NoError(s.db.Create(attachment).Error) + newestID = attachment.ID + } + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", fmt.Sprintf("/api/v1/accounts/%d/contacts/%d/attachments?per_page=5", s.account.ID, contact.ID), nil) + s.router.ServeHTTP(w, req) + + s.Equal(http.StatusOK, w.Code, w.Body.String()) + var resp map[string]any + s.NoError(json.Unmarshal(w.Body.Bytes(), &resp)) + s.Equal(float64(30), resp["meta"].(map[string]any)["total_count"]) + payload := resp["payload"].([]any) + s.Len(payload, 30) + first := payload[0].(map[string]any) + s.Equal(float64(newestID), first["id"]) + s.Equal(float64(displayID), first["conversation_id"]) +} + func (s *ContactHandlerCRUDTestSuite) TestMerge_ChatwootActionsPathReturnsRawContact() { base := &model.Contact{AccountID: s.account.ID, Name: "Base Contact", Email: "base@example.com"} mergee := &model.Contact{AccountID: s.account.ID, Name: "Mergee Contact", PhoneNumber: "+12212345"} diff --git a/internal/handler/api/v1/conversation_handler.go b/internal/handler/api/v1/conversation_handler.go index 9e21d68b..9dc87ce2 100644 --- a/internal/handler/api/v1/conversation_handler.go +++ b/internal/handler/api/v1/conversation_handler.go @@ -24,6 +24,8 @@ type ConversationHandler struct { auditSvc *service.AuditService } +const chatwootAttachmentResultsPerPage = 100 + // NewConversationHandler creates a new ConversationHandler. func NewConversationHandler(conversationSvc *service.ConversationService, messageSvc *service.MessageService) *ConversationHandler { return &ConversationHandler{conversationSvc: conversationSvc, messageSvc: messageSvc} @@ -763,12 +765,12 @@ func (h *ConversationHandler) ListAttachments(c *gin.Context) { return } - p := pagination.Parse(c) + _, offset := fixedPageOffset(c, chatwootAttachmentResultsPerPage) conversation, ok := h.resolveConversationRoute(c, accountID, conversationID) if !ok { return } - attachments, total, svcErr := h.messageSvc.ListAttachments(c.Request.Context(), accountID, conversation.ID, p.Offset, p.PerPage) + attachments, total, svcErr := h.messageSvc.ListAttachments(c.Request.Context(), accountID, conversation.ID, offset, chatwootAttachmentResultsPerPage) if svcErr != nil { handleServiceError(c, svcErr) return diff --git a/internal/handler/api/v1/conversation_handler_test.go b/internal/handler/api/v1/conversation_handler_test.go index fbc6295c..83a7e0fd 100644 --- a/internal/handler/api/v1/conversation_handler_test.go +++ b/internal/handler/api/v1/conversation_handler_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "strconv" "testing" + "time" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" @@ -446,6 +447,41 @@ func (s *ConversationHandlerTestSuite) TestListAttachmentsReturnsChatwootPayload assert.Contains(s.T(), item, "created_at") } +func (s *ConversationHandlerTestSuite) TestListAttachmentsUsesChatwootFixedPageSize() { + conversation := &model.Conversation{AccountID: s.testAccount.ID, InboxID: s.testConv.InboxID, ContactID: s.testConv.ContactID, Status: "open", ChannelType: "web_widget", Channel: "web_widget"} + s.Require().NoError(s.db.Create(conversation).Error) + message := &model.Message{AccountID: s.testAccount.ID, InboxID: conversation.InboxID, ConversationID: conversation.ID, Content: "files", MessageType: "incoming", ContentType: "text", Status: "sent"} + s.Require().NoError(s.db.Create(message).Error) + + baseTime := time.Now().Add(-time.Hour) + var newestID uint + for i := 0; i < 30; i++ { + attachment := &model.Attachment{ + Base: model.Base{CreatedAt: baseTime.Add(time.Duration(i) * time.Minute), UpdatedAt: baseTime.Add(time.Duration(i) * time.Minute)}, + AccountID: s.testAccount.ID, + MessageID: message.ID, + FileType: "file", + FileURL: fmt.Sprintf("https://cdn.example.com/file-%02d.txt", i), + FileName: fmt.Sprintf("file-%02d.txt", i), + } + s.Require().NoError(s.db.Create(attachment).Error) + newestID = attachment.ID + } + + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", fmt.Sprintf("%s/conversations/%d/attachments?per_page=5", s.accountURL(), conversation.ID), nil) + s.router.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusOK, w.Code, w.Body.String()) + var resp map[string]any + assert.NoError(s.T(), json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(s.T(), float64(30), resp["meta"].(map[string]any)["total_count"]) + payload := resp["payload"].([]any) + assert.Len(s.T(), payload, 30) + first := payload[0].(map[string]any) + assert.Equal(s.T(), float64(newestID), first["id"]) +} + func (s *ConversationHandlerTestSuite) TestToggleTyping_InvalidAccountID() { w := httptest.NewRecorder() req, _ := http.NewRequest("POST", "/api/v1/accounts/abc/conversations/1/toggle_typing", bytes.NewBufferString(`{"typing_status":"on"}`)) diff --git a/internal/handler/api/v1/helpers.go b/internal/handler/api/v1/helpers.go index 5eba1cd7..9f37db45 100644 --- a/internal/handler/api/v1/helpers.go +++ b/internal/handler/api/v1/helpers.go @@ -66,6 +66,14 @@ func parseIntQueryDefault(c *gin.Context, param string, fallback int) (int, erro return n, nil } +func fixedPageOffset(c *gin.Context, perPage int) (int, int) { + page, _ := strconv.Atoi(c.DefaultQuery("page", "1")) + if page < 1 { + page = 1 + } + return page, (page - 1) * perPage +} + func parseUintAnyParam(c *gin.Context, params ...string) (uint, error) { var lastErr error for _, param := range params {