[User model] Logging fixes #1194
Merged
Conversation
This file contains hidden or 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
nan-li
force-pushed
the
user_model/more_alpha_1_fixes
branch
from
dd537b3 to
42a11e3
Compare
* Replace all Swift `print()` calls with OneSignalLog. * Replace a few NSLogs in the SDK with OneSignalLog as well.
nan-li
force-pushed
the
user_model/use_onesignal_log
branch
from
aa59b9a to
02cfc96
Compare
Refine some more APIs for Swift
* For clarity, rename `OSUD_PLAYER_ID_TO` to `OSUD_PUSH_SUBSCRIPTION_ID` * And `OSUD_PUSH_TOKEN_TO` to `OSUD_PUSH_TOKEN` * Remove from Common Defines values no longer used
* Motivation: push sub behaves differently from other subscriptions and is not cleared from the store when a user changes. Separate it into its own store so that the email and sms subscriptions stores can be cleared but the push subscription store is not.
* Motivation: hydrating subscriptions can create a model and add to store if it doesn't exist locally. Guard against hydrating here as well so that a request isn't extraneously created.
* Motivation: Hydrating the push subscription is problematic. We will hydrate it if: - the push subscription ID does not already exist locally - the type of the returned subscription is iOSPush - the token at the time the request is made matches the returned token - we hydrate using the first matching subscription that is returned (there may be multiple iOS push subscriptions in the response but we will accept the first one that fulfills our conditions above) * This can still be faulty behavior but we get as close as we can to correct behavior for now.
* Add push subscription observer and permission observer * Missed this requirement for testing -> update bundle ID to staging, needs to be `com.onesignal.example.staging` * Move set log level up to capture logs earlier. * Not making any structural changes to example app, but link some buttons to new methods like login, logout
* Motivation: When initializing the delta queue and request queue, the executors need to hook up the Deltas' and Requests' models to the ones in the store, so that they can get the updated information needed to send the requests, and hydrate the correct models. * When model stores, deltas, and requests are initialized from cache, they lose their references as they are new instances. * To simplify uncaching, we maintain separate request queues for each request type in the executor * Also needed to update some `let` properties to be `var`
* Motivation: Must be careful of accessing `OneSignalUserManagerImpl.sharedInstance` before it's actually finished initializing, as the initializer for it does a lot of things! * What was happening was that during the process of initializing the `sharedInstance`, we initialized the executors, but the executors' initializer accessed model stores via OneSignalUserManagerImpl.sharedInstance` which is a problem. * Instead, initialize these in `start()`, but now they will be Optional. * And we may need to note if any other areas can have the same issue.
* Comments, TODOs, and swiftlint * Remove too extraneous logging * add informational logging
* On logout, we were setting the user to `nil` to early * When logout is called, we prepare for new user, and the operation repo and executors flush their Deltas. However, they still need access to user properties, so set user to `nil` after this.
* We want to remove this request from the request queue and cache when it returns successfully, even if we don't hydrate. * It can return just `success` with no subscription info if this subscription already exists.
* Update check for 2 tokens being equal, since "" is equal to nil
* Motivation: Create User's response won't send us the user's complete info if this user already existed. * For now, do both, as we should parse to get the subscription_id from this request. * TODO: Differentiate if we need to Fetch based on response code to not make extraneous call.
* It seems we cannot send `enabled = true` without sending `notification_types` for creating an email or SMS. Even sending -99 is ok. * Or, we send neither. * The server is still in flux. For now, don't send any of these, but we need to revisit
emawby
approved these changes
Jan 6, 2023
fhboswell
approved these changes
Jan 6, 2023
…_round_2 [user model] More fixes for alpha release (round 3)
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.
Description
One Line Summary
Remove Swift
print(updating the message to be cleaner) andNSLogfrom SDK, and use OneSignalLog instead.Motivation
Logging
Scope
Logging
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is