[expo-notification] Prevent from notification being dispatch twice #8437
Conversation
| public void onFailure() { | ||
| Log.w("expo-notifications", "Couldn't get experience from remote message.", null); | ||
| dispatchToNewNotificationModule(remoteMessage); | ||
| dispatchToOldNotificationModule(remoteMessage); | ||
| } |
There was a problem hiding this comment.
I'm not sure what should we do here 🤔
There was a problem hiding this comment.
Me too 🤔 Let's dispatch only to the old notifications module. 🤷 If no useNewNotificationsAPI falls back to using the old module, no manifest should fall back the same.
|
Native Component List for this branch is ready |
| public void onFailure() { | ||
| Log.w("expo-notifications", "Couldn't get experience from remote message.", null); | ||
| dispatchToNewNotificationModule(remoteMessage); | ||
| dispatchToOldNotificationModule(remoteMessage); | ||
| } |
There was a problem hiding this comment.
Me too 🤔 Let's dispatch only to the old notifications module. 🤷 If no useNewNotificationsAPI falls back to using the old module, no manifest should fall back the same.
| @@ -31,7 +38,40 @@ public void onMessageReceived(RemoteMessage remoteMessage) { | |||
| return; | |||
| } | |||
|
|
|||
| ExponentDB.experienceIdToExperience(remoteMessage.getData().get("experienceId"), new ExponentDB.ExperienceResultListener() { | |||
There was a problem hiding this comment.
Does searching for it asynchronously work ok even when the app is terminated? What I'd be afraid of is starting new threads in a time-limited call-to-service. I believe Android catches it and doesn't terminate the app while the asynchronous callback is being executed, but who knows.
There was a problem hiding this comment.
Just to be on the safe side — do you think this work ok in standalone apps too? Should we add a note somewhere to ensure test this there?
There was a problem hiding this comment.
So, I've tested it and it seems to work even if the app was killed. But maybe, we should switch to sync the call 🤔
I believe that will work in standalone apps. However, we should test it :D
android/expoview/src/main/java/host/exp/exponent/fcm/ExpoFcmMessagingService.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/fcm/ExpoFcmMessagingService.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/fcm/ExpoFcmMessagingService.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/fcm/ExpoFcmMessagingService.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/fcm/ExpoFcmMessagingService.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/fcm/ExpoFcmMessagingService.java
Outdated
Show resolved
Hide resolved
android/expoview/src/main/java/host/exp/exponent/fcm/ExpoFcmMessagingService.java
Outdated
Show resolved
Hide resolved
lukmccall
deleted the
@lukmccall/expo-notifications/fix-double-notification-dispatch
branch
Why
Parts of #8135.
How
Now, the user can specify which notification will be used on Android.
The scheme update: https://github.com/expo/universe/pull/5048
Test Plan