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
Allow non strings tobe nillable #265
Allow non strings tobe nillable #265
Conversation
src/parser/xmlHandler.js
Outdated
@@ -60,6 +60,7 @@ class XMLHandler { | |||
name = descriptor.qname.name; | |||
let isSimple = descriptor.isSimple; | |||
let attrs = null; | |||
if (val === null && descriptor.isNillable) descriptor.type.name = 'string'; |
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 is hacky. Can we fix https://github.com/strongloop/strong-soap/blob/f7430739366af6a24f1fdede0d1c6ec27c6a073f/src/parser/xmlHandler.js#L881 to return null
if val == 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.
I originally looked at that, but there are other date things that are going on before the code gets to the line to check for null and nilable. I do agree that this seems sort of hacky, but the more I thought about it the more I came to the conclusion that if the field is nilable and the value is null, the code should really not care about the data type from there on out.
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 agree. Can we refactor the code a bit to cover nillable
date?
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.
My apologies, it looks like getting toSmlDateOrTime to pass back null did work, not sure what I did different the first time I tried it.
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.
Great. Can you revert this change then?
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.
Done (I thought I has already but...well, it seems I am having a hell of a week).
@rsunbury Thank you. I cleaned up your commit history and pushed them back for the PR. |
Released as strong-soap@1.22.1 |
Description
Fixes issue with non-string fields (i.e. dates) not being able to be nilled.
Related issues
#263
Checklist
guide