Fix: Method optIn() not working after optOut() has been called
#1957
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
nan-li
force-pushed
the
fix/optIn_not_working
branch
2 times, most recently
from
dbe29cd to
958191a
Compare
nan-li
requested review from
jkasten2,
emawby,
jinliu9508,
shepherd-l and
jennantilla
jkasten2
approved these changes
Jan 10, 2024
...ignal/core/src/main/java/com/onesignal/user/internal/subscriptions/SubscriptionModelStore.kt
Outdated
Show resolved
Hide resolved
nan-li
force-pushed
the
fix/optIn_not_working
branch
from
958191a to
6319054
Compare
nan-li
changed the base branch from
dev_app/add_helper_buttons
to
user-model/main
nan-li
changed the base branch from
user-model/main
to
dev_app/add_helper_buttons
nan-li
force-pushed
the
fix/optIn_not_working
branch
from
6319054 to
e21d556
Compare
nan-li
changed the base branch from
dev_app/add_helper_buttons
to
user-model/main
Existing Problem: - If on a previous session, `optOut()` was called, we will hydrate the push subscription with the status of `UNSUBSCRIBE(-2)` on the next session. - However, we lose the true device-level subscription status in doing so. - Then, when `optIn()` is called, we were using `UNSUBSCRIBE(-2)` along with `optedIn` to calculate the data to send in the Update Subscription request. - This resulted in incorrect values being sent and reports of `optIn()` method not working. Solution: - We can still hydrate the push subscription model's `optedIn` property if we see either UNSUBSCRIBE(-2) or DISABLED_FROM_REST_API_DEFAULT_REASON(-30). - However, when the subscription model store is replacing all of the subscriptions in the refresh user operation, keep the existing push subscription `status`. - This way, when `optIn()` is called again, we can use device-level status with the `optedIn` boolean to calculate values to send to the server.
nan-li
force-pushed
the
fix/optIn_not_working
branch
2 times, most recently
from
5f19168 to
db99671
Compare
|
Noting that our Test/build setup Android step is failing, but previous runs of this PR did succeed. |
Merged
jinliu9508
pushed a commit
that referenced
this pull request
Jan 31, 2024
Fix: Method `optIn()` not working after `optOut()` has been called
jinliu9508
pushed a commit
that referenced
this pull request
Jan 31, 2024
Fix: Method `optIn()` not working after `optOut()` has been called
jinliu9508
pushed a commit
that referenced
this pull request
Feb 6, 2024
Fix: Method `optIn()` not working after `optOut()` has been called
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
Fix method
optIn()not working afteroptOut()has been called, by maintaining the local push subscription status.Details
Motivation
Existing Problem:
optOut()was called, that means we sent notification_types ofUNSUBSCRIBE(-2)to the server.UNSUBSCRIBE(-2)in the response on the next session.optIn()is called, we were usingUNSUBSCRIBE(-2)along withoptedInto calculate the data to send in the Update Subscription request.optIn()method not working.Solution:
optedInproperty if we see eitherUNSUBSCRIBE(-2)orDISABLED_FROM_REST_API_DEFAULT_REASON(-30)in the response.model.statusfrom the server. When the subscription model store is replacing all of the subscriptions in the refresh user operation, keep the existing push subscriptionstatus.optIn()is called again, we can use this device-level status with theoptedInboolean to calculate values to send to the server.Scope
DISABLED_FROM_REST_API_DEFAULT_REASON(-30)from the server to update localoptedInstategetSubscriptionEnabledAndStatus(here) so the model's own properties aren't directly sent.Testing
Unit testing
No unit test added, can consider adding.
Manual testing
Android emulator API 33
optIn()andoptOut()with different permissions, new session, cold starts, and logging into different usersAndroid emulator API 30
optIn()andoptOut()with new session, cold starts, and logging into different usersAffected code checklist
Checklist
Overview
Testing
Final pass
This change is