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

Add ignore_defaults option #491

Merged

Conversation

pezholio
Copy link
Contributor

@pezholio pezholio commented Aug 4, 2016

As outlined in #490 - it seems that grape-swagger adds a default 201 response for PUT and POST requests. If a PUT or POST request doesn't immediately create a resource, the correct response is 202 Accepted. In this case, we want to ignore the default 201 response, and only specify a 202.

There may be other use cases as well, so I've added support for an ignore_defaults option which only adds user-specified responses, and ignores the default responses specified in https://github.com/ruby-grape/grape-swagger/blob/master/lib/grape-swagger/doc_methods/status_codes.rb

@LeFnord
Copy link
Member

LeFnord commented Aug 4, 2016

@pezholio … thanks

please run bundle exec rake or bundle exec rubocop to see the warning 😉

@pezholio
Copy link
Contributor Author

pezholio commented Aug 4, 2016

Right, that should help 😄 I've also added a separate spec for the behaviour, rather than rolling it into the main api_swagger_v2_spec, which seemed more sensible, and also stopped a whole bunch of the Rubocop errors

let(:json) { JSON.parse(last_response.body) }

it 'only returns one response if ignore_defaults is specified' do
expect(json['paths']['/delay_thing']['post']['responses']).to eq('202' => { 'description' => 'OK' })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add also an expectation, which proofs that the default wasn't created:

expect(json['paths']['/delay_thing']['post']['responses'].keys).not_to include '201'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No probs 👍

@LeFnord
Copy link
Member

LeFnord commented Aug 4, 2016

👍 … cc/ @Bugagazavr, if you are fine with it, I'll cut the next release

@kzaitsev
Copy link
Contributor

kzaitsev commented Aug 4, 2016

LGTM

@LeFnord LeFnord merged commit 45c0956 into ruby-grape:master Aug 4, 2016
LeFnord pushed a commit to LeFnord/grape-swagger that referenced this pull request Feb 9, 2019
* Add `ignore_defaults` option

* Update Changelog

* Make Rubocop happy

* Add seperate spec for `ignore_defaults`

* Make tests more specific

* Revert Rubocop-pleasing changes

* Increase max line length
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants