Refactored ajax helpers so they use a little bit more coherent pattern; Removed code duplication from form_remote_tag

2 files changed, 116 insertions, 105 deletions

diff --git a/actionpack/lib/action_view/helpers/ajax_helper.rb b/actionpack/lib/action_view/helpers/ajax_helper.rb
index da639b0658..93a79766dc 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 extract_remote_attributes!(options)
+        attributes = options.delete(:html) || {}
+
+        attributes.merge!(extract_update_attributes!(options))
+        attributes.merge!(extract_request_attributes!(options))
+        attributes["data-js-type"] = options.delete(:js_type) || "remote"
+
+        attributes
+      end
+
       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)
@@ -18,26 +28,8 @@ module ActionView
         attributes.merge!(extract_remote_attributes!(options))
         attributes.merge!(options)
 
-        url = attributes.delete(:url)
-        form_tag(attributes.delete(:action) || url_for(url), attributes, &block)
-      end
-
-      def extract_remote_attributes!(options)
-        attributes = options.delete(:html) || {}
-        
-        update = options.delete(:update)
-        if update.is_a?(Hash)
-          attributes["data-update-success"] = update[:success]
-          attributes["data-update-failure"] = update[:failure]
-        else
-          attributes["data-update-success"] = update
-        end
-
-        attributes["data-update-position"] = options.delete(:position)
-        attributes["data-method"]          = options.delete(:method)
-        attributes["data-js-type"]         = "remote"
-
-        attributes
+        url = attributes.delete("data-url")
+        form_tag(attributes.delete(:action) || url, attributes, &block)
       end
 
       def link_to_remote(name, url, options = {})
@@ -56,64 +48,37 @@ module ActionView
         tag(:input, attributes)
       end
 
-      def submit_to_remote(name, value, options = {})
-        html_options = options.delete(:html) || {}
-        html_options.merge!(:name => name, :value => value, :type => "submit")
-
-        attributes = extract_remote_attributes!(options)
-        attributes.merge!(html_options)
-
-        tag(:input, attributes)
-      end
-
       def periodically_call_remote(options = {})
-        attributes = extract_observer_attributes!(options)
-        attributes["data-js-type"] = "periodical_executer"
-
-        script_decorator(attributes)
+#         frequency = options[:frequency] || 10 # every ten seconds by default
+#         code = "new PeriodicalExecuter(function() {#{remote_function(options)}}, #{frequency})"
+#         javascript_tag(code)
       end
 
-      #TODO: Should name change to a css query? - BR
       def observe_field(name, options = {})
-        options[:observed] = name
-        attributes = extract_observer_attributes!(options)
-        attributes["data-js-type"] = "field_observer"
-
-        script_decorator(attributes)
-      end
-
-      def observe_field(name, options = {})
-        url = options[:url]
-        options[:url] = url_for(url) if url && url.is_a?(Hash)
-        
+        attributes = extract_remote_attributes!(options)
+        callback = options.delete(:function)
         frequency = options.delete(:frequency)
-        if frequency && frequency != 0
-          options[:frequency] = frequency.to_i
-        end
 
-        if with = options[:with]
-          if with !~ /[\{=(.]/
-            options[:with] = "'#{options[:with]}=' + encodeURIComponent(value)"
-          else
-            options[:with] ||= 'value' unless options[:function]
-          end
-        end
+        attributes["data-name"] = name
+        attributes["data-js-type"] = "field_observer"
 
-        if function = options[:function]
-          statements = function # || remote_function(options) # TODO: Need to implement remote function - BR
-          options[:function] = JSFunction.new(statements, "element", "value")
+        if callback
+          attributes["data-observer-code"] = create_js_function(callback, "element", "value")
+        end
+        if frequency && frequency != 0
+          attributes["data-frequency"] = frequency.to_i
         end
-        options[:name] = name
 
-        script_decorator("field_observer", options)
+        script_decorator(attributes)
       end
 
-      def script_decorator(js_type, options)
-        attributes = [%(type="application/json"), %(data-js-type="#{js_type}")]
-        attributes += options.map{|k, v| %(data-#{k}="#{v}")}
+      def script_decorator(options)
+        attributes = %w(type="application/json")
+        attributes += options.map{|k, v| k + '="' + v.to_s + '"'}
         "<script " + attributes.join(" ") + "></script>"
       end
 
+      # TODO: All evaled goes here per wycats
       module Rails2Compatibility
         def set_callbacks(options, html)
           [:complete, :failure, :success, :interactive, :loaded, :loading].each do |type|
@@ -144,15 +109,50 @@ module ActionView
 
       private
 
-      # TODO: Move to javascript helpers - BR
-      class JSFunction
-        def initialize(statements, *arguments)
-          @statements, @arguments = statements, arguments
+      def extract_request_attributes!(options)
+        attributes = {}
+        attributes["data-method"] = options.delete(:method)
+
+        url = options.delete(:url)
+        attributes["data-url"] = url.is_a?(Hash) ? url_for(url) : url
+
+        #TODO: Remove all references to prototype - BR
+        if options.delete(:form)
+          attributes["data-parameters"] = 'Form.serialize(this)'
+        elsif submit = options.delete(:submit)
+          attributes["data-parameters"] = "Form.serialize('#{submit}')"
+        elsif with = options.delete(:with)
+          if with !~ /[\{=(.]/
+            attributes["data-with"] = "'#{with}=' + encodeURIComponent(value)"
+          else
+            attributes["data-with"] = with
+          end
         end
 
-        def to_s(options = nil)
-          "function(#{@arguments.join(", ")}) {#{@statements}}"
+        purge_unused_attributes!(attributes)
+      end
+
+      def extract_update_attributes!(options)
+        attributes = {}
+        update = options.delete(:update)
+        if update.is_a?(Hash)
+          attributes["data-update-success"] = update[:success]
+          attributes["data-update-failure"] = update[:failure]
+        else
+          attributes["data-update-success"] = update
         end
+        attributes["data-update-position"] = options.delete(:position)
+
+        purge_unused_attributes!(attributes)
+      end
+
+      def purge_unused_attributes!(attributes)
+        attributes.delete_if {|key, value| value.nil? }
+        attributes
+      end
+
+      def create_js_function(statements, *arguments)
+        "function(#{arguments.join(", ")}) {#{statements}}"
       end
 
     end
diff --git a/actionpack/test/javascript/ajax_test.rb b/actionpack/test/javascript/ajax_test.rb
index 5e62677a92..2ca47344e5 100644
--- a/actionpack/test/javascript/ajax_test.rb
+++ b/actionpack/test/javascript/ajax_test.rb
@@ -1,7 +1,5 @@
 require "abstract_unit"
 
-#TODO: Switch to assert_dom_equal where appropriate.  assert_html is not robust enough for all tests - BR
-
 class AjaxTestCase < ActionView::TestCase
   include ActionView::Helpers::AjaxHelper
 
@@ -28,20 +26,6 @@ class AjaxTestCase < ActionView::TestCase
     end
   end
 
-  def extract_json_from_data_element(data_element)
-    root = HTML::Document.new(data_element).root
-    script = root.find(:tag => "script")
-    cdata = script.children.detect {|child| child.to_s =~ /<!\[CDATA\[/ }
-    js = cdata.content.split("\n").map {|line| line.gsub(Regexp.new("//.*"), "")}.join("\n").strip!
-
-    ActiveSupport::JSON.decode(js)
-  end
-
-  def assert_data_element_json(actual, expected)
-    json = extract_json_from_data_element(actual)
-    assert_equal expected, json
-  end
-
   def self.assert_callbacks_work(&blk)
     define_method(:assert_callbacks_work, &blk)
 
@@ -140,8 +124,8 @@ class FormRemoteTagTest < AjaxTestCase
 
   # TODO: Play with using assert_dom_equal
   test "basic" do
-    assert_html form_remote_tag(:update => "#glass_of_beer", :url => { :action => :fast  }),
-      %w(form action="/url/hash" method="post" data-js-type="remote" data-update-success="#glass_of_beer")
+    assert_dom_equal %(<form action="/url/hash" method="post" data-js-type="remote" data-update-success="#glass_of_beer">),
+      form_remote_tag(:update => "#glass_of_beer", :url => { :action => :fast  })
   end
 
   test "when protect_against_forgery? is true" do
@@ -246,6 +230,42 @@ class Article
   end
 end
 
+class ExtractRemoteAttributesTest < AjaxTestCase
+  attr_reader :attributes
+
+  test "extract_remote_attributes! html" do
+    attributes = extract_remote_attributes!(:html => { :class => "css_klass", :style => "border:1px solid"})
+    assert_equal "css_klass", attributes[:class]
+    assert_equal "border:1px solid", attributes[:style]
+  end
+
+  test "extract_remote_attributes! update options when :update is a hash" do
+    attributes = extract_remote_attributes!(:update => { :success => "foo", :failure => "bar" })
+    assert_equal "foo", attributes["data-update-success"]
+    assert_equal "bar", attributes["data-update-failure"]
+  end
+
+  test "extract_remote_attributes! update options when :update is string" do
+    attributes = extract_remote_attributes!(:update => "baz")
+    assert_equal "baz", attributes["data-update-success"]
+  end
+
+  test "extract_remote_attributes! position" do
+    attributes = extract_remote_attributes!(:position => "before")
+    assert_equal "before", attributes["data-update-position"]
+  end
+
+  test "extract_remote_attributes! data-js-type when it is NOT passed" do
+    attributes = extract_remote_attributes!({})
+    assert_equal "remote", attributes["data-js-type"]
+  end
+
+  test "extract_remote_attributes! data-js-type when it passed" do
+    attributes = extract_remote_attributes!(:js_type => "some_type")
+    assert_equal "some_type", attributes["data-js-type"]
+  end
+end
+
 class RemoteFormForTest < AjaxTestCase
 
   def setup
@@ -321,11 +341,8 @@ class ButtonToRemoteTest < AjaxTestCase
   
   class StandardTest < ButtonToRemoteTest
     test "basic" do
-      button = button({:url => {:action => "whatnot"}}, {:class => "fine"})
-      [/input/, /class="fine"/, /type="button"/, /value="Remote outpost"/,
-       /data-url="\/whatnot"/].each do |match|
-         assert_match match, button
-      end
+      assert_html button({:url => {:action => "whatnot"}}, {:class => "fine", :value => "RemoteOutpost"}),
+        %w(input class="fine" type="button" value="RemoteOutpost" data-url="/url/hash")
     end
   end
   
@@ -336,20 +353,17 @@ class ButtonToRemoteTest < AjaxTestCase
       button(callback => "undoRequestCompleted(request)")
     end
   end
-<<<<<<< HEAD
-=======
 end
 
 class ScriptDecoratorTest < AjaxTestCase
   def decorator()
-    script_decorator("foo_type", :foo => "bar", :baz => "bang")
+    script_decorator("data-js-type" => "foo_type", "data-foo" => "bar", "data-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
@@ -385,11 +399,10 @@ class ObserveFieldTest < AjaxTestCase
     assert_no_match /frequency/, field(:frequency => 0)
   end
 
-  # TODO: Finish when remote_function or some equivilent is finished -BR
-#  def test_observe_field
-#    assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nnew Form.Element.Observer('glass', 300, function(element, value) {new Ajax.Request('http://www.example.com/reorder_if_empty', {asynchronous:true, evalScripts:true, parameters:value})})\n//]]>\n</script>),
-#      observe_field("glass", :frequency => 5.minutes, :url => { :action => "reorder_if_empty" })
-#  end
+  test "observe field with common options" do
+    assert_html observe_field("glass", :frequency => 5.minutes, :url => { :action => "reorder_if_empty" }),
+      %w(script data-name="glass" data-frequency="300" data-url="/url/hash")
+  end
 
   # 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
@@ -398,7 +411,6 @@ class ObserveFieldTest < AjaxTestCase
 
     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
@@ -408,7 +420,6 @@ class ObserveFieldTest < AjaxTestCase
 
   test "using :function for callback" do
     assert_html field(:function => "alert('Element changed')"),
-      %w(script data-function="function(element, value) {alert('Element changed')}")
-
+      %w(script data-observer-code="function(element, value) {alert('Element changed')}")
   end
 end