From 7e583b73c4e7ddad2241b2ca05b3c16a5fdf0cc6 Mon Sep 17 00:00:00 2001
From: Santiago Pastorino <santiago@wyeworks.com>
Date: Thu, 31 Dec 2015 00:40:33 -0300
Subject: Fix collection_radio_buttons' hidden_field name and make it appear
 before the radios

Fixes #22773
---
 actionview/CHANGELOG.md                            |  8 +++++
 .../helpers/tags/collection_check_boxes.rb         |  4 +++
 .../action_view/helpers/tags/collection_helpers.rb |  8 +++--
 .../test/template/form_collections_helper_test.rb  | 12 ++++----
 actionview/test/template/form_helper_test.rb       | 36 +++++++++++-----------
 5 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index 91d47cda11..dbdd4378d0 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -1,3 +1,11 @@
+*   Fix `collection_radio_buttons` hidden_field name and make it appear
+    before the actual input radio tags to make the real value override
+    the hidden when passed.
+
+    Fixes #22773
+
+    *Santiago Pastorino*
+
 *   `ActionView::TestCase::Controller#params` returns an instance of
     `ActionController::Parameters`.
 
diff --git a/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb b/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb
index 3256d44e18..3dda47a458 100644
--- a/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb
+++ b/actionview/lib/action_view/helpers/tags/collection_check_boxes.rb
@@ -23,6 +23,10 @@ module ActionView
         def render_component(builder)
           builder.check_box + builder.label
         end
+
+        def hidden_field_name #:nodoc:
+          "#{super}[]"
+        end
       end
     end
   end
diff --git a/actionview/lib/action_view/helpers/tags/collection_helpers.rb b/actionview/lib/action_view/helpers/tags/collection_helpers.rb
index b87b4281d6..1d3b1ecf0b 100644
--- a/actionview/lib/action_view/helpers/tags/collection_helpers.rb
+++ b/actionview/lib/action_view/helpers/tags/collection_helpers.rb
@@ -97,16 +97,20 @@ module ActionView
           # 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
+            hidden_field + rendered_collection
           else
             rendered_collection
           end
         end
 
         def hidden_field #:nodoc:
-          hidden_name = @html_options[:name] || "#{tag_name(false, @options[:index])}[]"
+          hidden_name = @html_options[:name] || hidden_field_name
           @template_object.hidden_field_tag(hidden_name, "", id: nil)
         end
+
+        def hidden_field_name #:nodoc:
+          "#{tag_name(false, @options[:index])}"
+        end
       end
     end
   end
diff --git a/actionview/test/template/form_collections_helper_test.rb b/actionview/test/template/form_collections_helper_test.rb
index b59be8e36c..4f7ea88169 100644
--- a/actionview/test/template/form_collections_helper_test.rb
+++ b/actionview/test/template/form_collections_helper_test.rb
@@ -202,35 +202,35 @@ class FormCollectionsHelperTest < ActionView::TestCase
     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
+    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][]" }
+    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
+    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
+    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
+    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
+    assert_select "input[type=hidden][name='user[category_ids]'][value='']", count: 0
   end
 
   # COLLECTION CHECK BOXES
diff --git a/actionview/test/template/form_helper_test.rb b/actionview/test/template/form_helper_test.rb
index 1f7ff3ca7c..bd40fa06ac 100644
--- a/actionview/test/template/form_helper_test.rb
+++ b/actionview/test/template/form_helper_test.rb
@@ -1600,11 +1600,11 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post", "new_post") do
+      "<input type='hidden' name='post[active]' value='' />" +
       "<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>" +
-      "<input type='hidden' name='post[active][]' value='' />"
+      "<label for='post_active_false'>false</label>"
     end
 
     assert_dom_equal expected, output_buffer
@@ -1622,13 +1622,13 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post", "new_post") do
+      "<input type='hidden' name='post[active]' value='' />" +
       "<label for='post_active_true'>"+
       "<input id='post_active_true' name='post[active]' type='radio' value='true' />" +
       "true</label>" +
       "<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='' />"
+      "false</label>"
     end
 
     assert_dom_equal expected, output_buffer
@@ -1648,13 +1648,13 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post_1", "new_post") do
+      "<input type='hidden' name='post[active]' value='' />" +
       "<label for='post_active_true'>"+
       "<input id='post_active_true' name='post[active]' type='radio' value='true' />" +
       "true</label>" +
       "<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
 
@@ -1670,11 +1670,11 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "foo_new_post", "new_post") do
+      "<input type='hidden' name='post[active]' value='' />" +
       "<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>" +
-      "<input type='hidden' name='post[active][]' value='' />"
+      "<label for='foo_post_active_false'>false</label>"
     end
 
     assert_dom_equal expected, output_buffer
@@ -1689,11 +1689,11 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post", "new_post") do
+      "<input type='hidden' name='post[1][active]' value='' />" +
       "<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>" +
-      "<input type='hidden' name='post[1][active][]' value='' />"
+      "<label for='post_1_active_false'>false</label>"
     end
 
     assert_dom_equal expected, output_buffer
@@ -1708,13 +1708,13 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post", "new_post") do
+      "<input name='post[tag_ids][]' type='hidden' value='' />" +
       "<input checked='checked' id='post_tag_ids_1' name='post[tag_ids][]' type='checkbox' value='1' />" +
       "<label for='post_tag_ids_1'>Tag 1</label>" +
       "<input id='post_tag_ids_2' name='post[tag_ids][]' type='checkbox' value='2' />" +
       "<label for='post_tag_ids_2'>Tag 2</label>" +
       "<input checked='checked' id='post_tag_ids_3' name='post[tag_ids][]' type='checkbox' value='3' />" +
-      "<label for='post_tag_ids_3'>Tag 3</label>" +
-      "<input name='post[tag_ids][]' type='hidden' value='' />"
+      "<label for='post_tag_ids_3'>Tag 3</label>"
     end
 
     assert_dom_equal expected, output_buffer
@@ -1732,6 +1732,7 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post", "new_post") do
+      "<input name='post[tag_ids][]' type='hidden' value='' />" +
       "<label for='post_tag_ids_1'>" +
       "<input checked='checked' id='post_tag_ids_1' name='post[tag_ids][]' type='checkbox' value='1' />" +
       "Tag 1</label>" +
@@ -1740,8 +1741,7 @@ class FormHelperTest < ActionView::TestCase
       "Tag 2</label>" +
       "<label for='post_tag_ids_3'>" +
       "<input checked='checked' id='post_tag_ids_3' name='post[tag_ids][]' type='checkbox' value='3' />" +
-      "Tag 3</label>" +
-      "<input name='post[tag_ids][]' type='hidden' value='' />"
+      "Tag 3</label>"
     end
 
     assert_dom_equal expected, output_buffer
@@ -1762,6 +1762,7 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post_1", "new_post") do
+      "<input name='post[tag_ids][]' type='hidden' value='' />"+
       "<label for='post_tag_ids_1'>" +
       "<input checked='checked' id='post_tag_ids_1' name='post[tag_ids][]' type='checkbox' value='1' />" +
       "Tag 1</label>" +
@@ -1771,7 +1772,6 @@ class FormHelperTest < ActionView::TestCase
       "<label for='post_tag_ids_3'>" +
       "<input checked='checked' id='post_tag_ids_3' name='post[tag_ids][]' type='checkbox' value='3' />" +
       "Tag 3</label>" +
-      "<input name='post[tag_ids][]' type='hidden' value='' />"+
       "<input id='post_id' name='post[id]' type='hidden' value='1' />"
     end
 
@@ -1788,9 +1788,9 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "foo_new_post", "new_post") do
+      "<input name='post[tag_ids][]' type='hidden' value='' />" +
       "<input checked='checked' id='foo_post_tag_ids_1' name='post[tag_ids][]' type='checkbox' value='1' />" +
-      "<label for='foo_post_tag_ids_1'>Tag 1</label>" +
-      "<input name='post[tag_ids][]' type='hidden' value='' />"
+      "<label for='foo_post_tag_ids_1'>Tag 1</label>"
     end
 
     assert_dom_equal expected, output_buffer
@@ -1806,9 +1806,9 @@ class FormHelperTest < ActionView::TestCase
     end
 
     expected = whole_form("/posts", "new_post", "new_post") do
+      "<input name='post[1][tag_ids][]' type='hidden' value='' />" +
       "<input checked='checked' id='post_1_tag_ids_1' name='post[1][tag_ids][]' type='checkbox' value='1' />" +
-      "<label for='post_1_tag_ids_1'>Tag 1</label>" +
-      "<input name='post[1][tag_ids][]' type='hidden' value='' />"
+      "<label for='post_1_tag_ids_1'>Tag 1</label>"
     end
 
     assert_dom_equal expected, output_buffer
-- 
cgit v1.2.3