Also, it seems like our implementation of checking push-notification permissions might have an issue: we take into account specific settings, not only authorization status
https://github.com/Softeq/XToolkit.WhiteLabel/blob/master/Softeq.XToolkit.Permissions.iOS/PermissionsService.cs#L54-L68
One thing is that settings are hardcoded, which is not good - again, seems like PushNotificationService should decide which settings are actually required. The second is that the logic itself might be a bit incorrect - if user grants permission for alert+sound, for instance, and then disables sounds in settings, then our implementation would say that notification permissions are denied, although it is not true and notifications would still be received and shown to user.
Steps to Reproduce
Request push notification permission using IPermissionsManager.CheckWithRequestAsync<NotificationsPermission>()
Use PushNotificationService to handle push notifications
Expected Behavior
PushNotificationService has all required options authorized
Actual Behavior
Only Alert and Sound options are authorized always
Possible Fix
I would suggest to remove push-notification permissions from the Permissions project and implement it in the PushNotifications project (as a part of IPushNotificationService, or as a separate service?), and also rework check permissions logic.
Basic Information
Version with issue: XToolkit.Permissions 2.1.0, XToolkit.PushNotifications 1.0.1
TimofeyBurak
commented
1 year ago
Description
Push-notifications permissions are requested with hardcoded options, even though the options should probably be defined by PushNotificationService functionality: https://github.com/Softeq/XToolkit.WhiteLabel/blob/master/Softeq.XToolkit.Permissions.iOS/PermissionsService.cs#L70-L78
Also, it seems like our implementation of checking push-notification permissions might have an issue: we take into account specific settings, not only authorization status https://github.com/Softeq/XToolkit.WhiteLabel/blob/master/Softeq.XToolkit.Permissions.iOS/PermissionsService.cs#L54-L68 One thing is that settings are hardcoded, which is not good - again, seems like PushNotificationService should decide which settings are actually required. The second is that the logic itself might be a bit incorrect - if user grants permission for alert+sound, for instance, and then disables sounds in settings, then our implementation would say that notification permissions are denied, although it is not true and notifications would still be received and shown to user.
Steps to Reproduce
IPermissionsManager.CheckWithRequestAsync<NotificationsPermission>()PushNotificationServiceto handle push notificationsExpected Behavior
PushNotificationServicehas all required options authorizedActual Behavior
Only
AlertandSoundoptions are authorized alwaysPossible Fix
I would suggest to remove push-notification permissions from the Permissions project and implement it in the PushNotifications project (as a part of
IPushNotificationService, or as a separate service?), and also rework check permissions logic.Basic Information