-
Notifications
You must be signed in to change notification settings - Fork 114
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
Feature branch: new session logic + outcomes #624
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added types for DeviceRecord serialization.
instead of direct api calls from the page
1. Perform validation for all required params to be present 2. If no session exists, create a new one with current moment as its start 3. If there is an existing session 3.1. check if it's already active, theoretically should never get to this point and should be safe to continue with no extra action; 3.2. for inactive session we verify when it was last marked as inactive. if it's been less than THRESHOLD seconds, mark as active 3.3. last case should not occur often, it should cover the case if we failed to report it on the previous attempt. May need extra thought.
1. If there is no existing session, invalid use case -> early return 2. If the session is already in inactive state -> invalid use case -> early return 3. Otherwise - mark as inactive - calculate total session time since last activated - update the record in the database - set up timeout to report session to on_focus and cleanup
It tracks switching to a different tab, fully covering page with another window, screen lock/unlock.
Don't have much time when tab is closing, skipping device record and directly notify SW for https. Does not include http support yet. Had to change SessionPayload interface to reflect optional device record fields.
Don't add focus and blur when page's visibility is hidden, i.e. rendered in background. As soon as becomes visible, set both blur and focus. If becomes hidden again, clear out event listeners. Done so to avoid double-firing of session events.
or deactivate the session. Currently done for https only. Http support to follow. Unfortunately, safari is special and would not give the correct focused status for its pages to the service worker as chrome does. Had to work around it with a messaging to the page: 1. sending a request to all active pages to return their focused states 2. setting a timeout of 500ms to wait for responses 3. added a handler on the page and in SW to react to the new events 4. after timeout goes off, check current clientsStatus in SW to trigger session update
to account for multiple session update requests, e.g. - closing several tabs fast - switching between tabs
It had wrong assumption about payload structure.
1. Including push token on service worker side if missing 2. Implementing or rather skipping implementation on page for getting push token.
and not using subscription workaround and first page view
Had to introduce an interface for SessionManager and use it in context instead of actual classed. By doing this each type of context can rely on its specific implementation while still complying with general interface.
when cancel happens.
regardless of notificationOpensLink flag value.
to not use page Context
in order not to break tests
until we finish splitting all managers clearly for page and sw
if sw got killed before carrying out finalize
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Session logic refactoring for web sdk
Motivation
Previously we were tracking sessions per tab, i.e. every new open tab was counted as a new session while page refresh was not even if page was left open in the background for a long time.
New way of session tracking should be much more similar to Android SDK and be more logical from user perspective:
Implementation
The refactored session tracking heavily relies on service workers (SW). Previously all api calls were performed from the page which made them somewhat fragile to page refreshes. The new approach moves api calls related to session tracking into the service worker.
Chrome and Firefox
For these browsers we already utilize SW since they are used for push notifications. It means there was no change needed to install SW as part of initialization flow. The main changes are in setting up additional messaging between page and SW to track session related events, with different handling of HTTPS and HTTP setups.
HTTP setup is implemented using the subscription workaround where we add an iframe pointing to a custom HTTPS OneSignal (OS) subdomain and install SW there. Page can talk to iframe, iframe talks to its SW and back.
Safari
Safari does not use SW for push but they became supported a few versions ago so we'll be installing SW on Safari for HTTPS sites (SW are not allowed for HTTP sites). For pure HTTP sites SW is not allowed at all so session is reported the old way on the first page load for a new tab/window.
General flow
Outcomes
Motivation
Outcomes is a feature recently introduced in OneSignal dashboard and currently supported by ios and android only. This PR adds outcomes support to Web SDK.
General flow
New session logic and outcomes implementation heavily rely on service worker (SW) and having push as part of SW so it's best supported by browsers that implemented push via SW events.
Safari's way of handling push makes it impossible to track direct and indirect outcomes making all of them to be unattributed.
This branch also includes improvements and bug fixes for session logic that became obvious during testing.
This change is