Skip to content
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

🚧 OneSignal WebSDK v16 Released (Beta 8)! 🚧 -- seeking feedback #980

Closed
rgomezp opened this issue Jan 10, 2023 · 17 comments
Closed

🚧 OneSignal WebSDK v16 Released (Beta 8)! 🚧 -- seeking feedback #980

rgomezp opened this issue Jan 10, 2023 · 17 comments

Comments

@rgomezp
Copy link
Contributor

rgomezp commented Jan 10, 2023

What's on your mind?

In this major version beta release for the OneSignal SDK, we are making a significant shift from a device-centered model to a user-centered model. A user-centered model allows for more powerful omni-channel integrations within the OneSignal platform.

For information please see the migration guide.

🚧 Beta 2 release. Please test thoroughly prior to production use. 🚧

Please post any feedback related to the beta under this issue.

@rgomezp rgomezp pinned this issue Jan 10, 2023
@netizen-ais
Copy link

I've reviewing this, but I couldn't find some key functions in those docs.
For example: Does the new user.login(external_user_id) method allow the old secured method with a hash as a second parameter?

@rgomezp
Copy link
Contributor Author

rgomezp commented Jan 27, 2023

@netizen-ais ,
Great question! Currently, no. But this feature is coming soon.

However, instead it will take a JWT token.

Did you have any other questions?

@rgomezp rgomezp changed the title 🚧 OneSignal WebSDK v16 Released (alpha)! 🚧 -- seeking feedback 🚧 OneSignal WebSDK v16 Released (Beta)! 🚧 -- seeking feedback Feb 6, 2023
@rgomezp rgomezp changed the title 🚧 OneSignal WebSDK v16 Released (Beta)! 🚧 -- seeking feedback 🚧 OneSignal WebSDK v16 Released (Beta 2)! 🚧 -- seeking feedback Feb 28, 2023
@lmeurs
Copy link

lmeurs commented Jun 23, 2023

Hi all, thanks for v11, the new user model is exactly what we need for our client. We experienced some issues, your support desk refered to this Github issue.

We are new to OneSignal, so we had to start from scratch. This did not work out for v11 due to the lack of updated documentation, so we decided to build a proof of concept based on v9. We wanted to explore all possibilities, so implemented all events and methods. This worked like a charm, then we tried migrating to v11 and ran into a few issues.

  1. At https://noti.bloen.nl/ is a working example using our v11 script from https://noti.bloen.nl/js/notifications.js. Method OneSignal.Notifications.getPermissionStatus() seemed to no longer work (saw this being changed in a previous release), so we implemented testing property OneSignal.Notifications.permissionNative. This value seems not updated right away, we had to implement a small timeout to make it work, but there must be a better way! 😀 Could not find a way to use a promise of callback, did we miss something? The same script without timeout can be found at https://noti.bloen.nl/without-timeout/ (check the console for logging and clear site data and reset permissions in case no error occurs)

  2. Also in the final part of the script we call OneSignal.login() which results in error: “Login: Error while identifying/upserting user: identifyUser failed: no identity found”, which did not occur in the v9 version calling OneSignal.setExternalUserId().

  3. Users appear to be able to alter their external ID's client side. I assume using JWT tokens mentioned by @rgomezp can help prevent this, is this documented somewhere?

All help is well appreciated! 😀

BTW: When do you expect a stable v11 version? The 15th of March Marc said “we expect it within weeks rather than months”. 😀

@rgomezp
Copy link
Contributor Author

rgomezp commented Jun 30, 2023

  1. At https://noti.bloen.nl/ is a working example using our v11 script from https://noti.bloen.nl/js/notifications.js. Method OneSignal.Notifications.getPermissionStatus() seemed to no longer work (saw this being changed in a previous release), so we implemented testing property OneSignal.Notifications.permissionNative. This value seems not updated right away, we had to implement a small timeout to make it work, but there must be a better way! 😀 Could not find a way to use a promise of callback, did we miss something? The same script without timeout can be found at https://noti.bloen.nl/without-timeout/ (check the console for logging and clear site data and reset permissions in case no error occurs)

The promise you're looking for is actually the init function:

    OneSignal.init({
      appId: NOTIFICATION_ONESIGNAL_APP_ID,
      safari_web_id: NOTIFICATION_ONESIGNAL_SAFARI_WEB_ID,
      notifyButton: {
        enable: !true,
      },
    }).then(() => {
      console.log("Opted in:", OneSignal.User.PushSubscription.optedIn);
      console.log("Push notifications enabled: " + OneSignal.User.PushSubscription.optedIn);
    })
  1. Also in the final part of the script we call OneSignal.login() which results in error: “Login: Error while identifying/upserting user: identifyUser failed: no identity found”, which did not occur in the v9 version calling OneSignal.setExternalUserId().

Same goes for logging in. Make sure the init function resolves first.

OneSignal.init({
      appId: NOTIFICATION_ONESIGNAL_APP_ID,
      safari_web_id: NOTIFICATION_ONESIGNAL_SAFARI_WEB_ID,
      notifyButton: {
        enable: !true,
      },
    }).then(() => {
      OneSignal.login("laurens");
    })
  1. Users appear to be able to alter their external ID's client side. I assume using JWT tokens mentioned by @rgomezp can help prevent this, is this documented somewhere?

JWT tokens (identity verification) are not currently supported. You can see a list of limitations here.

BTW: When do you expect a stable v11 version? The 15th of March Marc said “we expect it within weeks rather than months”. 😀

Could you clarify what you're basing the version numbers on? Did you mean v16?

@lmeurs
Copy link

lmeurs commented Jul 1, 2023

Hi @rgomezp, thank you for your reply and taking the time looking into our code!

I updated our code accordingly, waiting for the init to resolve fixed the first issue, now OneSignal.Notifications.permissionNative is updated when expected. :-)

The second issue still occurs, I tested this on Chrome 114 after resetting site's permissions and clearing site's data. Subscribing seems successful, but the error still occurs: Login: Error while identifying/upserting user: identifyUser failed: no identity found. At this time the OneSignal.User.PushSubscription.id and OneSignal.User.PushSubscription.token also are empty. The new user is present in the dashboard, but without the external user ID.

image

After waiting say 10 seconds, other errors occur: Database REMOVE Error: DOMException: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing.

When just refreshing the page, previously mentioned errors no longer occur, OneSignal.User.PushSubscription.id and OneSignal.User.PushSubscription.token have values and the external user ID has been added to the user. NB: on the first refresh an additional 409 error occurs for the PATCH request of https://onesignal.com/api/v1/apps/800f295a-a6b2-49c9-9fb1-2555e1e8df91/users/by/onesignal_id/44404c20-6ada-4d48-9849-a6b5ce48a870/identity.

About JWT, I misinterpreted your comment and now understand JWT will eventually be implemented instead of using a separate hash.

Version numbers confuse me. :-) With v11 I refered to versions as communicated at https://documentation.onesignal.com/v11.0/docs.

@lmeurs
Copy link

lmeurs commented Jul 9, 2023

Hi @rgomezp,

Login issues

Error Login: Error while identifying/upserting user: identifyUser failed: no identity found keeps occuring:

image

Steps to reproduce:

  1. Clear all site's data
  2. Load page with OneSignal and permissionChange event listener
  3. When permission changes to granted, call OneSignal.login(MY_ID).then(...)

The promise's success callback is called. Now the error occurs, both OneSignal.User.PushSubscription.id and OneSignal.User.PushSubscription.token are empty. After a few seconds the latter gets a value afterall, the first not. Also, in the backend a user has been created, but with no external ID.

When reloading the page, OneSignal.login(MY_ID).then(...) is called for the 2nd time: this time not from a permissionChange event, but on init. Now the error no longer occurs and both OneSignal.User.PushSubscription.id and OneSignal.User.PushSubscription.token have values. In the backend the user has an external ID.

Related question: does OneSignal.login(MY_ID).then(...) needs to be called on each page load, or just once?

Multiple users using the same browser

Our goal is to only provide web push notifications to users with accounts on our website, is this possible on shared devices? I tried using OneSignal.logout(), but the same subscription seems to be used for multiple users.

I read in the release notes (https://github.com/OneSignal/OneSignal-Website-SDK/blob/user-model/v1/MIGRATION_GUIDE.md#version-16-alpha) Switching between users via login() and logout() is unsafe. Please stick to single user testing. Is this related? If so, is this going to be fixed?

IDBDatabase issues (false alarm!)

NB: the mentioned errors Database REMOVE Error: DOMException: Failed to execute 'transaction' on 'IDBDatabase': The database connection is closing (see screenshot below) always only occur once after clearing the site's data, with or without refreshing the page. Probably caused (if possible) by open promises in the background?

image

Duplicate user ID's

When testing I delete all OneSignal subscriptions/users from the Users dashboard. When running the same test, I encounter a 409: One or more Aliases claimed by another User error. Is it to be expected that a match is found with a deleted user?

All help is appreciated, thank you so much!

@jkasten2
Copy link
Member

@lmeurs Thanks for all the details here!

Quick update on the "Error Login: Error while identifying/upserting user: identifyUser failed: no identity found keeps occuring:" issue. We have reproduced the issue and are looking to ship a fix for this in the next beta.

You should have to only call OneSignal.login(MY_ID) once, however it is also safe to call it on each page load since this function checks if the User is the same.

We will follow up on your other comments soon

@lmeurs
Copy link

lmeurs commented Jul 21, 2023

Hi @jkasten2, thank you for your reply. I am looking forward to receiving your reactions!

Login issues

In the meanwhile I tested the following:

  1. Clearing site data and resetted permissions
  2. Initialized OneSignal
  3. Requested and granted permission (see 1st custom log message)
  4. On the permissionChange event the user we call OneSignal.login(NOTIFICATION_JWT)
  5. When the login promise resolves I tested OneSignal.User.PushSubscription.id and OneSignal.User.PushSubscription.token, which where undefined and null respectively (see 2nd custom log message)

The console shows the "no identity found" error and I continued testing from the console:

  • OneSignal.User.PushSubscription.token got an expected value after a short delay
  • OneSignal.User.PushSubscription.id staid undefined, even after calling OneSignal.login(MY_USERNAME) multiple times.

image

In the background multiple user and subscription API calls were done, which did contain the exepected external ID, but OneSignal.User.PushSubscription.id never gets updated.

Only after reloading the page OneSignal.User.PushSubscription.id got an expected value right away, without calling OneSignal.login(MY_USERNAME) again.

Device/browser data per subscription

Currently hardly any browser data is provided per subscription, only OS (Linux for Android) and the browser version.

We would like to point our users at subscriptions at other devices/browsers, to clarify the way web push is device/browser depending. At https://cdn.onesignal.com/sdks/web/v16/OneSignalSDK.page.es6.js I saw some advanced device/browser sniffing being done, can the result in some form be shared per subscription?

@lmeurs
Copy link

lmeurs commented Jul 21, 2023

@jkasten2, @rgomezp: I have a new POC which I would like to share with you personally. In case of any interest, please contact me at lmeurs@gmail.com.

@jkasten2
Copy link
Member

@lmeurs Quick update, we have addressed some of the issue were you seeing in a v16.beta6 release today.
https://github.com/OneSignal/OneSignal-Website-SDK/releases/tag/160000.beta6

  • This fixes the OneSignal.login error “Login: Error while identifying/upserting user: identifyUser failed: no identity found” you reported.

We will be addressing your other issues soon.

@lmeurs
Copy link

lmeurs commented Jul 30, 2023

Hi @jkasten2: we can confirm the no identity found issue no longer occurs. Thank you for fixing this!

Unfortunately we ran into another issue: changing notification permissions on Chrome desktop from the Chrome UI (click the padlock icon in the address bar and alter permissions) does not always fire OneSignal.Notifications.addEventListener('permissionChange', function(permission) { /*...*/ }).

  • After granting/denying the very first time, not reloading the page and again granting/denying/resetting, the event gets fired just fine.
  • But when granted and reloading the page, the event does not get fired when again granting/denying/resetting.

NB: native notification permission change events (implemented like https://stackoverflow.com/a/59111264/328272) do get fired correctly.

This beta version still is too buggy, progression is slower than expected (in March a stable version was expected withing weeks) and unfortunately you did not accept our invitiation to get in touch and look into these issues together. We really prefered OneSignal, but we are going to look into other notification providers with an advanced user model + API like PushPad, Notificare or WonderPush.

@jkasten2 jkasten2 changed the title 🚧 OneSignal WebSDK v16 Released (Beta 2)! 🚧 -- seeking feedback 🚧 OneSignal WebSDK v16 Released (Beta 7)! 🚧 -- seeking feedback Aug 1, 2023
@jkasten2
Copy link
Member

jkasten2 commented Aug 1, 2023

@lmeurs Sorry for our delay here, some of the delay was due to getting more feedback. However we are now pushing through our finial testing and fixes to get this major release over the finish line. The v15 is currently the stable release and can be used today, however once v16 is released out of beta v15 won't get major feature updates.

We encourage you to reach out to support@onesignal.com and / or though the chat bubble from the onesignal dashboard so we can to assist you directly and with faster responses.

@lmeurs
Copy link

lmeurs commented Aug 2, 2023

Great to see beta 7 (https://github.com/OneSignal/OneSignal-Website-SDK/releases/tag/160000.beta7) having been released in the meanwhile, which seems to fix one of our issues (have not tested this yet).

Issue #1071 from 151604 seems similar to an issue we reported here (#980 (comment)), but this fix seems to not have made it to the beta. Do you think these issues are related?

A new finding is that new notification subscriptions are created each time when granting permissions: when granting for the first time, but also each time when granting again after denying. I guess this was not the case one or two weeks ago. Not sure what actually wanted/expected behaviour is?

@jkasten2
Copy link
Member

jkasten2 commented Aug 2, 2023

Issue #1071 from 151604 seems similar to an issue we reported here (#980 (comment)), but this fix seems to not have made it to the beta. Do you think these issues are related?

Yes, we have a matching PR #1072 that will go out in beta8 soon.

A new finding is that new notification subscriptions are created each time when granting permissions: when granting for the first time, but also each time when granting again after denying. I guess this was not the case one or two weeks ago. Not sure what actually wanted/expected behaviour is?

Looking back at my testing results I do see this happening back on July 20th (which would be beta5), but it could have been happening longer. Its possible however conditions have changed where this is happening more often now, so thank you for bring it up! We will also see if we get a fix for this in the next beta.

@jkasten2
Copy link
Member

jkasten2 commented Aug 5, 2023

@lmeurs v16 beta8 is now available with a fix for the permission toggle issues you noted (both the stop firing issue and new subscriptions being created), as well as a number of other fixes.

  • You may have to clear you browser cache if you are not seeing the changes, I believe the browser cache is set to 3 days.

@jkasten2 jkasten2 changed the title 🚧 OneSignal WebSDK v16 Released (Beta 7)! 🚧 -- seeking feedback 🚧 OneSignal WebSDK v16 Released (Beta 8)! 🚧 -- seeking feedback Aug 6, 2023
@jkasten2
Copy link
Member

v16 is now out of beta and has been released! See the full release notes:
https://github.com/OneSignal/OneSignal-Website-SDK/releases/tag/160000

This means no breaking changes will be made in v16. If you find a bug or have a feature require please create a new issue.

@jkasten2 jkasten2 unpinned this issue Aug 15, 2023
@netizen-ais
Copy link

@netizen-ais , Great question! Currently, no. But this feature is coming soon.

However, instead it will take a JWT token.

Is this already implemented? As I see in the web frontend SDK documentation it seems that a user could impersonate another and receive their notifications

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants