[IOCOM-1848] Analytics for Push Notifications Engagement #6598
Conversation
Tests for isForceDismissAndNotUnreadMessagesHiddenSelector, pushNotificationsBannerForceDismissionDateSelector, unreadMessagesCountAfterForceDismissionSelector selectors
Vangaorth
requested review from
ChrisMattew,
gispada,
freddi301 and
a team
as code owners
Jira Pull Request LinkThis Pull Request refers to the following Jira issue IOCOM-1848 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6598 +/- ##
==========================================
+ Coverage 49.33% 49.50% +0.16%
==========================================
Files 1565 1566 +1
Lines 32275 32370 +95
Branches 7290 7297 +7
==========================================
+ Hits 15923 16024 +101
+ Misses 16313 16307 -6
Partials 39 39
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
| event_type: "action", | ||
| banner_id: "push_notif_activation", | ||
| banner_page: | ||
| route === "MESSAGES_HOME" ? "MESSAGES_HOME" : "SETTINGS_MAIN", |
There was a problem hiding this comment.
| route === "MESSAGES_HOME" ? "MESSAGES_HOME" : "SETTINGS_MAIN", | |
| route, |
since this is a literal type, why not directly use it?
There was a problem hiding this comment.
Because it is a tracked data that appears in Mixpanel reports. Let's say that we just do banner_page: route. The test succeeds. Mixpanel boards are created, with this value in the reports (MESSAGES_HOME and SETTINGS_MAIN).
In the future, someone changes the value of the literal. Test still succeed, no warning at all but Mixpanel boards start receiving data that was not mapped (and nobody may be aware of it for a while).
With this redundancy in the test, if someone changes the literal value, the test fails so we have an error that should trigger the author (of the change) to reflect upon the impact on mixpanel data.
| event_type: "screen_view", | ||
| banner_id: "push_notif_activation", | ||
| banner_page: | ||
| route === "MESSAGES_HOME" ? "MESSAGES_HOME" : "SETTINGS_MAIN", |
| const eventName = "CLOSE_BANNER"; | ||
| const props = buildEventProperties("UX", "action", { | ||
| banner_id: "push_notif_activation", | ||
| banner_page: MESSAGES_ROUTES.MESSAGES_HOME, |
There was a problem hiding this comment.
since this banner is not attached to themessages_home screen, but rather to the current landing screen, would it make more sense to add the current route as a parameter?
There was a problem hiding this comment.
The event is related to the user manually closing the banner but (the banner) appears only in the messages' home and in the settings screen. Latter does not offer an option to close the banner so we do not have a use-case where the banner can be close anywhere but the messages' home
| discardModal.present(); | ||
| } else { | ||
| dispatch(setUserDismissedNotificationsBanner()); | ||
| closeHandler(); | ||
| } | ||
| }; | ||
|
|
||
| const onPress = React.useCallback(() => { | ||
| trackPushNotificationsBannerTap(MESSAGES_ROUTES.MESSAGES_HOME); |
There was a problem hiding this comment.
why not use the useRoute hook to get the current route?
There was a problem hiding this comment.
Because this banner does not have an use case for a screen that is not the messages' home
Short description
This PR adds the analytics for the disabled push notification permissions engagement banners.
List of changes proposed in this pull request
BANNER: for Itw, PushNotification and ProfileSettings bannersTAP_BANNER: for Itw, PushNotification and ProfileSettings bannersCLOSE_BANNER: for Itw, PushNotification and ProfileSettings bannersPUSH_NOTIF_SYSTEM_ALERT: for system permissions popupPUSH_NOTIF_THIRD_DISMISS_ALERT: for bottom sheet opening on third dismissal of the push notifications bannerPUSH_NOTIF_THIRD_DISMISS_ALERT_INTERACTION: for bottom sheet (above) interaction CTAPUSH_NOTIF_BANNER_FORCE_SHOW: when the dismissed push notification banner has to be shown againPUSH_NOTIF_BANNER_STILL_HIDDEN: when the dismissed push notification banner is not to be shown againHow to test
For every event, check that it is tracked with the proper properties