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

Correctly use the response's status code calling head #18275

Merged
merged 1 commit into from
Dec 31, 2014
Merged

Correctly use the response's status code calling head #18275

merged 1 commit into from
Dec 31, 2014

Conversation

robin850
Copy link
Member

Hello,

Commit 20fece1 introduced the _status_code method to fix calls to head :ok. This method has been added on both ActionController::Metal and ActionDispatch::Response.

As for the latter, this method is just equivalent to the response_code one so commit aefec3c removed it from the Reponse object so call to the _status_code method on an ActionController::Base instance would be handled by the Metal class (which Base inherits from) but the status code is not updated according to the response at this level.

The fix is to actually rely on response_code for ActionController::Base instances but this method doesn't exist for bare Metal controllers so we need to define it.

Note: the response.headers[] calls inside the tested controller's action seem awkward but they are actually needed to make sure the the else statement is reached as the Content-Length and Content-Type headers are most of the time not even filled when the request/response cycle ends.

/cc @carlosantoniodasilva @guilleiguaran

Have a nice day!

Commit 20fece1 introduced the `_status_code` method to fix calls to
`head :ok`. This method has been added on both ActionController::Metal
and ActionDispatch::Response.

As for the latter, this method is just equivalent to the `response_code`
one so commit aefec3c removed it from the `Reponse` object so call to
the `_status_code` method on an ActionController::Base instance would be
handled by the `Metal` class (which `Base` inherits from) but the status
code is not updated according to the response at this level.

The fix is to actually rely on `response_code` for ActionController::Base
instances but this method doesn't exist for bare Metal controllers so we
need to define it.
carlosantoniodasilva added a commit that referenced this pull request Dec 31, 2014
Correctly use the response's status code calling head
@carlosantoniodasilva carlosantoniodasilva merged commit 777c45e into rails:master Dec 31, 2014
@carlosantoniodasilva
Copy link
Member

Thanks!

carlosantoniodasilva added a commit that referenced this pull request Dec 31, 2014
Correctly use the response's status code calling head
@guilleiguaran
Copy link
Member

@robin850 great work!!!

@robin850
Copy link
Member Author

Thank you too guys! :-)

@robin850 robin850 deleted the head-status branch December 31, 2014 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants