Skip to content
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.

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

Respond with 422 Unprocessable Entity for non-GET HTML/JS requests with errors #223

Merged
merged 3 commits into from Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions CHANGELOG.md
@@ -1,8 +1,9 @@
## Unreleased

* Add support for Ruby 3.0, drop support for Ruby < 2.5
* Add support for Rails 6.1, drop support for Rails < 5.2
* Move CI to GitHub Actions
* Responding to an `HTML` or `JS` request that has errors on the resource now sets the status to `422 Unprocessable Entity`. (instead of the default of `200 OK`.) This makes it more consistent with other statuses more commonly used in APIs (JSON/XML for example), and works by default with Turbo/Hotwire which expects a 422 on form error HTML responses. Note that this change may break your application if you're relying on the previous 2xx status to handle error cases.
* Add support for Ruby 3.0, drop support for Ruby < 2.5.
* Add support for Rails 6.1, drop support for Rails < 5.2.
* Move CI to GitHub Actions.

## 3.0.1

Expand Down
3 changes: 2 additions & 1 deletion README.md
Expand Up @@ -212,7 +212,8 @@ assertions on this behavior for your controllers.
def create
@widget = Widget.new(widget_params)
@widget.errors.add(:base, :invalid)
# `respond_with` will render the `new` template again.
# `respond_with` will render the `new` template again,
# and set the status to `422 Unprocessable Entity`.
respond_with @widget
end
```
Expand Down
9 changes: 5 additions & 4 deletions lib/action_controller/respond_with.rb
Expand Up @@ -95,7 +95,8 @@ def clear_respond_to
# i.e. its +show+ action.
# 2. If there are validation errors, the response
# renders a default action, which is <tt>:new</tt> for a
# +post+ request or <tt>:edit</tt> for +patch+ or +put+.
# +post+ request or <tt>:edit</tt> for +patch+ or +put+,
# and the status is set to <tt>422 Unprocessable Entity</tt>.
# Thus an example like this -
#
# respond_to :html, :xml
Expand All @@ -116,8 +117,8 @@ def clear_respond_to
# format.html { redirect_to(@user) }
# format.xml { render xml: @user }
# else
# format.html { render action: "new" }
# format.xml { render xml: @user }
# format.html { render action: "new", status: :unprocessable_entity }
# format.xml { render xml: @user, status: :unprocessable_entity }
# end
# end
# end
Expand Down Expand Up @@ -194,7 +195,7 @@ def clear_respond_to
# need to render a template which is outside of controller's path or you
# want to override the default http <tt>:status</tt> code, e.g.
#
# respond_with(resource, render: { template: 'path/to/template', status: 422 })
# respond_with(resource, render: { template: 'path/to/template', status: 418 })
def respond_with(*resources, &block)
if self.class.mimes_for_respond_to.empty?
raise "In order to use respond_with, first you need to declare the " \
Expand Down
12 changes: 7 additions & 5 deletions lib/action_controller/responder.rb
Expand Up @@ -49,7 +49,7 @@ module ActionController #:nodoc:
# format.html { redirect_to(@user) }
# format.xml { render xml: @user, status: :created, location: @user }
# else
# format.html { render action: "new" }
# format.html { render action: "new", status: :unprocessable_entity }
# format.xml { render xml: @user.errors, status: :unprocessable_entity }
# end
# end
Expand Down Expand Up @@ -113,7 +113,7 @@ module ActionController #:nodoc:
# if @task.save
# flash[:notice] = 'Task was successfully created.'
# else
# format.html { render "some_special_template" }
# format.html { render "some_special_template", status: :unprocessable_entity }
# end
# end
# end
Expand Down Expand Up @@ -202,7 +202,7 @@ def navigation_behavior(error)
if get?
raise error
elsif has_errors? && default_action
render rendering_options
render error_rendering_options
else
redirect_to navigation_location
end
Expand Down Expand Up @@ -236,6 +236,8 @@ def resource_location
def default_render
if @default_response
@default_response.call(options)
elsif !get? && has_errors?
controller.render(options.merge(status: :unprocessable_entity))
else
controller.render(options)
end
Expand Down Expand Up @@ -300,11 +302,11 @@ def response_overridden?
@default_response.present?
end

def rendering_options
def error_rendering_options
if options[:render]
options[:render]
else
{ action: default_action }
{ action: default_action, status: :unprocessable_entity }
end
end
end
Expand Down
31 changes: 21 additions & 10 deletions test/action_controller/respond_with_test.rb
Expand Up @@ -170,6 +170,17 @@ def test_using_resource_with_js_simply_tries_to_render_the_template
get :using_resource
assert_equal "text/javascript", @response.media_type
assert_equal "alert(\"Hi\");", @response.body
assert_equal 200, @response.status
end

def test_using_resource_for_post_with_js_renders_the_template_and_yields_unprocessable_entity_on_failure
@request.accept = "text/javascript"
errors = { name: :invalid }
Customer.any_instance.stubs(:errors).returns(errors)
post :using_resource
assert_equal "text/javascript", @response.media_type
assert_equal "alert(\"Hi\");", @response.body
assert_equal 422, @response.status
end

def test_using_hash_resource_with_js_raises_an_error_if_template_cant_be_found
Expand Down Expand Up @@ -245,13 +256,13 @@ def test_using_resource_for_post_with_html_redirects_on_success
end
end

def test_using_resource_for_post_with_html_rerender_on_failure
def test_using_resource_for_post_with_html_rerender_and_yields_unprocessable_entity_on_failure
with_test_route_set do
errors = { name: :invalid }
Customer.any_instance.stubs(:errors).returns(errors)
post :using_resource
assert_equal "text/html", @response.media_type
assert_equal 200, @response.status
assert_equal 422, @response.status
assert_equal "New world!\n", @response.body
assert_nil @response.location
end
Expand Down Expand Up @@ -305,26 +316,26 @@ def test_using_resource_for_patch_with_html_redirects_on_success
end
end

def test_using_resource_for_patch_with_html_rerender_on_failure
def test_using_resource_for_patch_with_html_rerender_and_yields_unprocessable_entity_on_failure
with_test_route_set do
errors = { name: :invalid }
Customer.any_instance.stubs(:errors).returns(errors)
patch :using_resource
assert_equal "text/html", @response.media_type
assert_equal 200, @response.status
assert_equal 422, @response.status
assert_equal "Edit world!\n", @response.body
assert_nil @response.location
end
end

def test_using_resource_for_patch_with_html_rerender_on_failure_even_on_method_override
def test_using_resource_for_patch_with_html_rerender_and_yields_unprocessable_entity_on_failure_even_on_method_override
with_test_route_set do
errors = { name: :invalid }
Customer.any_instance.stubs(:errors).returns(errors)
@request.env["rack.methodoverride.original_method"] = "POST"
patch :using_resource
assert_equal "text/html", @response.media_type
assert_equal 200, @response.status
assert_equal 422, @response.status
assert_equal "Edit world!\n", @response.body
assert_nil @response.location
end
Expand All @@ -340,27 +351,27 @@ def test_using_resource_for_put_with_html_redirects_on_success
end
end

def test_using_resource_for_put_with_html_rerender_on_failure
def test_using_resource_for_put_with_html_rerender_and_yields_unprocessable_entity_on_failure
with_test_route_set do
errors = { name: :invalid }
Customer.any_instance.stubs(:errors).returns(errors)
put :using_resource

assert_equal "text/html", @response.media_type
assert_equal 200, @response.status
assert_equal 422, @response.status
assert_equal "Edit world!\n", @response.body
assert_nil @response.location
end
end

def test_using_resource_for_put_with_html_rerender_on_failure_even_on_method_override
def test_using_resource_for_put_with_html_rerender_and_yields_unprocessable_entity_on_failure_even_on_method_override
with_test_route_set do
errors = { name: :invalid }
Customer.any_instance.stubs(:errors).returns(errors)
@request.env["rack.methodoverride.original_method"] = "POST"
put :using_resource
assert_equal "text/html", @response.media_type
assert_equal 200, @response.status
assert_equal 422, @response.status
assert_equal "Edit world!\n", @response.body
assert_nil @response.location
end
Expand Down