invertase / react-native-firebase

🔥 A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.63k stars 2.2k forks source link

[📚] Support for Android 13 context permissions / or switch to react-native-permissions for all permissions work #6283

Open mikehardy opened 2 years ago

mikehardy commented 2 years ago

Discussed in https://github.com/invertase/react-native-firebase/discussions/6282

Originally posted by **MicaelRodrigues** May 31, 2022 With the upcoming [release](https://developer.android.com/about/versions/13/overview#timeline - late July) of `Android Tiramisu` apps will need to request permission to receive push notifications: - https://developer.android.com/about/versions/13/changes/notification-permission It will start working similarly to IOS, that shows a system dialog for user choice. So, as a suggestion `messaging().requestPermission` should open that system dialog if the user hasn't granted/rejected permssions yet.
fellipe-ribeiro commented 2 years ago

Any news for this?

mikehardy commented 2 years ago

@fellipe-ribeiro nope - it's open source, no need to ask for news, what you see is what you get --> https://medium.com/hackernoon/i-thought-i-understood-open-source-i-was-wrong-cf54999c097b

In the meantime, feel free to post a PR if this is important to your business, I'm traveling right now but have just enough time to collaborate on getting PRs in. I won't be able to do anything related to it for a month at least I think, and react-native-permissions should be considered your workaround until then

fellipe-ribeiro commented 2 years ago

Fixed with react-native-permissions. Thanks

github-actions[bot] commented 1 year ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

ghost commented 1 year ago

react-native-permissions helps solve the issue

mikehardy commented 1 year ago

Given the quality of react-native-permissions and that everyone (including myself!) has already had to handle this using it with android 13 out for a while... I think this actually calls for removing permissions from this library at the next breaking change

github-actions[bot] commented 1 year ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.

AlixH commented 1 year ago

react-native-permissions helps solve the issue

How ?

mikehardy commented 1 year ago

react-native-permissions helps solve the issue

How ?

It helps solve the issue because it is a complete, well-maintained package that supports the ability to interrogate current permission grant status on all useful permissions, and/or request permission from the user if the permission is not granted but you need it.

How could that not help solve a permission-related issue 🤔 ? It appears to be the canonical solution, in fact.

AlixH commented 1 year ago

react-native-permissions helps solve the issue

How ?

It helps solve the issue because it is a complete, well-maintained package that supports the ability to interrogate current permission grant status on all useful permissions, and/or request permission from the user if the permission is not granted but you need it.

How could that not help solve a permission-related issue 🤔 ? It appears to be the canonical solution, in fact.

I understand, but then it created a mix of library usage for notifications only. For iOS we do await messaging().requestPermission() and for Android we do await requestNotifications(['alert']). Is that even correct to have to following code ?

    if (authorizationStatus !== messaging.AuthorizationStatus.PROVISIONAL &&
        authorizationStatus !== messaging.AuthorizationStatus.AUTHORIZED ) {
      try {
        // requesting permission from user is required for iOS
        authorizationStatus = await messaging().requestPermission();
        if (Platform.OS === PLATFORM.ANDROID) {
          await requestNotifications(['alert']);
        }
       } catch ( error ) {
        console.error(error);
      }
    }
    if (authorizationStatus === messaging.AuthorizationStatus.AUTHORIZED || messaging.AuthorizationStatus.PROVISIONAL) {
      try {
        // Retrieve mobile token
        const fcmToken = await messaging().getToken();
        if ( fcmToken ) {
          this.token = fcmToken;
        }
      } catch ( error ) {
        console.error(error);
      }
    }
mikehardy commented 1 year ago

My best recommendation is to use react-native-permissions for all permission related activity in all places, in all code. Then there is no mix. It is what I do in fact, in my work projects and what I think most people do?

mikehardy commented 4 months ago

Re-opening this but note that it is to track what I consider to be the real work:

russellwheatley commented 1 month ago

Agree with @mikehardy , RNFB ought to get out of the business of permissions when it is nowhere near comprehensive enough.

We have a potential breaking change incoming in the following year so I'm marking this as something to remove.

github-actions[bot] commented 5 days ago

Hello 👋, to help manage issues we automatically close stale issues.

This issue has been automatically marked as stale because it has not had activity for quite some time.Has this issue been fixed, or does it still require attention?

This issue will be closed in 15 days if no further activity occurs.

Thank you for your contributions.