Add in app message lifecycle handler

[nan-li]

Description

One Line Summary

Adds the In-App Message Lifecycle Handler which includes up to 4 callbacks for the displaying and dismissing of In-App Messages.

Details

Motivation

This handler was added to give customers visibility into the display lifecycle of IAMs: when an IAM will be displayed, and also when it has been dismissed.

Scope

• Extra fields will show up if a user logs the OSInAppMessage instead of just its messageId property, which is fine.
• In-App Messages are only read and not modified.

Public API Changes

Yes, the documentation will need to be updated for react native to provide these new methods.

Previous WiP Questions

1. onWillDismissInAppMessage is logged before the inAppMessageClicked is logged even though clicking on the IAM is what causes it to be dismissed (is this problem?)
2. Can the InAppMessage be null? -> no, it should not be
3. The native code does call sendEvent for all 4 lifecycle events even if the user has not set all 4. They are just not "caught" by the JavaScript if the user hasn't set the event handler for a particular lifecycle event (native logcat shows all 4).
4. Should the native OSInAppMessage classes add toJSONObject (Android) or toDictionary or jsonRepresentation (iOS) as these exist on the other handler/observer objects -> yes, added.
5. Lastly, I noticed that RNOneSignal has different implementations for the other handlers and not sure what the differences are for (NotificationWillShowInForegroundHandler, NotificationOpenedHandler, ClickHandler) and if InAppMessageLifecycleHandler should be done differently.

Testing

Manual testing

Ran the code in example app on Android and iOS emulators.

Affected code checklist

• Notifications
  • Display
  • Open
  • Push Processing
  • Confirm Deliveries
• Outcomes
• Sessions
• In-App Messaging
• REST API requests
• Public API changes

Checklist

Overview

• I have filled out all REQUIRED sections above
• PR does one thing
• Any Public API changes are explained in the PR details and conform to existing APIs

Testing

• I have included test coverage for these changes, or explained why they are not needed
• All automated tests pass, or I explained why that is not possible
• I have personally tested this on my device, or explained why that is not possible

Final pass

• Code is as readable as possible.
• I have reviewed this PR myself, ensuring it meets each checklist item

---

This change is 

[emawby]

1. Is this the case in the native SDKs as well or does it log in the expected order in native?
2. I don't think the InAppMessage should be null. I can't think of a non-error scenario where it would be null
3. This is fine I think
4. Yes we probably should have the OSInAppMessage toJSONObject in native instead of RN so that we don't need to translate it in every wrapper. This will require native changes and release. Good call!
5. I think your approach here looks good to me, but I will defer to the RN experts.

Reviewable status: 0 of 11 files reviewed, all discussions resolved (waiting on @Jeasmine, @jkasten2, and @rgomezp)

[rgomezp]

Agree with emawby on all points. Let's make sure to get the native change done with a toJSONObject() before getting this out.

[rgomezp]

Looks good other than the small nits and waiting for the changes related to the native JSON stringifier.

[nan-li]

@emawby @rgomezp I have one more question:

I gave the react-native InAppMessage the property of messageId. In the native SDKs, they have this property with this same name. However, all the JSON used in the native SDKs pass it around as "id" (this exists on OSInAppMessageInternal):

json[@"id"] = self.messageId
message.messageId = json[@"id"]

Should I name the native JSON stringifier for the public OSInAppMessage as json[@"messageId"], differing from the Internal version, or rename the react-native InAppMessage property to id instead of messageId?

I believe the JSON keys must match over the bridge?

[rgomezp]

@emawby @rgomezp I have one more question:

I gave the react-native InAppMessage the property of messageId. In the native SDKs, they have this property with this same name. However, all the JSON used in the native SDKs pass it around as "id" (this exists on OSInAppMessageInternal):

json[@"id"] = self.messageId
message.messageId = json[@"id"]

Should I name the native JSON stringifier for the public OSInAppMessage as json[@"messageId"], differing from the Internal version, or rename the react-native InAppMessage property to id instead of messageId?

I believe the JSON keys must match over the bridge?

Let's stick to messageId since that's what we're doing natively

[nan-li]

Let's stick to messageId since that's what we're doing natively

Sounds good. Does this mean the native OSInAppMessage.toJSONObject() will have to be json["messageId"] instead of json["id"], which differs from OSInAppMessageInternal.toJSONObject()?

[jkasten2]

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @emawby, @Jeasmine, and @nan-li)

---

android/src/main/java/com/geektime/rnonesignalandroid/RNUtils.java, line 183 at r1 (raw file):

    /**
     * Converts an OSInAppMessage into a JSON object as OSInAppMessage lacks a toJSONObject() method.
     * TODO: add toJSONObject() to OSInAppMessage and remove this method.

Adding a comment here to ensure address this comment before merging.

[jkasten2]

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @emawby, @Jeasmine, @nan-li, and @rgomezp)