Skip to content

Commit

Permalink
Add a hidden field on the collection_radio_buttons
Browse files Browse the repository at this point in the history
This will avoid a error be raised when the only input on the form is the
`collection_radio_buttons`.
  • Loading branch information
maurogeorge committed Sep 24, 2015
1 parent bff28ba commit 491013e
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 36 deletions.
5 changes: 5 additions & 0 deletions actionview/CHANGELOG.md
@@ -1,3 +1,8 @@
* Add a `hidden_field` on the `collection_radio_buttons` to avoid raising a error
when the only input on the form is the `collection_radio_buttons`.

*Mauro George*

* `url_for` does not modify its arguments when generating polymorphic URLs.

*Bernerd Schaefer*
Expand Down
18 changes: 18 additions & 0 deletions actionview/lib/action_view/helpers/form_options_helper.rb
Expand Up @@ -644,6 +644,24 @@ def time_zone_options_for_select(selected = nil, priority_zones = nil, model = :
# collection_radio_buttons(:post, :author_id, Author.all, :id, :name_with_initial) do |b|
# b.label(:"data-value" => b.value) { b.radio_button + b.text }
# end
#
# ==== Gotcha
#
# The HTML specification says when nothing is select on a collection of radio buttons
# web browsers do not send any value to server.
# Unfortunately this introduces a gotcha:
# if a +User+ model has a +category_id+ field, and in the form none category is selected no +category_id+ parameter is sent. So,
# any strong parameters idiom like
#
# params.require(:user).permit(...)
#
# will raise a error since no +{user: ...}+ will be present.
#
# To prevent this the helper generates an auxiliary hidden field before
# every collection of radio buttons. The hidden field has the same name as collection radio button and blank value.
#
# In case if you don't want the helper to generate this hidden field you can specify
# <tt>include_hidden: false</tt> option.
def collection_radio_buttons(object, method, collection, value_method, text_method, options = {}, html_options = {}, &block)
Tags::CollectionRadioButtons.new(object, method, self, collection, value_method, text_method, options, html_options).render(&block)
end
Expand Down
25 changes: 2 additions & 23 deletions actionview/lib/action_view/helpers/tags/collection_check_boxes.rb
Expand Up @@ -9,41 +9,20 @@ class CollectionCheckBoxes < Base # :nodoc:
class CheckBoxBuilder < Builder # :nodoc:
def check_box(extra_html_options={})
html_options = extra_html_options.merge(@input_html_options)
html_options[:multiple] = true
@template_object.check_box(@object_name, @method_name, html_options, @value, nil)
end
end

def render(&block)
rendered_collection = render_collection do |item, value, text, default_html_options|
default_html_options[:multiple] = true
builder = instantiate_builder(CheckBoxBuilder, item, value, text, default_html_options)

if block_given?
@template_object.capture(builder, &block)
else
render_component(builder)
end
end

# Append a hidden field to make sure something will be sent back to the
# server if all check boxes are unchecked.
if @options.fetch(:include_hidden, true)
rendered_collection + hidden_field
else
rendered_collection
end
render_collection_for(CheckBoxBuilder, &block)
end

private

def render_component(builder)
builder.check_box + builder.label
end

def hidden_field
hidden_name = @html_options[:name] || "#{tag_name(false, @options[:index])}[]"
@template_object.hidden_field_tag(hidden_name, "", id: nil)
end
end
end
end
Expand Down
26 changes: 26 additions & 0 deletions actionview/lib/action_view/helpers/tags/collection_helpers.rb
Expand Up @@ -79,6 +79,32 @@ def render_collection #:nodoc:
yield item, value, text, default_html_options.merge(additional_html_options)
end.join.html_safe
end

def render_collection_for(builder_class, &block) #:nodoc:
options = @options.stringify_keys
rendered_collection = render_collection do |item, value, text, default_html_options|
builder = instantiate_builder(builder_class, item, value, text, default_html_options)

if block_given?
@template_object.capture(builder, &block)
else
render_component(builder)
end
end

# Append a hidden field to make sure something will be sent back to the
# server if all radio buttons are unchecked.
if options.fetch('include_hidden', true)
rendered_collection + hidden_field
else
rendered_collection
end
end

def hidden_field #:nodoc:
hidden_name = @html_options[:name] || "#{tag_name(false, @options[:index])}[]"
@template_object.hidden_field_tag(hidden_name, "", id: nil)
end
end
end
end
Expand Down
Expand Up @@ -14,15 +14,7 @@ def radio_button(extra_html_options={})
end

def render(&block)
render_collection do |item, value, text, default_html_options|
builder = instantiate_builder(RadioButtonBuilder, item, value, text, default_html_options)

if block_given?
@template_object.capture(builder, &block)
else
render_component(builder)
end
end
render_collection_for(RadioButtonBuilder, &block)
end

private
Expand Down
35 changes: 35 additions & 0 deletions actionview/test/template/form_collections_helper_test.rb
Expand Up @@ -198,6 +198,41 @@ def with_collection_check_boxes(*args, &block)
assert_select 'input[type=radio][value=false][checked=checked]'
end

test 'collection radio buttons generates only one hidden field for the entire collection, to ensure something will be sent back to the server when posting an empty collection' do
collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')]
with_collection_radio_buttons :user, :category_ids, collection, :id, :name

assert_select "input[type=hidden][name='user[category_ids][]'][value='']", count: 1
end

test 'collection radio buttons generates a hidden field using the given :name in :html_options' do
collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')]
with_collection_radio_buttons :user, :category_ids, collection, :id, :name, {}, { name: "user[other_category_ids][]" }

assert_select "input[type=hidden][name='user[other_category_ids][]'][value='']", count: 1
end

test 'collection radio buttons generates a hidden field with index if it was provided' do
collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')]
with_collection_radio_buttons :user, :category_ids, collection, :id, :name, { index: 322 }

assert_select "input[type=hidden][name='user[322][category_ids][]'][value='']", count: 1
end

test 'collection radio buttons does not generate a hidden field if include_hidden option is false' do
collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')]
with_collection_radio_buttons :user, :category_ids, collection, :id, :name, include_hidden: false

assert_select "input[type=hidden][name='user[category_ids][]'][value='']", count: 0
end

test 'collection radio buttons does not generate a hidden field if include_hidden option is false with key as string' do
collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')]
with_collection_radio_buttons :user, :category_ids, collection, :id, :name, 'include_hidden' => false

assert_select "input[type=hidden][name='user[category_ids][]'][value='']", count: 0
end

# COLLECTION CHECK BOXES
test 'collection check boxes accepts a collection and generate a series of checkboxes for value method' do
collection = [Category.new(1, 'Category 1'), Category.new(2, 'Category 2')]
Expand Down
13 changes: 9 additions & 4 deletions actionview/test/template/form_helper_test.rb
Expand Up @@ -1596,7 +1596,8 @@ def post.active; false; end
"<input id='post_active_true' name='post[active]' type='radio' value='true' />" +
"<label for='post_active_true'>true</label>" +
"<input checked='checked' id='post_active_false' name='post[active]' type='radio' value='false' />" +
"<label for='post_active_false'>false</label>"
"<label for='post_active_false'>false</label>" +
"<input type='hidden' name='post[active][]' value='' />"
end

assert_dom_equal expected, output_buffer
Expand All @@ -1619,7 +1620,8 @@ def post.active; false; end
"true</label>" +
"<label for='post_active_false'>"+
"<input checked='checked' id='post_active_false' name='post[active]' type='radio' value='false' />" +
"false</label>"
"false</label>" +
"<input type='hidden' name='post[active][]' value='' />"
end

assert_dom_equal expected, output_buffer
Expand All @@ -1645,6 +1647,7 @@ def post.id; 1; end
"<label for='post_active_false'>"+
"<input checked='checked' id='post_active_false' name='post[active]' type='radio' value='false' />" +
"false</label>"+
"<input type='hidden' name='post[active][]' value='' />" +
"<input id='post_id' name='post[id]' type='hidden' value='1' />"
end

Expand All @@ -1663,7 +1666,8 @@ def post.active; false; end
"<input id='foo_post_active_true' name='post[active]' type='radio' value='true' />" +
"<label for='foo_post_active_true'>true</label>" +
"<input checked='checked' id='foo_post_active_false' name='post[active]' type='radio' value='false' />" +
"<label for='foo_post_active_false'>false</label>"
"<label for='foo_post_active_false'>false</label>" +
"<input type='hidden' name='post[active][]' value='' />"
end

assert_dom_equal expected, output_buffer
Expand All @@ -1681,7 +1685,8 @@ def post.active; false; end
"<input id='post_1_active_true' name='post[1][active]' type='radio' value='true' />" +
"<label for='post_1_active_true'>true</label>" +
"<input checked='checked' id='post_1_active_false' name='post[1][active]' type='radio' value='false' />" +
"<label for='post_1_active_false'>false</label>"
"<label for='post_1_active_false'>false</label>" +
"<input type='hidden' name='post[1][active][]' value='' />"
end

assert_dom_equal expected, output_buffer
Expand Down

0 comments on commit 491013e

Please sign in to comment.