Nested attribute setters can be overridden. #2945

Merged
merged 1 commit into from Mar 30, 2012

7 participants

@Peeja

We needed to use some of the features of accepts_nested_attributes_for (such as :_destroy) but we needed to filter the incoming attributes. This patch puts the methods accepts_nested_attributes_for defines in a module which can sit behind a custom implementation so that it can call super.

@josevalim
Ruby on Rails member

I like this. However, instead of creating a new module, can't we simply use the original attributes module?

@Peeja

I don't think that's wise. ActiveModel::AttributeMethods owns that module, and this is independent of that system. For instance, calling attribute_method_prefix (or the other similar declarations) undefines all of the methods in that module. It would blow these away too, and not know to redefine them.

@josevalim
Ruby on Rails member

Good point. /cc @jonleighton @tenderlove

@jonleighton
Ruby on Rails member

we should use the generated_feature_methods module

@carlosantoniodasilva
Ruby on Rails member

@Peeja any possibility of changing the pull request to use the generated_feature_methods module as per @jonleighton suggestion? Thanks.

@Peeja

Yep, it's been sitting on my list for...boy, 2 months? Yeah, I'll get on it.

Jonathan Mukai & Peter Jaros Nested attribute setters can be overridden.
Overriding implementation can call super.
135d704
@Peeja

Updated, but I'm having trouble getting the pull request to register the change. Please stand by.

@Peeja

Commenting seems to have done it. :)

Question: Now NestedAttributes depends on Core (for generated_feature_methods). Is that kosher? Or does that dependency need to be explicit somehow (like includeing Core, which doesn't seem right either).

@carlosantoniodasilva
Ruby on Rails member

I think we can just think that Core should always be there. @jonleighton any thought? Thanks.

@jonleighton
Ruby on Rails member

Yeah that's fine. At the moment, the use of modules in AR is mainly for code organisation really - there are lots of dependencies between different modules. I'll merge this, thanks!

@jonleighton jonleighton merged commit 3a8c543 into rails:master Mar 30, 2012
@davisre

Is there any chance of getting this into 3.2? There's a workaround, but calling super would be much nicer than calling assign_nested_attributes_for_collection_association. And we already have it for associations and normal attributes.

@TylerRick

+1 for backporting to 3.2...

In the meantime, you can always do what I did and just copy and paste the fix into your project. Yay.

@rafaelfranca
Ruby on Rails member

I don't think we should backport this since it is more a new feature that a bug fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment