Skip to content
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

support custom DELETE responses with models #440

Conversation

hedgesky
Copy link
Contributor

If we create and endpoint with such a description:

desc 'Endpoint desc', http_codes: [{ code: 200, message: 'desc', model: ExampleEntity }]

the entity won't appear in swagger json because of this line. It sees default 204 response (which was built few lines upper) and immediately moves to next entry.
This PR fixes that.

@kzaitsev
Copy link
Contributor

what you think about skip default codes assigement if http_codes present?

@dblock
Copy link
Member

dblock commented May 27, 2016

This also needs tests and CHANGELOG to be merged.

@hedgesky
Copy link
Contributor Author

@Bugagazavr personally, I like your idea, but it could affect those users who add just error http codes and rely on the library regarding successful cases.

@LeFnord
Copy link
Member

LeFnord commented May 28, 2016

@hedgesky with this dsl/desc.rb in mind, the actual behavior is correct for such an endpoint:

desc 'deleting stuff'
delete ':id' do
  # do it
  body false
end

this returns status 204

think, what you want to try to achieve is something like this:

desc 'deleting stuff',
  success: ExampleEntity
delete ':id' do
  # do it
  present :something, with: ExampleEntity
end

this is also possible, but returns status 200

means: adding a 2xx to the failureshttp_codes array doesn't match the meaning of it, and we should be consistent with grape 😉

@Bugagazavr think skipping defaults isn't such a good idea, for reason one could see in the example above, and IMO we should document the status code of a response, cause it is always in the response

@hedgesky
Copy link
Contributor Author

hedgesky commented Jun 2, 2016

@LeFnord Thanks for pointing that failure == http_codes. Actually, it's written in Using an option hash section of Readme, but I've found out all http_codes thing in Response documentation section. And there is no mention about it's related only to errors there.
Of course, it would be better if a developer read whole readme at first, but maybe name like error_http_codes would be more meaningful, what do you think?

@LeFnord
Copy link
Member

LeFnord commented Jun 2, 2016

@hedgesky … the keys success/entity and failures/http_codes coming direct from grape,
mean changing one of them should be done there

feel free to update the README

@hedgesky hedgesky closed this Jun 3, 2016
@LeFnord LeFnord mentioned this pull request Jun 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants