Close
Glad You're Ready. Let's Get Started!

Let us know how we can contact you.

Thank you!

We'll respond shortly.

LABS
Beware the frumious nested attribute

Nested attribute assignment is one of the recent additions to Rails that made a great deal of sense, and made a lot of people happy. Chances are you’ve either used nested attribute assignment by now, or you worked on an older project that really could have used it. If you haven’t yet, check it out and see what you think.

Unfortunately, not all is well in Railstown. Nested attribute assignment is slick, and the related implementation of #fields_for makes it even slicker, but #fields_for can cause you some headaches if you’re not careful. Possibly if you are careful as well.

Consider a standard example of where you might want nested attribute assignment:

class CatLady < ActiveRecord::Base
  has_many :cats
  accepts_nested_attributes_for :cats

  def crazy?
    true
  end
end

And in your edit view:

<% form_for @cat_lady do |cat_lady_form| %>
  <table>
    <tbody>
      <% cat_lady_form.fields_for :cats do |cat_fields| %>
        <tr>
          <td><%= cat_fields.text_field :name %></td>
          <td><%= cat_fields.text_field :nickname %></td>
          <td><%= cat_fields.text_field :burial_preferences %></td>
        </tr>
      <% end %>
    </tbody>
  </table>
<% end %>

This seems fine, but when you look in more detail you discover that #fields_for will emit a hidden <input /> element for each cat associated to our cat lady. It does this at the point where you make the #fields_for call, much like the way #form_for emits the <form /> element. Unfortunately, that means that #fields_for emits the <input /> element as a sibling of the <tr /> element for the related cat; and thus, as a direct child of the <tbody /> element. Oops. The HTML standard doesn’t allow <tbody /> elements to have <input /> elements as children.

Most browsers won’t complain about this, but Safari 4 will (and so, I’d guess, will any other WebKit-based browser, like Chrome). Safari not only complains, it helpfully moves the <input /> element to a valid position. So, instead of this (you’ll have to imagine a bit; the markdown renderer for this blog is actually modifying my invalid HTML example to try to make it valid):

<table>
  <tbody>
    </tbody></table><input name="cat_lady[cats_attributes][0][id]" type="hidden" value="423" />
    <tr>
      <td><input name="cat_lady[cats_attributes][0][name]" type="text" /></td>
      ...
    </tr>
  </tbody>
</table>

you end up with this:

<input name="cat_lady[cats_attributes][0][id]" type="hidden" value="423" />
<table>
  <tbody>
    <tr>
      <td><input name="cat_lady[cats_attributes][0][name]" type="text" /></td>
      ...
    </tr>
  </tbody>
</table>

Seems innocuous enough, doesn’t it? After all, it’s still inside the form, so the browser will still submit the value along with everything else. However, the HTML that you sent to the browser is still invalid, and Safari still spits out the errors, which is probably not the best way to gain your users’ confidence. Also, any JavaScript you’ve written that depends on the DOM structure you lay out might fail, but only in some browsers (and not, for a change, only in IE!).

Now, someone will point out that you can solve this problem by not using tables. True, but that solution has two drawbacks: first, it’s entirely reasonable, even potentially very desirable, to use a table for this type of data; second, the hidden ID input will end up outside whatever container element you create for your nested model. This may not generate invalid HTML, but it may generate conceptually improper HTML. For instance, what if we change the above HTML to look like this:

<div class="menagerie">
  <input name="cat_lady[cats_attributes][0][id]" type="hidden" value="423" />
  <div class="cat">
    <input name="cat_lady[cats_attributes][0][name]" type="text" /></td>
    ...
  </div>
</div>

It doesn’t take too much imagination in the drag-and-drop Web 2.1 world to come up with some form of DOM manipulation that will dissociate the cat div from its associated ID element. And, of course, if the server receives the nested cat attributes without an ID it will helpfully make a new cat model. We don’t want this; crazy cat lady has enough cats already.

So, what to do?

We knocked around some ideas, and the most reasonable seems to be to add the capability to manually insert the hidden ID field (and, potentially, the hidden _destroy field) to the form builder object created by #fields_for. So, the #fields_for block from the edit form above would look something like this:

<% cat_lady_form.fields_for :cats, :omit_hidden_fields => true do |cat_fields| %>
  <tr>
    <%= cat_fields.hidden_fields %>
    <td><%= cat_fields.text_field :name %></td>
    <td><%= cat_fields.text_field :nickname %></td>
    <td><%= cat_fields.text_field :burial_preferences %></td>
  </tr>
<% end %>

It’s also possible to automatically determine if the block for each nested model called the #hidden_fields method, which would obviate the need for the explicit option; I haven’t decided if I like that approach.

I’m open to suggestions for better fixes, or tweaks to this one. In any case, look for a Rails patch for this some time in the coming week.

Comments
  1. I like your proposed fix; however, I’d like to see something a bit more conventional. For instance, if I define ANY form field within the fields_for block called :id, that should be enough to stop Rails from inserting the default, hidden id field.

  2. Adam Milligan says:

    Tim, [Will Read](http://pivotallabs.com/users/will) actually picked up the ball on this one and has submitted a patch [here](https://rails.lighthouseapp.com/projects/8994/tickets/3259-allow-explicit-placement-of-hidden-fields-with-nested-models).

    He has gone through a few iterations on the idea, and I’m sure he’s be open to comments.

Post a Comment

Your Information (Name required. Email address will not be displayed with comment.)

* Copy This Password *

* Type Or Paste Password Here *