Nested object validations return incorrect attribute IDs

Looks like my formatting got wrecked, and an entire code block has vanished. I apologize. The rendered field looks like:

input id="user_address_attributes_city" name="user[address_attributes][city]" size="30" type="text"


Hi Yuval,

thanks for your first report.
This is a clear problem that I experienced myself.

The question I was asking myself when reading this was whether it makes more sense to alter the errors attribute or rather change the code that does the highlighting of fields with errors.

What do you guys think?

Thanks Pascal. IMO, the error collection should absolutely return a correct representation of the attributes that failed. That's how it works in non-nested cases. Otherwise we're left doing brute force subs on our error handlers to look for specific objects that have been nested. It gets even worse if they are nested in some forms and not on others.

I am updating this ticket because the problem has become even worse in Rails 2.3.5. Nested attributes are now coming back with a period instead of an underscore. For instance, going back to the original post where user accepts nested attributes for address, I now get address.city as the failed attribute, rather than address_city. This is not only just weird, it breaks simple unit tests such as assert user.errors.invalid?(:billing_address_city)

Which Rails version this happened please?

I didn't run Rails 2.3.4 because of other bugs in that release, so I can't say if the erroneous period issue happened then or in 2.3.5. It definitely did not happen in 2.3.3.

As it stands, in 2.3.5 you can't even do basic unit tests on nested objects. In 2.3.3, with the code in the original post I could do a unit test such as this:

@@@ test "create with invalid nested billing address" do

user = User.new(:email_address => 'foo@bar.com', :first_name => 'Foo', :last_name => 'Bar', :password => 'foo',
  :billing_address_attributes => {})
assert !user.valid?
assert user.errors.invalid?(:address_line_1)
assert user.errors.invalid?(:address_city)
assert user.errors.invalid?(:address_postal_code)
assert user.errors.invalid?(:address_country_code)
assert !user.billing_address.valid?
assert !user.save

end

But this is no longer possible, as the attributes are being handled internally as address.line_1 etc.

Sorry for the formatting issues. And "billing_address" above should be "address".

As far as I know, this is feature. You should use :"address.line_1" instead of :address_line_1. And I will try to explain why.

On Rails < 2.3.5, when working with I18n, if you had a customized messages for Address, and then it becomes a nested association, you needed to customized them in two places:

en:
  activerecord:
    errors:
      models:
        address:
          attributes:
            line_1:
              invalid: some special error
            city:
              invalid: another special error
        user:
          attributes:
            address_line_1:
              invalid: some special error
            address_city:
              invalid: another special error

Now, if we use "." instead of "_", it becomes like this:

en:
  activerecord:
    errors:
      models:
        address:
          attributes:
            line_1:
              invalid: some special error
            city:
              invalid: another special error
        user:
          attributes:
            address:
              line_1:
                invalid: some special error
              city:
                invalid: another special error

It does not sounds like a big difference, unless we take into account the features available in the last I18n gem version, which actually allow us to do this:

en:
  activerecord:
    errors:
      models:
        address:
          attributes:
            line_1:
              invalid: some special error
            city:
              invalid: another special error
        user:
          attributes:
            address: :"activerecord.errors.models.address.attributes"

Your yml is DRYer and you likely won't make mistakes. Everything should work as expected. You should be able to install i18n >= 0.3.3 in a project and make use of such features. If you don't, then we will really have a bug.

About the f.fields_for, it was supposed to work because it will look for the error in the @user.address.first.errors, instead of @user.errors. I will try to reproduce and create a patch.

Thanks for the update José, that's what I thought too. But looking at the way one has to use a symbol, do you think it would be feasible to allow a string as well?

Ie: "address.line_1" instead of :"address.line_1"

I don't know if we can use strings. This is completely up to ActiveRecord#Errors implementation.

To me, :"address.line_1" is extremely bizarre notation. And by the time your error makes it to the front end, you have to do onerous transformations just to figure out what's in error.

For instance (and I'm going to try the formatting again, but I'll be damned if I can ever get it to work as intended):

Here are the models.

user.rb
class User < ActiveRecord::Base
  has_one :address
  accepts_nested_attributes_for :address
end

address.rb
class Address < ActiveRecord::Base
  validates_presence_of :line_1
end

So we have a user object that accepts nested attributes for an address object, and that address object has validations.

Here's the controller that creates a new user with a nested address:

def create
  user = User.new(params[:user])
  render :update do |page|
  if user.save
    page.redirect_to user
  else
    show_errors(page, user)
  end
end

So let's say line_1 was left blank. In my error handler, we look at the user model and output all the errors in whatever fashion we want. In my case, the meaty part looks something like this. Note the substitutions that are required just to obtain the correct fields from the nested address object.

def show_errors(page, obj)
  obj.errors.each do |attr, msg|
  model_prefix = obj.class.name.underscore.downcase + "_"
  # at this point, the attribute is being incorrectly returned as *address.line_1* instead of matching the rendered field id of *user_address_attributes_line_1*
  # note that if first_name was required on the user object and left blank, we would correctly receive *first_name* as the attribute
  # So now we are required to substitute an underscore for the period and also to do a very specific sub to find an attribute beginning with *address* so that *_attributes* can be added onto it.
  attr = model_prefix + s.sub('.', '_').sub(/(address)/,'\1_attributes')
  # Then we flag the field and show a message
end

Now obviously, this is a custom error handler. But you get my point. What's happening on the model side is not at all being represented on the controller/form side. Hope that clarifies.

Really wish I could edit ticket entries. :) s above is supposed to be attr.

Yes, I see your point. I was not the one who planned to use "user_address_attributes_line_1" instead of "user_address_line_1", so I cannot tell what motivated that difference in the first place. But regarding to "address.line_1" in error messages, I think this is an improvement compared with what we had before. I discussed several options extensively with different people and we settled on it. So I'm -1 for changing it unless you come up with a good solution. ;)

Ok, the reason for having params[:user][:address_attributes] instead of params[:user][:address] is because the second would do "user.address = params[:user][:address]", which wouldn't work. That's why we need to do: "user.address_attributes = params[:user][:address_attributes]".

So we could change errors messages to be: user.errors[:"address.attributes.line_1"]. But this is not getting anywhere prettier. Again, we need the dots so we can DRY up I18n files.

By the way, the dot actually helps you, because all you need to do (in Rails 2.3.5) is replace the '.' by '_attributes_'. Consider a relationship with underscore:

class Post
  has_one :super_user
end

And the error message like:

post.errors["super_user.special_comment"] #=> can't be blank

In your helper, to find the proper field, you can simply replace the dot. However, considering Rails 2.3.3 approach, this would be impossible, because there is nothing telling where the relationship starts and where the attribute start:

post.errors["super_user_special_comment"] #=> can't be blank

You would need to track the relationships, which is just more complicated.

This pretty much won't fix unless a good solution is proposed and we already tried several options, you can check tickets #2210 Validation on associations can't process I18n aware error message and #2904 Avoid copying errors from child to parent on autosave for some background. Does it make sense?

It does to me, imo the ticket can be closed.

I still feel like there's a disconnect here, especially with basic unit testing. Thanks anyway for your time. Happy new year!