From 7eeb0350c1a4d8afd38b63f8591884fcfff9bdb7 Mon Sep 17 00:00:00 2001
From: Subba Rao Pasupuleti <subbarao.pasupuleti@gmail.com>
Date: Fri, 13 Aug 2010 16:31:16 -0400
Subject: [PATCH] select tags coerce the :selected option, options
 to strings before comparison
 [#5056 state:resolved]

---
 .../lib/action_view/helpers/form_options_helper.rb |   48 ++++++++++++-------
 .../test/template/form_options_helper_test.rb      |   35 ++++++++++++++
 2 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb
index ee34452..7f43d7b 100644
--- a/actionpack/lib/action_view/helpers/form_options_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -294,18 +294,34 @@ module ActionView
       #     <option value="Free" selected="selected">Free</option>\n<option value="Basic">Basic</option>\n<option value="Advanced">Advanced</option>\n<option value="Super Platinum" disabled="disabled">Super Platinum</option>
       #
       # NOTE: Only the option tags are returned, you have to wrap this call in a regular HTML select tag.
-      def options_for_select(container, selected = nil)
-        return container if String === container
+      def options_for_select(container, *args)
+        if container && container.is_a?(String)
+         return container 
+        end
+
+        if container && container.is_a?(Hash)
+         container = container.to_a 
+        end
+
+        selected_options = args.extract_options!.symbolize_keys
+        selected = selected_options[:selected] || args.first
+        disabled = selected_options[:disabled]
 
-        container = container.to_a if Hash === container
-        selected, disabled = extract_selected_and_disabled(selected)
+        selected, disabled = [ selected, disabled ].map { |r| Array.wrap(r).map(&:to_s) }
 
         options_for_select = container.inject([]) do |options, element|
           html_attributes = option_html_attributes(element)
-          text, value = option_text_and_value(element)
-          selected_attribute = ' selected="selected"' if option_value_selected?(value, selected)
-          disabled_attribute = ' disabled="disabled"' if disabled && option_value_selected?(value, disabled)
-          options << %(<option value="#{html_escape(value.to_s)}"#{selected_attribute}#{disabled_attribute}#{html_attributes}>#{html_escape(text.to_s)}</option>)
+          text, value = option_text_and_value(element).map(&:to_s)
+
+          if selected && option_value_selected?(value, selected)
+            selected_attribute = ' selected="selected"' 
+          end
+
+          if disabled && option_value_selected?(value, disabled)
+            disabled_attribute = ' disabled="disabled"' 
+          end
+
+          options << %(<option value="#{html_escape(value)}"#{selected_attribute}#{disabled_attribute}#{html_attributes}>#{html_escape(text)}</option>)
         end
 
         options_for_select.join("\n").html_safe
@@ -335,11 +351,15 @@ module ActionView
       # Will not select a person with the id of 1 because 1 (an Integer) is not the same as '1' (a string)
       #   options_from_collection_for_select(@people, 'id', 'name', 1)
       # should produce the desired results.
-      def options_from_collection_for_select(collection, value_method, text_method, selected = nil)
+      def options_from_collection_for_select(collection, value_method, text_method, *args)
+        selected_options = args.extract_options!.symbolize_keys
+        selected = selected_options[:selected] || args.first
+        disabled = selected_options[:disabled]
+
         options = collection.map do |element|
           [element.send(text_method), element.send(value_method)]
         end
-        selected, disabled = extract_selected_and_disabled(selected)
+
         select_deselect = {}
         select_deselect[:selected] = extract_values_from_collection(collection, value_method, selected)
         select_deselect[:disabled] = extract_values_from_collection(collection, value_method, disabled)
@@ -527,14 +547,6 @@ module ActionView
           end
         end
 
-        def extract_selected_and_disabled(selected)
-          if selected.is_a?(Hash)
-            [selected[:selected], selected[:disabled]]
-          else
-            [selected, nil]
-          end
-        end
-
         def extract_values_from_collection(collection, value_method, selected)
           if selected.is_a?(Proc)
             collection.map do |element|
diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb
index d14e502..51d98ef 100644
--- a/actionpack/test/template/form_options_helper_test.rb
+++ b/actionpack/test/template/form_options_helper_test.rb
@@ -176,6 +176,41 @@ class FormOptionsHelperTest < ActionView::TestCase
     )
   end
 
+  def test_collection_options_with_preselected_value_as_string_and_option_value_is_integer
+    albums = [ Album.new(1, "first","rap"), Album.new(2, "second","pop")] 
+    assert_dom_equal(
+    %(<option selected="selected" value="1">rap</option>\n<option value="2">pop</option>),
+    options_from_collection_for_select(albums, "id", "genre", :selected => "1")
+    )
+  end
+
+  def test_collection_options_with_preselected_value_as_integer_and_option_value_is_string
+    albums = [ Album.new("1", "first","rap"), Album.new("2", "second","pop")] 
+
+    assert_dom_equal(
+    %(<option selected="selected" value="1">rap</option>\n<option value="2">pop</option>),
+    options_from_collection_for_select(albums, "id", "genre", :selected => 1)
+    )
+  end
+
+  def test_collection_options_with_preselected_value_as_string_and_option_value_is_float
+    albums = [ Album.new(1.0, "first","rap"), Album.new(2.0, "second","pop")] 
+
+    assert_dom_equal(
+    %(<option value="1.0">rap</option>\n<option value="2.0" selected="selected">pop</option>),
+    options_from_collection_for_select(albums, "id", "genre", :selected => "2.0")
+    )
+  end
+
+  def test_collection_options_with_preselected_values_as_string_array_and_option_value_is_float
+    albums = [ Album.new(1.0, "first","rap"), Album.new(2.0, "second","pop"), Album.new(3.0, "third","country") ] 
+
+    assert_dom_equal(
+    %(<option value="1.0" selected="selected">rap</option>\n<option value="2.0">pop</option>\n<option value="3.0" selected="selected">country</option>),
+    options_from_collection_for_select(albums, "id", "genre", ["1.0","3.0"])
+    )
+  end
+
   def test_option_groups_from_collection_for_select
     assert_dom_equal(
       "<optgroup label=\"&lt;Africa&gt;\"><option value=\"&lt;sa&gt;\">&lt;South Africa&gt;</option>\n<option value=\"so\">Somalia</option></optgroup><optgroup label=\"Europe\"><option value=\"dk\" selected=\"selected\">Denmark</option>\n<option value=\"ie\">Ireland</option></optgroup>",
-- 
1.7.0.4