Implement handling of Android actions in background #1074

dluksza · 2018-05-11T22:13:15Z

There are some cases when local notification action should be handled in
background eg. snoozing the reminder. In case of it launching app UI is
not necessary and would be confusing for the end user.

Therefore there should be a way to handle local notification action in
background.

For this reason new property 'runInBackground' was added to the
AndroidAction class and TypeScript type.

Also new broadcast receiver and service were implemented to handle
properly background actions.

In order to run particular action in background user need to set its
'runInBackground' property to 'true', eg:

  ...
  .android.addAction(new firebase.notifications.Android.Action("snooze",
      "ic_snooze", "Snooze").setRunInBackground(true))

Then create a background asynchronous handler:

  const handleAsyncTask = async (notificationOpen: NotifficationOpen) => {
    if (notificationOpen && notificationOpen.notification) {
      const action = notificationOpen.action;
      const notificationId = notificationOpen.notification.notificationId;
      if (action === "snooze") {
        console.log("Reschedule notification for later time", notificationId);
      } else {
        console.log("unsupported action", action);
      }
      // hide the notification
      firebase.notifications().removeDeliveredNotification(notificationId);
    }
  }

Next hander should be registered to an headless handler:

  AppRegistry.registerHeadlessTask('RNFirebaseBackgroundNotification', () => handleAsyncTask);

Finally AndroidManifest.xml file must be modified, to include receiver
and service definition:

  <receiver
      android:name="io.invertase.firebase.notifications.RNFirebaseBackgroundNotificationReceiver"
      android:exported="true">
    <intent-filter>
      <action android:name="io.invertase.firebase.notifications.BackgroundAction"/>
    </intent-filter>
  </receiver>
  <service android:name="io.invertase.firebase.notifications.RNFirebaseBackgroundNotificationsService"/>

Now when ever 'Snooze' action is pressed it will launch
'handleAsyncTask' function in the background and reschedule the
notification for the later time.

chrisbianca

@dluksza Thanks so much for submitting this PR, it's greatly appreciated. I've added a few comments with some suggested renames and a couple of cases which I think need to be considered before we can get this merged.

chrisbianca · 2018-05-15T09:28:45Z

.../main/java/io/invertase/firebase/notifications/RNFirebaseBackgroundNotificationReceiver.java

+    }
+  }
+
+  private boolean isAppOnForeground(Context context) {


Can you change this to use the method from the io.invertase.firebase.Utils class rather than duplicating?

chrisbianca · 2018-05-15T09:31:34Z

.../main/java/io/invertase/firebase/notifications/RNFirebaseBackgroundNotificationReceiver.java

+
+import java.util.List;
+
+public class RNFirebaseBackgroundNotificationReceiver extends BroadcastReceiver {


Can we rename this to RNFirebaseBackgroundNotificationActionReceiver to make it clear that it handles actions and not notifications themselves

chrisbianca · 2018-05-15T09:32:06Z

.../main/java/io/invertase/firebase/notifications/RNFirebaseBackgroundNotificationReceiver.java

+    serviceIntent.putExtras(intent.getExtras());
+    context.startService(serviceIntent);
+
+    if (!isAppOnForeground((context))) {


What happens if the app is in the foreground when the Snooze action is hit?
I think it will currently just get lost - instead we need to make sure that the action gets passed through into the standard onNotificationOpened handler so that it can be handled normally.

I need to test this. I'm not sure if going through standard onNotificationOpened way is the best solution, since this would require code duplication (the same handler function should be called from two places). Would rather handle everything in background.

There is a small drawback for the always background approach. When background action handler is changing the UI state this should be achieved using some kind of event or listener approach.

I believe it would be better to handle things separately in the foreground and background as it gives more flexibility to the developer if they're like their app to act differently. You've already touched on UI state which might need to be updated in the foreground.

The functionality that's shared can easily be wrapped inside a method which can be called from both the background and foreground handler to prevent code duplication.

chrisbianca · 2018-05-15T09:32:47Z

.../main/java/io/invertase/firebase/notifications/RNFirebaseBackgroundNotificationsService.java

+
+import io.invertase.firebase.messaging.MessagingSerializer;
+
+public class RNFirebaseBackgroundNotificationsService extends HeadlessJsTaskService {


As above, can this be renamed to RNFirebaseBackgroundNotificationActionsService?

chrisbianca · 2018-05-15T09:33:10Z

.../main/java/io/invertase/firebase/notifications/RNFirebaseBackgroundNotificationsService.java

+    if (extras != null) {
+      WritableMap notificationMap = Arguments.makeNativeMap(intent.getExtras());
+      return new HeadlessJsTaskConfig(
+        "RNFirebaseBackgroundNotification",


Can we change this to RNFirebaseBackgroundNotificationAction?

chrisbianca · 2018-05-15T09:39:12Z

.../main/java/io/invertase/firebase/notifications/RNFirebaseBackgroundNotificationsService.java

+  protected @Nullable HeadlessJsTaskConfig getTaskConfig(Intent intent) {
+    Bundle extras = intent.getExtras();
+    if (extras != null) {
+      WritableMap notificationMap = Arguments.makeNativeMap(intent.getExtras());


We need to make sure that the object being passed to the JS side conforms to the NotificationOpen structure. At the moment it doesn't allow for a snooze action with a remote input specified.

Take a look at the parseIntentForLocalNotification method in RNFirebaseNotifications to see how this is done.

There are some cases when local notification action should be handled in background eg. snoozing the reminder. In case of it launching app UI is not necessary and would be confusing for the end user. Therefore there should be a way to handle local notification action in background. For this reason new property 'runInBackground' was added to the AndroidAction class and TypeScript type. Also new broadcast receiver and service were implemented to handle properly background actions. In order to run particular action in background API consumer need to set its 'runInBackground' property to 'true', eg: ... .android.addAction(new firebase.notifications.Android.Action("snooze", "ic_snooze", "Snooze").setRunInBackground(true)) ... Then, there are two cases that API consumer needs to handle. First when app is in the foreground, standard notification and notification action code path will be executed. This mean, that: * onNotification() listener will be called (which should call displayNotification(), in order to show it to the user), * onNotificationOpen() listener will be called after the action is tapped by the user Secondly, when application is in background or it is not running new 'RNFirebaseBackgroundNotificationAction' handler will be called. To properly handle this case API consumer should create a background asynchronous handler: const handleAsyncTask = async (notificationOpen: NotifficationOpen) => { if (notificationOpen && notificationOpen.notification) { const action = notificationOpen.action; const notificationId = notificationOpen.notification.notificationId; if (action === "snooze") { console.log("Reschedule notification for later time", notificationId); } else { console.log("unsupported action", action); } // hide the notification firebase.notifications().removeDeliveredNotification(notificationId); } } Next hander should be registered to headless handler: AppRegistry.registerHeadlessTask('RNFirebaseBackgroundNotificationAction', () => handleAsyncTask); Finally AndroidManifest.xml file must be modified, to include receiver and service definition: <receiver android:name="io.invertase.firebase.notifications.RNFirebaseBackgroundNotificationActionReceiver" android:exported="true"> <intent-filter> <action android:name="io.invertase.firebase.notifications.BackgroundAction"/> </intent-filter> </receiver> <service android:name="io.invertase.firebase.notifications.RNFirebaseBackgroundNotificationActionsService"/> Now when ever 'Snooze' action is pressed it will launch 'handleAsyncTask' function in the background or onNotificationOpen() when app is in foreground. And reschedule the notification for the later time.

dluksza · 2018-05-16T06:39:53Z

Here is the updated version of previous PR. I've addressed all the comments, updated the commit message and rebased it on current master.

chrisbianca · 2018-05-16T09:26:02Z

...java/io/invertase/firebase/notifications/RNFirebaseBackgroundNotificationActionReceiver.java

+      return;
+    }
+
+    if (Utils.isAppInForeground(context)) {


Have you been able to test that this works in the foreground?

Generally the broadcast receiver runs outside the react context, so I'd be surprised if this did work as it is currently. The way we work around this in other areas is to use a local broadcast receiver which the react application listens to directly. I'm more than happy to merge and then make these changes myself, but wanted to understand if you'd managed to get it working this way before I do?

I've just retested it with HabitChallenge app in three scenarios:

app in foreground

app in background

app not running

In all of them everything worked as expected.

BTW. Test were done with react-native 0.55.4 and android simulator running Android 8.1.0

Great! In which case, I will get this merged and we can keep an eye on it.
If we get reports of issues in the foreground then I can easily switch it over to use the local broadcast.

Thank you again for such a comprehensive PR! I'll look to get some documentation added and we'll include this as part of a release today or tomorrow.

Cool! Looking forward for the release! :D

dluksza mentioned this pull request May 14, 2018

Add Android API to delete channel and channel group #1080

Merged

chrisbianca suggested changes May 15, 2018

View reviewed changes

dluksza force-pushed the android-notification-action-background-handler branch from fea93ed to 17f7f39 Compare May 16, 2018 06:38

chrisbianca reviewed May 16, 2018

View reviewed changes

chrisbianca approved these changes May 16, 2018

View reviewed changes

chrisbianca merged commit 640d7aa into invertase:master May 16, 2018

dluksza deleted the android-notification-action-background-handler branch May 16, 2018 09:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement handling of Android actions in background #1074

Implement handling of Android actions in background #1074

dluksza commented May 11, 2018

chrisbianca left a comment

chrisbianca May 15, 2018

dluksza May 15, 2018

chrisbianca May 15, 2018

dluksza May 15, 2018

chrisbianca May 15, 2018

chrisbianca May 15, 2018

dluksza May 15, 2018

chrisbianca May 15, 2018

chrisbianca May 15, 2018

dluksza May 15, 2018

chrisbianca May 15, 2018

chrisbianca May 15, 2018

dluksza May 15, 2018

chrisbianca May 15, 2018

chrisbianca May 15, 2018

dluksza May 15, 2018

chrisbianca May 15, 2018

dluksza commented May 16, 2018

chrisbianca May 16, 2018

dluksza May 16, 2018

dluksza May 16, 2018

chrisbianca May 16, 2018

dluksza May 16, 2018


		import java.util.List;

		public class RNFirebaseBackgroundNotificationReceiver extends BroadcastReceiver {


		import io.invertase.firebase.messaging.MessagingSerializer;

		public class RNFirebaseBackgroundNotificationsService extends HeadlessJsTaskService {

Implement handling of Android actions in background #1074

Implement handling of Android actions in background #1074

Conversation

dluksza commented May 11, 2018

chrisbianca left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dluksza commented May 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment