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

Push Notification permissions inconsistency #502

Open
TimofeyBurak opened this issue Nov 14, 2022 · 1 comment
Open

Push Notification permissions inconsistency #502

TimofeyBurak opened this issue Nov 14, 2022 · 1 comment
Labels
c/permissions Related to Permissions component. c/push-notifications Related to Push Notifications component.

Comments

@TimofeyBurak
Copy link
Contributor

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

  1. Request push notification permission using IPermissionsManager.CheckWithRequestAsync<NotificationsPermission>()
  2. 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
Copy link
Contributor Author

Related to #500

@wcoder wcoder added c/push-notifications Related to Push Notifications component. c/permissions Related to Permissions component. labels Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/permissions Related to Permissions component. c/push-notifications Related to Push Notifications component.
Projects
None yet
Development

No branches or pull requests

2 participants