Skip to content

Commit

Permalink
Fixed index and auto index for nested fields_for [#327 state:resolved]
Browse files Browse the repository at this point in the history 
Signed-off-by: Joshua Peek <josh@joshpeek.com>
  • Loading branch information
@kjg @josh
kjg authored and josh committed Jul 19, 2008
1 parent 706425e commit 1b4b1aa
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 10 deletions.
30 changes: 23 additions & 7 deletions actionpack/lib/action_view/helpers/form_helper.rb
View file
Expand Up @@ -528,10 +528,10 @@ class InstanceTag #:nodoc:

def initialize(object_name, method_name, template_object, object = nil)
@object_name, @method_name = object_name.to_s.dup, method_name.to_s.dup
@template_object= template_object
@template_object = template_object
@object = object
if @object_name.sub!(/\[\]$/,"")
if object ||= @template_object.instance_variable_get("@#{Regexp.last_match.pre_match}") and object.respond_to?(:to_param)
if @object_name.sub!(/\[\]$/,"") || @object_name.sub!(/\[\]\]$/,"]")
if (object ||= @template_object.instance_variable_get("@#{Regexp.last_match.pre_match}")) && object.respond_to?(:to_param)
@auto_index = object.to_param
else
raise ArgumentError, "object[] naming but object param and @object var don't exist or don't respond to to_param: #{object.inspect}"
Expand Down Expand Up @@ -708,7 +708,7 @@ def tag_id_with_index(index)
end

def sanitized_object_name
@sanitized_object_name ||= @object_name.gsub(/[^-a-zA-Z0-9:.]/, "_").sub(/_$/, "")
@sanitized_object_name ||= @object_name.gsub(/\]\[|[^-a-zA-Z0-9:.]/, "_").sub(/_$/, "")
end

def sanitized_method_name
Expand All @@ -726,6 +726,13 @@ class FormBuilder #:nodoc:
def initialize(object_name, object, template, options, proc)
@object_name, @object, @template, @options, @proc = object_name, object, template, options, proc
@default_options = @options ? @options.slice(:index) : {}
if @object_name.to_s.match(/\[\]$/)
if object ||= @template.instance_variable_get("@#{Regexp.last_match.pre_match}") and object.respond_to?(:to_param)
@auto_index = object.to_param
else
raise ArgumentError, "object[] naming but object param and @object var don't exist or don't respond to to_param: #{object.inspect}"
end
end
end

(field_helpers - %w(label check_box radio_button fields_for)).each do |selector|
Expand All @@ -738,16 +745,25 @@ def #{selector}(method, options = {})
end

def fields_for(record_or_name_or_array, *args, &block)
if options.has_key?(:index)
index = "[#{options[:index]}]"
elsif defined?(@auto_index)
self.object_name = @object_name.to_s.sub(/\[\]$/,"")
index = "[#{@auto_index}]"
else
index = ""
end

case record_or_name_or_array
when String, Symbol
name = "#{object_name}[#{record_or_name_or_array}]"
name = "#{object_name}#{index}[#{record_or_name_or_array}]"
when Array
object = record_or_name_or_array.last
name = "#{object_name}[#{ActionController::RecordIdentifier.singular_class_name(object)}]"
name = "#{object_name}#{index}[#{ActionController::RecordIdentifier.singular_class_name(object)}]"
args.unshift(object)
else
object = record_or_name_or_array
name = "#{object_name}[#{ActionController::RecordIdentifier.singular_class_name(object)}]"
name = "#{object_name}#{index}[#{ActionController::RecordIdentifier.singular_class_name(object)}]"
args.unshift(object)
end

Expand Down
116 changes: 113 additions & 3 deletions actionpack/test/template/form_helper_test.rb
View file
Expand Up @@ -6,7 +6,7 @@
alias_method :title_before_type_cast, :title unless respond_to?(:title_before_type_cast)
alias_method :body_before_type_cast, :body unless respond_to?(:body_before_type_cast)
alias_method :author_name_before_type_cast, :author_name unless respond_to?(:author_name_before_type_cast)
alias_method :secret?, :secret
alias_method :secret?, :secret

def new_record=(boolean)
@new_record = boolean
Expand All @@ -22,6 +22,7 @@ class Comment
attr_reader :post_id
def save; @id = 1; @post_id = 1 end
def new_record?; @id.nil? end
def to_param; @id; end
def name
@id.nil? ? 'new comment' : "comment ##{@id}"
end
Expand All @@ -30,7 +31,6 @@ def name

class Comment::Nested < Comment; end


class FormHelperTest < ActionView::TestCase
tests ActionView::Helpers::FormHelper

Expand Down Expand Up @@ -447,6 +447,117 @@ def test_nested_fields_for
assert_dom_equal expected, output_buffer
end

def test_nested_fields_for_with_nested_collections
form_for('post[]', @post) do |f|
concat f.text_field(:title)
f.fields_for('comment[]', @comment) do |c|
concat c.text_field(:name)
end
end

expected = "<form action='http://www.example.com' method='post'>" +
"<input name='post[123][title]' size='30' type='text' id='post_123_title' value='Hello World' />" +
"<input name='post[123][comment][][name]' size='30' type='text' id='post_123_comment__name' value='new comment' />" +
"</form>"

assert_dom_equal expected, output_buffer
end

def test_nested_fields_for_with_index
form_for('post', @post, :index => 1) do |c|
concat c.text_field(:title)
c.fields_for('comment', @comment, :index => 1) do |r|
concat r.text_field(:name)
end
end

expected = "<form action='http://www.example.com' method='post'>" +
"<input name='post[1][title]' size='30' type='text' id='post_1_title' value='Hello World' />" +
"<input name='post[1][comment][1][name]' size='30' type='text' id='post_1_comment_1_name' value='new comment' />" +
"</form>"

assert_dom_equal expected, output_buffer
end

def test_nested_fields_for_with_index
form_for(:post, @post, :index => 1) do |f|
f.fields_for(:comment, @post) do |c|
concat c.text_field(:title)
end
end

expected = "<form action='http://www.example.com' method='post'>" +
"<input name='post[1][comment][title]' size='30' type='text' id='post_1_comment_title' value='Hello World' />" +
"</form>"

assert_dom_equal expected, output_buffer
end

def test_nested_fields_for_with_index_on_both
form_for(:post, @post, :index => 1) do |f|
f.fields_for(:comment, @post, :index => 5) do |c|
concat c.text_field(:title)
end
end

expected = "<form action='http://www.example.com' method='post'>" +
"<input name='post[1][comment][5][title]' size='30' type='text' id='post_1_comment_5_title' value='Hello World' />" +
"</form>"

assert_dom_equal expected, output_buffer
end

def test_nested_fields_for_with_auto_index
form_for("post[]", @post) do |f|
f.fields_for(:comment, @post) do |c|
concat c.text_field(:title)
end
end

expected = "<form action='http://www.example.com' method='post'>" +
"<input name='post[123][comment][title]' size='30' type='text' id='post_123_comment_title' value='Hello World' />" +
"</form>"

assert_dom_equal expected, output_buffer
end

def test_nested_fields_for_with_auto_index_on_both
form_for("post[]", @post) do |f|
f.fields_for("comment[]", @post) do |c|
concat c.text_field(:title)
end
end

expected = "<form action='http://www.example.com' method='post'>" +
"<input name='post[123][comment][123][title]' size='30' type='text' id='post_123_comment_123_title' value='Hello World' />" +
"</form>"

assert_dom_equal expected, output_buffer
end

def test_nested_fields_for_with_index_and_auto_index
form_for("post[]", @post) do |f|
f.fields_for(:comment, @post, :index => 5) do |c|
concat c.text_field(:title)
end
end

form_for(:post, @post, :index => 1) do |f|
f.fields_for("comment[]", @post) do |c|
concat c.text_field(:title)
end
end

expected = "<form action='http://www.example.com' method='post'>" +
"<input name='post[123][comment][5][title]' size='30' type='text' id='post_123_comment_5_title' value='Hello World' />" +
"</form>" +
"<form action='http://www.example.com' method='post'>" +
"<input name='post[1][comment][123][title]' size='30' type='text' id='post_1_comment_123_title' value='Hello World' />" +
"</form>"

assert_dom_equal expected, output_buffer
end

def test_fields_for
fields_for(:post, @post) do |f|
concat f.text_field(:title)
Expand Down Expand Up @@ -831,7 +942,6 @@ def test_remote_form_for_with_html_options_adds_options_to_form_tag
assert_dom_equal expected, output_buffer
end


protected
def comments_path(post)
"/posts/#{post.id}/comments"
Expand Down

3 comments on commit 1b4b1aa

@glennpow
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am attempting to utilize mass-assign for my models, but can’t get the update_attributes to work correctly when I have association_collections as child models.
My create methods work fine, since the hash passes in an Array value for these collections. I.E.:
emails => [ {…}, {…} ] )
But, when I try to edit/update my model, the field names in the form naturally have the :id of the email included in them, so that the resulting value passed to the mass-assign is not an Array, but a Hash of this form:
emails => {“5” => {…}, “6” => {…}}
Where the 5 and 6 are the :id values of the respective emails. This gets passed into the update_attributes, which eventually gets to:
AssociationCollection.replace(other_array)
which assumes “emails” to be an Array. Shouldn’t this replace method be “smarter” so that if a Hash is passed in, it will ascertain the :id values from it, and then reassign the model attributes accordingly?
Furthermore, what would happen if there were a combination of updated models (emails) and perhaps one new email (that didn’t have an :id). Does the mass-assign functionality handle this situation?
Any help would be appreciated.
-Glenn

@acook8103
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generation of HTML element id and form field name are inconsistent after 2nd level of nesting.


<% form_for([: inventory, @request]) do |f| %>
    <% line_item = @request.line_items.first %>
    <% f.fields_for "existing_line_item_attributes[]", line_item do |li| %>
      <%= li.text_field :equipment_type_id %>
      <% equipment_li = line_item.equipment_line_items.first %>
      <% li.fields_for "existing_equipment_line_item_attributes[]", equipment_li do |eli| %>
        <%= eli.text_field :equipment_id %>
      <% end %>
    <% end %>
<% end %>

Generates this:


<form action="/inventory/requests/350903690" class="edit_request" id="edit_request_350903690" method="post">
      <input id="request_existing_line_item_attributes_51161156_equipment_type_id" name="request[existing_line_item_attributes][51161156][equipment_type_id]" size="30" type="text" value="691081111" />
      
        <input id="request_existing_line_item_attributes___existing_equipment_line_item_attributes_860256615_equipment_id" name="request[existing_line_item_attributes[]][existing_equipment_line_item_attributes][860256615][equipment_id]" size="30" type="text" value="1029003634" />

</form>

You notice that the in the 3rd level that the 2nd level ID is missing and that the “[]” are enclosed within the first field.

I was able to get the the above form to work as expected by removing the brackets and explicitly setting the id.Generation of HTML element id and form field name are inconsistent after 2nd level of nesting.


<% form_for([:inventory, @request]) do |f| %>
    <% line_item = @request.line_items.first %>
    <% f.fields_for "existing_line_item_attributes", line_item, :index => line_item.id do |li| %>
      <%= li.text_field :equipment_type_id %>
      <% equipment_li = line_item.equipment_line_items.first %>
      <% li.fields_for "existing_equipment_line_item_attributes", equipment_li, :index => equipment_li.id do |eli| %>
        <%= eli.text_field :equipment_id %>
      <% end %>
    <% end %>
<% end %>

I perused the code, but am not familiar enough with things to spot anything.

Thanks for the original patch. Made my code less ugly.

@acook8103
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Textile Reference I followed said to do pre and code.

Let’s try just code so you can see the html. I also took out lt and gt.

form action="/inventory/requests/350903690" class="edit_request" id="edit_request_350903690" method="post"> input id="request_existing_line_item_attributes_51161156_equipment_type_id" name="request[existing_line_item_attributes][51161156][equipment_type_id]" size="30" type="text" value="691081111" /

```
input id=“request_existing_line_item_attributes___existing_equipment_line_item_attributes_860256615_equipment_id” name=“request[existing_line_item_attributes[]][existing_equipment_line_item_attributes]860256615[equipment_id]” size=“30” type=“text” value=“1029003634” /
```

/form

Please sign in to comment.