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

[David Dollar]

Recently the following patch

http://github.com/rails/rails/commit/e0750d6a5c7f621e4ca12205137c0b135cab444a

was committed to rails trunk. This patch allows for associations to be
flagged as :accessible => true and then hydrated from nested hashes
(i.e. nested forms)

class Post < ActiveRecord::Base
belongs_to :author, :accessible => true
has_many :comments, :accessible => true
end

post = Post.create({
:title => 'Accessible Attributes',
:author => { :name => 'David Dollar' },
:comments => [
{ :body => 'First Post!' },
{ :body => 'Nested Hashes are great!' }
]
})

post.comments << { :body => 'Another Comment' }

I have done some work on another patch to allow this same mechanism to
be used for updating existing rows using nested hashes. This completes
the work as far as dynamically generating forms and being able to
simply and intuitively push them back into the database.

http://github.com/ddollar/rails/commit/14a16844bbb3ba9edb14269ce2d0b61c9a43637e

This allows for the following:

# create from a basic hash
> p = Post.create(:title => 'Test Post', :author => { :name => 'David' })
=> #<Post id: 8, author_id: 8, title: "Test Post", created_at:
"2008-07-14 15:22:53", updated_at: "2008-07-14 15:22:53">

# update a singular reference
> p.author = { :name => 'Joe' }
=> {:name=>"Joe"}

# it 'updates' the row in sql, notice the id is still the same
> p.author
=> #<Author id: 8, name: "Joe", created_at: "2008-07-14 15:22:53",
updated_at: "2008-07-14 15:23:03">


# create an author with posts from a hash
> a = Author.create(:name => 'David', :posts => [ { :title => 'Post 1' }, { :title => 'Post 2' } ])
=> #<Author id: 14, name: "David", created_at: "2008-07-14 15:38:18",
updated_at: "2008-07-14 15:38:18">

# show the posts
> a.posts
=> [#<Post id: 17, author_id: 14, title: "Post 1", created_at:
"2008-07-14 15:38:18", updated_at: "2008-07-14 15:38:18">, #<Post id:
18, author_id: 14, title: "Post 2", created_at: "2008-07-14 15:38:18",
updated_at: "2008-07-14 15:38:18">]

# use << to update existing entries (as well as add new ones,
demonstrated later)
> a.posts << { :id => 17, :title => 'Post 1 Updated' }
=> [#<Post id: 17, author_id: 14, title: "Post 1 Updated", created_at:
"2008-07-14 15:38:18", updated_at: "2008-07-14 15:38:53">, #<Post id:
18, author_id: 14, title: "Post 2", created_at: "2008-07-14 15:38:18",
updated_at: "2008-07-14 15:38:18">]

# show posts to verify the update
> a.posts
=> [#<Post id: 17, author_id: 14, title: "Post 1 Updated", created_at:
"2008-07-14 15:38:18", updated_at: "2008-07-14 15:38:53">, #<Post id:
18, author_id: 14, title: "Post 2", created_at: "2008-07-14 15:38:18",
updated_at: "2008-07-14 15:38:18">]

# can't update posts that don't belong to the author
> a.posts << { :id => 1, :title => 'Not Allowed' }
ActiveRecord::RecordNotFound: Couldn't find Post with ID=1 AND
("posts".author_id = 14)
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/base.rb:1393:in `find_one'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/base.rb:1376:in `find_from_ids'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/base.rb:537:in `find'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/associations/association_collection.rb:
47:in `find'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/associations/association_collection.rb:
103:in `<<'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/associations/association_collection.rb:
99:in `each'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/associations/association_collection.rb:
99:in `<<'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/connection_adapters/abstract/
database_statements.rb:66:in `transaction'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/transactions.rb:79:in `transaction'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/transactions.rb:98:in `transaction'
from /Users/ddollar/Code/EdgeRailsApp/vendor/rails/
activerecord/lib/active_record/associations/association_collection.rb:
98:in `<<'
from (irb):12

# use = to outright replace all posts
> a.posts = [ { :title => 'Replace Posts' } ]
=> [#<Post id: 19, author_id: 14, title: "Replace Posts", created_at:
"2008-07-14 15:40:30", updated_at: "2008-07-14 15:40:30">]

# can even 'replace' using existing posts, the post attributes will be
updated
> a.posts = [ { :id => 19, :title => 'Can Replace This Way Too' } ]
=> [#<Post id: 19, author_id: 14, title: "Can Replace This Way Too",
created_at: "2008-07-14 15:40:30", updated_at: "2008-07-14 15:40:49">]

# use << also for adding brand new items
> a.posts << { :title => 'New Post' }
=> [#<Post id: 19, author_id: 14, title: "Replace Posts", created_at:
"2008-07-14

The patch can be found at

http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/619-allow-accessible-true-associations-to-update-records

I was asked to submit this patch to the mailing list and solicit any
comments. Does anyone see any obvious holes or ways this functionality
should change?


Thanks,
David Dollar

[Pratik]

Has anyone any strong opinions against this ? I'm very interested in
hearing more thoughts on this and/or if you have any suggestions to
improvise David's approaches.

Thanks,
Pratik

On Jul 14, 6:38 pm, David Dollar <ddol...@gmail.com> wrote:
> Recently the following patch
>

> http://github.com/rails/rails/commit/e0750d6a5c7f621e4ca12205137c0b13...

>
> was committed to rails trunk. This patch allows for associations to be
> flagged as :accessible => true and then hydrated from nested hashes
> (i.e. nested forms)
>
> class Post < ActiveRecord::Base
>   belongs_to :author,   :accessible => true
>   has_many   :comments, :accessible => true
> end
>
> post = Post.create({
>   :title    => 'Accessible Attributes',
>   :author   => { :name => 'David Dollar' },
>   :comments => [
>     { :body => 'First Post!' },
>     { :body => 'Nested Hashes are great!' }
>   ]
>
> })
>
> post.comments << { :body => 'Another Comment' }
>
> I have done some work on another patch to allow this same mechanism to
> be used for updating existing rows using nested hashes. This completes
> the work as far as dynamically generating forms and being able to
> simply and intuitively push them back into the database.
>

> http://github.com/ddollar/rails/commit/14a16844bbb3ba9edb14269ce2d0b6...

> http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/61...

[Hique]

Wow, great news! I am very in favor of this patch. When learning
Rails, I was surprised that objects couldn't be created from a nested
hash. Now with this patch, it will be very useful in complex forms
(the problem is well covered in Railscasts Complex Forms series). I
will take a closer look at the patch soon.

A big +1 on this and thanks for the patch

Hique
http://hiquepedia.com

[Clemens]

I'm generally in favor of this for pretty much the same reason as
Hique.
One thing, though: If this kind of behavior was merged into core, the
fields_for method should also be updated to accept an array as its
first parameter to facilitate the use of nested hashes.

Right now, we have to do something like the following:
<% form_for :product do |f| %>
<p><%= f.text_field :name %></p>
<% fields_for "product[price]" do |f| -%>
<p><%= f.text_field :quantity %></p>
<p><%= f.text_field :price %></p>
<% end -%>
<% end %>

It would be great to have it like this:
<% form_for :product do |f| %>
<p><%= f.text_field :name %></p>
<% fields_for [:product, :price] do |f| -%>
<p><%= f.text_field :quantity %></p>
<p><%= f.text_field :price %></p>
<% end -%>
<% end %>

Currently, fields_for does accept an array but the implementation is
faulty/illogical. I've just posted a ticket that addresses this issue
(http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/
641-formhelper-fields_for-has-illogical-and-unused-option). It would
be great if the above code could produce a nested hash much like
form_for builds nested routes from an array.

[Jeff]

On Jul 16, 7:57 pm, Pratik <pratikn...@gmail.com> wrote:
> Has anyone any strong opinions against this ? I'm very interested in
> hearing more thoughts on this and/or if you have any suggestions to
> improvise David's approaches.
>
> Thanks,
> Pratik

Is the :accessible specifier necessary? In other words, are we afraid
something might break if ActiveRecord::Base#new and #create natively
understood nested hashes as well as the flat hashes that it can accept
today?

Jeff

[David Dollar]

> On Jul 16, 7:57 pm, Pratik <pratikn...@gmail.com> wrote:
>> Has anyone any strong opinions against this ? I'm very interested in
>> hearing more thoughts on this and/or if you have any suggestions to
>> improvise David's approaches.
>>
>> Thanks,
>> Pratik
>
> Is the :accessible specifier necessary? In other words, are we afraid
> something might break if ActiveRecord::Base#new and #create natively
> understood nested hashes as well as the flat hashes that it can accept
> today?
>
> Jeff

The main concern that I have seen presented is that form injection
could cause data to be created/updated around your database
that you did not intend should the attacker carefully craft a form for
submission.

David

[Ryan Bates]

Good point David, this can be very dangerous if always enabled due to
form injection. But when intentionally enabled, this has the potential
to greatly simplify multi-model forms - which I love!

Out of curiosity, have you tried making a multi-model form with this?
I can if you want. There are a lot of complexities involved,
specifically with regards to when the associated models are saved and
what happens when validation fails.

Ryan

>  smime.p7s
> 2KDownload

[Michael Koziarski]

> Good point David, this can be very dangerous if always enabled due to
> form injection. But when intentionally enabled, this has the potential
> to greatly simplify multi-model forms - which I love!
>
> Out of curiosity, have you tried making a multi-model form with this?
> I can if you want. There are a lot of complexities involved,
> specifically with regards to when the associated models are saved and
> what happens when validation fails.

It'd be a great idea to go through the requirements you cover in your
awesome screencasts, and see how this helps or hinders each case.
That'd help us get a handle on how this is going to play out.

--
Cheers

Koz

[Josh Susser]

On Jul 18, 2008, at 10:52 AM, Ryan Bates wrote:

>
> Good point David, this can be very dangerous if always enabled due to
> form injection. But when intentionally enabled, this has the potential
> to greatly simplify multi-model forms - which I love!
>
> Out of curiosity, have you tried making a multi-model form with this?
> I can if you want. There are a lot of complexities involved,
> specifically with regards to when the associated models are saved and
> what happens when validation fails.

Yep, exactly. I talked with David Dollar on IRC about this. I've
done this in a way that is similar to your recipe in ARR, but I create/
update the associated models in a before_validation callback, rather
than in the setter. That keeps everything in one transaction and
makes it a little easier to handle validation issues. I did like your
approach of splitting the setters into two pieces for create and
update. The one thing that keeps bugging me though is doing deletes -
I've still yet to figure out a really nice way to handle that.

--josh

--
Josh Susser
http://blog.hasmanythrough.com

[Ryan Bates]

I made a GitHub project to catalog the various approaches to the multi-
model form problem.
http://github.com/ryanb/complex-form-examples/

You can find my approach using David Dollar's fork here:
http://github.com/ryanb/complex-form-examples/tree/ddollar-accessible

It greatly simplifies the code, however there are a couple issues:
- an exception is raised when validation fails for task
- tasks are updated even if validation fails for project

I'm unsure the best way to solve these. If you have a solution, or an
entirely different approach, please fork this project and send a pull
request.

Ryan

[Ryan Bates]

On Jul 18, 1:47 pm, Josh Susser <j...@hasmanythrough.com> wrote:

> Yep, exactly.  I talked with David Dollar on IRC about this.  I've  
> done this in a way that is similar to your recipe in ARR, but I create/
> update the associated models in a before_validation callback, rather  
> than in the setter.  That keeps everything in one transaction and  
> makes it a little easier to handle validation issues.  I did like your  
> approach of splitting the setters into two pieces for create and  
> update.  The one thing that keeps bugging me though is doing deletes -  
> I've still yet to figure out a really nice way to handle that.

I'm going to have to try using a before_validation callback. Regarding
deletes, are you not happy with removing all children that are missing
in the params hash (as the recipe does)? I suppose it is a little
dangerous, but I can't think of an alternative solution which is close
to the same simplicity.

Ryan

[David Dollar]

On a related-yet-slightly-different-topic note:

p = Project.find(:first)
p.tasks.first.name = 'New Name'
p.save

does not update the Task in the database. I'd like to get some
thoughts/opinions on using dirty tracking to make cascadable saves
possible.

If I could cascade the saves, it becomes a lot easier to bubble up
validations in a format that makes sense for nested associations.

> --~--~---------~--~----~------------~-------~--~----~
> You received this message because you are subscribed to the Google
> Groups "Ruby on Rails: Core" group.
> To post to this group, send email to rubyonra...@googlegroups.com
> To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en
> -~----------~----~----~----~------~----~------~--~---
>

[Josh Susser]

On Jul 19, 2008, at 9:18 AM, David Dollar wrote:

> On a related-yet-slightly-different-topic note:
>
> p = Project.find(:first)
> p.tasks.first.name = 'New Name'
> p.save
>
> does not update the Task in the database. I'd like to get some
> thoughts/opinions on using dirty tracking to make cascadable saves
> possible.
>
> If I could cascade the saves, it becomes a lot easier to bubble up
> validations in a format that makes sense for nested associations.

Whoa pardner, that's dangerously close to the Unit-of-Work pattern
there! I like this idea (it's been discussed before many times), but
it's a huge change to how things work. Much moreso than just partial
updates. I'm all for it though.

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.

p = Project.find(some_id)
task = p.tasks.find_by_name("send invoice")
task.due_date = 3.days.from_now
p.build_task(task_params)
p.save

There's a lot that would need to happen to make that example work, and
maybe that's even reaching a bit too far. Anyway, just thinking out
loud.

[Drew]

On Jul 16, 6:57 pm, Pratik <pratikn...@gmail.com> wrote:
> Has anyone any strong opinions against this ?

I think the key here is that the new :accessible=>true option allows
for mass assignments on create, but doesn't support a good method for
doing updates. It should be both or neither, therefore I strongly
believe that this is something that belongs in core as opposed to a
plugin. We shouldn't have to add 'the other half' of the
functionality needed from a plugin. This patch works extremely well
and I think it's one of the best nested update systems available.

Drew

[zilkey]

I'd love to see something like this make it to core.

I'm a bit late in weighing in here, but for completeness, I should
mention that I attempted to solve the same problem with a plugin I
wrote called single_save (http://github.com/zilkey/single_save/tree/
master), based in large part off of Ryan's screencast. I also created
a demo app with it (http://github.com/zilkey/complex_forms_demo_app/
tree/master). There's also a similar attempt at
http://github.com/giraffesoft/attribute_fu/tree/master from James
Golick. I haven't had time to check out David's plugin in-depth, so I
apologize if anything here is duplicate info.

I think that the params should be explicit about what they are
deleting. Deleting everything that isn't in the params can lead to a
lot of unintended consequences re concurrency. If two people add
child items and click the save button at the same time, the added
items from one will be deleted. If instead there were an explicit
"marked_as_deleted" flag that was sent along in the params, both
people's input would be saved.

As far as deletes go, any parameter-hacking solution must also be able
to cover standard validation scenarios to be useful. If you remove an
child item from a form, and the form is invalid, when the form posts
back you should not child that you removed. If you cancel, the one
you removed should still be in the database. This should also hold
true even if the form is submitted multiple times with invalid data.

It seems to me that accessible attributes should be able to do the
following:

* if the parameter set is there, with an id, update it
* if the parameter set is there, without an id, add it
* if the parameter set is there, with an id, and it's set to
marked_as_deleted, then delete it as part of the transaction
after_save
* if there is an object in the database but no matching set of params,
do nothing

Another important point about deletes is that uniqueness validations
can become a problem - let's say you have a uniqueness constraint on a
task name, you load a project with those tasks, delete one, then add
one back:

class Task
validates_uniquess_of :name, :scope => :project
end

p = Project.first
p.tasks.first.name # => "do something"
p.attributes = some_hash_that_marks_the_first_task_as_deleted
p.tasks_without_deleted # => []
p.attributes = some_hash_that_adds_a_new_task_named_do_something
p.save! # => throws validation error because there is already a task
named "do something"

This is something I haven't solved yet, but I can imagine two complex
solutions:

* run all deletes, then run validations, then run the inserts/
updates. If the validations fail, roll it back.
* create validations that are smarter - that can execute exists
statements passing in the ids of the children who are marked for
deletion.

I suspect that dealing with uniqueness will be a problem if you
implement marked_as_deleted or not.

I'm psyched to see this discussion on the core mailing list - if we
can get this right I would use it in almost every app I've written.

[Tom Ward]

On Thu, Jul 17, 2008 at 12:14 PM, Clemens <cle...@railway.at> wrote:
>
> I'm generally in favor of this for pretty much the same reason as
> Hique.
> One thing, though: If this kind of behavior was merged into core, the
> fields_for method should also be updated to accept an array as its
> first parameter to facilitate the use of nested hashes.
>
> Right now, we have to do something like the following:
> <% form_for :product do |f| %>
> <p><%= f.text_field :name %></p>
> <% fields_for "product[price]" do |f| -%>
> <p><%= f.text_field :quantity %></p>
> <p><%= f.text_field :price %></p>
> <% end -%>
> <% end %>
>
> It would be great to have it like this:
> <% form_for :product do |f| %>
> <p><%= f.text_field :name %></p>
> <% fields_for [:product, :price] do |f| -%>
> <p><%= f.text_field :quantity %></p>
> <p><%= f.text_field :price %></p>
> <% end -%>
> <% end %>

Bit late to the party I know, but can't you just do:

<% form_for :product do |product| %>
<%= product.text_field :name %>
<% product.fields_for :price do |price| %>
<%= price.text_field :quantity %>
<% end %>
<% end %>

I hope you can, because I've been using it in apps for ages!

Tom

[Ryan Bates]

On Jul 20, 11:21 pm, "Tom Ward" <t...@popdog.net> wrote:
> <% form_for :product do |product| %>
> <%= product.text_field :name %>
> <% product.fields_for :price do |price| %>
> <%= price.text_field :quantity %>
> <% end %>
> <% end %>

Yep, that works (assuming product has only one price). However I'm not
sure if this fetches @price or @product.price object. I think the
former, but haven't tested.

Ryan

[raffael]

Sorry for a little off-topic but does anyone of know how to support an
object that inherits all attributes through STI when I
use :accessible=>true?

For example:

Object A belongs_to object B and B has_many A objects.
Object B is accessible for A.
Objects C and D inherit their fields through STI from the B object.

I want to create a form that will allow me to fill object's A fields
along with C or D's fields.
The form should have a type selector so that user can choose if he
wants to fill a C object or D.

Form has:
- object A fields
- choose object type list
- depending on the type selected in the list above a set of fields for
a specific B inheriting object (C or D)

[James Golick]

There's actually a somewhat major issue with this patch, with respect
to multi-model forms.

Because the elements are submitted as an array [{}, {}, {}], they are
completely incompatible with ajax submissions of forms. When rails
receives params in an array format (param[]), it reads each element in
the order it receives it, until it reaches a duplicate key, that's
when it decides to create a new element in the array that your
controller ultimately sees. When forms are submitted via ajax, it
isn't really feasible to make sure that elements are submitted in
order. This is partly why I wrote attribute_fu, even when Ryan Bates'
solution was wrapped up in the multi-model forms plugin.

The hash that a_f generates is ugly, but it works in all cases. This
patch, unless amended to support those ugly hashes instead, is going
to cause people a lot of headache when they can't figure out why their
ajax forms won't save right. Or worse, when the attributes are all
getting crossed up between the different elements.

[David Dollar]

Just wanted to give an update since I've been somewhat quiet on the
topic.

Thanks to the great suggestions both here and in #rails-contrib, I'm
working
on a slightly different angle to this patch. Hopefully I'll have the
code finished
in another day or two to post up here for review, but hopefully I can
address
the many legitimate concerns that have been raised.

- David

[Tom Locke]

> 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

[SillyDude]

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

[Dave Rothlisberger]

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 http://rails.lighthouseapp.com/projects/8994/tickets/127-params-hash-is-incorrect-for-nested-arrays

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"}}}}

On Jul 14, 12:38 pm, David Dollar <ddol...@gmail.com> wrote:
> Recently the following patch
>

> http://github.com/rails/rails/commit/e0750d6a5c7f621e4ca12205137c0b13...

>
> was committed to rails trunk. This patch allows for associations to be
> flagged as :accessible => true and then hydrated from nested hashes
> (i.e. nested forms)
>
> class Post < ActiveRecord::Base
>   belongs_to :author,   :accessible => true
>   has_many   :comments, :accessible => true
> end
>
> post = Post.create({
>   :title    => 'Accessible Attributes',
>   :author   => { :name => 'David Dollar' },
>   :comments => [
>     { :body => 'First Post!' },
>     { :body => 'Nested Hashes are great!' }
>   ]
>
> })
>
> post.comments << { :body => 'Another Comment' }
>
> I have done some work on another patch to allow this same mechanism to
> be used for updating existing rows using nested hashes. This completes
> the work as far as dynamically generating forms and being able to
> simply and intuitively push them back into the database.
>

> http://github.com/ddollar/rails/commit/14a16844bbb3ba9edb14269ce2d0b6...

> http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/61...

[Thomas Lee]

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

[Zack Chandler]

+ 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

[Thomas Lee]

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

Cheers,
T

[Thomas Lee]

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

[Tom Locke]

> 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

[Michael Koziarski]

>> 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.

--
Cheers

Koz

[Thomas Lee]

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

Cheers,
T

[felttippin]

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</
label>
<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</
label>

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

> >> mechanism! Seehttp://rails.lighthouseapp.com/projects/8994/tickets/127-params-hash-...

>
> >> 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/...

[zilkey]

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.

> ...
>
> read more »