GoDaddy improvements - round 2 - #61
Conversation
|
A note on response messages - copied from #52. Swagger provides two ways for specifying the response type:
In the screenshot above, I am proposing to include the response model in both places for now. We can later add a configuration option to customise this behaviour, if such thing is requested. |
|
The CI builds will be failing until strongloop/strong-remoting#113 is released in |
818abb4 to
9b21f40
Compare
|
Rebased on top of the current master, which uses |
|
@bajtos - We switched to always using responseMessage.responseModel, because we use different Models by Status, including a custom, reusable Model for Error responses. This is more natural for many of our endpoints that aren't CRUD-related. Also, I believe it better uses the space visually and with less confusion. You could change it to use For reference, see: https://developer.godaddy.com/doc#!/shoppers/Subaccount_create |
Agreed. @raymondfeng @ritch do you have any opinion on this? I am inclined to rework the implementation and drop route-level response type as originally proposed in #52. The only issue I am aware of is that the user cannot select response type (json, xml, etc.), as the combo-box is gone. It looks like a swagger ui bug to me though.
|
|
@bajtos - I have also moved the Response Content Type select to down by the "Try it out!" In shelbys/swagger-UI |
That makes a lot of sense to me, it is one more reason to upstream your changes. |
Users of loopback-explorer should not override the swagger version, as it's the explorer who decides what version of the Swagger Spec it implements.
Improve spacing of page elements when the browser window is small.
Models not attached to the app are included too.
Accepts/returns arguments allow anonymous object types, e.g.
{ 'arg': 'kvp', type: { 'name': 'string', 'value': 'string' } }
As of this commit, these types are converted to Swagger type 'object'.
Add a default "success" response message, the status code is 200 or 204 depending on whether the method returns any data. Append any error messages as specified in the `errors` property of method's remoting metadata. Move the description of operation's return type to the "success" response message. Include error message models in the API models.
9b21f40 to
d05dcb7
Compare
|
Test FAILed. |
1 similar comment
|
Test FAILed. |
Add tests verifying that Swagger docs include model description for recursively nested references to Models and Arrays of Models in properties, modelTo and modelThrough relations, accepts, returns and errors. Fix bugs discovered along the way.
fa3f822 to
6fb81c2
Compare
|
I have rebased the patch on top of the current master and modified the
I have added integration tests for that in the last commit and fixed bugs discovered along the way. @raymondfeng please review. |
|
Test FAILed. |
|
@slnode test please |
|
Test PASSed. |
|
LGTM |
…ound-2 GoDaddy improvements - round 2
|
From discussion on swagger-spec, it appears |
5 participants
Another follow-up pull request for #52.
opts.swaggerVersionresponseMessagesRequires strongloop/strong-remoting#113 (/cc @ritch)
/to @raymondfeng and/or @STRML please review
/cc @shelbys