-
Notifications
You must be signed in to change notification settings - Fork 9
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 properties for UTM codes #2056
Conversation
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
38c9a65
to
7e63fc9
Compare
Looks like |
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.
Looks like
test_Event_update_event_properties()
andtest_Event_update_user_properties()
intest_analytics.py
already test the feature added in this PR, so new tests may not be required.
True, those functions have been tested. But this PR is adding behavior to the ViewedPageEvent
, and so tests could (should) be written for that.
Thanks, that makes sense. I added 4 new tests to test the new behavior of |
23e7262
to
7bfe7f0
Compare
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 work to keep on this @lalver1! There's still some refinement needed for the tests, hopefully my comments make sense. LMK if you want to chat.
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.
Fantastic work @lalver1! Thanks for sticking with the tests, looks great!
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.
This looks great!
Thanks @thekaveman and @angela-tran! |
Closes #2034.
Uses request.GET.get() to get the UTM code. If a UTM code is not present in the query string, returns
null
for that UTM code.How to test
Using a local instance of
benefits
, send a request that looks something likehttp://localhost:port/?utm_campaign=transit
. Verify that the propertyutm_campaign
has been added toevent_properties
anduser_properties
.