Skip to content
This repository

Inconsistency between ActiveModel::Errors and ActiveResource::Errors #2959

Closed
dim opened this Issue · 7 comments

5 participants

Dimitrij Denissenko Isaac Sanders Carlos Antonio da Silva Steve Klabnik Uģis Ozols
Dimitrij Denissenko

Hi,

There seems to be an inconsistency between ActiveModel::Errors and ActiveResource::Errors when it comes to serialization. While XML works just fine, JSON-serialized errors seem to cause problems. Here an example:

# app/models/user.rb
class User < ActiveRecord::Base
  validates :email, :presence => true
end

# app/models/user_res.rb
class UserRes < ActiveResource::Base
end

# app/controllers/users_controller.rb
class UsersController < ApplicationController
  respond_to :xml, :json

  def create
    @user = User.create params[:user]
    respond_with @user
  end
end

Assuming we have a resource and a record:

res = UserRes.new :email => ''
rec = User.new.tap(&:valid?)

Exchanging errors between ActiveRecord and ActiveResource via XML works as expected:

res.errors.from_xml(rec.errors.to_xml)
res.errors.to_hash # => #<OrderedHash {:email=>["can't be blank"]}>

Exchanging errors between ActiveRecord and ActiveResource via JSON doesn't:

res.errors.from_json(rec.errors.to_json)
res.errors.to_hash # => #<OrderedHash {}>

Here is the reason:

# ActiveRecord::Errors#to_xml serializes full_messages
Hash.from_xml(rec.errors.to_xml) # => {"errors"=>{"error"=>["Email can't be blank"]}}

# ActiveRecord::Errors#to_json serializes the error hash
ActiveSupport::JSON.decode(rec.errors.to_json) # => {"email"=>["can't be blank"]}

In ActiveResource::Errors method #from_xml & #from_json seem to work consistently, I assume this is a bug in ActiveModel::Errors#as_json - shouldn't it be something like:

def as_json(options=nil)
  { "errors" => to_a }
end

Thanks,
D

Isaac Sanders

@dim Is this still an issue?

Carlos Antonio da Silva

ActiveModel::Errors#as_json accepts an option argument with :full_messages option, that could give you the full messages instead. I'm not sure about wrapping it in an another hash with errors, though.

@josevalim thoughts on this one?

Carlos Antonio da Silva

@josevalim do you think we could close this one?

Steve Klabnik
Collaborator

Three months with no comments. Also we're removing ActiveSupport in Rails anyway, so I'm giving it a close. @dim if you'd like this behavior changed, please submit a pull request. Thanks.

Uģis Ozols

Also we're removing ActiveSupport in Rails anyway

@steveklabnik could you share more info about this?

Steve Klabnik
Collaborator

Whoah, that's a big mistype. I meant to say "ActiveResource:" http://github.com/rails/activeresource

Uģis Ozols

Gotcha :wink:

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.