This differential implements function that parses data received in CommNotificationsHandler when application is in the foreground into object that directly maps to what we expect to receive on the JavaScript side to display foreground notification. Relevant broadcast receiver is implemented
as well.
Details
Details
Build the app. Temporarily make parsing method as public static and call it in CommNotificationsHandler on RemoteMessage instance. App must not crash.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsEventEmitter.java | ||
|---|---|---|
| 61 ↗ | (On Diff #20973) | This shouldn't be a part of CommAndroidNotificationsEventEmitter - can we include this in CommAndroidNotificationsForegroundMessageReceiver? |
| 66 ↗ | (On Diff #20973) | We can probably avoid this flag by early returning. |
| 68–78 ↗ | (On Diff #20973) | We can avoid repeatedly creating these arrays by making a static final consts with them. Also, if we want true immutability, we should avoid using arrays and instead use e.g. Set.of. |
| 80–87 ↗ | (On Diff #20973) | Is this code really correct? Intuitively we should say that dataComplete = true when all the obligatoryKeys are present and not when any of them is present. |
| 94–97 ↗ | (On Diff #20973) | Just a nit, but we can avoid calling the same method twice |
| 125 ↗ | (On Diff #20973) | Could you bring back the newline? |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsEventEmitter.java | ||
|---|---|---|
| 61 ↗ | (On Diff #20973) | This method will be moved to CommAndroidNotifications module and exposed as public static.It will be needed in CommAndroidNotifications to implement getInitialNotification method. Alternatively it is possible to create separate file with class RemoteMessageParser that exposes this method as public. We can't include this in CommAndroidNotificationsForegroundMessageReceiver since it will be needed in other places. |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationParser.java | ||
|---|---|---|
| 10 | A good convention is to write const names using uppercase letters | |
| 45 | Please add a newline | |
| native/android/app/src/main/java/app/comm/android/notifications/CommAndroidNotificationsEventEmitter.java | ||
| 61 ↗ | (On Diff #20973) | Making it a separate class was a good idea |