[Android] Finalize checkPermissions

[paintedbicycle]

1. Android permissions check should check if multiple permissions have been granted (READ_CONTACTS, WRITE_CONTACTS and READ_PROFILE I believe are the ones). Right now Android does a basic check, but it would be smart to check multiple permissions in an array and return this to the developer in a smart way. Keep in mind we should try to match iOS functionality but on both platforms do it in a platform expected way.

2. A popup needs to be built to show the permissions request information as per best practices in permissionsAndroid at runtime. See ActivityCompat.shouldShowRequestPermissionRationale() and onRequestPermissionsResult()

An example here: https://www.sitepoint.com/requesting-runtime-permissions-in-android-m-and-n/

3. A developer could perhaps pass in what permissions they need in the app JS (READ, WRITE)

4. A developer should be able to pass in string for what message to show user in the popup

5. We need to handle what happens if you're already asked. A user on Android can say "never ask again" and this should match the iOS functionality where you can only ever ask once (effectively the same as saying "never ask again"). You can check for the on Android and devs should be able to check that and show different UI

[joshuapinter]

Reading Contacts is pretty much good to go, besides maybe allowing for custom text in the modal to show users and handling when a user checks the "don't ask again".

Still need to do Write and maybe the Profile one as well.

[paintedbicycle]

Cool!

I'm working through testing this. The new permissions seems to be mostly working, except right after requesting permissions for the first time I got this crash:

So to summarize:

• 1. The "alreadyRequestedAccessToContacts" check in my app went fine and returned false showing the "request" button
• 2. Then I clicked a button to request access - the pop up appears fine
• 3. I click "Allow" on the pop up
• 4. App crashes

Paul

[joshuapinter]

Just pushed an update. Try with the latest in master.

[paintedbicycle]

Hmm, now my app is back to crashing when interacting with the photo module.

[joshuapinter]

Hmm, and you're using this: Contacts.requestAccessToContactsAsPromise?

[paintedbicycle]

No I haven't moved to the promise version of anything. I need to keep parity between iOS and Android

[paintedbicycle]

It was working fine before that last change you did

[joshuapinter]

Okay, I'm going to remove the promise stuff for now. It's just causing confusion. We'll replace all the callbacks with Promises later, and do it with iOS so your app stays in parity for both platforms.

Give me two seconds to push a new update.

[paintedbicycle]

Ok cool. I think the idea of moving to promises is great. But indeed, it'd have to be the same for both platforms.

[joshuapinter]

Okay, give that a whirl. And make sure to reload the Android Studio and to reload the react native bundle.

All Promises have been removed so there shouldn't be any confusion there and the errors shouldn't reference a Promise anywhere.

[paintedbicycle]

Tested it out - yes, it did indeed fix the activity issue (crashing when interacting with the camera), but there is an API mismatch between iOS and Android.

If I run your current exampleApp in iOS, it crashes. On Android your exampleApp works fine.

I looked into it and realized that your API is different between iOS and Android. Not the method names, but the return values.

In Android: Contacts.requestAccessToContacts( ( errors, canUserAccessContacts ) => {}

In iOS (and what is documented in ReadMe): Contacts.requestAccessToContacts( (canUserAccessContacts ) => {}

So this is causing weirdness in Android for me since I'm using the ReadMe documented format which matches iOS. I think you'll either have to update iOS to include an error return, or review Android to match iOS.

PS I hope app my comments and testing is coming across as working with you to fully test and helping to get this ready, rather than an unwanted endless stream of bug tickets.

[joshuapinter]

Haha. No, definitely not annoying. This is super useful and very much appreciated. You are a collaborator after all so it feels like we're working through these together. :)

Now, on to issue. Yup, you're totally right. I don't know why I used different callback returns for different methods. Makes no sense to me. I'll update Android to match the existing documentation and iOS.

Another nice thing about using Promises later is that it will eliminate this potential confusion and simplify things.

Give me a minute and I'll push a new commit.

[joshuapinter]

Okay, pushed a new update that should fix that. I made the callbacks match the README. The fact that some return ( errors, result ) and some return just ( result ) is mind-boggling dumb. Shame on my past self for doing that. 🙈

Give it a try when you can and confirm it's all good.

[paintedbicycle]

Yep, permissions seem good to go!

[joshuapinter]

👍 Should we close this or leave it open for writing contacts, etc.?

[paintedbicycle]

Let's close it, as Android doesn't currently allow writing. We can increase permissions as part of that ticket

[joshuapinter]

💯 👍