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

Handle validation errors #49

Open
jbbarth opened this issue Aug 15, 2013 · 5 comments
Open

Handle validation errors #49

jbbarth opened this issue Aug 15, 2013 · 5 comments

Comments

@jbbarth
Copy link

jbbarth commented Aug 15, 2013

It seems for now roar-rails' responder doesn't handle validation errors transparently (correct me if I'm wrong) like rails' builtin responder does, see ActionController::Responder docs. Today I write this:

def create
  singer = Singer.new(params[:singer])
  if singer.save
    respond_with singer
  else
    render json: { message: "Validation Failed", errors: singer.errors }.to_json
  end
end

but I'd like this to automatically happen (depending if resource has_errors?..) so we can write this:

def create
  respond_with Singer.create(params[:singer])
end

Could you :

  • confirm this doesn't work like that for now ? (just tested it with rails 4 / ruby 2.0)
  • tell me if this could be interesting or if it's achievable in an other way ?

If it sounds good I can work on that and submit a pull request in a few weeks.

@fltiago
Copy link
Contributor

fltiago commented Feb 12, 2014

@apotonick +1. Seems like it doesn't look for a Representer when there is a validation error. That's right?

@apotonick
Copy link
Member

That would be an ActiveModel-related extension of Roar. How would the output of the errors property look like?

@cutalion
Copy link

FYI, I've solved this with redefined render method in base controller class.
I needed one-level errors hash in my API, instead of 2-level ActiveModel::Errors hash.

{"errors" => ["full error message"]} instead of {"errors" => { "base" => ["error message"]}}

  def render(*args)
    options = args.dup.extract_options!

    if options[:json] && options[:json].is_a?(Hash) &&
      options[:json][:errors].is_a?(ActiveModel::Errors)

      options[:json][:errors] = options[:json][:errors].full_messages

      super options
    else
      super
    end
  end

@apotonick
Copy link
Member

If you want to return a generic errors document, there should be a generic Representer::JSON::Errors that kicks in when respond_with (or whatever renders) detects an error.

Introducing that, you wouldn't have to override render but could simply provide an optional errors representer.

@cutalion
Copy link

I guess it won't work with current implementation of roar-rails.
The main issue with Roar::Rails::Responder is that it gets control only on rendering model (overrides display).

Rails won't render resource if it has errors (https://github.com/rails/rails/blob/master/actionpack/lib/action_controller/metal/responder.rb#L179). It will explicitly call resource.errors, which are not decorated.

I think that we can try to decorate resource earlier to take control over errors method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants