This project is archived and is in readonly mode.
ActiveResource validation error not recognized as belonging to a certain field
Reported by Yuri | March 13th, 2009 @ 03:25 PM | in 3.x
If the server side ActiveRecord class contains a validates_presence_of constraint, and this is exposed as a resource, saving a new ActiveResource instance without providing this obligatory attribute will result in the validation error assigned to base instead of this attribute.
Explaining in more details:
Underlying ActiveRecord
class Shop < ActiveRecord::Base
validates_presence_of :name
end
Creating an ActiveResource:
class Shop < < ActiveResource::Base
self.site = ...
end
shop = Shop.new(:some_other_attribute => 'foo')
shop.save #=> false, server side validation failed
obj.errors.on(:name) #=> nil !!!
The incoming XML containing the output of @my_shop_active_record.errors.to_xml is parsed incorrectly and such validation error is placed to base instead of being recognized as specific to a certain attribute.
The reason lies in the following code from validation.rb (activeresource/lib/active_resource/validations.rb):
# Grabs errors from the XML response.
def from_xml(xml)
clear
humanized_attributes = @base.attributes.keys.inject({}) { |h, attr_name| h.update(attr_name.humanize => attr_name) }
messages = Array.wrap(Hash.from_xml(xml)['errors']['error']) rescue []
messages.each do |message|
attr_message = humanized_attributes.keys.detect do |attr_name|
if message[0, attr_name.size + 1] == "#{attr_name} "
add humanized_attributes[attr_name], message[(attr_name.size + 1)..-1]
end
end
add_to_base message if attr_message.nil?
end
end
The problem with this code is that while parsing the xml it relies on the initial hash submitted to the constructor of the ActiveResource instance, iterating through humanized_attributes. All other errors go to :base.
I expect obj.errors.on(:name) to return the validation error and I think this would be the correct behavior.
I monkey-patched ActiveResource and ActiveRecord to fix this.
First, dealing with a structured format like XML we can perfectly embed any info into XML attributes and the just parse it, instead of (1) Humanizing the name of the attribute (2) Appending it to the error message (what for?) (3) Applying some perlish string manipulations on the client ActiveResource side trying to guess which attribute this error refers to
Here's first my fix for AR to add field name to error.to_xml
module ActiveRecord
class Errors
def each_attribute_and_message(options = {})
@errors.each_key do |attr|
@errors[attr].each do |message|
next unless message
if attr == "base"
yield [:base, message]
else
yield [attr, message]
end
end
end
end
def to_xml(options={})
options[:root] ||= "errors"
options[:indent] ||= 2
options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent])
options[:builder].instruct! unless options.delete(:skip_instruct)
options[:builder].errors do |e|
each_attribute_and_message do |attr, msg|
e.error(msg, :attr => attr)
end
end
end
end
end
The generated xml looks like:
<?xml version="1.0" encoding="UTF-8"?>
<errors>
<error attr="name">shop.error.presence_of_name</error>
</errors>
And now let's rewrite from_xml to parse it:
module ActiveResource #:nodoc:
class Errors #:nodoc:
def from_xml(xml)
clear
REXML::XPath.each( REXML::Document.new(xml), "//error") do |element|
attrs = element.attributes
if attrs['attr']
add attrs['attr'], element.text
else
add_to_base element.text
end
end
end
end
end
Now obj.errors.on(:name) will return the appropriate error message.
I don't know if there are any standards as to how XML is parsed in Rails, you might feel like changing it, fine.
Also, one might notice that my to_xml does not append the humanized name of the field to the message, it's just that I consider this as confusing redundancy, I need the message to be exactly as I specified it in ActiveRecord, but that's not the main point of this bugreport.
Comments and changes to this ticket
-
Taryn East July 27th, 2010 @ 05:49 PM
Neat solution.
In from_xml - I'd do a check to make sure we have the attribute before trying to add an error onto it (or preferably, auto-add the attribute if it doesn't exist yet).
May run into backwards-compatibility issues though - what if we're talking to an old Rails app (that still spits out errors the old way) ? Do we need to try your way first, then also check for an attribute-name, just to maintain existing functionality?
-
Santiago Pastorino February 9th, 2011 @ 12:31 AM
- State changed from new to open
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino February 9th, 2011 @ 12:31 AM
- State changed from open to stale
-
csnk May 18th, 2011 @ 08:24 AM
We are the professional uniforms manufacturer, uniforms supplier, uniforms factory, custom uniforms.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Tags
Referenced by
- 1472 ActiveResource errors.from_xml Can't handle symboled attribute keys because of humanize call I have just posted a bug report where the cause of the pr...