-
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 onesignalid is local when identity verification off #2242
base: identity_verification_beta
Are you sure you want to change the base?
Fix onesignalid is local when identity verification off #2242
Conversation
This is tagged WIP, is it still in WIP? |
I just removed the tag and it is now ready for review. |
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.
Can you test a new install using this branch with identity verification turned off?
I am getting an immediate 400 on the user creation request - the push subscription is missing. May be reproducible or just me.
… identity verification is off Also removed backend operation suppression; fixed test units using identity operation executor
d07bbef
to
b968181
Compare
Thanks for pointing out. I just pushed a change that is tested for a new install and the steps to reproduce. |
I think these changes require more testing and I am not sure the changes are the right solution - I don't think they get to the root issue. Using the latest changes, If I run the app, and then kill it and reopen to drive a new cold start, I am seeing the same Look at Additionally, we should consider how to get devices out of this stuck state on upgrade. |
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.
Put in another comment
Description
One Line Summary
Fix a bug that a local onesignal ID is used for logging in when identity verification is turned off.
Details
Motivation
The beta for identity verification has a bug that incorrectly uses a local OneSignal ID in a login request. This may result in a 400 error and cause the login to fail. Some users who unknowingly adopt the beta version may experience a change in behavior. This PR aims to resolve the bug in the beta and ensure stability for the identity verification feature, whether it is enabled or disabled.
Scope
Added a check for the useIdentityVerification status before composing a login request.
Testing
Manual testing
Steps to reproduce the bug with a 400 error:
After the fix, we no longer receive the 400 error and the logout is successful.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is