nesquena / rabl

General ruby templating with json, bson, xml, plist and msgpack support
http://blog.codepath.com/2011/06/27/building-a-platform-api-on-rails/
MIT License
3.65k stars 335 forks source link

Rails, respond_with and status #88

Closed Dagnan closed 13 years ago

Dagnan commented 13 years ago

Hi.

When creating an object using respond_with, it renders the json or xml with status 201 (Created). I think with rabl it just renders the template with the status code 200.

Is it a bug or a normal behavior? And is there a way to merge both behaviors?

Dagnan commented 13 years ago

I was able to perform what I'm talking about in an initializer, maybe it can help seeing the big picture:

ActionController::Responder.class_eval do
  def to_format
    if has_errors?
      api_behavior(nil) and return
    end

    controller.response.status = :created if post? && ! has_errors?

    default_render
  rescue ActionView::MissingTemplate => e
    api_behavior(e)
  end
end

Now I understand a lot better Rails view stack. As for my problem, the culprit is this line in mime_responds.rb:

collector = Collector.new { default_render }

which calls the default view without options (I guess this is a functionality and not a bug since it's name is "default_render").

haxney commented 13 years ago

You can also achieve this by creating a subclass of ActionController::Responder and passing it to respond_with like this:

class MyResponder < ActionController::Responder
  def to_format
    case
    when has_errors?
      controller.response.status = :unprocessable_entity
    when post?
      controller.response.status = :created
    end

    binding.pry
    default_render
  rescue ActionView::MissingTemplate => e
    api_behavior(e)
  end
end

class UsersController < ApplicationController
  respond_to :json, :xml, :html

  def update
    @user.update_attributes(params[:user])
    respond_with(@user, :responder => MyResponder)
  end
end

This allows you to swap in and out responders as necessary, and avoids having to monkey-patch ActionController::Responder. If you want that behavior by default, you can add self.responder = MyResponder to your controllers or even your ApplicationController.

It might be worth including something like this in Rabl out of the box, since it is rather surprising behavior.

nesquena commented 13 years ago

Interesting, thanks for sharing the code snippet. I could see something like this being very useful. I typically use Rabl in Padrino and Rails 2.3 just because that's where my apps are which is why something like this isn't there by default. This is something I think would go well in the wiki though (added just now): https://github.com/nesquena/rabl/wiki/Response-Codes.

Dagnan commented 13 years ago

Thanks Haxney, this is certainly cleaner your way :)

haxney commented 13 years ago

Whoops, there shouldn't be that binding.pry in there. I had been using Pry for testing purposes and forgot to take it out!

If you want this behavior throughout all of your controllers, you can simply put self.responder = MyResponder in your ApplicationController, so that you don't have to specify respond_with(@user, :responder => MyResponder) in every function in your controllers.

Govinda-Fichtner commented 12 years ago

In the solution above I missed the errors in case something goes wrong. In the original responders there is this display_errors method which renders the errors.

class ApiResponder < ActionController::Responder
  def to_format
    case
    when has_errors?
      controller.response.status = :unprocessable_entity
      display_errors
    when post?
      controller.response.status = :created
      default_render
    end

  rescue ActionView::MissingTemplate => e
    api_behavior(e)
  end
end

It might be also a good idea to add this to the wiki page. It took me a while to find this page and to find out that it is RABL's fault that I get a 200 instead of a 201 for a create action and not mine. What is the reason that RABL doesn't do the above by default?

mxrguspxrt commented 12 years ago

For not having to change controller code, I use config/initializers/rabl_fix_status_codes.rb

class ActionController::Responder

  alias :to_format_original :to_format

  def to_format
    if post? || !has_errors?
      controller.response.status = :created
    end
    to_format_original
  end

end
ansel1 commented 11 years ago

can use super too. And don't need to check for has_errors?. controller.response.status will be overwritten later in default to_format

In your application controller:

class MyAppResponder < ActionController::Responder
  def to_format
    controller.response.status = :created if post?
    super
  end
end

self.responder = MyAppResponder

super will change the status code to :unprocessable_entity if necessary. This code just sets the default status, which can still be overridden by render options, respond_with options, etc.

nesquena commented 11 years ago

If anyone wants to update /improve this page https://github.com/nesquena/rabl/wiki/Response-Codes that would helpful because it's where I point people who are interested in a 'smart' rabl responder.