Applied Yehuda's patch; Sharing extract_object_name_for_form! between form_helper and ajax_helper; Added script_decorator helper

3 files changed, 79 insertions, 59 deletions

diff --git a/actionpack/lib/action_view/helpers/ajax_helper.rb b/actionpack/lib/action_view/helpers/ajax_helper.rb
index f4da540f42..d6c43a1a15 100644
--- a/actionpack/lib/action_view/helpers/ajax_helper.rb
+++ b/actionpack/lib/action_view/helpers/ajax_helper.rb
@@ -3,6 +3,16 @@ module ActionView
     module AjaxHelper
       include UrlHelper
 
+      def remote_form_for(record_or_name_or_array, *args, &proc)
+        options = args.extract_options!
+        object_name = extract_object_name_for_form!(args, options, record_or_name_or_array)
+
+        concat(form_remote_tag(options))
+        fields_for(object_name, *(args << options), &proc)
+        concat('</form>'.html_safe!)
+      end
+      alias_method :form_remote_for, :remote_form_for
+
       def form_remote_tag(options = {}, &block)
         attributes = {}
         attributes.merge!(extract_remote_attributes!(options))
@@ -50,41 +60,35 @@ module ActionView
       end
 
       def observe_field(name, options = {})
-        if options[:url]
-          options[:url] = options[:url].is_a?(Hash) ? url_for(options[:url]) : options[:url]
-        end
+        url = options[:url]
+        options[:url] = url_for(url) if url && url.is_a?(Hash)
         
-        if options[:frequency]
-          case options[:frequency]
-            when 0
-              options.delete(:frequency)
-            else
-              options[:frequency] = options[:frequency].to_i
-          end
+        frequency = options.delete(:frequency)
+        if frequency && frequency != 0
+          options[:frequency] = frequency.to_i
         end
 
-        if options[:with]
-          if options[:with] !~ /[\{=(.]/
+        if with = options[:with]
+          if with !~ /[\{=(.]/
             options[:with] = "'#{options[:with]}=' + encodeURIComponent(value)"
           else
             options[:with] ||= 'value' unless options[:function]
           end
         end
 
-        if options[:function]
-          statements = options[:function] # || remote_function(options) # TODO: Need to implement remote function - BR
+        if function = options[:function]
+          statements = function # || remote_function(options) # TODO: Need to implement remote function - BR
           options[:function] = JSFunction.new(statements, "element", "value")
         end
-
         options[:name] = name
 
-        <<-SCRIPT
-        <script type="application/json" data-js-type="field_observer">
-        //<![CDATA[
-          #{options.to_json}
-        // ]]>
-        </script>
-        SCRIPT
+        script_decorator("field_observer", options)
+      end
+
+      def script_decorator(js_type, options)
+        attributes = [%(type="application/json"), %(data-js-type="#{js_type}")]
+        attributes += options.map{|k, v| %(data-#{k}="#{v}")}
+        "<script " + attributes.join(" ") + "></script>"
       end
 
       module Rails2Compatibility
@@ -124,11 +128,11 @@ module ActionView
           @statements, @arguments = statements, arguments
         end
 
-        def as_json(options = nil)
+        def to_s(options = nil)
           "function(#{@arguments.join(", ")}) {#{@statements}}"
         end
       end
 
     end
   end
-end
\ No newline at end of file
+end
diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index 20e9916d62..1615f135b4 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -262,23 +262,8 @@ module ActionView
       # FormTagHelper#form_tag.
       def form_for(record_or_name_or_array, *args, &proc)
         raise ArgumentError, "Missing block" unless block_given?
-
         options = args.extract_options!
-
-        case record_or_name_or_array
-        when String, Symbol
-          object_name = record_or_name_or_array
-        when Array
-          object = record_or_name_or_array.last
-          object_name = ActionController::RecordIdentifier.singular_class_name(object)
-          apply_form_for_options!(record_or_name_or_array, options)
-          args.unshift object
-        else
-          object = record_or_name_or_array
-          object_name = ActionController::RecordIdentifier.singular_class_name(object)
-          apply_form_for_options!([object], options)
-          args.unshift object
-        end
+        object_name = extract_object_name_for_form!(args, options, record_or_name_or_array)
 
         concat(form_tag(options.delete(:url) || {}, options.delete(:html) || {}))
         fields_for(object_name, *(args << options), &proc)
@@ -742,6 +727,25 @@ module ActionView
       def radio_button(object_name, method, tag_value, options = {})
         InstanceTag.new(object_name, method, self, options.delete(:object)).to_radio_button_tag(tag_value, options)
       end
+
+      private
+        def extract_object_name_for_form!(args, options, record_or_name_or_array)
+          case record_or_name_or_array
+          when String, Symbol
+            object_name = record_or_name_or_array
+          when Array
+            object = record_or_name_or_array.last
+            object_name = ActionController::RecordIdentifier.singular_class_name(object)
+            apply_form_for_options!(record_or_name_or_array, options)
+            args.unshift object
+          else
+            object = record_or_name_or_array
+            object_name = ActionController::RecordIdentifier.singular_class_name(object)
+            apply_form_for_options!([object], options)
+            args.unshift object
+          end
+          object_name
+        end
     end
 
     module InstanceTagMethods #:nodoc:
diff --git a/actionpack/test/javascript/ajax_test.rb b/actionpack/test/javascript/ajax_test.rb
index 41bfdbd9b9..5e62677a92 100644
--- a/actionpack/test/javascript/ajax_test.rb
+++ b/actionpack/test/javascript/ajax_test.rb
@@ -188,15 +188,10 @@ class FormRemoteTagTest < AjaxTestCase
   end
 
   test "using a :method option" do
-<<<<<<< HEAD
-    expected_form_attributes = %w(form action="/url/hash" method="post" data-remote="true" data-update-success="#glass_of_beer")
-    # TODO: Ask Katz: Why does rails do this?  Some web servers don't allow PUT or DELETE from what I remember... - BR
-=======
     expected_form_attributes = %w(form action="/url/hash" method="post" data-js-type="remote" data-update-success="#glass_of_beer")
     # TODO: Experiment with not using this _method param.  Apparently this is done to address browser incompatibilities, but since
     # we have a layer between the HTML and the JS libs now, we can probably get away with letting JS the JS libs handle the requirement
     # for an extra field if needed.
->>>>>>> 6af9c2f... Changed data-remote='true' to data-js-type='remote'
     expected_input_attributes = %w(input name="_method" type="hidden" value="put")
 
     assert_html form_remote_tag(:update => "#glass_of_beer", :url => { :action => :fast  }, :html => { :method => :put }),
@@ -341,6 +336,20 @@ class ButtonToRemoteTest < AjaxTestCase
       button(callback => "undoRequestCompleted(request)")
     end
   end
+<<<<<<< HEAD
+=======
+end
+
+class ScriptDecoratorTest < AjaxTestCase
+  def decorator()
+    script_decorator("foo_type", :foo => "bar", :baz => "bang")
+  end
+
+  test "basic" do
+    expected = %(<script type="application/json" data-js-type="foo_type" data-foo="bar" data-baz="bang"></script>)
+    assert_dom_equal expected, decorator
+  end
+>>>>>>> bd54253... Applied Yehuda's patch; Sharing extract_object_name_for_form! between form_helper and ajax_helper; Added script_decorator helper
 end
 
 class ObserveFieldTest < AjaxTestCase
@@ -358,18 +367,18 @@ class ObserveFieldTest < AjaxTestCase
   end
 
   test "using a url string" do
-    assert_data_element_json field(:url => "/some/other/url"),
-      "url" => "/some/other/url", "name" => "title"
+    assert_html field(:url => "/some/other/url"),
+      %w(script data-url="/some/other/url" data-name="title")
   end
 
   test "using a url hash" do
-    assert_data_element_json field(:url => {:controller => :blog, :action => :update}),
-      "url" => "/url/hash", "name" => "title"
+    assert_html field(:url => {:controller => :blog, :action => :update}),
+      %w(script data-url="/url/hash" data-name="title")
   end
 
   test "using a :frequency option" do
-    assert_data_element_json field(:url => { :controller => :blog }, :frequency => 5.minutes),
-      "url" => "/url/hash", "name" => "title", "frequency" => 300
+    assert_html field(:url => { :controller => :blog }, :frequency => 5.minutes),
+      %w(script data-url="/url/hash" data-name="title" data-frequency="300")
   end
 
   test "using a :frequency option of 0" do
@@ -384,19 +393,22 @@ class ObserveFieldTest < AjaxTestCase
 
   # TODO: Consider using JSON instead of strings.  Is using 'value' as a magical reference to the value of the observed field weird? (Rails2 does this) - BR
   test "using a :with option" do
-    assert_data_element_json field(:with => "foo"),
-      "name" => "title", "with" => "'foo=' + encodeURIComponent(value)"
-    assert_data_element_json field(:with => "'foo=' + encodeURIComponent(value)"),
-      "name" => "title", "with" => "'foo=' + encodeURIComponent(value)"
+    assert_html field(:with => "foo"),
+      %w(script data-name="title" data-with="'foo=' + encodeURIComponent(value)")
+
+    assert_html field(:with => "'foo=' + encodeURIComponent(value)"),
+      %w(script data-name="title" data-with="'foo=' + encodeURIComponent(value)")
+
   end
 
   test "using json in a :with option" do
-    assert_data_element_json field(:with => "{'id':value}"),
-      "name" => "title", "with" => "{'id':value}"
+    assert_html field(:with => "{'id':value}"),
+      %w(script data-name="title" data-with="{'id':value}")
   end
 
   test "using :function for callback" do
-    assert_data_element_json field(:function => "alert('Element changed')"),
-      "name" => "title", "function" => "function(element, value) {alert('Element changed')}"
+    assert_html field(:function => "alert('Element changed')"),
+      %w(script data-function="function(element, value) {alert('Element changed')}")
+
   end
 end