Skip to content

Redirect as plain text with message. #1194

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 11, 2015
Merged

Redirect as plain text with message. #1194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
9 changes: 9 additions & 0 deletions UPGRADING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions lib/grape/dsl/inside_route.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 11 additions & 2 deletions spec/grape/endpoint_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down