Page MenuHomePhabricator

Implement native method to get device token and use it in JavaScript
ClosedPublic

Authored by marcin on Jan 25 2023, 1:46 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 5, 12:48 AM
Unknown Object (File)
Nov 16 2024, 5:18 AM
Unknown Object (File)
Nov 15 2024, 8:35 PM
Unknown Object (File)
Nov 15 2024, 7:57 PM
Unknown Object (File)
Nov 15 2024, 7:37 PM
Unknown Object (File)
Nov 8 2024, 3:04 AM
Unknown Object (File)
Nov 8 2024, 3:04 AM
Unknown Object (File)
Nov 8 2024, 3:04 AM
Subscribers

Details

Summary

This differential implements native method to get notifications token and uses it in JavaScript. Relevant types are updated as well

Test Plan

Build app but make sure you are not installing fresh. Notifuications token is send from CommNotificationsHandler when app is installed, so it would interfere with this test plan. Make sure all
notifications functionality works. Without ability to get notifications token it won't.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

tomek requested changes to this revision.Jan 26 2023, 3:21 AM

The test plan worries me a bit. What happens when users update the application? Do they need to fresh install the app? Or maybe, we assume that the existing users already received the token and this function will be called just for the new users?

native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java
107–117 ↗(On Diff #21294)

Not sure if OnCompleteListener is annotated by FunctionalInterface, but if that's the case, we can simplify the code

This revision now requires changes to proceed.Jan 26 2023, 3:21 AM
In D6381#191282, @tomek wrote:

The test plan worries me a bit. What happens when users update the application? Do they need to fresh install the app? Or maybe, we assume that the existing users already received the token and this function will be called just for the new users?

We need notifications token in JavaScript memory for notifications to work. Assuming that code I implemented in this differential is incorrect, but the application is build and freshly installed, JavaScript memory will still get the token from service callback, which according to docs: https://firebase.google.com/docs/reference/android/com/google/firebase/messaging/FirebaseMessagingService#onNewToken(java.lang.String), is fired when application is installed or new token is generated. I advised against using fresh install to ensure we that code that populates token in JavaScript works correctly.

Token is delivered via FirebaseMessagingService callback only on fresh install of the app or token change. Therefore many developers tend to save it persistent memory on the device. We do not persist token on the device therefore every time we open the app after killing it the token is missing. But it is also valid approach since we have FirebaseMessaging#getToken() method that according to docs I linked returns the same token. Nevertheless having service callback deliver the token is not redundant, since we will be notified immediately if token changes (we would need some active pooling if rely on getToken() method exclusively).

I probably should have been more specific here. I shouldn't advise against using fresh install of the app but rather not testing with app directly opened after fresh install (since this is when we get token from service, not via getToken method). If we fresh install, open, kill, then open again we can also proceed with testing this differential since device token will be obtained from getToken method.

marcin added inline comments.
native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java
107–117 ↗(On Diff #21294)
native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java
107–117 ↗(On Diff #21294)

It doesn't have to be annotated explicitly, IIRC any single-function interface can be replaced with lambda:

Screenshot 2023-01-26 at 15.04.07.png (308×592 px, 49 KB)

tomek added inline comments.
native/android/app/build.gradle
668–669 ↗(On Diff #21370)

Why these changes were introduced?

This revision is now accepted and ready to land.Jan 26 2023, 9:36 AM
native/android/app/build.gradle
668–669 ↗(On Diff #21370)

In react-native-firebase version that we were using getToken implementation was implemented in the following way:

String senderId = FirebaseApp.getInstance().getOptions().getGcmSenderId();
String token = FirebaseInstanceId
       .getInstance()
       .getToken(senderId, FirebaseMessaging.INSTANCE_ID_SCOPE);

FirebaseInstanceId is deprecated: https://stackoverflow.com/questions/51125169/what-method-should-i-use-now-since-firebaseinstanceid-getinstance-gettoken-i. In order to use newer API I had to bump versions. We already use some deprecated features in this stack, but in this particular case it was straightforward to use newer API.

native/android/app/build.gradle
668–669 ↗(On Diff #21370)

Just FYI, in our codebase we seem to do it this way right now:

const fcmToken = await firebase.messaging().getToken();
native/android/app/build.gradle
668–669 ↗(On Diff #21370)

I was talking about native implementation. firebase.messaging().getToken() calls lines I included above under the hood.

native/android/app/build.gradle
668–669 ↗(On Diff #21370)

Ah sorry!!