-
Notifications
You must be signed in to change notification settings - Fork 3
TASK-1816943: Added support for startingFields for WKWebView based en… #52
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
Conversation
…gine. Extended API to support openAssignment action.
ecce1a9 to
e32a89b
Compare
| } | ||
|
|
||
|
|
||
| object ConstellationSdkKActionSerializer : KSerializer<ConstellationSdkAction> { |
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.
Most probably this custom serializer is not needed as kotlin should handle serialization of sealed classes out of the box. You just need to add @Serialiable to sealed class and subclasses and use type as a class discriminator instead of actionType.
| import kotlinx.serialization.json.Json | ||
|
|
||
| @Serializable | ||
| data class EngineConfiguration( |
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.
This seems to be implementation-detail of specific engine. I'd move it to engine-webview > commonMain and make it internal, as it is not something to be used by SDK clients. Btw - commonMain sourceset was recently created here: https://github.com/pegasystems/constellation-mobile-sdk/pull/49/files#diff-2398e22a8cacbeea18a4b5885f734cb50ef9029a687327c36481db76d377e40a
|
|
||
| private const val TAG = "JsonHelper" | ||
|
|
||
| fun Any.toJsonElement(): JsonElement? = when (this) { |
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.
It should be internal
eee20c5 to
634eaa9
Compare
| val jsonElements = this.mapNotNull { | ||
| it?.toJsonElement() ?: run { | ||
| val type = it?.let { it::class } ?: "null" | ||
| Log.w(TAG, "Unsupported type $type, cannot convert to JsonElement.") |
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.
Won't it result in duplicated logs for the same type? It seems that the same log will be printed from else branch (as we're calling toJsonElement recursively). I think we could get rid of that run {} block.
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.
good point - will refactor it
| if (config.action.actionType === "CreateCase") { | ||
| await createCase(config.action.caseClassName, config.action.startingFields); | ||
| } else if (config.action.actionType === "OpenAssignment") { | ||
| await openAssignment(config.action.assignmentId); |
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 think that it is okay for now, but in the future we should consider making it possible to "interact" with the application/engine instead of just reinitializing everything each time we want to open or create something.
| }; | ||
|
|
||
| console.log("[OpenAssignment]", "Opening assignment: " + assignmentId); | ||
| await PCore.getMashupApi().openAssignment(assignmentId, PCore.getConstants().APP.APP, options); |
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.
Wow, that's so simple 😎
lukaszgajewski-pega
left a comment
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.
Code review completed, please take a look at my comments.
…gine. Extended API to support openAssignment action.