Require visitor match for affiliate attribution cookies#200
Conversation
Greptile SummaryThis PR tightens affiliate attribution by requiring that a tracking-code cookie click belong to the same
Confidence Score: 5/5Safe to merge — the visitor-id guard is narrowly scoped and the fallback limit fix is a correctness improvement with no behavioral regression risk. Both changed paths are exercised by dedicated regression tests. The visitor-id filter is additive and the .limit(1) relocation in the fallback path aligns the code with how the Supabase builder applies filters before execution. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[findAttribution called] --> B{trackingCode present?}
B -- Yes --> C[Lookup affiliate application]
C --> D{app found & approved?}
D -- No --> E[Fall through to visitor fallback]
D -- Yes --> F[Build click query\ntracking_code + offer_id + window]
F --> G{visitorId present?}
G -- Yes --> H[Add .eq visitor_id filter]
G -- No --> I[No visitor filter]
H --> J[.limit 1 execute query]
I --> J
J --> K{clicks found?}
K -- No --> L[return affiliated: false]
K -- Yes --> M[return affiliated: true with click_id]
B -- No --> E
E --> N{visitorId present?}
N -- No --> O[return null]
N -- Yes --> P[Build click query offer_id + window + ORDER DESC]
P --> Q[Add .eq visitor_id filter]
Q --> R[.limit 1 execute query]
R --> S{clicks found?}
S -- No --> T[return affiliated: false]
S -- Yes --> U[return affiliated: true with click_id and tracking_code]
Reviews (3): Last reviewed commit: "Fix affiliate attribution visitor matchi..." | Re-trigger Greptile |
| if (visitorId) { | ||
| query = query.eq("visitor_id", visitorId); | ||
| } |
There was a problem hiding this comment.
Redundant
visitorId guard in fallback path
The if (visitorId) block at line 223 is unreachable-false: the early return at line 211 (if (!visitorId) return null) guarantees visitorId is always truthy here. This is pre-existing, but the refactor in this PR (moving the query from a chained .limit(1) call to a separate .limit(1) at the end) is a good opportunity to remove the dead branch and call .eq("visitor_id", visitorId) unconditionally, which would make the intent clearer.
Fixes #199.
Summary
visitor_idwhen one is availablelimit(1)so the latest matching visitor click is selectedValidation
./node_modules/.bin/vitest run src/lib/affiliates/tracking.test.ts./node_modules/.bin/eslint src/lib/affiliates/tracking.ts src/lib/affiliates/tracking.test.ts./node_modules/.bin/tsc --noEmit --pretty false./node_modules/.bin/prettier --check src/lib/affiliates/tracking.ts src/lib/affiliates/tracking.test.tsgit diff --check -- src/lib/affiliates/tracking.ts src/lib/affiliates/tracking.test.ts