search

ryanb / nested_form

Rails plugin to conveniently handle multiple models in a single form.
MIT License
1.79k stars 505 forks source link

_destroy hidden field is true when validation fails #193

Closed anthonyalberto closed 12 years ago

anthonyalberto commented 12 years ago

Hello,

Just came across a problem while using this amazing gem :

I have a model with a has_many association. Let's call them Parent and Child. I have a validation on the Parent model that prevents erasing all Children from a Parent. A parent must have at least one Child.

I'm editing a Parent that has one Child. I click on the link generated by link_to_remove. It sets _destroy to true. I submit the form. Validation fails. I'm back on the form that displays the Child again since it wasn't destroyed. But _destroy is still set to true for this child. Therefore, the user has no idea that by resubmitting the form, it'll try to destroy the Child again!

hidden_field injected here : https://github.com/ryanb/nested_form/blob/master/lib/nested_form/builder_mixin.rb#L60

I think the hidden_field should force the value to false, unless there's a reason it shouldn't?

Just adding object[:_destroy] = false before line 60 should do the trick, I'll try it anyway.

I'll also be able to provide a pull request if you acknowledge the problem.

Thanks

lest commented 12 years ago

It's related to #83. Nested records are marked for destruction that is why there are _destroy fields with true value.

anthonyalberto commented 12 years ago

Ok, here's what I've done quickly : https://github.com/anthonyalberto/nested_form/blob/master/lib/nested_form/builder_mixin.rb#L60

Works for me so far, behaves as expected. Would there be any reason to output a hidden_field :_destroy set to true? My thought would be that only JS should set it to true, but I may be wrong.

Thanks

lest commented 12 years ago

In 2444edb74ed24c415928c3e559cfd8d68dc752a1 an additional class is added on the div if the object is marked for destruction so you can hide such elements using css.

anthonyalberto commented 12 years ago

Ok well, I'll stick with my branch for now, makes more sense for me to cancel the destruction of the object since it could be that it wasn't correct to destroy it in the first place and that's why the form submission failed.

Thanks!