-
Notifications
You must be signed in to change notification settings - Fork 63
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
Sets request body required property to false when all action parameters are optional #591
base: master
Are you sure you want to change the base?
Conversation
Sets requestBody required property based on this evaluation. If all parameters are nullable, it sets the required value to false, otherwise it sets it to true
src/Microsoft.OpenApi.OData.Reader/Generator/OpenApiRequestBodyGenerator.cs
Outdated
Show resolved
Hide resolved
Quality Gate failedFailed conditions |
foreach (var parameter in action.Parameters.Skip(skip)) | ||
{ | ||
allParamsNullable &= parameter.Type.IsNullable; |
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.
One last question here. Should we be checking if the parameter is of type IEdmOptionalParameter
instead? Is it possible for required parameter to be nullable?
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 indeed! We should. I will add a check for the annotation: Org.OData.Core.V1.OptionalParameter
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 for now let's only update the check to only cater for the annotation Org.OData.Core.V1.OptionalParameter
as that's the scenario we want to cater for in #582. We can always come back and do that for nullable types.
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.
Okay
Fixes #582