-
Notifications
You must be signed in to change notification settings - Fork 264
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
Remove early return of trackReceivedEvent to allow for os_notification_influence_open event to be sent to Firebase #1241
Conversation
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, but let's see about if we want to change the Firebase influenced attribution time from 2 minutes.
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jkasten2 and @shepherd-l)
We should take it on as a separate fix to make the firebase influences match OneSignal influences (remove 2 minute logic) |
Cherry pick of #1241 Also adding crash protection against a nil notificaiton id and campaign
Cherry pick of #1241 Also adding crash protection against a nil notificaiton id and campaign
Cherry pick of #1241 Also adding crash protection against a nil notificaiton id and campaign
Cherry pick of #1241 Also adding crash protection against a nil notificaiton id and campaign
Description
One Line Summary
Remove early return of
trackReceivedEvent
method to allow for theos_notification_influence_open
event to be sent to Firebase.Details
Motivation
The
os_notification_influence_open
event is not currently logged in Firebase. While initially thought to be an issue with thetrackInfluenceOpenEvent
method logic, it was eventually determined that the previous method,trackReceivedEvent
, returned early and prevented the influence event from being sent.Other
Since iOS does not support this
os_notification_received
event, thelogEventWithName
method for this event was also removed.Our documentation currently states that the
os_notification_influence_open
event will be sent if the app is opened within two minutes of receiving a notification. This was originally thought to be the flawed logic preventing the event from being sent altogether. However, after updating thetrackReceivedEvent
method, the original logic works. However, if we determine to change the duration, that can be easily updated.Testing
Unit testing
No unit testing was used.
Manual testing
Tested on an iPad running iOS 15.2.
Test Scenarios:
os_notification_influence_open
event was successfully logged:os_notification_influence_open
event was logged in Firebase DebugView.Affected code checklist
Checklist
Overview
Testing
Final pass
This change is