Feat/popups v2 rules - #84
anyelopetit wants to merge 35 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ce1f901 to
385a6da
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce1f90104a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd3d873ba5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de17524ce2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Adds the browser half of the split evaluation. The server decides profile conditions and sends only the page conditions of the lanes that survived, so this treats the payload as the whole remaining question: lanes are OR'd, conditions inside a lane are AND'd, and no lanes means the popup may display. PopupDisplayRules mirrors Popup::DisplayRules::PageEvaluator, including complement semantics for unset values, so Test a URL and the runtime cannot disagree. Scroll depth and time on page only grow, so a popup gated on them is re-checked on scroll and once a second instead of being decided on connect. Watching starts only when a rule needs a measurement, and stops the moment the popup displays: a popup counts as shown when it actually appears, never when its rules merely match. An eligible: false response carries no markup and is treated as nothing to render rather than a failure.
de17524 to
beec397
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: beec397f97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0d07955a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38acfaea16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f048e59a7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70df204cba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d15985a71c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
| return | ||
|
|
||
| if (response.data.status === 202) continue |
There was a problem hiding this comment.
If the status endpoint remains at 202 through the eighth attempt—such as when the identification job takes longer than roughly 16 seconds—this continue exits the loop without clearing identificationPending. Because identify() already unmounted the active popup and loadPopup() refuses to run while that flag is set, popup evaluation then remains disabled for the rest of the visit. Clear the pending state and restore anonymous popup evaluation when the retry budget is exhausted.
Useful? React with 👍 / 👎.
| // `eligible: false` with no markup when this visitor does not qualify. That is a | ||
| // deliberate outcome rather than an error, so it is treated the same as "nothing to | ||
| // render" instead of surfacing as a failure. | ||
| if (data.eligible === false || !data.html) return null |
There was a problem hiding this comment.
When the initial response is eligible: false, this early return means no popup controller is mounted, but PopupController.watchNavigation() is the only runtime component that records SPA route changes. For example, an anonymous visitor can navigate through several routes and then identify into an eligible audience; the reloaded popup receives a stale page-view count and a session.page_views rule may stay false. The remaining uncovered case is specifically this no-markup response, so route tracking needs to live outside the rendered popup controller or remain active while eligibility is false.
Useful? React with 👍 / 👎.
| ) | ||
| return | ||
|
|
||
| User.remember(details.externalId, details.source, details.fingerprint) |
There was a problem hiding this comment.
When an app awaits identify() and then logs the user out before receipt polling completes, identify() has already returned but forget() does not invalidate the background identification. A later successful poll reaches this line and recreates the user cookies that logout just cleared, so subsequent activity can be attributed to the previous user. forget() must advance the identification version, cancel polling, and clear the pending state before deleting the identity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c0848e9a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const identificationVersion = ++this.identificationVersion | ||
| const businessId = this.visitBusinessId | ||
| const session = this.session | ||
| this.identificationPending = true | ||
| this.cancelIdentificationPolling() |
There was a problem hiding this comment.
When user A's receipt is still polling and the app switches back to the previously remembered user B, identify(B) returns from the fingerprint-match branch before reaching this version increment. The request for A therefore remains current and can later overwrite B's identity cookies, despite B being the newest identification request. Advance the identification version and cancel pending polling before applying the already-identified shortcut.
Useful? React with 👍 / 👎.
| watchNavigation() { | ||
| if (this.onNavigation) return | ||
|
|
||
| this.lastRoute = this.pageRoute() |
There was a problem hiding this comment.
If an undisplayed popup element is detached, the SPA changes routes, and the same element is reinserted, disconnect() removes the only navigation listeners and this assignment accepts the new route without recording it. The controller also retains its old connectedAt, so the new page is omitted from session.page_views and inherits the previous page's elapsed time, potentially making page-count or time-on-page rules evaluate incorrectly. Compare the reconnect route with the previously observed route and update both counters before evaluating.
Useful? React with 👍 / 👎.
1 participant
Summary
Implements the browser-runtime half of Popup Rules V2 for the paired Rails change in hellotext#5833.
The SDK evaluates page, session, and activity conditions in the browser, follows SPA navigation, declares the rules contract to Rails, and coordinates late identification with the new Rails receipt protocol before re-evaluating popup eligibility. Profile, audience, and signal conditions stay on the server and are never evaluated from browser-visible contact data.
Important
Versioning is TENTATIVE.
package.jsondeclares2.6.1and the Rails branch pins@hellotext/hellotext2.6.0. Both numbers were assumptions. Ahmed should set the final version here, and the matching Rails pin, before merge.Runtime contract with Rails
X-Hellotext-Popup-Rules: 1(POPUP_RULES_CONTRACTinsrc/api/popups.js).{ eligible: false }responses, with or without areason, are a deliberate outcome and render nothing.Rails withholds a popup whose every alternative needs browser conditions from a runtime that does not send the header (
reason: 'runtime_unsupported'). An older SDK installed on a store therefore never shows a page-targeted popup on every page. It gets that popup once the store updates. The Rails editor warns the merchant while an outdated runtime has asked for a popup during the last 7 days, even if up-to-date browsers are also seen.The client evaluator expects the canonical
{ lanes: [...] }document:The composer limits a popup to three alternatives, but the runtime stays defensive when evaluating persisted data.
Supported client-side context
page.path. Matches the storefront path (see normalization below).document.titledocument.referrernavigator.languages[0], falling back tonavigator.languageDevice is not a rule: it stays a popup design setting and is fixed when the popup controller connects.
Navigation and route normalization
The popup runtime watches:
pushStateandreplaceState;popstateandhashchange;document.titlemutations.Routes are canonicalized with the same rules as the Rails normalizer and editor preview, so a saved Page URL value always compares against what the SDK reads:
#/…,#!/…) are kept as the route;is/is not,/sale,/sale/, and/sale/index.htmlare the same page;contains/does not contain, a trailing slash typed by the merchant is kept;/a%2Fbis not/a/b);Merchants may enter a path or an
http(s)://URL. An exact path gains its leading slash (sale→/sale); a contained fragment is kept as typed (rojomatches/zapato-rojo). Rails refuses bare hosts (shop.example/sale), empty or/-only fragments, and non-web schemes.UTM semantics
hello_utmattribution cookie.utm_source,utm_medium, orutm_campaign, those values are used alone and are remembered insessionStoragefor the rest of the tab visit. Otherwise the remembered campaign is used. The two are never mixed.#/route?utm_…) are read only when the address itself carries none, so/?utm_source=a#/sale?utm_source=breadsa.URLSearchParams, compared case-insensitively, and+reads as a space.Activity semantics
Activity is scoped to the current business and the current browser tab. Once an event occurs, it counts on every later page in that tab, and a new tab starts without it. Events from another business or from before the tab visit do not unlock a rule.
The commerce activities are product decisions, but each integration must emit the event at a successful storefront action. Shopify, Wix, and VTEX must be smoke-tested:
Late identification receipt protocol
Popup eligibility can change after
identify(), because a visitor may meet a profile, audience, or signal condition only once Rails has mapped the identification.identify()submits the identification and receives a short-lived receipt.GET /v1/public/identifications/:idwith the session header, using bounded backoff.Safety properties:
Backward compatibility of that legacy path covers identification only. Page, session, and activity rules depend on the Rails capability gate described above.
Failure behavior and privacy boundaries
no-store, and the SDK never uses a receipt from another session.Known issue (not introduced here)
Rails renders
data-hellotext--popup-delay-value, butpopup_controller.jsdoes not declare or read adelayvalue, so trigger delay has no effect.Build and dependency updates
lib/anddist/artifacts are included.Validation
npm test→ 45 suites, 975 tests passing, including theX-Hellotext-Popup-Rulesheader assertion.npm run build→ completed successfully.npx prettier --checkon the files changed by the latest commits → passing. The repository has no format script, and this PR does not reformat untouched files.Release checklist
identify();Intentionally out of scope