-
Notifications
You must be signed in to change notification settings - Fork 1
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(analytics): other iOS GA updates #652
Conversation
public func locationFetcherDidChangeAuthorization(_ fetcher: LocationFetcher) { | ||
authorizationStatus = fetcher.authorizationStatus | ||
analytics.recordSession( | ||
locationAccess: fetcher.authorizationStatus, | ||
locationAccuracy: fetcher.accuracyAuthorization | ||
) |
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.
For some reason, this gets called far more often than the authorization actually changes; this shouldn't actually be a problem, but it does flood the log with 11.5.0 - [FirebaseAnalytics][I-ACS023081] Analytics is disabled. User property not set
repeatedly when running the staging app locally; I could try to check that things have actually changed before sending the variable to Firebase if we thought it was worth the effort.
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.
That seems like a good quick fix, and close enough to the ticket scope. If it turns out to not be quick for whatever eldritch reason we can always just make it a ticket.
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.
Remembering the previous fetcher.authorizationStatus
and fetcher.authorizationAccuracy
didn't actually work - these were still being sent to Firebase way too often - but some judicious breakpoints led me to
locationDataManager: LocationDataManager = .init(distanceFilter: 1), |
LocationDataManager
every time the ContentView
renders.
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.
👍
avoids creating a new one every time ContentView renders and thereby spamming GA with redundant location permission updates
# Conflicts: # iosApp/iosApp.xcodeproj/project.pbxproj
Summary
Ticket: GA Updates
Handles the Search, Combined Stop + Trip Details, and User settings elements. Nearby Transit was covered in #650 since it had some architectural consequences that these do not.
It's not clear that there's actually a way to track things per session, so I'm tracking things per user instead. I think I've set up the custom definitions correctly in GA, but I'm not sure how to be certain.
iOS
[ ] If you added any user-facing strings on iOS, are they included in Localizable.xcstrings?[ ] Add temporary machine translations, marked "Needs Review"android
[ ] All user-facing strings added to strings resource[ ] Expensive calculations are run inwithContext(Dispatchers.Default)
where possibleTesting
Manually verified that everything is called when it should be.