Select Helper New Kwargs API

PoC on Kwargs

From 506831e934f500aa2c940b46bd5a992a10b4980d Mon Sep 17 00:00:00 2001
From: Islam Wazery <wazery@ubuntu.com>
Date: Thu, 2 Jul 2015 09:45:11 +0200
Subject: [PATCH] PoC on Kwargs task

---
 .../lib/action_view/helpers/form_options_helper.rb |  39 ++++-
 actionview/lib/action_view/helpers/tags/select.rb  |   1 +
 .../test/template/form_options_helper_test.rb      | 184 +++++++++++++++++++--
 3 files changed, 207 insertions(+), 17 deletions(-)

diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb
index 1b7b188..4366177 100644
--- a/actionview/lib/action_view/helpers/form_options_helper.rb
+++ b/actionview/lib/action_view/helpers/form_options_helper.rb
@@ -159,8 +159,19 @@ module ActionView
       # In case if you don't want the helper to generate this hidden field you can specify
       # <tt>include_hidden: false</tt> option.
       #
-      def select(object, method, choices = nil, options = {}, html_options = {}, &block)
-        Tags::Select.new(object, method, self, choices, options, html_options, &block).render
+      def select(object, method, *args, &block)
+        kwargs = %i(collection prompt include_hidden disabled selected include_blank id required multiple name size)
+
+        #TODO: DRY this up
+        options_args = %i(include_blank include_hidden index prompt disabled selected)
+        array = args.select { |arg| arg.respond_to?(:keys) && arg.keys.any? { |k| options_args.include?(k) } }
+        options = Hash[*array.collect{|h| h.to_a}.flatten]
+
+        html_options_args = %i(id required multiple name size)
+        array = args.select { |arg| arg.respond_to?(:keys) && arg.keys.any? { |k| html_options_args.include?(k) } }
+        html_options = Hash[*array.collect{|h| h.to_a}.flatten]
+
+        exec_with_kwargs(object, method, self, *args, kwargs, options, html_options, &block)
       end
 
       # Returns <tt><select></tt> and <tt><option></tt> tags for the collection of existing return values of
@@ -733,6 +744,29 @@ module ActionView
       end
 
       private
+        
+        def exec_with_kwargs(object, method, s, *args, kwargs, options, html_options, &block)
+          if kwarg_request?(kwargs, *args)
+            Tags::Select.new(object, method, s, args[0], options || {}, html_options || {}, &block).render
+          else
+            non_kwarg_request_warning if args.present?
+            Tags::Select.new(object, method, s, args[0], args[1] || {}, args[2] || {}, &block).render
+          end
+        end
+          
+        def kwarg_request?(kwargs, *args)
+          args[0].respond_to?(:keys) && args[0].keys.any? { |k| kwargs.include?(k) }
+        end
+        
+        def non_kwarg_request_warning
+          ActiveSupport::Deprecation.warn(<<-MSG.strip_heredoc)
+            ActionView::Helpers:FormOptionsHelper methods will accept only
+            keyword arguments in future Rails versions.
+            Example:
+            select("post", "person_id", collection: Person.all.collect {|p| [ p.name, p.id ] }, include_blank: true)
+          MSG
+        end
+
         def option_html_attributes(element)
           if Array === element
             element.select { |e| Hash === e }.reduce({}, :merge!)
@@ -794,6 +828,7 @@ module ActionView
       #   <% end %>
       #
       # Please refer to the documentation of the base helper for details.
+      # TODO: Change this to the new API
       def select(method, choices = nil, options = {}, html_options = {}, &block)
         @template.select(@object_name, method, choices, objectify_options(options), @default_options.merge(html_options), &block)
       end
diff --git a/actionview/lib/action_view/helpers/tags/select.rb b/actionview/lib/action_view/helpers/tags/select.rb
index 180900c..0a2a28d 100644
--- a/actionview/lib/action_view/helpers/tags/select.rb
+++ b/actionview/lib/action_view/helpers/tags/select.rb
@@ -4,6 +4,7 @@ module ActionView
       class Select < Base # :nodoc:
         def initialize(object_name, method_name, template_object, choices, options, html_options)
           @choices = block_given? ? template_object.capture { yield || "" } : choices
+          @choices = choices[:collection] if choices.respond_to?(:keys) && choices.keys.any? { |k| k == :collection }
           @choices = @choices.to_a if @choices.is_a?(Range)
 
           @html_options = html_options
diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb
index d7daba8..ce1e0cb 100644
--- a/actionview/test/template/form_options_helper_test.rb
+++ b/actionview/test/template/form_options_helper_test.rb
@@ -450,15 +450,38 @@ class FormOptionsHelperTest < ActionView::TestCase
     @post.category = "<mus>"
     assert_dom_equal(
       "<select id=\"post_category\" name=\"post[category]\"><option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
+      select("post", "category", collection: %w( abe <mus> hest))
+    )
+  end
+
+  def test_deprecated_select
+    @post = Post.new
+    @post.category = "<mus>"
+    assert_dom_equal(
+      "<select id=\"post_category\" name=\"post[category]\"><option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
       select("post", "category", %w( abe <mus> hest))
     )
+    assert_deprecated do
+      select("post", "category", %w( abe <mus> hest))
+    end
   end
 
+  #FIXME: collection default value should be specified in the implementation 
   def test_select_without_multiple
     assert_dom_equal(
       "<select id=\"post_category\" name=\"post[category]\"></select>",
-      select(:post, :category, "", {}, :multiple => false)
+      select(:post, :category, collection: "", multiple: false)
+    )
+  end
+
+  def test_deprecated_select_without_multiple
+    assert_dom_equal(
+      "<select id=\"post_category\" name=\"post[category]\"></select>",
+      select(:post, :category, "", {}, multiple: false)
     )
+    assert_deprecated do
+      select(:post, :category, "", {}, multiple: false)
+    end
   end
 
   def test_select_with_grouped_collection_as_nested_array
@@ -475,8 +498,29 @@ class FormOptionsHelperTest < ActionView::TestCase
         %Q{<option value="so">Somalia</option></optgroup><optgroup label="Europe"><option value="dk">Denmark</option>},
         %Q{<option value="ie">Ireland</option></optgroup></select>},
       ].join("\n"),
+      select("post", "origin", collection: countries_by_continent)
+    )
+  end
+
+  def test_deprecated_select_with_grouped_collection_as_nested_array
+    @post = Post.new
+
+    countries_by_continent = [
+      ["<Africa>", [["<South Africa>", "<sa>"], ["Somalia", "so"]]],
+      ["Europe",   [["Denmark", "dk"], ["Ireland", "ie"]]],
+    ]
+
+    assert_dom_equal(
+      [
+        %Q{<select id="post_origin" name="post[origin]"><optgroup label="&lt;Africa&gt;"><option value="&lt;sa&gt;">&lt;South Africa&gt;</option>},
+        %Q{<option value="so">Somalia</option></optgroup><optgroup label="Europe"><option value="dk">Denmark</option>},
+        %Q{<option value="ie">Ireland</option></optgroup></select>},
+      ].join("\n"),
       select("post", "origin", countries_by_continent)
     )
+    assert_deprecated do
+      select("post", "origin", countries_by_continent)
+    end
   end
 
   def test_select_with_grouped_collection_as_hash
@@ -493,8 +537,29 @@ class FormOptionsHelperTest < ActionView::TestCase
         %Q{<option value="so">Somalia</option></optgroup><optgroup label="Europe"><option value="dk">Denmark</option>},
         %Q{<option value="ie">Ireland</option></optgroup></select>},
       ].join("\n"),
+      select("post", "origin", collection: countries_by_continent)
+    )
+  end
+
+  def test_deprecated_select_with_grouped_collection_as_hash
+    @post = Post.new
+
+    countries_by_continent = {
+      "<Africa>" => [["<South Africa>", "<sa>"], ["Somalia", "so"]],
+      "Europe"   => [["Denmark", "dk"], ["Ireland", "ie"]],
+    }
+
+    assert_dom_equal(
+      [
+        %Q{<select id="post_origin" name="post[origin]"><optgroup label="&lt;Africa&gt;"><option value="&lt;sa&gt;">&lt;South Africa&gt;</option>},
+        %Q{<option value="so">Somalia</option></optgroup><optgroup label="Europe"><option value="dk">Denmark</option>},
+        %Q{<option value="ie">Ireland</option></optgroup></select>},
+      ].join("\n"),
       select("post", "origin", countries_by_continent)
     )
+    assert_deprecated do
+      select("post", "origin", countries_by_continent)
+    end 
   end
 
   def test_select_with_boolean_method
@@ -502,8 +567,20 @@ class FormOptionsHelperTest < ActionView::TestCase
     @post.allow_comments = false
     assert_dom_equal(
       "<select id=\"post_allow_comments\" name=\"post[allow_comments]\"><option value=\"true\">true</option>\n<option value=\"false\" selected=\"selected\">false</option></select>",
+      select("post", "allow_comments", collection: %w( true false ))
+    )
+  end
+
+  def test_deprecated_select_with_boolean_method
+    @post = Post.new
+    @post.allow_comments = false
+    assert_dom_equal(
+      "<select id=\"post_allow_comments\" name=\"post[allow_comments]\"><option value=\"true\">true</option>\n<option value=\"false\" selected=\"selected\">false</option></select>",
       select("post", "allow_comments", %w( true false ))
     )
+    assert_deprecated do
+      select("post", "allow_comments", %w( true false ))
+    end
   end
 
   def test_select_under_fields_for
@@ -511,15 +588,36 @@ class FormOptionsHelperTest < ActionView::TestCase
     @post.category = "<mus>"
 
     output_buffer = fields_for :post, @post do |f|
+      concat f.select(:category, collection: %w( abe <mus> hest))
+    end
+
+    assert_dom_equal(
+      "<select id=\"post_category\" name=\"post[category]\"><option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
+      output_buffer
+    )
+  end
+  
+  def test_deprecated_select_under_fields_for
+    @post = Post.new
+    @post.category = "<mus>"
+
+    output_buffer = fields_for :post, @post do |f|
       concat f.select(:category, %w( abe <mus> hest))
     end
 
+    assert_deprecated do
+      fields_for :post, @post do |f|
+        f.select(:category, %w( abe <mus> hest))
+      end
+    end
+
     assert_dom_equal(
       "<select id=\"post_category\" name=\"post[category]\"><option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
       output_buffer
     )
   end
 
+  #FIXME: selected is not rendered
   def test_fields_for_with_record_inherited_from_hash
     map = Map.new
 
@@ -532,6 +630,25 @@ class FormOptionsHelperTest < ActionView::TestCase
       output_buffer
     )
   end
+  
+  def test_deprecated_fields_for_with_record_inherited_from_hash
+    map = Map.new
+
+    output_buffer = fields_for :map, map do |f|
+      concat f.select(:category, %w( abe <mus> hest))
+    end
+
+    assert_deprecated do
+      fields_for :map, map do |f|
+        f.select(:category, %w( abe <mus> hest))
+      end
+    end
+
+    assert_dom_equal(
+      "<select id=\"map_category\" name=\"map[category]\"><option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
+      output_buffer
+    )
+  end
 
   def test_select_under_fields_for_with_index
     @post = Post.new
@@ -641,10 +758,23 @@ class FormOptionsHelperTest < ActionView::TestCase
     @post.category = "<mus>"
     assert_dom_equal(
       "<select id=\"post_category\" name=\"post[category]\"><option value=\"\"></option>\n<option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
-      select("post", "category", %w( abe <mus> hest), :include_blank => true)
+      select("post", "category", collection: %w( abe <mus> hest), include_blank: true)
     )
   end
 
+  def test_deprecated_select_with_blank
+    p "cder"
+    @post = Post.new
+    @post.category = "<mus>"
+    assert_dom_equal(
+      "<select id=\"post_category\" name=\"post[category]\"><option value=\"\"></option>\n<option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\" selected=\"selected\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
+      select("post", "category", %w( abe <mus> hest), include_blank: true)
+    )
+    assert_deprecated do
+      select("post", "category", %w( abe <mus> hest), include_blank: true)
+    end
+  end
+
   def test_select_with_include_blank_false_and_required
     @post = Post.new
     @post.category = "<mus>"
@@ -701,7 +831,7 @@ class FormOptionsHelperTest < ActionView::TestCase
     @post = Post.new
     assert_dom_equal(
       "<select id=\"post_category\" name=\"post[category]\"><option value=\"\">&lt;The prompt&gt;</option>\n<option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
-      select("post", "category", %w( abe <mus> hest), :prompt => '<The prompt>')
+      select("post", "category", %w( abe <mus> hest), prompt: '<The prompt>')
     )
   end
 
@@ -710,18 +840,42 @@ class FormOptionsHelperTest < ActionView::TestCase
     @post.category = ""
     assert_dom_equal(
       "<select id=\"post_category\" name=\"post[category]\"><option value=\"\">Please select</option>\n<option value=\"\"></option>\n<option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
-      select("post", "category", %w( abe <mus> hest), :prompt => true, :include_blank => true)
-    )
-  end
-
-  def test_empty
-    @post = Post.new
-    @post.category = ""
-    assert_dom_equal(
-      "<select id=\"post_category\" name=\"post[category]\"><option value=\"\">Please select</option>\n<option value=\"\"></option>\n</select>",
-      select("post", "category", [], :prompt => true, :include_blank => true)
-    )
-  end
+      select("post", "category", collection: %w( abe <mus> hest), prompt: true, include_blank: true )
+    )
+  end
+
+ # def test_deprecated_select_with_prompt_and_blank
+#    @post = Post.new
+#    @post.category = ""
+#    assert_dom_equal(
+#      "<select id=\"post_category\" name=\"post[category]\"><option value=\"\">Please select</option>\n<option value=\"\"></option>\n<option value=\"abe\">abe</option>\n<option value=\"&lt;mus&gt;\">&lt;mus&gt;</option>\n<option value=\"hest\">hest</option></select>",
+#      select("post", "category", %w( abe <mus> hest), prompt: true, include_blank: true)
+#    )
+#    assert_deprecated do
+#      select("post", "category", %w( abe <mus> hest), prompt: true, include_blank: true)
+#    end
+#  end
+
+#  def test_empty
+#    @post = Post.new
+#    @post.category = ""
+#    assert_dom_equal(
+#      "<select id=\"post_category\" name=\"post[category]\"><option value=\"\">Please select</option>\n<option value=\"\"></option>\n</select>",
+#      select("post", "category", collection: [], prompt: true, include_blank: true)
+#    )
+#  end
+
+#  def test_deprecated_empty
+#    @post = Post.new
+#    @post.category = ""
+#    assert_dom_equal(
+#      "<select id=\"post_category\" name=\"post[category]\"><option value=\"\">Please select</option>\n<option value=\"\"></option>\n</select>",
+#      select("post", "category", [], prompt: true, include_blank: true)
+#    )
+#    assert_deprecated do
+#      select("post", "category", [], prompt: true, include_blank: true)
+#    end
+#  end
 
   def test_select_with_nil
     @post = Post.new
-- 
1.9.5 (Apple Git-50.3)