feat: add contacts management and thread resolution#957
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dd82cae-8bc6-4eaa-9b90-95073720c577
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8dd82cae-8bc6-4eaa-9b90-95073720c577
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move sanitizeUniqueStrings into the shared requests helper. Preserve first-seen ordering after normalization. Add a direct ToContacts nil-properties regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allow .csv extension only; accept common browser MIME fallbacks for uploads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add contacts query parsing and optional ContactDetails enrichment through a contact map provider. DI passes nil until Task 8 wires ContactService. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add ContactHandler with authenticated /v1/contacts routes: GET/POST /v1/contacts, POST /v1/contacts/upload, PUT/DELETE /v1/contacts/:contactID. - Load user-scoped contact before Update/Delete so a missing or cross-user contact returns 404 (avoids the repo's zero-row Delete returning 204 while advertising 404). - Support one-and-many JSON create via ContactStoreRequest and reuse ContactService.CreateMany so cache invalidation stays single-shot for Store and CSV upload alike. - Add DI getters ContactRepository, ContactService, ContactHandlerValidator, ContactHandler and RegisterContactRoutes; register the routes alongside RegisterMessageThreadRoutes. - Finalise Task 7 by injecting container.ContactService() into MessageThreadService, replacing the temporary nil dependency. - Cover all five routes with real Fiber requests plus a shared ContactRepository fake that verifies service/repository effects, and pin the DI wiring with a compile-time test that constructs MessageThreadService with the ContactService. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make emails/properties optional in contact request DTOs. Regenerate Swagger docs and add request JSON regression tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ialogs Add the /contacts page grounded in the existing Vuetify theme. Header uses text-display-large; debounced search drives the API ?query= param. VDataTable columns: Name, Phone Numbers, Emails, Created, Updated, Actions. Rows show avatar initials, phone chips and relative timestamps via humanizeTime. Add/edit/delete/import-CSV dialogs use opacity 0.9 and warning-colored Close controls. Add/Edit has repeatable phone numbers, emails and free-form properties, preserved on edit. CSV import surfaces row-indexed API validation errors inline, not only via a toast. loadContacts forwards the trimmed search value as the query param, keeping Task 11's contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve contact details across mark-read updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend the contact repository/service/handler so GET /v1/contacts returns a top-level total count for the same user/query filter, independent of skip/limit, enabling true server-side pagination. - add ContactRepository.Count reusing a shared scopedContactQuery helper so Index and Count filters can never drift; Count ignores limit/offset - add ContactService.Count and handler responseOKWithTotal; Index returns total - add Total to responses.ContactsResponse and regenerate Swagger - sanitize parsed CSV rows before validation so CSV and JSON accept the same phone/email formats; drop the now-redundant re-sanitize on upload - assert Scan error behaviour instead of the unexported stacktrace type - tests for count filter parity, total propagation, and CSV sanitization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- store total now reflects the server count; loadContacts accepts skip and limit and remembers the current window so mutation refreshes stay in place - contacts page uses VDataTableServer with items-length bound to the server total; page/size changes fetch the correct skip/limit and a debounced query resets to page 1 without duplicate requests - delete bumps the load generation so an in-flight load cannot resurrect a deleted contact, without leaving loading stuck or hiding delete errors - remove the dead filteredContacts computed and add defensive null coalescing for emails/phone_numbers in the table display - regenerate API models with the new contacts total field Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove misleading sort affordances from the contacts table. Keep the subtitle aligned with the active search state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 50 minor |
🟢 Metrics 518 complexity · 201 duplication
Metric Results Complexity 518 Duplication 201
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Greptile SummaryThis PR introduces a full contacts management feature: a new
Confidence Score: 3/5Safe to merge with caution — one data-retention gap needs to be addressed before this goes to production for any deployment that supports account deletion. The contacts feature is well-structured and thoroughly tested, but
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant ContactHandler
participant ContactHandlerValidator
participant ContactService
participant ContactRepository
participant Cache
Client->>ContactHandler: POST /v1/contacts (JSON array or object)
ContactHandler->>ContactHandlerValidator: ValidateStore(request)
ContactHandlerValidator-->>ContactHandler: url.Values (errors)
ContactHandler->>ContactService: CreateMany(userID, contacts)
ContactService->>ContactRepository: Store(contacts)
ContactRepository-->>ContactService: nil
ContactService->>Cache: Set(key, empty, 24h) [invalidate]
ContactService-->>ContactHandler: nil
ContactHandler-->>Client: 201 Created
Client->>ContactHandler: "GET /v1/threads?contacts=true"
ContactHandler->>MessageThreadService: "GetThreads(WithContacts=true)"
MessageThreadService->>ContactService: GetContactMap(userID)
ContactService->>Cache: Get(key)
alt cache hit
Cache-->>ContactService: JSON map
ContactService-->>MessageThreadService: "map[phone]*Contact"
else cache miss or invalidated
Cache-->>ContactService: empty or error
ContactService->>ContactRepository: FetchAll(userID)
ContactRepository-->>ContactService: []Contact
ContactService->>Cache: Set(key, encodedMap, 24h)
ContactService-->>MessageThreadService: "map[phone]*Contact"
end
MessageThreadService-->>ContactHandler: []MessageThread with ContactDetails
ContactHandler-->>Client: 200 OK
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant ContactHandler
participant ContactHandlerValidator
participant ContactService
participant ContactRepository
participant Cache
Client->>ContactHandler: POST /v1/contacts (JSON array or object)
ContactHandler->>ContactHandlerValidator: ValidateStore(request)
ContactHandlerValidator-->>ContactHandler: url.Values (errors)
ContactHandler->>ContactService: CreateMany(userID, contacts)
ContactService->>ContactRepository: Store(contacts)
ContactRepository-->>ContactService: nil
ContactService->>Cache: Set(key, empty, 24h) [invalidate]
ContactService-->>ContactHandler: nil
ContactHandler-->>Client: 201 Created
Client->>ContactHandler: "GET /v1/threads?contacts=true"
ContactHandler->>MessageThreadService: "GetThreads(WithContacts=true)"
MessageThreadService->>ContactService: GetContactMap(userID)
ContactService->>Cache: Get(key)
alt cache hit
Cache-->>ContactService: JSON map
ContactService-->>MessageThreadService: "map[phone]*Contact"
else cache miss or invalidated
Cache-->>ContactService: empty or error
ContactService->>ContactRepository: FetchAll(userID)
ContactRepository-->>ContactService: []Contact
ContactService->>Cache: Set(key, encodedMap, 24h)
ContactService-->>MessageThreadService: "map[phone]*Contact"
end
MessageThreadService-->>ContactHandler: []MessageThread with ContactDetails
ContactHandler-->>Client: 200 OK
Reviews (1): Last reviewed commit: "fix(web): mark contacts headers unsortab..." | Re-trigger Greptile |
| Delete(ctx context.Context, userID entities.UserID, contactID uuid.UUID) error | ||
|
|
||
| // DeleteAllForUser deletes all contacts for a user. | ||
| DeleteAllForUser(ctx context.Context, userID entities.UserID) error |
There was a problem hiding this comment.
DeleteAllForUser never wired to the UserAccountDeleted event
Every other entity in the codebase — billing, Discord, heartbeat, message threads, phone notifications, webhooks, etc. — has a dedicated listener that calls DeleteAllForUser when a UserAccountDeleted event fires (see e.g. billing_listener.go, message_thread_listener.go). The ContactRepository defines this method and ContactService could expose it, but no ContactListener was registered. As a result, all contacts for a deleted user remain in the database indefinitely, constituting a data-retention/compliance gap.
| if len(query) > 0 { | ||
| queryPattern := "%" + query + "%" |
There was a problem hiding this comment.
User-supplied query is used verbatim as a LIKE pattern, enabling wildcard leakage
queryPattern is built by concatenating % around the raw query string. If a user includes SQL LIKE wildcard characters (% or _) in their search term, they act as wildcards against the columns. For example, searching _ matches every entry (any single character). The query is parameterized so there is no SQL injection risk, but the semantics are incorrect — a user expecting an exact substring match gets a much broader result. Escaping the user input before inserting it into the pattern fixes this.
| if len(query) > 0 { | |
| queryPattern := "%" + query + "%" | |
| if len(query) > 0 { | |
| escaped := strings.NewReplacer(`%`, `\%`, `_`, `\_`).Replace(query) | |
| queryPattern := "%" + escaped + "%" |
| contacts, err := service.repository.FetchAll(ctx, userID) | ||
| if err != nil { | ||
| return nil, service.tracer.WrapErrorSpan(span, stacktrace.Propagatef(err, "cannot fetch contacts to build map for user [%s]", userID)) | ||
| } | ||
|
|
||
| result := make(map[string]*entities.Contact, len(*contacts)) | ||
| for index := range *contacts { | ||
| // Take a fresh copy of the slice element so the map holds a stable pointer | ||
| // that does not alias the underlying slice storage. | ||
| contact := (*contacts)[index] | ||
| for _, number := range contact.PhoneNumbers { | ||
| result[number] = &contact | ||
| } | ||
| } |
There was a problem hiding this comment.
FetchAll loads every contact for a user with no upper bound
GetContactMap calls FetchAll which issues a SELECT * with no LIMIT. On every cache miss or post-mutation rebuild, all of a user's contacts are pulled into memory at once. A power user with tens of thousands of contacts would trigger a very large in-memory allocation and a long-running query. Given the 24 h TTL and the fact that every mutation (create, update, delete) invalidates the cache, heavy writers could trigger this repeatedly within a day. Consider streaming or paginating the build, or at minimum adding an application-level cap with an error when the contact list exceeds a safe threshold.
Summary
Testing
cd api && go test -vet=off ./...cd api && go build ./...cd web && pnpm lintcd web && pnpm generate