enhancement and fix for LinkedIn CAPI tag#3
Conversation
|
Great work @otyeung 🎉 Just one small note: According to Google's server-side common event data doc, it should be |
|
Hi @mithredate Great catch! I've fixed this in this branch, please kindly review & take a look! Apart from that, I've added several enhancements:
Ping me via Slack or Teams if you want to discsuss. |
|
Thank you so much @otyeung for raising this PR! Sorry we missed seeing this PR earlier. We'll be gradually updating the template with enhancements and incorporate most of your updates. We'll keep you posted. cc @SurendraJambeLI |
ZLeventer
left a comment
There was a problem hiding this comment.
Solid set of improvements. A few observations from working in this codebase recently (PRs #15–#19):
-
The
conversionValuehandling needs a guard againstundefined— if the GTM variable isn't set, the API call fails silently. I addressed this specifically in #15. -
For the email hashing logic: the CAPI spec accepts both
SHA256_EMAIL(pre-hashed) and plaintext email (which the tag should hash client-side before sending). If this PR touches email handling, worth confirming it normalizes (lowercase + trim) before hashing per LinkedIn's docs. -
The
li_fat_idcookie read is a good addition if included here — that's the LinkedIn first-party ads tracking ID and is the highest-signal match key after email.
Overall this looks like a meaningful improvement over the baseline template. Would be good to get it or a subset merged — the template has had several overlapping PRs open for 1-2 years addressing similar gaps.
Changes made :