From f0479cbbd52961c75906022003d4f03aa39556e0 Mon Sep 17 00:00:00 2001
From: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Date: Sat, 14 May 2011 20:58:02 -0300
Subject: [PATCH] Refactor fields_for to make the api more clear, and fix usage
 with non nested attributes and without object

---
 .../lib/action_view/helpers/form_helper.rb    | 45 ++++++++-----------
 actionpack/test/template/form_helper_test.rb  | 14 ++++++
 2 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index bfd04f163d..cc8b61228f 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -1216,35 +1216,30 @@ module ActionView
         RUBY_EVAL
       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
+      def fields_for(record_name, record_object = nil, fields_options = {}, &block)
+        fields_options, record_object = record_object, nil if record_object.is_a?(Hash)
+        fields_options[:builder] ||= options[:builder]
+        fields_options[:parent_builder] = self
 
-        args << {} unless args.last.is_a?(Hash)
-        args.last[:builder] ||= options[:builder]
-        args.last[:parent_builder] = self
-
-        case record_or_name_or_array
+        case record_name
         when String, Symbol
-          if nested_attributes_association?(record_or_name_or_array)
-            return fields_for_with_nested_attributes(record_or_name_or_array, args, block)
-          else
-            name = record_or_name_or_array
+          if nested_attributes_association?(record_name)
+            return fields_for_with_nested_attributes(record_name, record_object, fields_options, block)
           end
         else
-          object = record_or_name_or_array.is_a?(Array) ? record_or_name_or_array.last : record_or_name_or_array
-          name   = ActiveModel::Naming.param_key(object)
-          args.unshift(object)
+          record_object = record_name.is_a?(Array) ? record_name.last : record_name
+          record_name   = ActiveModel::Naming.param_key(record_object)
         end
-        name = "#{object_name}#{index}[#{name}]"
 
-        @template.fields_for(name, *args, &block)
+        index = if options.has_key?(:index)
+          "[#{options[:index]}]"
+        elsif defined?(@auto_index)
+          self.object_name = @object_name.to_s.sub(/\[\]$/,"")
+          "[#{@auto_index}]"
+        end
+        record_name = "#{object_name}#{index}[#{record_name}]"
+
+        @template.fields_for(record_name, record_object, fields_options, &block)
       end
 
       def label(method, text = nil, options = {}, &block)
@@ -1333,10 +1328,8 @@ module ActionView
           @object.respond_to?("#{association_name}_attributes=")
         end
 
-        def fields_for_with_nested_attributes(association_name, args, block)
+        def fields_for_with_nested_attributes(association_name, association, options, block)
           name = "#{object_name}[#{association_name}_attributes]"
-          options = args.extract_options!
-          association = args.shift
           association = convert_to_model(association)
 
           if association.respond_to?(:persisted?)
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index c25c850eb3..286bfb4d04 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -1725,6 +1725,20 @@ class FormHelperTest < ActionView::TestCase
     assert_dom_equal expected, output_buffer
   end
 
+  def test_form_for_and_fields_for_with_non_nested_association_and_without_object
+    form_for(@post) do |f|
+      concat f.fields_for(:category) { |c|
+        concat c.text_field(:name)
+      }
+    end
+
+    expected = whole_form('/posts/123', 'edit_post_123', 'edit_post', 'put') do
+      "<input name='post[category][name]' type='text' size='30' id='post_category_name' />"
+    end
+
+    assert_dom_equal expected, output_buffer
+  end
+
   class LabelledFormBuilder < ActionView::Helpers::FormBuilder
     (field_helpers - %w(hidden_field)).each do |selector|
       class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1