Permalink
Browse files

Do not call fields_for from form_for, to avoid instantiating two buil…

…ders

Conflicts:
	actionpack/lib/action_view/helpers/form_helper.rb
	actionpack/test/template/form_helper_test.rb
  • Loading branch information...
1 parent 1506d4d commit 756188b512ca11e24262a74856e3bc11b9b2dbc9 @carlosantoniodasilva carlosantoniodasilva committed Jan 5, 2013
Showing with 19 additions and 9 deletions.
  1. +6 −8 actionpack/lib/action_view/helpers/form_helper.rb
  2. +13 −1 actionpack/test/template/form_helper_test.rb
View
14 actionpack/lib/action_view/helpers/form_helper.rb
@@ -331,9 +331,9 @@ def convert_to_model(object)
# In many cases you will want to wrap the above in another helper, so you
# could do something like the following:
#
- # def labelled_form_for(record_or_name_or_array, *args, &proc)
+ # def labelled_form_for(record_or_name_or_array, *args, &block)
# options = args.extract_options!
- # form_for(record_or_name_or_array, *(args << options.merge(:builder => LabellingFormBuilder)), &proc)
+ # form_for(record_or_name_or_array, *(args << options.merge(:builder => LabellingFormBuilder)), &block)
# end
#
# If you don't need to attach a form to a model instance, then check out
@@ -355,7 +355,7 @@ def convert_to_model(object)
# <%= form_for @invoice, :url => external_url, :authenticity_token => false do |f|
# ...
# <% end %>
- def form_for(record, options = {}, &proc)
+ def form_for(record, options = {}, &block)
raise ArgumentError, "Missing block" unless block_given?
options[:html] ||= {}
@@ -374,12 +374,10 @@ def form_for(record, options = {}, &proc)
options[:html][:method] = options.delete(:method) if options.has_key?(:method)
options[:html][:authenticity_token] = options.delete(:authenticity_token)
- builder = options[:parent_builder] = instantiate_builder(object_name, object, options, &proc)
- fields_for = fields_for(object_name, object, options, &proc)
+ builder = options[:parent_builder] = instantiate_builder(object_name, object, options, &block)
+ output = capture(builder, &block)
default_options = builder.multipart? ? { :multipart => true } : {}
- output = form_tag(options.delete(:url) || {}, default_options.merge!(options.delete(:html)))
- output << fields_for
- output.safe_concat('</form>')
+ form_tag(options.delete(:url) || {}, default_options.merge!(options.delete(:html))) { output }
end
def apply_form_for_options!(object_or_array, options) #:nodoc:
View
14 actionpack/test/template/form_helper_test.rb
@@ -755,7 +755,6 @@ def test_fields_for_with_file_field_generate_multipart
assert_dom_equal expected, output_buffer
end
-
def test_form_for_with_format
form_for(@post, :format => :json, :html => { :id => "edit_post_123", :class => "edit_post" }) do |f|
concat f.label(:title)
@@ -2217,6 +2216,19 @@ def test_fields_for_returns_block_result
assert_equal "fields", output
end
+ def test_form_for_only_instantiates_builder_once
+ initialization_count = 0
+ builder_class = Class.new(ActionView::Helpers::FormBuilder) do
+ define_method :initialize do |*args|
+ super(*args)
+ initialization_count += 1
+ end
+ end
+
+ form_for(@post, :builder => builder_class) { }
+ assert_equal 1, initialization_count, 'form builder instantiated more than once'
+ end
+
protected
def protect_against_forgery?
false

0 comments on commit 756188b

Please sign in to comment.