-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix: resolve facebook ads context readability (externalAdReply) #2397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts WhatsApp message parsing to correctly extract and append Facebook/Instagram Click-to-WhatsApp ad context from externalAdReply in extendedTextMessage, while making location parsing safer and excluding helper-only fields from type detection. Sequence diagram for parsing externalAdReply Facebook ad contextsequenceDiagram
participant WhatsAppAPI
participant WebhookHandler
participant getConversationMessage
participant getTypeMessage
participant getMessageContent
participant BaseChatbotService
participant TypebotService
WhatsAppAPI->>WebhookHandler: incoming msg (extendedTextMessage.contextInfo.externalAdReply)
WebhookHandler->>getConversationMessage: msg
getConversationMessage->>getTypeMessage: msg
getTypeMessage->>getTypeMessage: extract conversation, extendedTextMessage, etc.
getTypeMessage->>getTypeMessage: read locationMessage using degreesLatitude?.toString()
getTypeMessage->>getTypeMessage: read externalAdReply from msg.message.extendedTextMessage.contextInfo
alt externalAdReply.body defined
getTypeMessage-->>getConversationMessage: types.externalAdReplyBody = externalAdReplyBody|body
else externalAdReply.title defined
getTypeMessage-->>getConversationMessage: types.externalAdReplyBody = externalAdReplyBody|title
else no externalAdReply
getTypeMessage-->>getConversationMessage: types.externalAdReplyBody = undefined
end
getConversationMessage->>getMessageContent: types
getMessageContent->>getMessageContent: select typeKey excluding externalAdReplyBody and messageType
getMessageContent-->>getConversationMessage: messageContent (possibly with externalAdReplyBody appended)
getConversationMessage-->>WebhookHandler: final message string
WebhookHandler-->>BaseChatbotService: user message with ad context
WebhookHandler-->>TypebotService: user message with ad context
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- The new
externalAdReplyBodylogic now only checksmsg.message.extendedTextMessage.contextInfo, which may drop support for payloads that previously usedmsg.contextInfo.externalAdReply; consider preserving the old path as a fallback. - The repeated access chain
msg.message.extendedTextMessage.contextInfo.externalAdReplycould be simplified via optional chaining with a local variable (e.g.const adReply = msg?.message?.extendedTextMessage?.contextInfo?.externalAdReply) to improve readability and reduce duplication. - The string key
'externalAdReplyBody'is now hard-coded both when buildingtypesand when filtering keys ingetMessageContent; consider extracting this into a shared constant to avoid drift if the key name changes later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `externalAdReplyBody` logic now only checks `msg.message.extendedTextMessage.contextInfo`, which may drop support for payloads that previously used `msg.contextInfo.externalAdReply`; consider preserving the old path as a fallback.
- The repeated access chain `msg.message.extendedTextMessage.contextInfo.externalAdReply` could be simplified via optional chaining with a local variable (e.g. `const adReply = msg?.message?.extendedTextMessage?.contextInfo?.externalAdReply`) to improve readability and reduce duplication.
- The string key `'externalAdReplyBody'` is now hard-coded both when building `types` and when filtering keys in `getMessageContent`; consider extracting this into a shared constant to avoid drift if the key name changes later.
## Individual Comments
### Comment 1
<location> `src/utils/getConversationMessage.ts:54-56` </location>
<code_context>
- ? `externalAdReplyBody|${msg.contextInfo.externalAdReply.body}`
+
+ // --- FIX FACEBOOK ADS START ---
+ externalAdReplyBody: msg?.message?.extendedTextMessage?.contextInfo?.externalAdReply?.body
+ ? `externalAdReplyBody|${msg.message.extendedTextMessage.contextInfo.externalAdReply.body}`
+ : msg?.message?.extendedTextMessage?.contextInfo?.externalAdReply?.title
+ ? `externalAdReplyBody|${msg.message.extendedTextMessage.contextInfo.externalAdReply.title}`
: undefined,
</code_context>
<issue_to_address>
**issue (bug_risk):** Narrowing `externalAdReplyBody` to only `extendedTextMessage.contextInfo` may regress other message types that used the top-level `contextInfo`.
The previous code read from `msg?.contextInfo?.externalAdReply`, which covers cases where `contextInfo` is attached at the top level or under non–extended text messages. The new version only checks `msg.message.extendedTextMessage.contextInfo`, so those other shapes may lose ad text. Consider using the new path first and falling back to the old one, for example:
```ts
externalAdReplyBody:
msg?.message?.extendedTextMessage?.contextInfo?.externalAdReply?.body
? `externalAdReplyBody|${msg.message.extendedTextMessage.contextInfo.externalAdReply.body}`
: msg?.message?.extendedTextMessage?.contextInfo?.externalAdReply?.title
? `externalAdReplyBody|${msg.message.extendedTextMessage.contextInfo.externalAdReply.title}`
: msg?.contextInfo?.externalAdReply?.body
? `externalAdReplyBody|${msg.contextInfo.externalAdReply.body}`
: msg?.contextInfo?.externalAdReply?.title
? `externalAdReplyBody|${msg.contextInfo.externalAdReply.title}`
: undefined,
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Description
This PR resolves an issue where the API fails to parse the context of Facebook/Instagram Click-to-WhatsApp ads (
externalAdReply). Previously, if a user clicked an ad but sent only the default pre-filled message, the API would treat it as an empty or unknown message type because the ad payload resides incontextInfo, not the main text body.Changes
src/utils/getConversationMessage.ts.externalAdReplyinmsg.message.extendedTextMessage.contextInfo.body(ortitleas fallback) from the ad payload.Testing
externalAdReply.base-chatbotandtypebotservices receive the concatenated string correctly.Summary by Sourcery
Handle Facebook/Instagram Click-to-WhatsApp ad messages by correctly extracting and appending external ad reply context to the conversation message.
Bug Fixes: