-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MOBILE-4239] Fix Airship Actions running #557
Conversation
ios/AirshipReactNative.swift
Outdated
actionName, | ||
value: try AirshipJSON.wrap(actionValue) | ||
action["_name"] as! String, | ||
value: action["_value"] is NSNull ? nil : try AirshipJSON.wrap(action["_value"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to make sure I pass nil if it's a NSNull.
Otherwise the JSON wrapping fails: the NSNull Object don't go through the guard else, is that normal @crow ?
public static func wrap(_ value: Any?, encoder: JSONEncoder = AirshipJSON.defaultEncoder) throws -> AirshipJSON {
guard let value = value else {
return .null
}
.
.
.
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
/** | ||
* Gets the event's interaction ID. | ||
*/ | ||
get interactionId(): string | undefined { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need these changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not particularly, I noticed we handle those in the proxy so I just added them
What do these changes do?
Update the actions running code to encapsulate the action into a dictionary as the React conversion can't handle dynamic type like JsonValue directly.
Why are these changes necessary?
To handle actions with different value types.
How did you verify these changes?
Tested several actions making sure I test every values types possible.
New arch and old arch are working fine on Android. iOS is fine too.