Skip to content

Commit

Permalink
sets 204 as default status for delete
Browse files Browse the repository at this point in the history
- adds changelog entry
- addresses comments
  • Loading branch information
LeFnord committed Dec 5, 2016
1 parent 152fa40 commit 257861b
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 21 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Next Release

* [#1503](https://github.com/ruby-grape/grape/pull/1503): Allow to use regexp validator with arrays - [@akoltun](https://github.com/akoltun).
* [#1507](https://github.com/ruby-grape/grape/pull/1507): Add group attributes for parameter definitions - [@304](https://github.com/304).
* [#1532](https://github.com/ruby-grape/grape/pull/1532): Sets 204 as default status for delete - [@LeFnord](https://github.com/LeFnord).
* Your contribution here.

#### Fixes
Expand All @@ -15,7 +16,7 @@ Next Release
* [#1510](https://github.com/ruby-grape/grape/pull/1510): Fix: inconsistent validation for multiple parameters - [@dgasper](https://github.com/dgasper).
* [#1526](https://github.com/ruby-grape/grape/pull/1526): Reduce warnings caused by instance variables not initialized - [@cpetschnig](https://github.com/cpetschnig).
* [#1531](https://github.com/ruby-grape/grape/pull/1531): Updates gem dependencies - [@LeFnord](https://github.com/LeFnord).

* Your contribution here.

0.18.0 (10/7/2016)
==================
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1741,7 +1741,7 @@ cookies.delete :status_count, path: '/'

## HTTP Status Code

By default Grape returns a 200 status code for `GET`-Requests and 201 for `POST`-Requests.
By default Grape returns a 201 for `POST`-Requests, 204 for `DELETE`-Requests and 200 status code for all other Requests.
You can use `status` to query and set the actual HTTP Status Code

```ruby
Expand Down
15 changes: 15 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,21 @@ Prior to this version the response would be `one is missing`.

See [#1510](https://github.com/ruby-grape/grape/pull/1510) for more information.

#### The default status code for DELETE is now 204 instead of 200.

Breaking change: Sets the default response status code for a delete request to 204.
A status of 204 makes the response more distinguishable and therefore easier to handle on the client side, particularly because a DELETE request typically returns an empty body as the resource was deleted or voided.

To achieve the old behavior, one has to set it explicitly:
```ruby
delete :id do
status 200
'foo successfully deleted'
end
```

For more information see: [#1532](https://github.com/ruby-grape/grape/pull/1532).

### Upgrading to >= 0.17.0

#### Removed official support for Ruby < 2.2.2
Expand Down
2 changes: 2 additions & 0 deletions lib/grape/dsl/inside_route.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ def status(status = nil)
case request.request_method.to_s.upcase
when Grape::Http::Headers::POST
201
when Grape::Http::Headers::DELETE
204
else
200
end
Expand Down
8 changes: 7 additions & 1 deletion spec/grape/dsl/inside_route_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def initialize
end

describe '#status' do
%w(GET PUT DELETE OPTIONS).each do |method|
%w(GET PUT OPTIONS).each do |method|
it 'defaults to 200 on GET' do
request = Grape::Request.new(Rack::MockRequest.env_for('/', method: method))
expect(subject).to receive(:request).and_return(request)
Expand All @@ -105,6 +105,12 @@ def initialize
expect(subject.status).to eq 201
end

it 'defaults to 204 on DELETE' do
request = Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE'))
expect(subject).to receive(:request).and_return(request)
expect(subject.status).to eq 204
end

it 'returns status set' do
subject.status 501
expect(subject.status).to eq 501
Expand Down
69 changes: 51 additions & 18 deletions spec/grape/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1095,32 +1095,65 @@ def memoized
end

context 'anchoring' do
verbs = %w(post get head delete put options patch)
describe 'delete 204' do
it 'allows for the anchoring option with a delete method' do
subject.send(:delete, '/example', anchor: true) {}
send(:delete, '/example/and/some/more')
expect(last_response.status).to eql 404
end

verbs.each do |verb|
it "allows for the anchoring option with a #{verb.upcase} method" do
subject.send(verb, '/example', anchor: true) do
verb
end
send(verb, '/example/and/some/more')
it 'anchors paths by default for the delete method' do
subject.send(:delete, '/example') {}
send(:delete, '/example/and/some/more')
expect(last_response.status).to eql 404
end

it "anchors paths by default for the #{verb.upcase} method" do
subject.send(verb, '/example') do
verb
it 'responds to /example/and/some/more for the non-anchored delete method' do
subject.send(:delete, '/example', anchor: false) {}
send(:delete, '/example/and/some/more')
expect(last_response.status).to eql 204
expect(last_response.body).to be_empty
end
end

describe 'delete 200, with response body' do
it 'responds to /example/and/some/more for the non-anchored delete method' do
subject.send(:delete, '/example', anchor: false) do
status 200
body 'deleted'
end
send(verb, '/example/and/some/more')
expect(last_response.status).to eql 404
send(:delete, '/example/and/some/more')
expect(last_response.status).to eql 200
expect(last_response.body).not_to be_empty
end
end

it "responds to /example/and/some/more for the non-anchored #{verb.upcase} method" do
subject.send(verb, '/example', anchor: false) do
verb
describe 'all other' do
%w(post get head put options patch).each do |verb|
it "allows for the anchoring option with a #{verb.upcase} method" do
subject.send(verb, '/example', anchor: true) do
verb
end
send(verb, '/example/and/some/more')
expect(last_response.status).to eql 404
end

it "anchors paths by default for the #{verb.upcase} method" do
subject.send(verb, '/example') do
verb
end
send(verb, '/example/and/some/more')
expect(last_response.status).to eql 404
end

it "responds to /example/and/some/more for the non-anchored #{verb.upcase} method" do
subject.send(verb, '/example', anchor: false) do
verb
end
send(verb, '/example/and/some/more')
expect(last_response.status).to eql verb == 'post' ? 201 : 200
expect(last_response.body).to eql verb == 'head' ? '' : verb
end
send(verb, '/example/and/some/more')
expect(last_response.status).to eql verb == 'post' ? 201 : 200
expect(last_response.body).to eql verb == 'head' ? '' : verb
end
end
end
Expand Down

0 comments on commit 257861b

Please sign in to comment.