-
Notifications
You must be signed in to change notification settings - Fork 28
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: Publish onboarding events #1147
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Changes have been made to critical files, which contain lines commonly executed in production. Learn more ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #1147 +/- ##
=======================================
Coverage 96.03% 96.04%
=======================================
Files 837 837
Lines 19769 19781 +12
=======================================
+ Hits 18986 18998 +12
Misses 783 783
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
✅ All tests successful. No failed tests were found. 📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
❌ 2 Tests Failed:
View the top 2 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
dbd2986
to
2106b3d
Compare
"App Installed", | ||
{ | ||
"user_ownerid": installer.ownerid | ||
if installer is not None |
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 think this check fails if installer is none because we're already doing the access on line 523. I don't really know how python handles it tbh but this example on a python sandbox gave me an error
stuff = {
"ownerid": 1
}
stuff2 = stuff3["ownerid"] if stuff3 is not None else stuff["ownerid"]
print(stuff2)
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.
It think it's okay. installer
will be defined but possibly None.
>>> stuff = { "ownerid": 1 }
>>> stuff2 = stuff3["ownerid"] if stuff3 is not None else stuff["ownerid"]
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
stuff2 = stuff3["ownerid"] if stuff3 is not None else stuff["ownerid"]
^^^^^^
NameError: name 'stuff3' is not defined. Did you mean: 'stuff'?
>>> stuff3 = None
>>> stuff2 = stuff3["ownerid"] if stuff3 is not None else stuff["ownerid"]
>>> stuff2
1
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.
oh yeah okay I missed that part, sounds good
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.
lgtm!
Publishes Amplitude events for App Installed, User Created, and User Logged in.
Closes codecov/engineering-team#3179