Fix direct tracking

[Jeasmine]

• Direct session tracking is not working when OneSignal opening application is disabled on the manifest. If the user opens the application on the open handler callback no direct session is being tracked
• Create a complete method under OSNotificationOpenedResult to track direct opening by notification click

To achieve the fix we have a timeout handler of 5 seconds, this time is the max time that the application will wait for the application to start. This way we can track that the app was started by a notification click activity open handle by the user, this will able Session DIRECT tracking

This change is 

[emawby]

Reviewed 4 of 5 files at r1.
Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @emawby, @Jeasmine, @jkasten2, and @rgomezp)

---

Examples/OneSignalDemo/app/src/main/AndroidManifest.xml, line 67 at r1 (raw file):

            android:value="com.onesignal.sdktest.notification.NotificationServiceExtension" />

        <meta-data android:name="com.onesignal.NotificationOpened.DEFAULT" android:value="DISABLE" />

Do we want to commit this change and always test with this setting or is this something we should remove after testing?

---

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationOpenedResult.java, line 71 at r1 (raw file):

   /**
    * Method indicating OneSignal that application was opened by user.
    * This method must be call before startActivity or equivalents are called

What happens if it is not called before startActivity? With our 5 second timer won't we automatically call complete after startActivity often? Also do we need to update documentation indicating that the opened handler should complete its work in 5 seconds?

[rgomezp]

Reviewable status: 4 of 5 files reviewed, 6 unresolved discussions (waiting on @emawby, @Jeasmine, @jkasten2, and @rgomezp)

---

Examples/OneSignalDemo/app/src/main/AndroidManifest.xml, line 67 at r1 (raw file):

Previously, emawby (Elliot Mawby) wrote…

Do we want to commit this change and always test with this setting or is this something we should remove after testing?

Probably not. Especially since we are just logging the event in the callback and not actually starting the activity via startActivity as indicated by the docs.

---

OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java, line 305 at r1 (raw file):

      // Fire with the last appEntryState that just ended.
      void onEntryStateChange(AppEntryAction appEntryState);
   }

Nit: newline

---

OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java, line 306 at r1 (raw file):

      void onEntryStateChange(AppEntryAction appEntryState);
   }
   private static List<EntryStateListener> entryStateListeners = new ArrayList<>();

Nit: newline

---

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationOpenedResult.java, line 4 at r1 (raw file):

 * Modified MIT License
 *
 * Copyright 2016 OneSignal

2020

---

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationOpenedResult.java, line 66 at r1 (raw file):

         }
      };
      timeoutHandler.startTimeout(PROCESS_NOTIFICATION_TIMEOUT, timeoutRunnable);

Can you add a comment expanding on why this timeout is needed, why you chose 5 seconds, and how will this impact existing implementations?

Also would be good to add to the PR for high level.

[rgomezp]

Reviewable status: 4 of 5 files reviewed, 7 unresolved discussions (waiting on @emawby, @Jeasmine, and @jkasten2)

---

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationOpenedResult.java, line 65 at r1 (raw file):

            complete(false);
         }
      };

Nit: newine

[Jeasmine]

Reviewed 4 of 5 files at r1.
Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @emawby, @Jeasmine, @jkasten2, and @rgomezp)

Examples/OneSignalDemo/app/src/main/AndroidManifest.xml, line 67 at r1 (raw file):

            android:value="com.onesignal.sdktest.notification.NotificationServiceExtension" />

        <meta-data android:name="com.onesignal.NotificationOpened.DEFAULT" android:value="DISABLE" />

Do we want to commit this change and always test with this setting or is this something we should remove after testing?

OneSignalSDK/onesignal/src/main/java/com/onesignal/OSNotificationOpenedResult.java, line 71 at r1 (raw file):

   /**
    * Method indicating OneSignal that application was opened by user.
    * This method must be call before startActivity or equivalents are called

What happens if it is not called before startActivity? With our 5 second timer won't we automatically call complete after startActivity often? Also do we need to update documentation indicating that the opened handler should complete its work in 5 seconds?

thanks for the review! Good catch, we don't want to commit the DISABLE conf. And the comment was old because my first approach was that the user call complete, now it's only called internally

[rgomezp]

Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @emawby, @Jeasmine, and @jkasten2)