-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add getters and watchers for oneSignalId and externalId #690
Conversation
8a38e60
to
9fb0d58
Compare
d36f8db
to
d57c900
Compare
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.
Just had some nits, looks good!
Could you also update the Manual testing section in your PR to include what devices and version of Unity you tested on? I think it would be helpful to have it documented
WIP: I will test the PR on devices later
@shepherd-l Thanks for the review! I have made changes addressing all the concerns. Please have a look again. |
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.
LGTM!
Tested new install, login, and logout on an emulated Pixel 4 with Android 12 and physical iPhone 12 with iOS 15.5 built with Unity 2022.3.10f1 with the OneSignal example app
a1e313e
to
b14828f
Compare
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.
Some more nits
b14828f
to
2435b2a
Compare
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.
LGTM!
2435b2a
to
6656aaf
Compare
Add getters and watchers for oneSignalId and externalId
Add getters and watchers for oneSignalId and externalId
Add getters and watchers for oneSignalId and externalId
Add getters and watchers for oneSignalId and externalId
Add getters and watchers for oneSignalId and externalId
Add getters and watchers for oneSignalId and externalId
Add getters and watchers for oneSignalId and externalId
Add getters and watchers for oneSignalId and externalId
Description
One Line Summary
Add getters and observer for onesignalId and externalId.
Details
Motivation
Exposing the onesignal ID and external ID with getters.
Also grant developers the ability to add observers that can be called when there is a change in user state.
Scope
UserState contains onesignalId and externalId (both can be empty strings if not available or not set).
Testing
Unit testing
No unit testing at this time
Manual testing
I have added two testing buttons in the test scene for getting onesignalId and externalId. I have also add observer to test the callback when the ids are changed.
Unity version: 2022.3.10f1
For Android: Emulator Pixel 3a api34
For ios: iPhone15 Simulator
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is