Implement sticky referrer tracking with first-touch attribution#271
Conversation
document.referrer is set by the browser on every internal navigation with the previous page URL, so the existing event-time precedence overwrote the session's entry referrer the moment a user clicked any internal link. Flip the OR fallback so the stored value wins, matching the sticky behaviour already used for ref, UTM, and click-ID fields.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the traffic source attribution logic in EventFactory.ts to make the referrer sticky across a session, preferring the stored referrer over the context referrer. It also adds comprehensive unit tests to verify this behavior. The review feedback highlights a critical bug where a direct visit followed by internal navigation can incorrectly attribute the session to an internal referrer, and suggests filtering out same-domain referrers along with adding a corresponding test case.
| referrer: | ||
| contextTrafficSources.referrer || storedTrafficSources?.referrer || "", | ||
| storedTrafficSources?.referrer || contextTrafficSources.referrer || "", |
There was a problem hiding this comment.
There is a subtle but critical bug when a user lands on the site directly (with no referrer).
Because a direct visit has an empty referrer, nothing is stored in the session storage. On the subsequent internal navigation, storedTrafficSources?.referrer is empty, so the logic falls back to contextTrafficSources.referrer (which the browser automatically populates with the previous internal page URL). This internal URL then gets stored in the session storage, incorrectly attributing the rest of the session to an internal referrer.
To prevent this, we should filter out same-domain (internal) referrers so they are never captured or stored.
referrer: (() => {
if (storedTrafficSources?.referrer) return storedTrafficSources.referrer;
const ref = contextTrafficSources.referrer;
if (!ref) return "";
try {
const refUrl = new URL(ref);
const currentHost = globalThis.location?.hostname;
if (currentHost && refUrl.hostname === currentHost) return "";
} catch {}
return ref;
})(),| setMockLocation("https://formo.so/dashboard"); | ||
| const freshContext = await getPageContext(); | ||
| expect(freshContext.referrer).to.equal(""); | ||
| }); |
There was a problem hiding this comment.
Add a test case to verify that direct landing followed by internal navigation does not incorrectly capture the internal page as the referrer.
});
it("should not treat internal navigation as referrer when landing directly", async () => {
// Direct landing: no referrer.
setMockLocation("https://formo.so/");
const landingContext = await getPageContext();
expect(landingContext.referrer).to.equal("");
// Internal navigation: browser sets document.referrer to the previous page.
setMockLocation("https://formo.so/dashboard", "https://formo.so/");
const secondContext = await getPageContext();
expect(secondContext.referrer).to.equal("");
});
|
@codex review |
After a direct landing (empty document.referrer), the next internal navigation reports document.referrer as the previous same-host page, which the prior change would have stored as the session's entry referrer for the rest of the session. Strip same-host referrers at the source via getExternalReferrer so they never enter the data path.
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Summary
This PR implements sticky referrer tracking to ensure first-touch attribution is preserved across a user's session. Previously, the referrer value could be overwritten during internal navigation because the browser automatically updates
document.referrerto the previous page URL. Now, the stored referrer value takes precedence, maintaining accurate session source attribution.Key Changes
setMockLocation()helper to accept an optionalreferrerparameter for testing different referrer scenariosImplementation Details
The fix leverages session storage to maintain the entry referrer across page navigations. The logic now follows:
storedTrafficSources?.referrer || contextTrafficSources.referrer || "", ensuring that once a referrer is captured at session start, it remains sticky throughout the session while still allowing fallback to document.referrer for fresh sessions with no stored value.https://claude.ai/code/session_018WEzHcE3XDMxo5pqn27vLw
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.