feat: add gwpApproved setting for gift-with-purchase conversions#2
feat: add gwpApproved setting for gift-with-purchase conversions#2mvedma0405 wants to merge 2 commits into
Conversation
Maps a configured custom event name (or comma-separated list) to a new
gwpApproved Pay+ signal, following the same pattern as the other custom
event settings: the signal posts { source, type: 'gwpApproved', detail:
<event attributes>, trigger } to the embedding plugin.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| { setting: 'pendingSuccessEventName', signal: SIGNALS.PENDING_SUCCESS }, | ||
| { setting: 'closeEventName', signal: SIGNALS.CLOSE }, | ||
| { setting: 'removeLoadingOverlayEventName', signal: SIGNALS.REMOVE_LOADING_OVERLAY }, | ||
| { setting: 'gwpApproved', signal: SIGNALS.GWP_APPROVED }, |
There was a problem hiding this comment.
Nit: I see you have a lot of these settings declared in your README.md but the new setting isn't reflected there. I would suggest removing the list from settings since that's going to be a headache to maintain.
There was a problem hiding this comment.
Agreed on the maintenance headache (this PR proved it by drifting on the first settings change). Removed both per-setting tables from the README and replaced them with a one-line description of the convention (each custom-event setting maps an event name to the signal of the same name), pointing at the dashboard fields for the full self-describing list. Kept the conceptual prose (page views vs custom events, screen_name, defaults) since that doesn't drift.
| removeLoadingOverlayEventName?: string; | ||
| conversionEventName?: string; | ||
| // Custom event name(s) that signal a gift-with-purchase conversion. | ||
| gwpApproved?: string; |
There was a problem hiding this comment.
nit: gwpApproved doesn't match the existing convention of settings. If we can't change it server side, it's not a dealbreaker.
There was a problem hiding this comment.
You're right, and the escape hatch doesn't apply: nothing has shipped server-side yet (the mPServer registration for this setting is still a draft), so the rename was free. Renamed to gwpApprovedEventName to match the <signal>EventName convention; the server-side SettingTemplate row will use the same key.
… settings tables - rename the setting key gwpApproved -> gwpApprovedEventName to match the <signal>EventName convention of the other custom-event settings (nothing has shipped server-side yet, so the rename is free) - replace the README per-setting tables with a drift-proof description of the naming convention; the dashboard fields are self-describing Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Both nits addressed: renamed the setting to |
3 participants
Summary
Adds a new dashboard setting,
gwpApproved, that maps a custom event name (or comma-separated list) to a newgwpApprovedPay+ signal, for gift-with-purchase conversion flows. It follows the exact pattern of the existing custom-event settings (approvedEventName,purchaseCompletedEventName, etc.):SIGNALS.GWP_APPROVED = 'gwpApproved'EVENT_SETTING_TO_SIGNALentry:gwpApproved→gwpApprovedRoktPayPlusKitSettings.gwpApproved?: stringWhen the advertiser logs a custom event whose name matches the configured value, the kit posts:
to
window.parent(iframe-only, as with all signals).Notes for reviewers
gwpApproved(noEventNamesuffix), matching the mPServer-side registration. If the server registration lands with a different key, this is a one-line change inEVENT_SETTING_TO_SIGNAL+ the interface.MessageEventTypeentry (e.g. wired tosignalGWPConversion()) for this signal to act in production; today GWP conversion fires internally offpurchaseCompletedin the STARZ flow.Testing Plan
npm run typecheck,npm run lint,npm run buildall cleannpm run test: 13/13, including two new tests: configuredgwpApprovedevent emits the signal with the event attributes asdetail(and notapproved), and configuring it suppresses the defaultconversionfallback (existing explicit-mapping semantics).