RFC: associations with :accessible => true should allow updating

rubyonrails-core
21

The big issue I see is finding the associated objects that need
saving. AR isn't good at dealing with partially loaded has_many
association collections. I think you'd need a new API on associations
to enumerate the already loaded records and see which are dirty and
need saving.

It seems like some form of cascading save is important if :accessible => true is going to apply to updates, given that those updates need to be in the same transaction. Perhaps a simplistic API to tell AR about related objects would be a good stepping-stone to a full solution:

p = Project.find(some_id) task = p.tasks.find_by_name("send invoice") task.due_date = 3.days.from_now p.include_in_save(task) # <--- p.save

In the case of a mass-attribute assignment, #include_in_save could be called automatically.

Tom

22

Hi Guys,

I'm a bit late to join this conversation, and honestly I haven't read it very closely yet. But I wrote something like this a few months ago and attempted to address a lot of the edge cases and issues (updating existing rows, etc). I have tests for it but they are integrated too tightly with my application so I'll just give you the code to the plugin. I'm using Javascript heavily with the forms so I haven't thought much about how this might fit in with rails form helpers but I don't imagine there would be much work.

http://gist.github.com/1612

Best, Kris Rasmussen Rivalmap

23

I would also love to see something like this make it into core.

I had half-implemented a crappy version of this myself for my app, by monkey-patching the setter methods generated by the has_many association (e.g. "contacts=") to accept a hash.

The main problem I ran into was with creating new records with several nested levels of association, e.g. { "school" => {     "contact" => [         { "name" => "abc",           "telephone" => [             { "area_code" => "nnn", "number" => "nnn" },             { "area_code" => "nnn", "number" => "nnn" } ] } ] } }

i.e. There is a single form for a school, from which you can add/edit/ delete contacts and their associated telephones. Each school has_many contacts and each contact has_many telephones.

The above hash should work perfectly; however it is impossible to generate that hash from a form with the existing parameter-parsing mechanism! See #127 Params hash is incorrect for nested arrays - Ruby on Rails - rails

Updating existing contacts/telephones works ok, because you don't have nested arrays but nested hashes keyed by (if you're using the technique from Ryan Bates' recipe) the record's id, e.g. "contact" => {     "15" => {         "name" => "abc",         "telephone" => {             "33" => { ... },             "34" => { ... } } } }

My workaround was to always use nested hashes, but keyed by a loop index (0, 1, 2, etc) and differentiating new records from existing records by the absence or presence of the id parameter.

A *very* nice-looking solution that builds on this is Thomas Lee's post here: http://groups.google.com/group/rubyonrails-core/browse_thread/thread/370ce12a7ac656eb

where parsing "a[b][0][c]=6" yields {"a" => {"b" => [{"c" => "6"}]}} instead of the current {"a" => {"b" => {"0" => {"c" => "6"}}}}

24

FWIW, I'm happy to write a patch for this -- the plugin was simply to get feedback on a potential implementation and the semantics. Similar patches have been rejected in the past because they changed the semantics and just tried to "work around" the existing broken parser code. A patch based off the plugin would both simplify the parser implementation and fix the "array" semantics.

Since I haven't really seen much outside of your response and Koz's, I assumed that either nobody has any feedback or nobody has really run into this problem and so doesn't particularly care. I find both hard to believe -- I know how opinionated the ruby community can get ;).

I understand it's a subtle change, but I would have thought the scenario you just described is relatively common. I know we've run into it more than once, and it was the reason the plugin came about in the first place.

In any case, thanks for putting in your two cents.

Cheers, T

Dave Rothlisberger wrote:

25

+ 1 for keeping and improving mass assignment support in core.

In fact I find that the initial checkin (e0750d6a5c7f621e4ca12205137c0b135cab444a) works quite well.

I've swapped out a few custom association_attributes= style sections (ala Ryan Bates complex forms style) for the new accessible => true support and it really seems to work great - even with updates and deletes.

Best,

Zack

26

What about has_many associations for Rails models containing more than one field?

Cheers, T

Zack Chandler wrote:

27

Sorry, I should clarify:

What about has_many associations to models containing more than one field?

e.g.

class Student < ...   # has a name, age, year_level end

class School < ...   has_many :students end

Cheers, T

Thomas Lee wrote:

28

My workaround was to always use nested hashes, but keyed by a loop index (0, 1, 2, etc) and differentiating new records from existing records by the absence or presence of the id parameter.

I've also found a need to implement this in the past, so definitely +1 for

A *very* nice-looking solution that builds on this is Thomas Lee's post here:http://groups.google.com/group/rubyonrails-core/browse_thread/thread/…

where parsing "a[b][0][c]=6" yields {"a" => {"b" => [{"c" => "6"}]}} instead of the current {"a" => {"b" => {"0" => {"c" => "6"}}}}

Tom

29

A *very* nice-looking solution that builds on this is Thomas Lee's post here:http://groups.google.com/group/rubyonrails-core/browse_thread/thread/…

where parsing "a[b][0][c]=6" yields {"a" => {"b" => [{"c" => "6"}]}} instead of the current {"a" => {"b" => {"0" => {"c" => "6"}}}}

The main concern I have is the obvious backwards compatibility and security issues which were covered in that email. We have an, allegedly, pluggable parameter parsing scheme. It should be possible to plug this behaviour in without using a patch? At least that'd let us test the implications of making the changes.

30

Michael Koziarski wrote:

A *very* nice-looking solution that builds on this is Thomas Lee's post here:http://groups.google.com/group/rubyonrails-core/browse_thread/thread/…

where parsing "a[b][0][c]=6" yields {"a" => {"b" => [{"c" => "6"}]}} instead of the current {"a" => {"b" => {"0" => {"c" => "6"}}}}        The main concern I have is the obvious backwards compatibility and security issues which were covered in that email. We have an, allegedly, pluggable parameter parsing scheme. It should be possible to plug this behaviour in without using a patch? At least that'd let us test the implications of making the changes.

Spot on. That's exactly why it's a plugin rather than a patch. :slight_smile:

Cheers, T

31

Just an FYI, I've used Ryan Bate's complex forms before and it works great with textfields and textareas; however, it breaks with radiobuttons and checkboxes with multiple options.

Example, if u have the following form code for radio buttons:

<%- fields_for "survey[answer_attributes]", answer do |answer_form| -%>   <%= answer_form.hidden_field :question_id %>   <%= answer_form.radio_button :content, 'yes' %>   <%= answer_form.label :content, 'yes' %>   <br/>   <%= answer_form.radio_button :content, 'no' %>   <%= answer_form.label :content, 'no' %> <%- end -%>

Yields these input fields

<input id="survey_answer_attributes__content_no" name="survey[answer_attributes][content]" type="radio" value="no" /> <label for="cart_item_survey_attributes__answer_attributes__content_no">no</

<br/> <input id="survey_answer_attributes__content_yes" name="survey[answer_attributes][content]" type="radio" value="yes" /

<label for="cart_item_survey_attributes__answer_attributes__content_yes">yes</

And when it post you see that the 'no' radio button isn't associated..

"answer_attributes"=>[   {"question_id"=>"1", "content"=>"11111"},   {"content"=>"0"} ]

Not to mention if you have multiple input fields in the form it bumps the content value into the next hash. So if you had a textfield with question_id = 2 just below the radio buttons the params would be like this:

"answer_attributes"=>[   {"question_id"=>"1", "content"=>"11111"},   {"question_id"=>"2", "content"=>"0"},   {"content"=>"text field text"} ]

Would love to know if anyone has a solution for this...

Ryan

32

Your particular issue is probably better suited for the general mailing list, since it doesn't speak to an issue in core. Checkout how I solved it at http://github.com/zilkey/complex_forms_demo_app/tree/master, you can follow up with me there with any questions.