Rails core - Sep 2009 - association_valid? error message weirdness. Should it be changed?

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

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:
https://rails.lighthouseapp.com/projects/8994/tickets/2904-avoid-copying-errors-from-child-to-parent-on-autosave

Cheers,
Eloy

On 9 sep 2009, at 19:36, Josh wrote:
>
> 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
> >

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] https://rails.lighthouseapp.com/projects/8994/tickets/3147
[2] https://rails.lighthouseapp.com/projects/8994/tickets/2904


On Wed, Sep 9, 2009 at 7:36 PM, Josh<josh.m.sharpe@gmail.com>
wrote:
>
> 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
> >
>