Use 200 as default status for deletes that reply with content

jthornec · jthornec · commit 553331a696f5 · 2017-01-04T12:48:40.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,7 @@
 #### Fixes
 
 * [#1548](https://github.com/ruby-grape/grape/pull/1548): Avoid failing even if given path does not match with prefix - [@thomas-peyric](https://github.com/thomas-peyric), [@namusyaka](https://github.com/namusyaka).
+* [#1550](https://github.com/ruby-grape/grape/pull/1550): Use 200 as default status for deletes that reply with content - [@jthornec](http://github.com/jthornec).
 * Your contribution here.
 
 ### 0.19.0 (12/18/2016)
diff --git a/README.md b/README.md
@@ -1764,7 +1764,7 @@ cookies.delete :status_count, path: '/'
 
 ## HTTP Status Code
 
-By default Grape returns a 201 for `POST`-Requests, 204 for `DELETE`-Requests and 200 status code for all other Requests.
+By default Grape returns a 201 for `POST`-Requests, 204 for `DELETE`-Requests that don't return any content, and 200 status code for all other Requests.
 You can use `status` to query and set the actual HTTP Status Code
 
 ```ruby
diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb
@@ -130,7 +130,11 @@ def status(status = nil)
           when Grape::Http::Headers::POST
             201
           when Grape::Http::Headers::DELETE
-            204
+            if @body.present?
+              200
+            else
+              204
+            end
           else
             200
           end
diff --git a/spec/grape/dsl/inside_route_spec.rb b/spec/grape/dsl/inside_route_spec.rb
@@ -111,6 +111,13 @@ def initialize
       expect(subject.status).to eq 204
     end
 
+    it 'defaults to 200 on DELETE with a body present' do
+      request = Grape::Request.new(Rack::MockRequest.env_for('/', method: 'DELETE'))
+      subject.body 'content here'
+      expect(subject).to receive(:request).and_return(request)
+      expect(subject.status).to eq 200
+    end
+
     it 'returns status set' do
       subject.status 501
       expect(subject.status).to eq 501

Original file line number	Diff line number	Diff line change
`@@ -130,7 +130,11 @@ def status(status = nil)`
`130`	`130`	`when Grape::Http::Headers::POST`
`131`	`131`	`201`
`132`	`132`	`when Grape::Http::Headers::DELETE`
`133`		`- 204`
	`133`	`+ if @body.present?`
	`134`	`+ 200`
	`135`	`+ else`
	`136`	`+ 204`
	`137`	`+ end`
`134`	`138`	`else`
`135`	`139`	`200`
`136`	`140`	`end`