-
Notifications
You must be signed in to change notification settings - Fork 22
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
Fix type resolution in sub execution #209
Conversation
… TextType, rather return ParameterType<Text> error message for EthereumAddress more useful
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.
Added two minor comments about the tests.
engine.execute(mainTemplate, TemplateParameters(), Map(TemplateSourceIdentifier(TemplateTitle("clause")) -> clauseTemplate,TemplateSourceIdentifier(TemplateTitle("clause")) -> clauseTemplate)) match { | ||
case Right(result) => | ||
val variable = VariableDefinition(VariableName("Parties"), Some(VariableTypeDefinition("Collection",Some(VariableTypeDefinition("Parefhuzegfty Info",None))))) | ||
getCollection(variable, result, "") |
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.
What about adding an assert here to check if the returned type is indeed a collection value type with the expected values? If the returned collection is empty, should it break the test?
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.
The collection will be empty because the internal format value is an empty string.
The test here is more to check that the call actually works, there was an issue with this call previously
} | ||
|
||
def getCollection(variable:VariableDefinition, executionResult: TemplateExecutionResult, value:String):CollectionValue = { | ||
println(variable.variableTypeDefinition) |
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 believe we can remove this println
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.
you're right
It seems like when the parameter is not found, then the type returned is Text instead of ParameterType
This made issues in Collection definition.
This shows another issue that I need to look at but that's still an issue we need fixing
I'm not necessarily super happy with this solution (see #210 ) but it's clearly an improvement
This change is