struggling with accepts_nested_attributes_for

Hi,

I've been struggling with getting accepts_nested_attributes_for to work
perfectly with a nested model form. This is a project that was started
before Rails 2.0, and I'm trying to use new techniques in the new
functionality. I'm presently using Rails 2.3.4. I'm down to one last
issue, and I'm not finding an answer so far.

The system I'm building deals with Items that have ResponseItem's as
children (think questions with multiple choices). Creating a new item
works fine. By default, the item has 12 potential responses. A form is
rendered that allows the user to enter up to 12 responses. The saving
of the new item works fine.

The issue is with editing. Say the item was created with 4 responses.
When the edit form is rendered, 4 of the responses are shown, and the
other 8 are blank (only the first 4 are representing objects in the
databases, the blank ones have no database records). When the form is
submitted, I'm receiving an HTTP 500 error, with this output in the log:

Completed in 176ms (View: 95, DB: 22) | 200 OK
[http://localhost/items/edit/648?assessment_id=1]
/!\ FAILSAFE /!\ Sun May 02 21:04:01 UTC 2010
  Status: 500 Internal Server Error
  expected Array (got Hash) for param `response_items_attributes'
    /opt/jruby-1.4.0/lib/ruby/gems/1.8/gems/rack-1.0.1/lib/rack/utils.rb:85:in
`normalize_params'
    /opt/jruby-1.4.0/lib/ruby/gems/1.8/gems/rack-1.0.1/lib/rack/utils.rb:94:in
`normalize_params'
    /opt/jruby-1.4.0/lib/ruby/gems/1.8/gems/rack-1.0.1/lib/rack/utils.rb:62:in
`parse_nested_query'
    /opt/jruby-1.4.0/lib/ruby/gems/1.8/gems/rack-1.0.1/lib/rack/utils.rb:60:in
`each'
    /opt/jruby-1.4.0/lib/ruby/gems/1.8/gems/rack-1.0.1/lib/rack/utils.rb:60:in
`parse_nested_query'
    /opt/jruby-1.4.0/lib/ruby/gems/1.8/gems/rack-1.0.1/lib/rack/request.rb:140:in
`POST'
....

The call stack does not even show my code, it's all framework so far.

The models look like this:

class Item < ActiveRecord::base
  ...
  attr_accessor :response_item_ids
  has_many :response_items
  accepts_nested_attributes_for :response_items, :allow_destroy => true,
    :reject_if => proc { |attributes| attributes['Response'].blank? }
  ...
end

class ResponseItem < ActiveRecord::base
  ...
  belongs_to :item
  ...
end

The only real clue so far, is the names of the fields on the edit form
are different whether it's a pre-filled response, or a blank one.

This is for a response that has an item in the database:
<td>
    <input id="item_response_items_attributes_1311_Response"
name="item[response_items_attributes][1311][Response]" size="30"
type="text" value="Very familiar - I could explain common
product/service offerings" />
  </td>

This is for a blank response item, with no object in the database:
<td>
    <input id="item_response_items_attributes__Response"
name="item[response_items_attributes][][Response]" size="30" type="text"
/>
  </td>

My hunch is that mixing a form with some items that have an ID, and some
that are blank for an ID, is potentially causing trouble in the
normalize_params function?

One possible solution could be that for blank responses, I create a
temporary database item, so that they have an ID field filled out. When
I try to edit an item with all 12 responses, it does work because
there's an ID field for each response.

Clear as mud? :slight_smile: Tough to explain this in a forum. If anyone has any
thoughts, I'd appreciate it.

Thanks,
Kevin

Hello,
I think the issue could be in the 'Response' attribute in :

accepts_nested_attributes_for :response_items, :allow_destroy =>
true,
    :reject_if => proc { |attributes| attributes['Response'].blank? }

The way you wrote this method means that creations (or updates) of
ResponseItem instances are being rejected if the attribute named
'Response' is blank. I don't think ResponseItem has a 'Response'
attribute, which by the way is an invalid name (capitalized) for an
attribute name.

If you correct the statement above with this:

accepts_nested_attributes_for :response_items, :allow_destroy =>
true,
    :reject_if => proc { |attributes| attributes.all? {|k,v|
v.blank?} }

ResponseItem instances will be rejected (not saved) when all
attributes are blank.
If you need to reject instances when a specific attribute is blank you
could substitute the 'proc' with :

proc { |attributes| attributes['attribute_name'].blank? }

Actually, "Response" is the name of the column in the database.
Unfortunately, the schema I was asked to use was not as RoR as I would
have preferred. But you think even if that is the real column name, it
could be tripping up Rails?

I'll try your suggestion out tonight..

Thanks,
Kevin

You're right, "Response" is a perfect valid name for a column name, I
just never used a capitalized string for a column name :slight_smile:

Anyway, I just reproduced your app according to the (little) details
you've given. It just works fine.

This my input tag for a blank response_item for a new Item :

<input type="text" size="30" name="item[response_items_attributes][0]
[Response]" id="item_response_items_attributes_0_Response">

and when editing an existing Item record:

<input type="text" value="bla1" size="30"
name="item[response_items_attributes][0][Response]"
id="item_response_items_attributes_0_Response">

Mmh, are you passing "form.fields_for :response_items" in your
form_for block ?

Could you post the lines of the form_for block in your new/edit
templates for Item model?

How are you building the response_items for the Item instance in the
controller?

In my test app, the new action for ItemsController is :

def new
  @item.new
  12.times do
    @item.build
  end
end

If the max number of response_items for a given item is 12 you could
do this in your edit action:

def edit
  @item = Item.find(params[:id])
  difference = 12 - @item.response_items.count
  difference.times do
    @item.build
  end
end

But I'm going too far, I think :slight_smile:

kc00l wrote:

You're right, "Response" is a perfect valid name for a column name, I
just never used a capitalized string for a column name :slight_smile:

Anyway, I just reproduced your app according to the (little) details
you've given. It just works fine.

This my input tag for a blank response_item for a new Item :

<input type="text" size="30" name="item[response_items_attributes][0]
[Response]" id="item_response_items_attributes_0_Response">

and when editing an existing Item record:

<input type="text" value="bla1" size="30"
name="item[response_items_attributes][0][Response]"
id="item_response_items_attributes_0_Response">

Mmh, are you passing "form.fields_for :response_items" in your
form_for block ?

Could you post the lines of the form_for block in your new/edit
templates for Item model?

How are you building the response_items for the Item instance in the
controller?

In my test app, the new action for ItemsController is :

def new
  @item.new
  12.times do
    @item.build
  end
end

If the max number of response_items for a given item is 12 you could
do this in your edit action:

def edit
  @item = Item.find(params[:id])
  difference = 12 - @item.response_items.count
  difference.times do
    @item.build
  end
end

But I'm going too far, I think :slight_smile:

Thanks kc00l for all your help. I have not yet had a chance to try
using the .build function. That's a new one to me, wasn't around when I
originally worked on this.

Here's the new function in my controller. The response items are
implicitly loaded. This isn't the exact code, but the essence of it. I
haven't really made any of this production quality either, trying to
focus on getting it working first (just making a disclaimer in-case any
future employers stumble onto this post :slight_smile:

def new
    @item = Item.create_item(params[:item_type_id].to_i, params[:item])
    @max_responses = 12
end

here's my edit function:

def edit
    @item = Item.find(params[:id])
    @max_responses = 12
end

The form is made up of two parts. Here's the main portion of the
response form:

  <tbody>
    <% nResponseIndex = 1 %>
    <% item.response_items.each do |response| %>
      <tr>
        <%= render :partial => "new_response_item",
          :locals => { :nResponseIndex => nResponseIndex, :response_item
=> response, :f => f } %>
      </tr>
      <% nResponseIndex += 1 %>
    <% end %>

    <% until nResponseIndex > max_responses %>
      <tr>
        <%= render :partial => "new_response_item",
          :locals => { :nResponseIndex => nResponseIndex, :response_item
=> ResponseItem.new, :f => f } %>
      </tr>
      <% nResponseIndex += 1 %>
    <% end %>
  </tbody>

The item response fields are a partial, that is rendered 12 times:

<% fields_for "item[response_items_attributes][]", response_item do

fields> %>

  <td align="center">
    <%= "#{nResponseIndex}" %>
  </td>

  <td>
    <%= fields.text_field :Response %>
  </td>

  <td>
    <%= fields.text_field :Weight %>
  </td>
<% end %>

I apologize for the ugly code...I'm sure things could be done more
efficiently overall. I'm a java/c++ guy by day, I know enough RoR to be
dangerous, but am by no means an expert. I'll read up on that build
function, and see if that solves my problems.

Thanks,
Kevin

I'll let you go on by your own, I'm pretty sure you'll figure out how
to refactor your code using the Model.association.build method.
The code I suggested should do the trick, it's just some Rails
magic :slight_smile:

Some tips :
http://guides.rubyonrails.org/association_basics.html#has-many-association-reference

and

http://guides.rubyonrails.org/getting_started.html#building-a-multi-model-form

Just my 2 cents.

kc00l wrote:

I'll let you go on by your own, I'm pretty sure you'll figure out how
to refactor your code using the Model.association.build method.
The code I suggested should do the trick, it's just some Rails
magic :slight_smile:

Some tips :
http://guides.rubyonrails.org/association_basics.html#has-many-association-reference

and

http://guides.rubyonrails.org/getting_started.html#building-a-multi-model-form

Just my 2 cents.

kc00l,

I've been trying out our suggestions and reading those articles. Things
are still not 100%. For the subform, I was missing the form.fields_for,
and instead was using two independent fields_for statements. I made
this change to the response_items portion of the form:

<% form_for :item, :url => { :action => 'update', :id => @item } do |f|
%>

   ...item form elements...

<% f.fields_for :response_items do |response_fields| %>

<table>

  <thead>
  <tr>
      <td align="center"><strong>Index</strong></td>
      <td><strong>Response</strong></td>
      <td><strong>Weight</strong></td>
      <td><strong>Delete</strong></td>
    </tr>
  </thead>

  <tbody>
    <% nResponseIndex = 1 %>
    <% item.response_items.each do |response| %>
      <tr>
        <%= render :partial => "new_response_item",
          :locals => { :nResponseIndex => nResponseIndex, :response_item
=> response, :fields => response_fields } %>
      </tr>
      <% nResponseIndex += 1 %>
    <% end %>

  </tbody>
</table>

<% end %>
<% end %>

_new_response_item.rhtml:

<td align="center">
    <%= "#{nResponseIndex}" %>
  </td>

  <td>
    <%= fields.text_field :Response %>
  </td>

  <td>
    <%= fields.text_field :Weight %>
  </td>

  <td>
    <% unless fields.object.nil? || fields.object.new_record? %>
      <%= fields.label :_delete, 'Remove:' %>
      <%= fields.check_box :_delete %>
    <% end %>
  </td>

My controller looks the same as what you suggested...but after reading
the articles, I realized I needed @items.response_items.build.

Another area I had to do differently in the parent form, was to declare
the fields_for statement like this:

<% form_for :item, :url => { :action => 'update', :id => @item } do |f|
%>

The reason I can't do form_for(@item) is that @item is a polymorphic
object. When I try it like that, I get another error that I haven't
seen before (haven't tried to debug that yet).

The issue is that after these changes, my response_item portion of the
form is missing a '[]' in the name of the html element. Each response
item has the same HTML name attribute:

<td>
    <input id="item_response_items_Response"
name="item[response_items][Response]" size="30" type="text" />
  </td>

  <td>
    <input id="item_response_items_Weight"
name="item[response_items][Weight]" size="30" type="text" />

  </td>

Which of course doesn't work with a :has_many attribute with multiple
responses. There needs to be an extra [] element to make this work.

could the fields_for(@item) vs. fields_for :item be making a big
difference here?

I have the edit working now:

<% form_for :item, @item, :url => { :action => 'update' } do |f| %>

instead of

<% form_for :item, :url => { :action => 'update' } do |f| %>

Adding the @item to the form made all the difference, and I'm seeing the
same results that you are.

Thanks for all your help!

-Kevin