diff --git a/CHANGELOG.md b/CHANGELOG.md index f6c029b..add2428 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index 7cd3cbf..f8b28f9 100644 --- a/README.md +++ b/README.md @@ -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 ``` diff --git a/lib/action_controller/respond_with.rb b/lib/action_controller/respond_with.rb index 7d3d01e..1da4a05 100644 --- a/lib/action_controller/respond_with.rb +++ b/lib/action_controller/respond_with.rb @@ -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 :new for a - # +post+ request or :edit for +patch+ or +put+. + # +post+ request or :edit for +patch+ or +put+, + # and the status is set to 422 Unprocessable Entity. # Thus an example like this - # # respond_to :html, :xml @@ -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 @@ -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 :status 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 " \ diff --git a/lib/action_controller/responder.rb b/lib/action_controller/responder.rb index 7d91465..5751458 100644 --- a/lib/action_controller/responder.rb +++ b/lib/action_controller/responder.rb @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 diff --git a/test/action_controller/respond_with_test.rb b/test/action_controller/respond_with_test.rb index dc4bdc5..7b7c267 100644 --- a/test/action_controller/respond_with_test.rb +++ b/test/action_controller/respond_with_test.rb @@ -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 @@ -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 @@ -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 @@ -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