This differential implements native method to get notifications token and uses it in JavaScript. Relevant types are updated as well
Details
Details
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
Diff Detail
- Repository
- rCOMM Comm
- Branch
- marcin/eng-2501
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
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 | Not sure if OnCompleteListener is annotated by FunctionalInterface, but if that's the case, we can simplify the code | |
Comment Actions
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.
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java | ||
|---|---|---|
| 107–117 | Looking at: https://developers.google.com/android/reference/com/google/android/gms/tasks/OnCompleteListener, it probably isn't. | |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotifications.java | ||
|---|---|---|
| 107–117 | It doesn't have to be annotated explicitly, IIRC any single-function interface can be replaced with lambda:  | |
| native/android/app/build.gradle | ||
|---|---|---|
| 668–669 ↗ | (On Diff #21370) | Why these changes were introduced? |
| 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!! |