Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly deprecate the block argument in AV FormBuilder #5561

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 9 additions & 3 deletions actionpack/lib/action_view/helpers/form_helper.rb
Expand Up @@ -9,6 +9,7 @@
require 'active_support/core_ext/object/blank'
require 'active_support/core_ext/string/output_safety'
require 'active_support/core_ext/array/extract_options'
require 'active_support/deprecation'

module ActionView
# = Action View Form Helpers
Expand All @@ -22,7 +23,7 @@ module Helpers
# being routed to the appropriate controller action (with the appropriate <tt>:id</tt>
# parameter in the case of an existing resource), (ii) input fields should
# be named in such a way that in the controller their values appear in the
# appropriate places within the +params+ hash, and (iii) for an existing record,
# appropriate places within the +params+ hash, and (iii) for an existing record,
# when the form is initially displayed, input fields corresponding to attributes
# of the resource should show the current values of those attributes.
#
Expand Down Expand Up @@ -156,7 +157,7 @@ def convert_to_model(object)
# if <tt>:person</tt> also happens to be the name of an instance variable
# <tt>@person</tt>, the default value of the field shown when the form is
# initially displayed (e.g. in the situation where you are editing an
# existing record) will be the value of the corresponding attribute of
# existing record) will be the value of the corresponding attribute of
# <tt>@person</tt>.
#
# The rightmost argument to +form_for+ is an
Expand Down Expand Up @@ -1057,7 +1058,12 @@ def to_model
self
end

def initialize(object_name, object, template, options)
def initialize(object_name, object, template, options, block=nil)
if block
ActiveSupport::Deprecation.warn(
"Giving a block to FormBuilder is deprecated and has no effect anymore.")
end

@nested_child_index = {}
@object_name, @object, @template, @options = object_name, object, template, options
@parent_builder = options[:parent_builder]
Expand Down
12 changes: 12 additions & 0 deletions actionpack/test/template/form_helper_test.rb
Expand Up @@ -2223,6 +2223,18 @@ def test_fields_for_returns_block_result
assert_equal "fields", output
end

def test_form_builder_block_argument_deprecation
builder_class = Class.new(ActionView::Helpers::FormBuilder) do
def initialize(object_name, object, template, options, block)
super
end
end

assert_deprecated /Giving a block to FormBuilder is deprecated and has no effect anymore/ do
builder_class.new(:foo, nil, nil, {}, proc {})
end
end

protected

def hidden_fields(method = nil)
Expand Down