Skip to content
This repository has been archived by the owner on Feb 27, 2019. It is now read-only.

ConfiguredPermissions and manual request methods #130

Open
nmccann opened this issue Nov 21, 2015 · 5 comments
Open

ConfiguredPermissions and manual request methods #130

nmccann opened this issue Nov 21, 2015 · 5 comments
Labels

Comments

@nmccann
Copy link

nmccann commented Nov 21, 2015

Hi,

I'm trying to integrate PermissionScope with an existing UI for which the documentation suggests using the various request... methods. When doing this I initially ran into a problem when requesting notifications - my onAuthChange callback wasn't being called. After some digging it appears that this occurred because the configuredPermissions array used in getResultsForConfig was empty.

I was able to resolve this by calling addPermission but that seems odd for two reasons:

  • Adding a permission without a message causes a failed assertion, but including a message isn't useful as my existing UI already has a message.
  • It would be possible to infer what permissions I'm asking for based on the request... method being executed, so it seems like duplication.

At the very least it would be good to make the documentation more explicit about the need for addPermission when using request... - I struggled with this for a while.

That being said, the framework is very nice - thank you for the work you've put into it.

@nickoneill nickoneill added the bug label Nov 21, 2015
@nickoneill
Copy link
Owner

Hey there @nmccann, thanks for the feedback.

The manual request methods are definitely the way to go here but I didn't even think people would use them in conjunction with the onAuthChange callback so behavior there is currently undefined. One should not have to addPermission before using the manual methods! We'll for sure fix this in an upcoming release. Related to #125.

@nmccann
Copy link
Author

nmccann commented Nov 21, 2015

Interesting - the "calling request* methods directly" section of the documentation indicates that onAuthChange should be set.

@nickoneill
Copy link
Owner

Yep, the perils of working on software with a team! Some things are just assumed to happen :) We'll fix it up.

@nmccann
Copy link
Author

nmccann commented Nov 21, 2015

lol well thank you for the quick response :)

@nickoneill nickoneill mentioned this issue Nov 22, 2015
6 tasks
@bre7
Copy link
Collaborator

bre7 commented Nov 28, 2015

request/status methods should include the necessary parameters then: e.g. notificationCategories for Notifications, etc.

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

No branches or pull requests

3 participants