From f43ca571c200513047f6248e4a059bd84067ca09 Mon Sep 17 00:00:00 2001 From: Rogee Date: Sun, 7 Jun 2026 04:21:45 +0800 Subject: [PATCH] feat(conversations): align filter query operators --- docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md | 16 +++-- .../api/v1/conversation_handler_crud_test.go | 63 +++++++++++++++++++ internal/service/conversation_service.go | 31 ++++++++- 3 files changed, 102 insertions(+), 8 deletions(-) diff --git a/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md b/docs/CHATWOOT_PARITY_DEVELOPMENT_PLAN.md index 6b90b7ee..cd96e009 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.64 conversation filter payload parity, prepared as `feat(conversations): align filter payload basics`. -- Latest implementation checkpoint: this checkpoint, prepared as `feat(conversations): align filter payload basics`. -- Latest documentation/tooling checkpoint: this tracker update records Chatwoot `Conversations::FilterService` frontend payload basics for `status`, `priority`, and `display_id`, including raw `422 { error }` invalid-filter responses. No route artifacts change. +- Current tracking checkpoint: 2026-06-07 P3.65 conversation filter query-operator parity, prepared as `feat(conversations): align filter query operators`. +- Latest implementation checkpoint: this checkpoint, prepared as `feat(conversations): align filter query operators`. +- Latest documentation/tooling checkpoint: this tracker update records Chatwoot `Conversations::FilterService` frontend `query_operator` chain behavior and invalid query-operator error shape. 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: conversation filter now accepts Chatwoot frontend `{ payload: [...] }` conditions for standard status/priority/display-id filters, returns raw `422 { error }` for invalid attributes/operators, and computes filter counts from the filtered relation. P3.63 search message-order parity, P3.62 trimmed search payload fields, P3.61 sender-name, P3.60 numeric message-type parity, and prior report/profile/channel/contact checkpoints remain implemented. Live API/browser/enterprise smoke still needs the full PostgreSQL/Redis/Meilisearch/GoChat/Vite/Chrome stack. +- Worktree status at this implementation checkpoint: conversation filter payload conditions now honor Chatwoot `query_operator` chains with SQL `AND`/`OR` precedence and raw invalid-query-operator errors, while retaining P3.64 standard status/priority/display-id filtering and filtered counts. P3.63 search message-order parity, P3.62 trimmed search payload fields, P3.61 sender-name, P3.60 numeric message-type parity, and prior report/profile/channel/contact checkpoints remain implemented. 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 deeper report data-source/metric-builder drift or 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,7 +156,8 @@ This table is the shortest authoritative handoff view. If an older lower section | Priority | Workstream | Current state | Next checkpoint | Commit close rule | | --- | --- | --- | --- | --- | -| 0 | P3.64 conversation filter payload basics | Implemented for reused dashboard advanced conversation filters: GoChat now accepts Chatwoot `{ payload: [...] }` bodies for `status`, `priority`, and `display_id` with `equal_to`/`not_equal_to`, computes meta counts from the filtered relation, and returns raw `422 { error }` for invalid attributes/operators. | Keep in Review; reopen from B12 filter smoke or fresh reference evidence for query-operator chains, custom/additional attributes, labels, dates, assignee/inbox/team/message filters, or full `Conversations::PermissionFilterService` depth. | Focused conversation filter handler tests, full `go test ./...`, and `git diff --check` must pass. No route artifacts change. | +| 0 | P3.65 conversation filter query-operator parity | Implemented for reused dashboard advanced conversation filters: `{ payload: [...] }` conditions now honor `query_operator` chains (`AND`/`OR`) using the same SQL precedence as Chatwoot's concatenated filter query, and invalid query operators return raw `422 { error }`. | Keep in Review; reopen from B12 filter smoke or fresh reference evidence for custom/additional attributes, labels, dates, assignee/inbox/team/message filters, or full `Conversations::PermissionFilterService` depth. | Focused conversation filter handler tests, full `go test ./...`, and `git diff --check` must pass. No route artifacts change. | +| 0 | P3.64 conversation filter payload basics | Implemented for reused dashboard advanced conversation filters: GoChat now accepts Chatwoot `{ payload: [...] }` bodies for `status`, `priority`, and `display_id` with `equal_to`/`not_equal_to`, computes meta counts from the filtered relation, and returns raw `422 { error }` for invalid attributes/operators. | Keep in Review; reopen from B12 filter smoke or fresh reference evidence for custom/additional attributes, labels, dates, assignee/inbox/team/message filters, or full `Conversations::PermissionFilterService` depth. | Focused conversation filter handler tests, full `go test ./...`, and `git diff --check` must pass. No route artifacts change. | | 0 | P3.63 conversation search message-order parity | Implemented for reused dashboard conversation search results: GoChat now follows Chatwoot `Message` `default_scope { order(created_at: :asc) }` when serializing `conversation.messages` inside search payload items. | Keep in Review; reopen from B12 conversation search smoke or fresh reference evidence for sender polymorphic types beyond Contact/User or additional search model serializer fields. | Focused conversation search handler test, full `go test ./...`, and `git diff --check` must pass. No route artifacts change. | | 0 | P3.62 conversation search payload-field parity | Implemented for reused dashboard conversation search results: GoChat now follows `conversations/search.json.jbuilder` plus the search `_conversation` partial by omitting the normal conversation `meta` object from each search payload item. | Keep in Review; reopen from B12 conversation search smoke or fresh reference evidence for sender polymorphic types beyond Contact/User or additional search model serializer fields. | Focused conversation search handler test, full `go test ./...`, and `git diff --check` must pass. No route artifacts change. | | 0 | P3.61 conversation search sender-name parity | Implemented for reused dashboard conversation search results: GoChat now follows inspected `api/v1/models/_conversation.json.jbuilder` by emitting `sender_name` from `message.sender.name` for persisted contact/user senders in search result messages. | Keep in Review; reopen from B12 conversation search smoke or fresh reference evidence for message ordering, sender polymorphic types beyond Contact/User, or additional search model serializer fields. | Focused conversation search handler test, full `go test ./...`, and `git diff --check` must pass. No route artifacts change. | @@ -227,6 +228,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.65 conversation filter query-operator parity | `internal/service/conversation_service.go`, conversation filter handler tests | `reference/chatwoot/app/services/filter_service.rb#query_builder`, `reference/chatwoot/app/helpers/filters/filter_helper.rb#validate_single_condition`, `reference/chatwoot/spec/services/conversations/filter_service_frontend_alignment_spec.rb` | Conversation filter payload conditions now append `query_operator` into one SQL predicate chain, matching Chatwoot's `A AND B OR C` / `A OR B AND C` frontend alignment behavior and returning raw `422 { error: 'Query operator must be either "AND" or "OR".' }` for invalid operators. | Review by `feat(conversations): align filter query operators`; focused filter handler tests assert OR/AND precedence and invalid query-operator error; full `go test ./...` and `git diff --check` must pass. No route artifacts change. | | P3.64 conversation filter payload basics | `internal/handler/api/v1/conversation_handler.go`, `internal/service/conversation_service.go`, conversation filter handler tests | `reference/chatwoot/app/controllers/api/v1/accounts/conversations_controller.rb#filter`, `reference/chatwoot/app/services/conversations/filter_service.rb`, `reference/chatwoot/app/services/filter_service.rb`, `reference/chatwoot/lib/filters/filter_keys.yml`, `reference/chatwoot/spec/controllers/api/v1/accounts/conversations_controller_spec.rb` | Conversation filter now accepts the reused dashboard's `{ payload: [...] }` body for `status`, `priority`, and `display_id` with `equal_to`/`not_equal_to`, computes `mine_count`, `assigned_count`, `unassigned_count`, and `all_count` from the filtered relation, and returns raw Chatwoot-style `422 { error }` for invalid attributes/operators. | Review by `feat(conversations): align filter payload basics`; focused filter handler tests assert payload status filtering, filtered counts, raw invalid-attribute/operator errors, full `go test ./...`, and `git diff --check`. No route artifacts change. | | P3.63 conversation search message-order parity | `internal/handler/api/v1/conversation_serializer.go`, conversation search handler tests | `reference/chatwoot/app/views/api/v1/models/_conversation.json.jbuilder`, `reference/chatwoot/app/models/message.rb` | Conversation search result messages now serialize in Chatwoot association order by `created_at ASC`, matching `Message` default scope when the search partial iterates `conversation.messages`. GoChat keeps `id ASC` only as a deterministic tie-breaker for same-timestamp rows. | Review by `feat(conversations): align search message order`; focused search handler tests assert created-at ordering across messages inserted out of chronological order; full `go test ./...` and `git diff --check` must pass. No route artifacts change. | | P3.62 conversation search payload-field parity | `internal/handler/api/v1/conversation_serializer.go`, conversation search handler tests | `reference/chatwoot/app/views/api/v1/accounts/conversations/search.json.jbuilder`, `reference/chatwoot/app/views/api/v1/models/_conversation.json.jbuilder` | Conversation search payload items now omit the normal list/show `meta` object, matching the dedicated search partial fields: `id`, `uuid`, `created_at`, `contact`, `inbox`, `messages`, and `account_id`. | Review by `feat(conversations): trim search payload meta`; focused search handler tests assert the root `meta` still exists but each payload item has no `meta`; full `go test ./...` and `git diff --check` must pass. No route artifacts change. | @@ -316,7 +318,8 @@ This ledger records the committed parity checkpoints that future slices should b | Commit | Scope | Verification summary | Follow-up state | | --- | --- | --- | --- | -| `feat(conversations): align filter payload basics` | Advances P3.64 with Chatwoot conversation filter payload basics. `POST /conversations/filter` now understands the frontend `{ payload: [...] }` filter format for standard `status`, `priority`, and `display_id` attributes, emits raw `422 { error }` for invalid attributes/operators, and makes filter meta counts come from the filtered relation instead of the whole account. | `go test ./internal/handler/api/v1 -run 'ConversationCrudTestSuite/TestFilter' -count=1`; `go test ./internal/repository ./internal/service ./internal/handler/api/v1 -run 'ConversationRepo_(GetMeta\|ListForFinder)\|Conversation.*(Meta\|List\|Search\|Filter)\|ConversationCrudTestSuite/Test(Filter\|Search\|List)\|ConversationService' -count=1`; `go test ./...`; `git diff --check`. No route artifacts change. | Move P3.64 to Review; continue Phase 2/3 drift audit for full filter query-operator/custom-attribute depth, Phase 6 placeholder audit, or B12 live smoke. | +| `feat(conversations): align filter query operators` | Advances P3.65 with Chatwoot conversation filter frontend alignment. Payload conditions now form a single SQL predicate chain with `AND`/`OR` operators and database precedence, so `A OR B AND C` and `A AND B OR C` behave like the reference `FilterService#query_builder`; invalid query operators return raw `422 { error }`. | `go test ./internal/handler/api/v1 -run 'ConversationCrudTestSuite/TestFilter' -count=1`; `go test ./internal/repository ./internal/service ./internal/handler/api/v1 -run 'ConversationRepo_(GetMeta\|ListForFinder)\|Conversation.*(Meta\|List\|Search\|Filter)\|ConversationCrudTestSuite/Test(Filter\|Search\|List)\|ConversationService' -count=1`; `go test ./...`; `git diff --check`. No route artifacts change. | Move P3.65 to Review; continue Phase 2/3 drift audit for full filter custom/additional-attribute depth, Phase 6 placeholder audit, or B12 live smoke. | +| `feat(conversations): align filter payload basics` | Advances P3.64 with Chatwoot conversation filter payload basics. `POST /conversations/filter` now understands the frontend `{ payload: [...] }` filter format for standard `status`, `priority`, and `display_id` attributes, emits raw `422 { error }` for invalid attributes/operators, and makes filter meta counts come from the filtered relation instead of the whole account. | `go test ./internal/handler/api/v1 -run 'ConversationCrudTestSuite/TestFilter' -count=1`; `go test ./internal/repository ./internal/service ./internal/handler/api/v1 -run 'ConversationRepo_(GetMeta\|ListForFinder)\|Conversation.*(Meta\|List\|Search\|Filter)\|ConversationCrudTestSuite/Test(Filter\|Search\|List)\|ConversationService' -count=1`; `go test ./...`; `git diff --check`. No route artifacts change. | Move P3.64 to Review; continue Phase 2/3 drift audit for full filter custom-attribute depth, Phase 6 placeholder audit, or B12 live smoke. | | `feat(conversations): align search message order` | Advances P3.63 with Chatwoot conversation search message-order parity. Search result messages now use `created_at ASC, id ASC`, matching the reference `_conversation` partial iterating `conversation.messages` under `Message` default `created_at ASC` scope while keeping deterministic ties. | `go test ./internal/handler/api/v1 -run 'ConversationCrudTestSuite/TestSearch' -count=1`; `go test ./internal/repository ./internal/service ./internal/handler/api/v1 -run 'ConversationRepo_(GetMeta\|ListForFinder)\|Conversation.*(Meta\|List\|Search)\|ConversationCrudTestSuite/Test(Search\|List)' -count=1`; `go test ./...`; `git diff --check`. No route artifacts change. | Move P3.63 to Review; continue Phase 2/3 drift audit, Phase 6 placeholder audit, or B12 live smoke. | | `feat(conversations): trim search payload meta` | Advances P3.62 with Chatwoot conversation search payload-field parity. Search result payload items now use only the dedicated search `_conversation` partial fields and no longer include the normal list/show `meta` object, while preserving the root search `meta` counts and P3.61/P3.60 message fields. | `go test ./internal/handler/api/v1 -run 'ConversationCrudTestSuite/TestSearch' -count=1`; `go test ./internal/repository ./internal/service ./internal/handler/api/v1 -run 'ConversationRepo_(GetMeta\|ListForFinder)\|Conversation.*(Meta\|List\|Search)\|ConversationCrudTestSuite/Test(Search\|List)' -count=1`; `go test ./...`; `git diff --check`. No route artifacts change. | Move P3.62 to Review; continue Phase 2/3 drift audit, Phase 6 placeholder audit, or B12 live smoke. | | `feat(conversations): align search sender names` | Advances P3.61 with Chatwoot conversation search model serializer parity. Search result messages now resolve persisted contact/user senders and emit `sender_name` using the same `message.sender.name if message.sender` behavior as the reference `_conversation` Jbuilder, while preserving P3.60 numeric `message_type` and P3.59 root search payload behavior. | `go test ./internal/handler/api/v1 -run 'ConversationCrudTestSuite/TestSearch' -count=1`; `go test ./internal/repository ./internal/service ./internal/handler/api/v1 -run 'ConversationRepo_(GetMeta\|ListForFinder)\|Conversation.*(Meta\|List\|Search)\|ConversationCrudTestSuite/Test(Search\|List)' -count=1`; `go test ./...`; `git diff --check`. No route artifacts change. | Move P3.61 to Review; continue Phase 2/3 drift audit, Phase 6 placeholder audit, or B12 live smoke. | @@ -2637,3 +2640,4 @@ Verification milestone gates: - 2026-06-07: P3.62 conversation search payload-field checkpoint prepared as `feat(conversations): trim search payload meta`; audited Chatwoot `conversations/search.json.jbuilder` and the dedicated search `_conversation` partial. GoChat search payload items now omit the normal conversation `meta` object and keep only the search partial fields while preserving root search `meta` counts. Focused search handler tests passed; full `go test ./...` and `git diff --check` passed. No route artifacts change. - 2026-06-07: P3.63 conversation search message-order checkpoint prepared as `feat(conversations): align search message order`; audited Chatwoot search `_conversation` partial and `Message` default scope. GoChat search result messages now order by `created_at ASC, id ASC` instead of `id ASC`, matching `conversation.messages` chronological rendering while keeping deterministic ties. Focused search handler tests passed; full `go test ./...` and `git diff --check` passed. No route artifacts change. - 2026-06-07: P3.64 conversation filter payload basics checkpoint prepared as `feat(conversations): align filter payload basics`; audited Chatwoot `ConversationsController#filter`, `Conversations::FilterService`, base `FilterService`, filter key config, and controller specs. GoChat `POST /conversations/filter` now accepts frontend `{ payload: [...] }` filters for `status`, `priority`, and `display_id`, returns raw `422 { error }` for invalid attributes/operators, and computes filter meta counts from the filtered relation. Focused filter handler tests passed; full `go test ./...` and `git diff --check` passed. No route artifacts change. +- 2026-06-07: P3.65 conversation filter query-operator checkpoint prepared as `feat(conversations): align filter query operators`; audited Chatwoot `FilterService#query_builder`, `Filters::FilterHelper#validate_single_condition`, and frontend alignment specs. GoChat filter payload chains now honor `AND`/`OR` query operators with SQL precedence and return raw invalid-query-operator errors. Focused filter handler tests passed; full `go test ./...` and `git diff --check` passed. No route artifacts change. diff --git a/internal/handler/api/v1/conversation_handler_crud_test.go b/internal/handler/api/v1/conversation_handler_crud_test.go index df4059d1..59773e6f 100644 --- a/internal/handler/api/v1/conversation_handler_crud_test.go +++ b/internal/handler/api/v1/conversation_handler_crud_test.go @@ -1052,6 +1052,69 @@ func (s *ConversationCrudTestSuite) TestFilter_ChatwootPayloadStatus() { assert.Equal(s.T(), float64(s.testConv.ID), resp.Payload[0]["id"]) } +func (s *ConversationCrudTestSuite) TestFilter_ChatwootPayloadQueryOperatorChain() { + primaryID := uint(12345) + s.testConv.Status = "open" + s.testConv.Priority = "urgent" + s.testConv.DisplayID = &primaryID + s.Require().NoError(s.db.Save(s.testConv).Error) + + secondaryID := uint(67890) + secondary := &model.Conversation{AccountID: s.testAccount.ID, InboxID: s.testInbox.ID, ContactID: s.testContact.ID, DisplayID: &secondaryID, Status: "resolved", Priority: "low", ChannelType: "web_widget", Channel: "web_widget"} + s.Require().NoError(s.db.Create(secondary).Error) + + nonMatchingID := uint(11111) + nonMatching := &model.Conversation{AccountID: s.testAccount.ID, InboxID: s.testInbox.ID, ContactID: s.testContact.ID, DisplayID: &nonMatchingID, Status: "resolved", Priority: "low", ChannelType: "web_widget", Channel: "web_widget"} + s.Require().NoError(s.db.Create(nonMatching).Error) + + body, _ := json.Marshal(map[string]interface{}{ + "payload": []map[string]interface{}{ + {"attribute_key": "status", "filter_operator": "equal_to", "values": []string{"open"}, "query_operator": "OR"}, + {"attribute_key": "priority", "filter_operator": "equal_to", "values": []string{"low"}, "query_operator": "AND"}, + {"attribute_key": "display_id", "filter_operator": "equal_to", "values": []string{"67890"}}, + }, + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", s.accountURL()+"/conversations/filter", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + s.router.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusOK, w.Code) + var resp struct { + Meta struct { + AllCount int64 `json:"all_count"` + } `json:"meta"` + Payload []map[string]interface{} `json:"payload"` + } + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(s.T(), int64(2), resp.Meta.AllCount) + s.Require().Len(resp.Payload, 2) + ids := []float64{resp.Payload[0]["id"].(float64), resp.Payload[1]["id"].(float64)} + assert.ElementsMatch(s.T(), []float64{float64(primaryID), float64(secondaryID)}, ids) +} + +func (s *ConversationCrudTestSuite) TestFilter_ChatwootPayloadInvalidQueryOperator() { + body, _ := json.Marshal(map[string]interface{}{ + "payload": []map[string]interface{}{ + { + "attribute_key": "status", + "filter_operator": "equal_to", + "values": []string{"open"}, + "query_operator": "XOR", + }, + }, + }) + w := httptest.NewRecorder() + req, _ := http.NewRequest("POST", s.accountURL()+"/conversations/filter", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + s.router.ServeHTTP(w, req) + + assert.Equal(s.T(), http.StatusUnprocessableEntity, w.Code) + var resp map[string]string + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &resp)) + assert.Equal(s.T(), "Query operator must be either \"AND\" or \"OR\".", resp["error"]) +} + func (s *ConversationCrudTestSuite) TestFilter_ChatwootPayloadInvalidAttribute() { body, _ := json.Marshal(map[string]interface{}{ "payload": []map[string]interface{}{ diff --git a/internal/service/conversation_service.go b/internal/service/conversation_service.go index 09bfd4fd..8af193b8 100644 --- a/internal/service/conversation_service.go +++ b/internal/service/conversation_service.go @@ -893,9 +893,12 @@ func (s *ConversationService) Filter(ctx context.Context, accountID uint, userID } func (s *ConversationService) applyConversationFilterPayload(query *gorm.DB, payload []ConversationFilterCondition) (*gorm.DB, error) { + clauses := make([]string, 0, len(payload)*2) + args := make([]any, 0, len(payload)) for _, condition := range payload { attribute := strings.TrimSpace(condition.AttributeKey) operator := strings.TrimSpace(condition.FilterOperator) + queryOperator := strings.ToUpper(strings.TrimSpace(condition.QueryOperator)) values := conversationFilterStringValues(condition.Values) column, allowedOperators, err := conversationFilterColumn(attribute) @@ -909,16 +912,40 @@ func (s *ConversationService) applyConversationFilterPayload(query *gorm.DB, pay return nil, fmt.Errorf("Invalid value for %s", attribute) } + if len(clauses) > 0 && !conversationFilterIsQueryOperator(clauses[len(clauses)-1]) { + clauses = append(clauses, "AND") + } switch operator { case "equal_to": - query = query.Where(column+" IN ?", values) + clauses = append(clauses, column+" IN ?") + args = append(args, values) case "not_equal_to": - query = query.Where(column+" NOT IN ?", values) + clauses = append(clauses, column+" NOT IN ?") + args = append(args, values) + } + + if queryOperator != "" { + if queryOperator != "AND" && queryOperator != "OR" { + return nil, fmt.Errorf("Query operator must be either \"AND\" or \"OR\".") + } + clauses = append(clauses, queryOperator) } } + if len(clauses) == 0 { + return query, nil + } + last := clauses[len(clauses)-1] + if conversationFilterIsQueryOperator(last) { + clauses = clauses[:len(clauses)-1] + } + query = query.Where(strings.Join(clauses, " "), args...) return query, nil } +func conversationFilterIsQueryOperator(value string) bool { + return value == "AND" || value == "OR" +} + func conversationFilterColumn(attribute string) (string, []string, error) { switch attribute { case "status":