Redirect as plain text with optional message override.

tylerdooling · tylerdooling · commit d1bba79de66c · 2015-11-10T13:31:21.000-06:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,7 @@
 
 #### Fixes
 
+* [#1194](https://github.com/ruby-grape/grape/pull/1194): Redirect as plain text with message - [@tylerdooling](https://github.com/tylerdooling).
 * [#1185](https://github.com/ruby-grape/grape/pull/1185): Use formatters for custom vendored content types - [@tylerdooling](https://github.com/tylerdooling).
 * [#1156](https://github.com/ruby-grape/grape/pull/1156): Fixed `no implicit conversion of Symbol into Integer` with nested `values` validation - [@quickpay](https://github.com/quickpay).
 * [#1153](https://github.com/ruby-grape/grape/pull/1153): Fixes boolean declaration in an external file - [@towanda](https://github.com/towanda).
diff --git a/UPGRADING.md b/UPGRADING.md
@@ -23,6 +23,15 @@ be bypassed when processing responses for status codes defined [by rack](https:/
 
 See [#1190](https://github.com/ruby-grape/grape/pull/1190) for more information.
 
+#### Redirects respond as plain text with message
+
+`#redirect` now uses `text/plain` regardless of whether that format has
+been enabled. This prevents formatters from attempting to serialize the
+message body and allows for a descriptive message body to be provided - and
+optionally overridden - that better fulfills the theme of the HTTP spec.
+
+See [#1194](https://github.com/ruby-grape/grape/pull/1194) for more information.
+
 ### Upgrading to >= 0.12.0
 
 #### Changes in middleware
diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb
@@ -91,19 +91,25 @@ def error!(message, status = nil, headers = nil)
       # @param url [String] The url to be redirect.
       # @param options [Hash] The options used when redirect.
       #                       :permanent, default false.
+      #                       :body, default a short message including the URL.
       def redirect(url, options = {})
-        merged_options = options.reverse_merge(permanent: false)
-        if merged_options[:permanent]
+        permanent = options.fetch(:permanent, false)
+        body_message = options.fetch(:body, nil)
+        if permanent
           status 301
+          body_message ||= "This resource has been moved permanently to #{url}."
         else
           if env[Grape::Http::Headers::HTTP_VERSION] == 'HTTP/1.1' && request.request_method.to_s.upcase != Grape::Http::Headers::GET
             status 303
+            body_message ||= "An alternate resource is located at #{url}."
           else
             status 302
+            body_message ||= "This resource has been moved temporarily to #{url}."
           end
         end
         header 'Location', url
-        body ''
+        content_type 'text/plain'
+        body body_message
       end
 
       # Set or retrieve the HTTP status code.
diff --git a/spec/grape/endpoint_spec.rb b/spec/grape/endpoint_spec.rb
@@ -752,7 +752,7 @@ def app
       get '/hey'
       expect(last_response.status).to eq 302
       expect(last_response.headers['Location']).to eq '/ha'
-      expect(last_response.body).to eq ''
+      expect(last_response.body).to eq 'This resource has been moved temporarily to /ha.'
     end
 
     it 'has status code 303 if it is not get request and it is http 1.1' do
@@ -762,6 +762,7 @@ def app
       post '/hey', {}, 'HTTP_VERSION' => 'HTTP/1.1'
       expect(last_response.status).to eq 303
       expect(last_response.headers['Location']).to eq '/ha'
+      expect(last_response.body).to eq 'An alternate resource is located at /ha.'
     end
 
     it 'support permanent redirect' do
@@ -771,7 +772,15 @@ def app
       get '/hey'
       expect(last_response.status).to eq 301
       expect(last_response.headers['Location']).to eq '/ha'
-      expect(last_response.body).to eq ''
+      expect(last_response.body).to eq 'This resource has been moved permanently to /ha.'
+    end
+
+    it 'allows for an optional redirect body override' do
+      subject.get('/hey') do
+        redirect '/ha', body: 'test body'
+      end
+      get '/hey'
+      expect(last_response.body).to eq 'test body'
     end
   end
 
