-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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:ref attributes convert list #1168
base: master
Are you sure you want to change the base?
Conversation
} | ||
if (Boolean(this.$minOccurs)) { | ||
typeElement['minOccurs'] = this.$minOccurs | ||
} |
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.
please add a test for 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.
please add a test for these changes
Are you referring to minOccurs to add the test?
Sorry, about the minOccurs, it may not be necessary, just to match the maxOccurs.
It's only the maxOccurs part I want to fix.
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.
yes. sorry for the late reply. please add a test to verify that this works as expected now and in the future.
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.
@momo1006 have you solved this issue without this PR?
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.
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.
@ydaniju test is required for these changes. Seems like this PR was abandoned. I will try to have a look on this, just not sure when I have time.
Feels free to help with this and provide a test, that would make it much easier for me.
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.
Thanks @w666 for the quick response. I created a test for this here https://github.com/vpulim/node-soap/pull/1260/files . I don't know how to add the test in this PR.
I will attempt a fix for the next few hours.
} | ||
if (Boolean(this.$minOccurs)) { | ||
typeElement['minOccurs'] = this.$minOccurs | ||
} |
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.
yes. sorry for the late reply. please add a test to verify that this works as expected now and in the future.
Consider this XSD:
I expected DummyList and DummyValues to be returned in List
sample response SOAP:
The current value being returned.
When ref is used to set maxoccurs, if the returned value is a single value, it will not be converted to an array.
Response I'm expecting.
Similar to issues#1100