Validation of autosaved associations is broken

At first I had indeed implemented it so that one could change the option on the reflection and it would automatically be enabled. But the speed improvement of not doing that was big enough for me to decide against it.

We could add methods that one can call to enable validation, but it would need some thought. So, what's your use case?

I'm sorry, I don't quite understand: I'm reporting a regression, not requesting a feature. Autosaved associations should be validated, and accepts_nested_attributes_for automatically enables autosave for its associations. It's what the tests were doing before this change: declaring an association, calling accepts_nested_attributes_for on that association (as in Pirate and Ship), and then expecting that association to behave like any other autosaved association (as in test_should_automatically_validate_the_associated_model, test_should_merge_errors_on_the_associated_model_onto_the_parent_even_if_it_is_not_valid etc).

The change in #3161 leaves accepts_nested_attributes_for associations, or any other associations which have been set to autosave after their initial declaration, in a weird halfway state whereby they do still get autosaved (because autosaving is triggered by the :autosave option at save time) but don't get validated (because validation is triggered by the :autosave option at declaration time). And as the ActiveRecord::AutosaveAssociation docs say:

Validation is performed on the parent as usual, but also on all autosave enabled associations. If any of the associations fail validation, its error messages will be applied on the parents errors object and validation of the parent will fail.

For example, any applications which were relying on the correct behaviour of accepts_nested_attributes_for, i.e. enabling both autosaving and validation, will now be broken. So I guess that's a use case.

Ping.

For now it'd be better just to revert this commit than to leave autosaved associations broken in 2-3-stable. Any regression-free performance improvements can be applied later.

I agree with Tom that the commit in question should be reverted. And I'll go one step further: I've got several models with before_validation callbacks that are not longer being invoked. Optimizing the validations into nothingness is one thing, but skipping the validation callbacks is a whole new layer of surprise.

I don't think the best course of action is reverting the commit but instead fixing the shortcomings of the speedup.

Tom and Chris, it would be great if you could supply a patch for that.

(from [f125a34501e21b1e0da2b80d149df7a739482804]) Define autosave association callbacks when using accepts_nested_attributes_for.

This way we don't define all the validation methods for all associations by
default, but only when needed.

[#3355 state:resolved] http://github.com/rails/rails/commit/f125a34501e21b1e0da2b80d149df7...