flagsmith icon indicating copy to clipboard operation
flagsmith copied to clipboard

chore: Linkedin partner tracking

Open kyle-ssg opened this issue 1 year ago • 4 comments

Thanks for submitting a PR! Please check the boxes below:

  • [x] I have run pre-commit to check linting
  • [x] I have added information to docs/ if required so people know about the feature!
  • [x] I have filled in the "Changes" section below?
  • [x] I have filled in the "How did you test this code" section below?
  • [x] I have used a Conventional Commit title for this Pull Request

Changes

Adds linkedin partner tracking when LINKEDIN_PARTNER_TRACKING=true

How did you test this code?

  • Ran the frontend with LINKEDIN_PARTNER_TRACKING=true, validated conversion events were sent.
  • Validated no requests are sent to Linkedin when the variable is disabled

kyle-ssg avatar May 03 '24 09:05 kyle-ssg

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 3, 2024 10:29am
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 3, 2024 10:29am
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 3, 2024 10:29am

vercel[bot] avatar May 03 '24 09:05 vercel[bot]

Uffizzi Preview deployment-51197 was deleted.

github-actions[bot] avatar May 03 '24 09:05 github-actions[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.87%. Comparing base (54c2603) to head (7dab079). Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3879   +/-   ##
=======================================
  Coverage   95.86%   95.87%           
=======================================
  Files        1130     1131    +1     
  Lines       35795    35854   +59     
=======================================
+ Hits        34315    34374   +59     
  Misses       1480     1480           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar May 03 '24 10:05 codecov-commenter

Added one minor specific comment based on the implementation but I'm also keen to make sure that we've verified this doesn't cause any unnecessary outbound requests when the option is disabled, e.g. for self hosted installations. I can't see any reference to it in the test section of the PR description.

Yeah, it wouldn't. The script only gets added to the page if the env var is set https://github.com/Flagsmith/flagsmith/pull/3879/files#diff-d2af3ec690c6d6d6c4691ed1d668ec2cf39c84b774cef27c7346e5efd555d321R106

kyle-ssg avatar May 03 '24 10:05 kyle-ssg