Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Default responder doesn't honor the overwritten response block #4796

Closed
sikachu opened this Issue · 5 comments

3 participants

Prem Sichanugrist Santiago Pastorino José Valim
Prem Sichanugrist
Collaborator

Consider this snippet:

def create
  @user = User.create(params[:user])
  respond_with(@user) do |format|
    format.json do
      if @user.errors.blank?
        render :json => @user
      else
        render :json => @user.errors
      end
    end
  end
end

The given block will get passed into the responder as a :default_response(https://github.com/rails/rails/blob/v3.2.1/actionpack/lib/action_controller/metal/mime_responds.rb#L231-240) Hoever, it will not get called due to the fact that to_format does not honor the :default_response at all (https://github.com/rails/rails/blob/v3.2.1/actionpack/lib/action_controller/metal/responder.rb#L174-182). I think we should fix this to allow the override.

Setting this to 3.2.2, and I will work on this.

Prem Sichanugrist sikachu was assigned
Santiago Pastorino
Owner

@sikachu can you explain better what doesn't work here?. I've just tested and works fine for me

José Valim
Owner

@sikachu if you are passing the json block, in theory the responder won't even be invoked for json requests.

José Valim
Owner

Hrm, I am wrong. Yeah, we should probably fix this in a way the responder won't even be invoked.

Prem Sichanugrist
Collaborator

@spastorino it should not work for you. I have a failing test case which I'm going to include with my patch.

@josevalim yep, we have this problem ourselves where responder just ignore our overridden block. We want to return the errors without the root "errors" object, so this is annoying.

Prem Sichanugrist sikachu referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
Prem Sichanugrist sikachu referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
Santiago Pastorino
Owner

Ahh I see it depends if the resource has errors or not

Prem Sichanugrist sikachu referenced this issue from a commit in sikachu/rails
Prem Sichanugrist sikachu Fix override API response bug in respond_with
Default responder was only using the given respond block when user
requested for HTML format, or JSON/XML format with valid resource. This
fix the responder so that it will use the given block regardless of the
validity of the resource. Note that in this case you'll have to check
for object's validity by yourself in the controller.

Fixes #4796
3def1c8
Prem Sichanugrist sikachu referenced this issue from a commit in sikachu/rails
Prem Sichanugrist sikachu Fix override API response bug in respond_with
Default responder was only using the given respond block when user
requested for HTML format, or JSON/XML format with valid resource. This
fix the responder so that it will use the given block regardless of the
validity of the resource. Note that in this case you'll have to check
for object's validity by yourself in the controller.

Fixes #4796
567ac65
Prem Sichanugrist sikachu closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.