-
Notifications
You must be signed in to change notification settings - Fork 429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add hubspot cookie tracking #4539
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Skipped Deployments
|
Docker builds report
|
Uffizzi Preview |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4539 +/- ##
=======================================
Coverage 97.15% 97.16%
=======================================
Files 1157 1160 +3
Lines 40023 40107 +84
=======================================
+ Hits 38886 38970 +84
Misses 1137 1137 ☔ View full report in Codecov by Sentry. |
api/organisations/invites/views.py
Outdated
hubspot_cookie = request.COOKIES.get(HUBSPOT_COOKIE_NAME) | ||
|
||
if hubspot_cookie: | ||
if not HubspotTracker.objects.filter(user=request.user).exists(): | ||
HubspotTracker.objects.create( | ||
user=request.user, | ||
hubspot_cookie=hubspot_cookie, | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to avoid code duplication, which occurs further in join_organisation_from_link
. This code can be extracted to either of the following:
- A
HubspotTracker
class method - A function in
services.py
— that's how I'd do it, see existingservices.py
modules for reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure this is easy.
api/organisations/invites/views.py
Outdated
hubspot_cookie = request.COOKIES.get(HUBSPOT_COOKIE_NAME) | ||
|
||
if hubspot_cookie: | ||
if not HubspotTracker.objects.filter(user=request.user).exists(): | ||
HubspotTracker.objects.create( | ||
user=request.user, | ||
hubspot_cookie=hubspot_cookie, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See here.
api/organisations/views.py
Outdated
hubspot_cookie = request.COOKIES.get(HUBSPOT_COOKIE_NAME) | ||
|
||
if hubspot_cookie: | ||
if not HubspotTracker.objects.filter(user=request.user).exists(): | ||
HubspotTracker.objects.create( | ||
user=request.user, | ||
hubspot_cookie=hubspot_cookie, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See here.
api/organisations/invites/views.py
Outdated
|
||
if hubspot_cookie: | ||
if not HubspotTracker.objects.filter(user=request.user).exists(): | ||
HubspotTracker.objects.create( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update the stored cookie in case it's regenerated for some reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I've switched the code to an update_or_create
method to handle that if it ever comes up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got a nitpick, a code improvement proposal, and a question.
@@ -58,6 +58,12 @@ def create_lead(self, user: FFAdminUser, organisation: Organisation = None) -> N | |||
|
|||
HubspotLead.objects.create(user=user, hubspot_id=response["id"]) | |||
|
|||
if HubspotTracker.objects.filter(user=user).exists(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we can use walrus here and get rid of the exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch 👍🏼
Co-authored-by: Kim Gustyr <[email protected]>
Changes
Fixes #3855
This change adds in a form in the background of a Hubspot contact creation to add in the special Hubspot tracking cookie to give extra visibility in our user's journey in places like the activity tab of the Hubspot dashboard.
How did you test this code?
Manually tested it with my own Hubspot cookie and I added programatic tests after gaining confidence that it worked.