-
Notifications
You must be signed in to change notification settings - Fork 199
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
Update Push Subscription/Permission Getters to Async #977
Conversation
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.
We also should change the getId and getToken to return null
when it is not available, right now it is still taking what Android gives, which is ""
.
Also same for the push subscription observer, when you log the event and something is not available, it should log {"optedIn": "false", "id": null, token: null}
.
Thank you so much for all the feedback, @nan-li! Question about iOS:
Currently the push subscription listener on iOS gives a JSONRepresentation of |
You'll have to extract them with state.id, state.token etc just like you did for the user observer. |
Instead of returning an object with a boolean value
- Create new async methods - Mark old getters as deprecated
- Ensure ids are nullable and remove undefined - Update push optedIn getter to return boolean in callback
…er logging - Will log null instead of 'nil'
fcd8b2a
to
4304794
Compare
Rebased on |
* Instead of making dictionaries just to pass strings over the bridge, pass the strings directly, when appropriate. * Do this by creating helper methods in iOS and Android called `callbackSuccessString` and `successCallbackString`.
* To explain why they are still there
* Move Android requestPermission resolving into the bridge to improve readability of the method.
Pushed some changes, these are my testing scenarios:
OneSignal.initialize("YOUR_APP_ID")
const perm = await OneSignal.Notifications.getPermissionAsync() // false
const optedIn = await OneSignal.User.pushSubscription.getOptedInAsync() // false
const token = await OneSignal.User.pushSubscription.getTokenAsync() // null
const id = await OneSignal.User.pushSubscription.getIdAsync() // null
// User grants permission and permission observer fires
// Once the observer fires, Immediately call these methods
// New async getter returns `true` correctly
const optedIn = await OneSignal.User.pushSubscription.getOptedInAsync(); // true
// Old getter returns `false` still as it has not been updated yet
const optedIn = OneSignal.User.pushSubscription.optedIn; // still false
OneSignal.initialize("YOUR_APP_ID")
const perm = await OneSignal.Notifications.getPermissionAsync() // true
const optedIn = await OneSignal.User.pushSubscription.getOptedInAsync() // true
const token = await OneSignal.User.pushSubscription.getTokenAsync() // some real value
const id = await OneSignal.User.pushSubscription.getIdAsync() // some real value
const perm = OneSignal.Notifications.hasPermission(); // false
const optedIn OneSignal.User.pushSubscription.optedIn; // false
const token = OneSignal.User.pushSubscription.token; // undefined
const id = OneSignal.User.pushSubscription.id; // undefined
// Called several seconds after SDK initialization returns the correct data
const perm = OneSignal.Notifications.hasPermission(); // true
const optedIn OneSignal.User.pushSubscription.optedIn; // true
const token = OneSignal.User.pushSubscription.token; // some real value
const id = OneSignal.User.pushSubscription.id; // some real value
interface PushSubscriptionState {
id ?: string;
token ?: string;
optedIn : boolean;
} However, we had actually been passing along the empty string // Observer now firing when subscription ID is received has null properties when logged
{
"current": { "id": "abcd-abcd-abcd-abcd", "token": null, "optedIn":false},
"previous": {"id": null, "token": null, "optedIn": false}
} |
Description
One Line Summary
Update Push Subscription/Permission Getters to Async
Details
Motivation
We received reports of listeners not behaving as expected; certain getters were returning null due to the value not being available until returned from the native bridge. Updating getter methods to async will address this issue.
Scope
New async methods were added while also marking old methods as deprecated. Also updated
requestPermission
to account for changes made to these getters.Testing
Manual testing
Tested running and building app on Android 14 emulator and iOS 17.2 Emulator, to ensure new methods work as expected.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is