association_valid? error message weirdness. Should it be changed?

rubyonrails-core
1

This line o' code in autosave_association.rb is a little bit weird imo.

attribute = "#{reflection.name}_#{attribute}"

Line 252 on master at the moment.

It's used in at least two places I can tell (I haven't looked very hard), accepts_nested_attributes and validates_associated. The string that is generated is then applied to #errors on the parent record as the base name. So, for example:

class Profile < AR::Base   has_one :address   accepts_nested_attributes_for :address end

class Address < AR::Base   validates_presence_of :city end

p = Profile.new p.build_address p.save p.errors.full_messages # => ['Address City is invalid']

#or something like that.

The issue is that "Address City" is meaningless to our users. How many of you label your "City" text field as "Address City" on your "Profile" form?

Was this added in order to disambiguate if the same attribute exists on both the parent and the child? If so, can we do away with that? We can't save bad developers from themselves.

Changing the above line to

attribute = "#{attribute}"

only breaks a handful of tests and only in TestNestedAttributes and TestAutosaveAssociation, so I don't think it's widely used.

Thoughts?

Thanks, Josh

2

Hey Josh,

I think this patch by José Valim, which is in my queue to work on this
week, will solve this problem for you: #2904 Avoid copying errors from child to parent on autosave - Ruby on Rails - rails

Cheers, Eloy

3

This, and issues relating to it in one way or another, was recently discussed in ticket #3147 [1] and the proposed solution there was backporting ticket #2904 [2] to 2-3-stable.

#2904 should get rid of the quite confusing "#{reflection.name}_#{attribute}" "hack" and solve a few issues in 2.3.4 (unannounced changes I18N-keys for error messages in autosave associations, broken value-interpolation in error messages for autosave associations). There's some more details and examples available in the lighthouse tickets.

What do you think about our conclusions?

-- Anders Carling

[1] #3147 ActiveRecord::Error#full_message broken when used with I18n and autosave associations - Ruby on Rails - rails [2] #2904 Avoid copying errors from child to parent on autosave - Ruby on Rails - rails