-
Notifications
You must be signed in to change notification settings - Fork 376
fix: login with the same externalId fails to update the subscription #2406
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
base: main
Are you sure you want to change the base?
Conversation
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LoginHelper.kt
Show resolved
Hide resolved
OneSignalSDK/onesignal/core/src/main/java/com/onesignal/user/internal/LoginHelper.kt
Show resolved
Hide resolved
nan-li
left a comment
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.
Blocking while reviewing
|
What do you mean by "if the user’s external ID had been manually removed from the backend" and "remove the externalID through the dashboard"? |
abdulraqeeb33
left a comment
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.
Connected with @nan-li and @jinliu9508 , realized the current solution is not the ideal solution. We need to figure out a better solution for this issue.
| currentIdentityExternalId = identityModelStore.model.externalId | ||
| currentIdentityOneSignalId = identityModelStore.model.onesignalId | ||
|
|
||
| if (currentIdentityExternalId == externalId) { |
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.
This same behaviour exists in iOS, so we need to figure out what is it thats different we are doing.
is it possible that the login method is called so quickly and identityModelStore is not setup yet?
Description
One Line Summary
Fix issue where calling login with the same externalId as the previous app open fails to update the subscription in the backend.
Details
Motivation
Previously, when login was called using the same externalId as the previous app open, the SDK detected no change in the externalId and skipped creating a login request. Instead, it issued a refresh user request.
However, if the user’s external ID had been manually removed from the backend (for example, via the OneSignal Dashboard or calling Logout outside of SDK), this refresh request no longer linked the subscription with the intended externalId. The result was that the backend subscription remained unassociated with the external ID after login.
This change ensures that a login request is always created when login is called — even if the same externalId is passed again — so that the backend subscription is correctly updated and re-linked to that external ID. The subscription ID associated to the OneSignalID will still be reused.
Scope
Testing
Unit testing
Added a test unit to ensure a Login request is always created
Manual testing
Step to reproduce:
After fix: The subscription has successfully restored the externalID after the second Login.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is