search

shinyorg / shiny

.NET Framework for Backgrounding & Device Hardware Services (iOS, Android, & Catalyst)
https://shinylib.net
MIT License
1.43k stars 227 forks source link

[Bug]: Shiny Unregister() #1487

Closed BrandanN21 closed 2 months ago

BrandanN21 commented 2 months ago

Component/Nuget

Push - Azure Notification Hub (Shiny.Push.AzureNotificationHubs)

What operating system(s) are effected?

Version(s) of Operation Systems

Using a wide variety of Android OS versions on multiple devices. Keep in mind this only impacts android FMCv1

Hosting Model

Steps To Reproduce

  1. When turning off push notifications for a user using the .Unregister() method it unregisters ALL users, not just the one that turned it off. ie no options to pass parameters to .Unregister()
  2. Using Azure Notification Hub
  3. Using Nuget feeds below: `

    `

Expected Behavior

See Shiny Documentation: https://shinylib.net/server/push/#unregister-all-users

Given the documentation out there we should be able to specify the platform and the device token to unregister the users device from push notifications.

Actual Behavior

There are no optional parameters for the .Unregister() method in the source code of Shiny. When viewing this in the git repository as well as consuming it via the Nuget Feed there is no way to specify what device or user to unregister. This meant that when we were using this method if one person toggled off their notifications it would unregister all the tokens and no one would receive any notifications.

We were able to resolve / prove this theory by removing the use of the Shiny Unregister method in our code and when user's would turn on and off push notification's it still worked for all of our other users.

The Shiny documentation is not very clear at what this Unregister method is actually doing, and based on the docs we should be able to specify what userId, platform and/or device token we want to unregister but in the Shiny source code that is not an option.

It's worth noting that we are using Azure Notification Hubs and migrating our notifications to FCMv1 in case that provides more context around what this Unregister method is doing.

Since our work around was being able to remove the Unreigster() method and that seems to pose no issues on our end and fix the problem we are curious what exactly this Unregister() method is doing, why we can't specify the device token / platform, and what are the risks if we choose to not use the Unregister() method since it is causing conflicts within the notification process?

Thanks!!

Exception or Log output

No response

Code Sample

await this._pushManager.UnRegister();

Code of Conduct

aritchie commented 2 months ago

Server is not the client library. You are getting the docs completely cross wired.

BrandanN21 commented 2 months ago

@aritchie Would this then be the correct docs for client? https://shinylib.net/client/push/

If that is the case then there still isn't anything describing what .UnRegister() is actually doing and if it's necessary for us to do? The only references to the UnRegister method in the client/push documentation is below:

public async Task OnUnRegistered(string token) { // fires when IPushManager.UnRegister is called // or on startup when permissions are denied to push }

If it's not necessary for us to do anything with UnRegister() that seems to line up with what we found, but given we have the option to use the OnUnRegistered() method above, it would be great to know exactly how that works / what it is doing.

Thanks!

aritchie commented 2 months ago

UnRegister tells the platform to remove the native push token. If the push provider (in this case azure notification hubs) also has an UnRegister, it will tell the provider to also kill whatever token it is tracking. In the case of ANH, it is telling it to delete the installation ID

The OnUnRegistered as part of the IPushDelegate is called as an event should you choose to do something additional with the token. Maybe call your server to say that the token is going away. If I didn't expose this event, it would simply happen in the background without you even knowing.

If the docs aren't answering your questions, go to the code. It is all out in the open & free. Please note, this is not a product and issues are not here for support cases.