feat(whatsapp): add analytics v2 event capture - #420
Merged
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds persistent WhatsApp analytics fields and indexes, a NestJS WhatsAppAnalyticsService (Redis turn counting + Prisma fallback), returns structured analytics from product/image flows, upserts session state with IDs, and instruments WhatsAppService to record analytics best-effort after processing. ChangesWhatsApp Analytics End-to-End
Sequence DiagramsequenceDiagram
participant User
participant Webhook
participant WhatsAppService
participant SessionService
participant ProductDiscovery
participant ImageSearch
participant AnalyticsService
participant Redis
participant Prisma
User->>Webhook: inbound message
Webhook->>WhatsAppService: processMessage(phone, input)
WhatsAppService->>SessionService: recordInbound(phone)
SessionService->>Prisma: whatsAppSession.upsert()
Prisma-->>SessionService: session record (id)
SessionService-->>WhatsAppService: consent state + id
alt product search
WhatsAppService->>ProductDiscovery: sendGenericProductSearch(query)
ProductDiscovery->>Prisma: search/filter products
Prisma-->>ProductDiscovery: results
ProductDiscovery-->>WhatsAppService: WhatsAppProductSearchAnalytics
else image search
WhatsAppService->>ImageSearch: handleImageSearch(imageId)
ImageSearch->>Prisma: match products
Prisma-->>ImageSearch: results
ImageSearch-->>WhatsAppService: WhatsAppImageSearchAnalytics
end
WhatsAppService->>AnalyticsService: recordMessage(analytics) [finally]
AnalyticsService->>Redis: INCR(phone_turn_key)
alt Redis ok
Redis-->>AnalyticsService: turn
AnalyticsService->>Prisma: whatsAppAnalytics.create()
else Redis fails
AnalyticsService->>Prisma: whatsAppAnalytics.count(phone)
Prisma-->>AnalyticsService: count
AnalyticsService->>Prisma: whatsAppAnalytics.create()
end
Prisma-->>AnalyticsService: created
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/backend/src/channels/whatsapp/whatsapp-product-discovery.service.ts (1)
262-273:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDifferentiate invalid numeric picks from ambiguous matches.
If the shopper replies with an out-of-range number, this branch sends the ambiguous-match copy and records
AMBIGUOUS_PRODUCT_SELECTION, even though there was no matching row at all. That will mislead the user and skew the new selection-failure analytics.Suggested fix
+const INVALID_SELECTION_MESSAGE = + "I couldn't find that number in the recent results. Please choose one of the listed options."; + const selected = results.find((result) => result.rank === indexReference); if (!selected) { await this.interactiveService.sendTextMessage( phone, - AMBIGUOUS_SELECTION_MESSAGE, + INVALID_SELECTION_MESSAGE, ); return { handled: true, intentSuccessful: false, - errorType: "AMBIGUOUS_PRODUCT_SELECTION", - dropOffStep: "product_selection_ambiguous", + errorType: "INVALID_PRODUCT_SELECTION", + dropOffStep: "product_selection_invalid", }; }apps/backend/src/channels/whatsapp/whatsapp.service.ts (1)
62-110:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPaused/resumed messages still bypass the new analytics flow.
processMessagecan return from the Redis pause branch beforerecordInbound,analyticsinitialization, and thefinallyblock run, so those inbound WhatsApp messages never get awhatsapp_analyticsrow. That leaves a durable-capture gap for an entire production branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp.service.ts` around lines 62 - 110, processMessage currently returns early from the Redis "paused" branch before recordInbound runs and analytics is initialized, so paused inbound messages never reach the finally block and no whatsapp_analytics row is created; fix by ensuring recordInbound and analytics initialization always run before any early return: either move the pause check to after obtaining normalizedPhone/userId/session or, if you must keep the pause check early, call this.whatsappSessionService.recordInbound(...) and populate the analytics object (same shape as the existing block) before returning, so the finally block can persist analytics; reference processMessage, redisService.get(pauseKey), recordInbound, analytics, and the finally block when making the change.
🧹 Nitpick comments (2)
apps/backend/src/channels/whatsapp/whatsapp-session.service.spec.ts (1)
95-103: ⚡ Quick winThe
upsertmock never exercises the update payload.This mock always echoes
create, so the cached-inbound tests cannot catch regressions in the realupdatebranch. Please assert theupsert(...).updateobject, or model an existing session separately, so null-clobbering bugs onuserIdandconsentAtfail here.Also applies to: 129-170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp-session.service.spec.ts` around lines 95 - 103, The upsert mock in the test for prisma.whatsAppSession.upsert currently always returns the create payload so the update branch (and potential null-clobbering of fields like userId and consentAt) is never exercised; update the test to either assert the shape of the upsert call's update argument or change the mockImplementation to detect when an update is intended and return an object that merges existing session values with the provided update (e.g., read args.update and args.create from UpsertSessionArgs and return a merged result), and apply the same change to the other similar mocks referenced around lines 129-170 so cached-inbound tests fail on regressions.apps/backend/src/channels/whatsapp/whatsapp.service.spec.ts (1)
54-91: ⚡ Quick winUse a full session fixture now that analytics reads
session.id.Most tests still stub
recordInboundwith{ isConsentGiven: ... }, butprocessMessagenow always readssession.idand sometimessession.userIdbefore recording analytics. A sharedmakeSession()helper withid,userId, andisConsentGivenwould keep these tests representative and avoid false positives when the session contract changes again.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/backend/src/channels/whatsapp/whatsapp.service.spec.ts` around lines 54 - 91, Tests currently stub analytics inputs with only isConsentGiven, but processMessage (in WhatsAppService) and whatsappAnalyticsService.recordMessage now read session.id and session.userId; create a shared makeSession() fixture that returns { id, userId, isConsentGiven } and update all test stubs (where recordInbound/analytics/session is mocked) to use that full session object so tests supply id and userId consistently; update any uses in the spec that call whatsappAnalyticsService.recordMessage or invoke WhatsAppService.processMessage to pass or return the full session from whatsappSessionService.normalizePhone / session mocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/backend/src/channels/whatsapp/image-search.service.ts`:
- Around line 268-278: The helper emptyImageAnalytics currently always sets
zeroResults: true which causes transport/vision/download/term-extraction
failures to be recorded as zero-result searches; change
emptyImageAnalytics(errorType: string, zeroResults = false) to accept an
explicit zeroResults boolean (default false), set zeroResults to that parameter,
and update all callers (e.g., the code paths that handle download failures,
vision/term-extraction failures, and the catch-all error path) to pass false for
actual failures and only pass true when you legitimately want to record a
zero-result image search; keep errorType populated so analytics can distinguish
failure vs. genuine zero-result searches.
In `@apps/backend/src/channels/whatsapp/whatsapp-analytics.service.ts`:
- Around line 99-114: The current try/catch around both
this.redisService.incr(key) and expire() discards a successfully allocated Redis
turn when expire() fails; change the flow so incr is awaited first and its
returned turn is kept, then call expire() in a separate try/catch: call const
turn = await this.redisService.incr(key); then try { await
this.redisService.expire(key, WA_ANALYTICS_TURN_TTL); } catch (error) {
this.logger.warn(`... ${error instanceof Error ? error.message : 'unknown
error'}`); } and finally return turn; only fall back to
this.prisma.whatsAppAnalytics.count({ where: { phone } }) + 1 if incr itself
throws (i.e., catch only the incr failure).
In `@apps/backend/src/channels/whatsapp/whatsapp-session.service.ts`:
- Around line 45-50: The upsertSessionState call sites (e.g., the call at
whatsapp-session.service.ts lines shown) pass null for userId/consent values
which causes upsertSessionState to overwrite existing whatsAppSession.userId and
whatsAppSession.consentAt; change upsertSessionState (or its callers) so that
when userId or consent values are null it preserves the existing whatsAppSession
values instead of writing null: inside upsertSessionState, fetch the existing
whatsAppSession by normalizedPhone (or use an existing loader like
getSession/state lookup), and only overwrite userId and consentAt when the
incoming parameters are non-null/defined—otherwise reuse
existing.whatsAppSession.userId and existing.whatsAppSession.consentAt—then
persist the merged session; apply the same preservation logic for all other call
sites that currently pass null.
---
Outside diff comments:
In `@apps/backend/src/channels/whatsapp/whatsapp.service.ts`:
- Around line 62-110: processMessage currently returns early from the Redis
"paused" branch before recordInbound runs and analytics is initialized, so
paused inbound messages never reach the finally block and no whatsapp_analytics
row is created; fix by ensuring recordInbound and analytics initialization
always run before any early return: either move the pause check to after
obtaining normalizedPhone/userId/session or, if you must keep the pause check
early, call this.whatsappSessionService.recordInbound(...) and populate the
analytics object (same shape as the existing block) before returning, so the
finally block can persist analytics; reference processMessage,
redisService.get(pauseKey), recordInbound, analytics, and the finally block when
making the change.
---
Nitpick comments:
In `@apps/backend/src/channels/whatsapp/whatsapp-session.service.spec.ts`:
- Around line 95-103: The upsert mock in the test for
prisma.whatsAppSession.upsert currently always returns the create payload so the
update branch (and potential null-clobbering of fields like userId and
consentAt) is never exercised; update the test to either assert the shape of the
upsert call's update argument or change the mockImplementation to detect when an
update is intended and return an object that merges existing session values with
the provided update (e.g., read args.update and args.create from
UpsertSessionArgs and return a merged result), and apply the same change to the
other similar mocks referenced around lines 129-170 so cached-inbound tests fail
on regressions.
In `@apps/backend/src/channels/whatsapp/whatsapp.service.spec.ts`:
- Around line 54-91: Tests currently stub analytics inputs with only
isConsentGiven, but processMessage (in WhatsAppService) and
whatsappAnalyticsService.recordMessage now read session.id and session.userId;
create a shared makeSession() fixture that returns { id, userId, isConsentGiven
} and update all test stubs (where recordInbound/analytics/session is mocked) to
use that full session object so tests supply id and userId consistently; update
any uses in the spec that call whatsappAnalyticsService.recordMessage or invoke
WhatsAppService.processMessage to pass or return the full session from
whatsappSessionService.normalizePhone / session mocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 79108276-76db-4096-9b48-ed5aff581b45
📒 Files selected for processing (12)
apps/backend/prisma/migrations/20260611120000_add_whatsapp_analytics_v2_fields/migration.sqlapps/backend/prisma/schema.prismaapps/backend/src/channels/whatsapp/image-search.service.tsapps/backend/src/channels/whatsapp/whatsapp-analytics.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp-analytics.service.tsapps/backend/src/channels/whatsapp/whatsapp-product-discovery.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp-product-discovery.service.tsapps/backend/src/channels/whatsapp/whatsapp-session.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp-session.service.tsapps/backend/src/channels/whatsapp/whatsapp-shared.module.tsapps/backend/src/channels/whatsapp/whatsapp.service.spec.tsapps/backend/src/channels/whatsapp/whatsapp.service.ts
onerandomdevv
approved these changes
Jun 10, 2026
This was referenced Jun 11, 2026
This was referenced Jul 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds durable WhatsApp analytics v2 event capture for inbound shopper messages. The patch expands the existing WhatsAppAnalytics schema, records best-effort structured rows from consent, linking, image search, product search, product selection, support, fallback, and error paths, and keeps message handling resilient if analytics persistence fails.
Type of change
Area affected
How to test this
Expected result: WhatsApp tests, lint, typecheck, Prisma validation/generation, and backend build all pass. Analytics writes are best-effort and do not block replies.
Pre-commit checklist
Screenshots
N/A - backend analytics capture only.
Notes for reviewer
Migration scope is limited to whatsapp_analytics: v2 analytics columns, requested indexes, and an optional user_id FK. It does not alter WIZZA reply behavior, search ranking, account linking rules, prompts, checkout, payments, orders, delivery, web, or shared package behavior. Prisma migrate dev could not be used locally because the shared dev database reported pre-existing drift in older migrations, so the migration SQL was created manually and inspected.
Summary by CodeRabbit
New Features
Database
Tests