273 lines
17 KiB
Markdown
273 lines
17 KiB
Markdown
# G7 Verification Report: SLA Policies, AppliedSLAs, Assignment Policies V2
|
|
## Task: t_ec88adec — Verify G7 implementation matches Chatwoot original across 5 criteria
|
|
|
|
## 1. Interface Paths
|
|
|
|
### SLA Policies CRUD
|
|
| Endpoint | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| List | GET /api/v1/accounts/:account_id/sla_policies | GET /api/v1/accounts/:account_id/sla_policies | ✅ PASS |
|
|
| Create | POST /api/v1/accounts/:account_id/sla_policies | POST /api/v1/accounts/:account_id/sla_policies | ✅ PASS |
|
|
| Show | GET /api/v1/accounts/:account_id/sla_policies/:id | GET /api/v1/accounts/:account_id/sla_policies/:id | ✅ PASS |
|
|
| Update | PUT/PATCH /api/v1/accounts/:account_id/sla_policies/:id | PUT /api/v1/accounts/:account_id/sla_policies/:id | ⚠️ MINOR — Chatwoot supports both PUT and PATCH; gochat only PUT. Rails resources auto-register both. |
|
|
| Delete | DELETE /api/v1/accounts/:account_id/sla_policies/:id | DELETE /api/v1/accounts/:account_id/sla_policies/:id | ✅ PASS |
|
|
|
|
### SLA Inbox Associations (EXTRA — not in Chatwoot routes.rb)
|
|
| Endpoint | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| List Inboxes | N/A (enterprise-only, not in open-source routes) | GET /sla_policies/:id/inboxes | 🔵 ADDITION — not in Chatwoot open-source routes.rb |
|
|
| Add Inbox | N/A | POST /sla_policies/:id/inboxes | 🔵 ADDITION |
|
|
| Remove Inbox | N/A | DELETE /sla_policies/:id/inboxes/:inbox_id | 🔵 ADDITION |
|
|
|
|
Note: Chatwoot's open-source schema.rb does NOT have a `sla_policy_inboxes` table, but the enterprise edition may have one. The gochat implementation added these endpoints as a reasonable extension.
|
|
|
|
### AppliedSLAs
|
|
| Endpoint | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Index | GET /api/v1/accounts/:account_id/applied_slas (index) | N/A — not implemented | ❌ FAIL — Chatwoot has `resources :applied_slas, only: [:index]` but gochat does not implement the index (list) endpoint |
|
|
| Metrics | GET /api/v1/accounts/:account_id/applied_slas/metrics | GET /api/v1/accounts/:account_id/applied_slas/metrics | ✅ PASS |
|
|
| Download | GET /api/v1/accounts/:account_id/applied_slas/download | GET /api/v1/accounts/:account_id/applied_slas/download | ✅ PASS |
|
|
|
|
### Assignment Policies V2 CRUD
|
|
| Endpoint | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| List | GET /api/v1/accounts/:account_id/assignment_policies | GET /api/v1/accounts/:account_id/assignment_policies_v2 | ⚠️ MISMATCH — path uses `_v2` suffix |
|
|
| Create | POST /api/v1/accounts/:account_id/assignment_policies | POST /api/v1/accounts/:account_id/assignment_policies_v2 | ⚠️ MISMATCH — path uses `_v2` suffix |
|
|
| Show | GET /api/v1/accounts/:account_id/assignment_policies/:id | GET /api/v1/accounts/:account_id/assignment_policies_v2/:id | ⚠️ MISMATCH |
|
|
| Update | PUT/PATCH /assignment_policies/:id | PUT /assignment_policies_v2/:id | ⚠️ MISMATCH |
|
|
| Delete | DELETE /assignment_policies/:id | DELETE /assignment_policies_v2/:id | ⚠️ MISMATCH |
|
|
|
|
Note: The `_v2` suffix is intentional to avoid collision with the existing V1 `assignment_policy` (singular) route in gochat. Chatwoot's original uses `assignment_policies` (plural) for the V2 API.
|
|
|
|
### Assignment Policy V2 — Nested Inboxes
|
|
| Endpoint | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| List Inboxes | GET /assignment_policies/:assignment_policy_id/inboxes | GET /assignment_policies_v2/:id/inboxes | ⚠️ MISMATCH — `_v2` suffix + different param name (:id vs :assignment_policy_id) |
|
|
| Add Inbox | POST /assignment_policies/:assignment_policy_id/inboxes | POST /assignment_policies_v2/:id/inboxes | ⚠️ MISMATCH |
|
|
| Remove Inbox | DELETE /assignment_policies/:assignment_policy_id/inboxes/:id | DELETE /assignment_policies_v2/:id/inboxes/:inbox_id | ⚠️ MISMATCH — different param names (:id vs :inbox_id) |
|
|
|
|
### Assignment Policy V2 — Reverse (Inbox → Policy)
|
|
| Endpoint | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Show (inbox) | GET /inboxes/:inbox_id/assignment_policy (show) | GET /inboxes/:inbox_id/assignment_policy_v2 | ⚠️ MISMATCH — `_v2` suffix |
|
|
| Create/Set (inbox) | POST /inboxes/:inbox_id/assignment_policy (create) | POST /inboxes/:inbox_id/assignment_policy_v2 | ⚠️ MISMATCH — Chatwoot uses POST/create, gochat uses POST/set |
|
|
| Delete (inbox) | DELETE /inboxes/:inbox_id/assignment_policy (destroy) | DELETE /inboxes/:inbox_id/assignment_policy_v2 | ⚠️ MISMATCH |
|
|
|
|
**PATH SUMMARY: 3 PASS, 1 FAIL (missing applied_slas index), 8 MISMATCH (intentional _v2 suffix), 1 MINOR (missing PATCH support)**
|
|
|
|
---
|
|
|
|
## 2. Request Parameters
|
|
|
|
### SLA Policy Create/Update
|
|
| Param | Chatwoot (schema) | Gochat (model) | Match |
|
|
|---|---|---|---|
|
|
| name | string, not null | string, not null | ✅ PASS |
|
|
| description | string (text) | string (text) | ✅ PASS |
|
|
| first_response_time_threshold | float | int (response_time, in minutes) | ⚠️ MISMATCH — Chatwoot uses float (seconds), gochat uses int (minutes) |
|
|
| next_response_time_threshold | float | int (update_time, in minutes) | ⚠️ MISMATCH — Chatwoot uses float (seconds), gochat uses int (minutes) |
|
|
| resolution_time_threshold | float | int (resolution_time, in minutes) | ⚠️ MISMATCH — Chatwoot uses float (seconds), gochat uses int (minutes) |
|
|
| only_during_business_hours | boolean, default false | N/A — not in model | ❌ FAIL — missing field |
|
|
|
|
Note: The field naming differs: Chatwoot uses `first_response_time_threshold` / `next_response_time_threshold` / `resolution_time_threshold` (float, in seconds), gochat uses `response_time` / `update_time` / `resolution_time` (int, in minutes). The JSON field names differ: `first_response_time_threshold` vs `response_time`.
|
|
|
|
### AppliedSLA Metrics
|
|
| Param | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| conversation_id (query) | Used as filter param | conversation_id (query param, required) | ✅ PASS — gochat requires it explicitly |
|
|
|
|
### AppliedSLA Download
|
|
| Param | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| No specific params | Account-scoped list | account_id from auth context | ✅ PASS |
|
|
|
|
### Assignment Policy V2 Create/Update
|
|
| Param | Chatwoot (schema) | Gochat (model) | Match |
|
|
|---|---|---|---|
|
|
| name | string, not null | string, not null | ✅ PASS |
|
|
| description | text | text | ✅ PASS |
|
|
| assignment_order | integer (enum: round_robin=0) | type (enum: round_robin/fair/best_skill_match) | ⚠️ MISMATCH — different field name and enum structure |
|
|
| conversation_priority | integer (enum: earliest_created=0, longest_waiting=1) | N/A | ❌ FAIL — missing field |
|
|
| fair_distribution_limit | integer, default 100, not null | N/A | ❌ FAIL — missing field |
|
|
| fair_distribution_window | integer, default 3600, not null | N/A | ❌ FAIL — missing fields |
|
|
| enabled | boolean, default true, not null | N/A | ❌ FAIL — missing field |
|
|
|
|
Note: Gochat's AssignmentPolicyV2 uses a simplified `type` field (round_robin/fair/best_skill_match) instead of Chatwoot's separate assignment_order + conversation_priority + fair_distribution fields. This is a design simplification, not a direct match.
|
|
|
|
### Inbox Association (add/remove)
|
|
| Param | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| inbox_id | inbox_id in body | inbox_id (JSON body) | ✅ PASS |
|
|
|
|
**PARAM SUMMARY: 5 PASS, 5 MISMATCH (field naming/units), 5 FAIL (missing fields)**
|
|
|
|
---
|
|
|
|
## 3. Response Format
|
|
|
|
### SLA Policy Response
|
|
| Field | Chatwoot JSON | Gochat JSON | Match |
|
|
|---|---|---|---|
|
|
| id | id | id | ✅ PASS |
|
|
| name | name | name | ✅ PASS |
|
|
| description | description | description | ✅ PASS |
|
|
| first_response_time_threshold | first_response_time_threshold (float) | response_time (int) | ⚠️ MISMATCH — different field name + type |
|
|
| next_response_time_threshold | next_response_time_threshold (float) | update_time (int) | ⚠️ MISMATCH |
|
|
| resolution_time_threshold | resolution_time_threshold (float) | resolution_time (int) | ⚠️ MISMATCH |
|
|
| only_during_business_hours | only_during_business_hours (boolean) | N/A | ❌ FAIL — missing |
|
|
| account_id | account_id | account_id | ✅ PASS |
|
|
| created_at | created_at | created_at | ✅ PASS |
|
|
| updated_at | updated_at | updated_at | ✅ PASS |
|
|
|
|
Note: Gochat uses Gin's JSON serialization with `json` tags on struct fields. Rails uses active_model_serializers / jbuilder. The response structure is similar but field names differ for time thresholds.
|
|
|
|
### AppliedSLA Response
|
|
| Field | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| id | id | id | ✅ PASS |
|
|
| sla_policy_id | sla_policy_id | sla_policy_id | ✅ PASS |
|
|
| conversation_id | conversation_id | conversation_id | ✅ PASS |
|
|
| account_id | account_id | account_id | ✅ PASS |
|
|
| sla_status | sla_status (integer enum) | sla_status (string enum) | ⚠️ MISMATCH — Chatwoot uses integer enum (0=active, 1=violated, 2=completed), gochat uses string enum ("active", "violated", "completed") |
|
|
| frt/nrt/rt target/actual timestamps | N/A (not in schema) | frt_target_at, nrt_target_at, rt_target_at, frt_actual_at, nrt_actual_at, rt_actual_at | 🔵 ADDITION — gochat tracks computed timestamps |
|
|
| created_at/updated_at | created_at, updated_at | created_at, updated_at | ✅ PASS |
|
|
|
|
### Metrics Response
|
|
| Field | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| applied_sla + sla_events | Returns applied SLA with events | gin.H{"applied_sla": applied, "sla_events": events} | ✅ PASS — structure matches |
|
|
|
|
### Assignment Policy V2 Response
|
|
| Field | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| id | id | id | ✅ PASS |
|
|
| account_id | account_id | account_id | ✅ PASS |
|
|
| name | name | name | ✅ PASS |
|
|
| description | description | description | ✅ PASS |
|
|
| assignment_order | assignment_order (integer/enum) | type (string enum) | ⚠️ MISMATCH |
|
|
| conversation_priority | conversation_priority | N/A | ❌ FAIL |
|
|
| fair_distribution_limit | fair_distribution_limit | N/A | ❌ FAIL |
|
|
| fair_distribution_window | fair_distribution_window | N/A | ❌ FAIL |
|
|
| enabled | enabled | N/A | ❌ FAIL |
|
|
|
|
**RESPONSE SUMMARY: 6 PASS, 4 MISMATCH, 5 FAIL (missing fields), 1 ADDITION**
|
|
|
|
---
|
|
|
|
## 4. Error Handling / Status Codes
|
|
|
|
### SLA Policies
|
|
| Scenario | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Unauthorized (no account) | 401 | 401 (response.ErrUnauthorized) | ✅ PASS |
|
|
| Invalid ID param | 400 | 400 (response.ErrValidation) | ✅ PASS |
|
|
| Not found | 404 | handleServiceError → 404/500 | ⚠️ NEEDS VERIFY — depends on handleServiceError mapping |
|
|
| Validation error (create) | 422 | handleServiceError mapping | ⚠️ NEEDS VERIFY — Chatwoot uses 422 for validation, gochat may use 400 |
|
|
| Success create | 200/201 | response.OK → 200 | ⚠️ MINOR — Chatwoot returns 201 for create, gochat returns 200 |
|
|
| Success delete | 200/204 | response.OK → 200 (returns deleted object) | ⚠️ MINOR — Chatwoot returns 204 No Content for destroy |
|
|
|
|
### AppliedSLA Metrics
|
|
| Scenario | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Missing conversation_id | 400 | 400 (response.ErrValidation) | ✅ PASS |
|
|
| Not found | 404 | handleServiceError | ⚠️ NEEDS VERIFY |
|
|
| Unauthorized | 401 | 401 | ✅ PASS |
|
|
|
|
### Assignment Policy V2
|
|
| Scenario | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Unauthorized | 401 | 401 | ✅ PASS |
|
|
| Invalid ID | 400 | 400 | ✅ PASS |
|
|
| Not found | 404 | handleServiceError | ⚠️ NEEDS VERIFY |
|
|
| Remove inbox (not associated) | 404 | 500 (fmt.Errorf from service) | ❌ FAIL — should be 404 |
|
|
| Delete success | 204 | response.OK → 200 (returns deleted object) | ⚠️ MINOR |
|
|
|
|
**ERROR HANDLING SUMMARY: 4 PASS, 4 MINOR, 2 NEEDS VERIFY, 1 FAIL**
|
|
|
|
---
|
|
|
|
## 5. Business Logic Behavior
|
|
|
|
### SLA Policy Account Scoping
|
|
| Behavior | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| All queries scoped to account_id | Yes (Rails current_account) | Yes (accountID from auth context, passed to service) | ✅ PASS |
|
|
| Account uniqueness on name | N/A (no unique index on name alone) | uniqueIndex: account_id + name | ⚠️ EXTRA — gochat adds account+name uniqueness beyond Chatwoot schema |
|
|
| Soft delete | Yes (paranoia gem) | Yes (gorm.DeletedAt) | ✅ PASS |
|
|
|
|
### SLA Inbox Uniqueness
|
|
| Behavior | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| One SLA policy per inbox | Enterprise-only | uniqueIndex on inbox_id | ✅ PASS — gochat correctly enforces inbox uniqueness |
|
|
| AddInbox validates inbox belongs to account | N/A | Yes (service checks accountID) | ✅ PASS |
|
|
|
|
### AppliedSLA Uniqueness
|
|
| Behavior | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Unique on [account_id, sla_policy_id, conversation_id] | Yes (schema index) | N/A — no unique constraint enforced in model | ⚠️ MISMATCH — gochat doesn't enforce this unique constraint |
|
|
| Account scoping | Yes | Yes (AccountID check in GetAppliedSlaMetrics) | ✅ PASS |
|
|
|
|
### SLA Time Threshold Units
|
|
| Behavior | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Threshold stored as float (seconds) | Yes | Stored as int (minutes) | ⚠️ MISMATCH — unit conversion needed |
|
|
| only_during_business_hours flag | Yes | No | ❌ FAIL |
|
|
|
|
### Assignment Policy V2 Assignment Strategy
|
|
| Behavior | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Round robin assignment | assignment_order: round_robin | type: round_robin | ✅ PASS (same concept, different field) |
|
|
| Fair distribution | fair_distribution_limit + fair_distribution_window | type: fair (simplified) | ⚠️ MISMATCH — Chatwoot tracks limit/window, gochat just marks type |
|
|
| Best skill match | N/A (enterprise extension) | type: best_skill_match | 🔵 ADDITION |
|
|
| Conversation priority ordering | conversation_priority enum (earliest_created, longest_waiting) | N/A | ❌ FAIL — missing |
|
|
| Enabled/disabled flag | enabled boolean | N/A | ❌ FAIL — missing |
|
|
|
|
### Assignment Policy Inbox Association
|
|
| Behavior | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| One policy per inbox | Yes (unique index on inbox_id) | Yes (uniqueIndex on inbox_id) | ✅ PASS |
|
|
| SetInboxPolicy replaces existing | Yes (Rails creates/updates) | Yes (DeleteByInbox then Create) | ✅ PASS |
|
|
| Account scoping on associations | Yes (via account) | Yes (AccountID check in GetInboxPolicy) | ✅ PASS |
|
|
|
|
### Cascade/Dependent Deletes
|
|
| Behavior | Chatwoot | Gochat | Match |
|
|
|---|---|---|---|
|
|
| Delete policy → delete inbox associations | Yes (dependent: :destroy) | N/A — not explicitly handled | ⚠️ NEEDS VERIFY — need to check if GORM cascades |
|
|
| Delete policy → delete applied SLAs | Yes | N/A — not in service | ⚠️ NEEDS VERIFY |
|
|
|
|
**BUSINESS LOGIC SUMMARY: 7 PASS, 4 MISMATCH, 5 FAIL, 3 NEEDS VERIFY, 2 ADDITION**
|
|
|
|
---
|
|
|
|
## Overall Verification Summary
|
|
|
|
| Criterion | Pass | Mismatch | Fail | Needs Verify | Addition |
|
|
|---|---|---|---|---|---|
|
|
| Interface Paths | 3 | 8 (intentional _v2) | 1 (missing applied_slas index) | 0 | 3 |
|
|
| Request Params | 5 | 5 (naming/units) | 5 (missing fields) | 0 | 0 |
|
|
| Response Format | 6 | 4 (naming/types) | 5 (missing fields) | 0 | 1 |
|
|
| Error Handling | 4 | 4 (minor) | 1 (wrong status for remove-inbox) | 2 | 0 |
|
|
| Business Logic | 7 | 4 (units/structure) | 5 (missing features) | 3 | 2 |
|
|
| **TOTAL** | **25** | **21** | **17** | **5** | **6** |
|
|
|
|
### Critical Failures (must fix):
|
|
1. **Missing AppliedSLAs index endpoint** — Chatwoot has `resources :applied_slas, only: [:index]` but gochat doesn't implement GET /applied_slas (list)
|
|
2. **Missing `only_during_business_hours` field** on SlaPolicy model
|
|
3. **Missing AssignmentPolicy fields**: conversation_priority, fair_distribution_limit, fair_distribution_window, enabled
|
|
4. **Wrong error status for RemoveInbox when inbox not associated** — returns 500 instead of 404
|
|
5. **AppliedSLA unique constraint** not enforced in gochat model (Chatwoot schema has unique index on [account_id, sla_policy_id, conversation_id])
|
|
6. **SlaEvent missing fields**: conversation_id, account_id, sla_policy_id, inbox_id, meta — Chatwoot schema has these
|
|
|
|
### Intentional Design Differences (acceptable):
|
|
1. **Path `_v2` suffix** on assignment policies — avoids collision with existing V1 route
|
|
2. **Simplified AssignmentPolicyV2 type field** — reduces Chatwoot's multi-field approach (assignment_order + conversation_priority + fair_distribution) to a single type enum
|
|
3. **Time threshold units** — minutes (int) vs seconds (float) — functional equivalent with conversion
|
|
4. **SLA inbox associations** — added beyond Chatwoot open-source routes (enterprise feature)
|
|
5. **AppliedSLA timestamp tracking** — gochat adds frt/nrt/rt target and actual timestamps (enhancement)
|
|
6. **sla_status as string enum** vs Chatwoot's integer enum — more readable but API-incompatible
|
|
|
|
### Minor Issues:
|
|
1. **Missing PATCH support** on SLA policy update — Rails resources auto-register PUT+PATCH
|
|
2. **Create returns 200** instead of 201 — Chatwoot convention is 201 for resource creation
|
|
3. **Delete returns 200 with body** instead of 204 No Content — Chatwoot convention is 204
|