-
Notifications
You must be signed in to change notification settings - Fork 371
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
Fix cold start to create a new session regardless of backgrounded time #1932
Closed
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
[User Model] Location Unit Tests
[User Model] IAM Unit Tests (partial)
[User model] backend integration
Pre-alpha polish and fixes
* Make `OneSignal` module a psuedo project which brings in all subprojects. * Move `OneSignal` functionality down to new `core` subproject. * All other subprojects now depend on `OneSignal:core` instead of `OneSignal`. * Finalize new build/publishing scheme.
[User Model] Core Module
[User Model] Add name to each module for maven publish pom
* Remember device's push subscription ID in config and ensure that push subscription is returned in IUserManager.pushSubscription. * Add TransferSubscriptionOperation
[User Model] alpha bug fixes
* Updates to support creating a subscription when the subscription already exists for the owner.
…dy-exists [User Model] Subscription Already Exists
* Add app_version to SubscriptionObject * Add migration guide * Make OneSignal.User pascal-case
[User Model] Final touches
Release 5.0.0-alpha1
* Update ApplicationService to more accurately detect when the app goes in/out of focus * Always switch to main thread for RequestPermission calls, to ensure that (1) a suspend occurs and (2) any UI operations happen on the main thread.
* Add ToJsonObject to NotificationClickResult, NotificationAction, and InAppMessageClickResult. * Expose groupedNotifications in INotification
[User Model] Application Focus and Threading
Add public get tags method
…o_create_user Add refresh_device_metadata to create user so country / IP can be set
Subscription Manager contains a method previously called `addOrUpdatePushSubscription` that is meant to be used in relation to updating push tokens. This method is renamed to `addOrUpdatePushSubscriptionToken` so as to not confuse with other updates on the push subscription such as detecting app version changes between app opens, which is a newly added feature
Subscription Model now has sdk, deviceOS, carrier, and appVersion as properties so that they can persist. The getter is defaulted to "" (the empty string) as these properties do not exist prior to v5.0.5. They are only read when detecting changes so it is ok that they default to "" as the empty string will not be sent.
Additionally set these properties when a push subscription model is created: - sdk, deviceOS, carrier, appVersion
Let the SubscriptionManager refresh the push subscription model on new sessions.
When hydrating an existing push subscription model, use existing local device-scoped information instead of remote information for: - sdk - deviceOS - carrier - appVersion This information should always come from the local device. The reason for this change is that on a new session, we may detect a change to one of these properties and we do not want to overwrite it with old remote data from the get_user response.
Update push subscription model properties between sessions
[v5] Pause operation repo and retry failed user create
Release 5.0.5
- Now that cold starts will trigger a new session, the logic for storing active duration is updated. - Instead of active duration being reset to 0 at the start of every new session, it will reset to 0 after the session time is sent to the server. This is because cold starts may not have sent the old session time yet, so we should not reset to 0 here. - Additionally, the logic to run the session service's background task, which is sending the session time, will be triggered by there being an active duration instead of whether the session is valid. This is because a force killed app will initialize OneSignal to run this background task. By initializing OneSignal, the SDK believes this is a cold start and set the isValid property on the session model to `false`. However, we want to send off this accumulated session time to the server.
18 tasks
- On cold starts, the app may not be in the background for over 30 seconds, which is when the session time is sent and the session model's `isValid` property is set to false. - Therefore, after the session model is read from cache, reset it's `isValid` property to `false` in order to drive the creation of a new session on cold starts.
nan-li
force-pushed
the
fix_cold_start
branch
from
December 5, 2023 21:01
9c938b3
to
07a3647
Compare
nan-li
commented
Dec 5, 2023
Comment on lines
+8
to
+13
SimpleModelStore( | ||
{ SessionModel() }, | ||
"session", | ||
prefs, | ||
), | ||
) { |
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.
To me, this doesn't look as good as a one-liner. But linty lint was nitty picky.
jinliu9508
force-pushed
the
user-model/main
branch
3 times, most recently
from
February 6, 2024 16:38
2072eac
to
d73bfc6
Compare
Closing due to staleness for updated PR #2113 |
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.
❌ TODO: Test with influences and upgrading influences
❌ TODO: Add tesitng
Basic testing shows these are fulfilled on cold start
Description
One Line Summary
REQUIRED - Very short description that summaries the changes in this PR.
Details
Motivation
REQUIRED - Why is this code change being made? Or what is the goal of this PR? Examples: Fixes a specific bug, provides additional logging to debug future issues, feature to allow X.
Scope
RECOMMEND - OPTIONAL - What is intended to be effected. What is known not to change. Example: Notifications are grouped when parameter X is set, not enabled by default.
OPTIONAL - Other
OPTIONAL - Feel free to add any other sections or sub-sections that can explain your PR better.
Testing
Unit testing
OPTIONAL - Explain unit tests added, if not clear in the code.
Manual testing
RECOMMEND - OPTIONAL - Explain what scenarios were tested and the environment.
Example: Tested opening a notification while the app was foregrounded, app build with Android Studio 2020.3 with a fresh install of the OneSignal example app on a Pixel 6 with Android 12.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is