diff --git a/UPGRADING.md b/UPGRADING.md index 418cc89b1a..5f778176c9 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -41,6 +41,19 @@ Prior to this version the response would be `one is missing`. See [#1510](https://github.com/ruby-grape/grape/pull/1510) for more information. +#### Change default status code of DELETE to 204 + +**Breaking change**: Sets the default response status code for a delete request to 204. +Doing it makes the response more explicit, cause a delete request returns in most cases an empty body – the resource was deleted/voided. By setting it to 204, other as 200, makes the handling on consumer side easier. + +To achieve the old behaviour, one has to set it explicitly: +```ruby +delete :id do + status 200 + 'foo successfully deleted' +end +``` + ### Upgrading to >= 0.17.0 #### Removed official support for Ruby < 2.2.2 diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 22f13e017a..76a74c65be 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/lib/grape/dsl/inside_route.rb @@ -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 diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb index cf6dc31cf2..f262851662 100644 --- a/spec/grape/dsl/inside_route_spec.rb +++ b/spec/grape/dsl/inside_route_spec.rb @@ -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) @@ -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 diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb index 2fc94e5340..bf17c641e5 100644 --- a/spec/grape/endpoint_spec.rb +++ b/spec/grape/endpoint_spec.rb @@ -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