Fixed bug with :success/:failure callbacks for the JavaScriptHelper methods #1730 [court3nay/Thomas Fuchs]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1835 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 45 insertions, 3 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index db1a67f8f9..a034da7ae9 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed bug with :success/:failure callbacks for the JavaScriptHelper methods #1730 [court3nay/Thomas Fuchs]
+
 * Added named_route method to RouteSet instances so that RouteSet instance methods do not prevent certain names from being used. [Nicholas Seckar]
 
 * Fixed routes so that routes which do not specify :action in the path or in the requirements have a default of :action => 'index', In addition, fixed url generation so that :action => 'index' does not need to be provided for such urls. [Nicholas Seckar, Markjuh]
diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb
index 0c22996e02..43bedbd366 100644
--- a/actionpack/lib/action_view/helpers/javascript_helper.rb
+++ b/actionpack/lib/action_view/helpers/javascript_helper.rb
@@ -17,7 +17,7 @@ module ActionView
     module JavaScriptHelper      
       unless const_defined? :CALLBACKS
         CALLBACKS       = 
-          [:uninitialized, :loading, :loaded, :interactive, :complete, :failure].push((100..599).to_a).flatten
+          [:uninitialized, :loading, :loaded, :interactive, :complete, :failure, :success].push((100..599).to_a).flatten
         AJAX_OPTIONS    = [ :before, :after, :condition, :url, :asynchronous, :method, 
           :insertion, :position, :form, :with, :update, :script ].concat(CALLBACKS)
         JAVASCRIPT_PATH = File.join(File.dirname(__FILE__), 'javascripts')
diff --git a/actionpack/lib/action_view/helpers/javascripts/prototype.js b/actionpack/lib/action_view/helpers/javascripts/prototype.js
index 37635ccfab..5feddb6dda 100644
--- a/actionpack/lib/action_view/helpers/javascripts/prototype.js
+++ b/actionpack/lib/action_view/helpers/javascripts/prototype.js
@@ -268,7 +268,7 @@ Ajax.Request.prototype = (new Ajax.Base()).extend({
 
     if (event == 'Complete')
       (this.options['on' + this.transport.status]
-       || this.options['on' + this.responseIsSuccess() ? 'Success' : 'Failure']
+       || this.options['on' + (this.responseIsSuccess() ? 'Success' : 'Failure')]
        || Prototype.emptyFunction)(this.transport);
 
     (this.options['on' + event] || Prototype.emptyFunction)(this.transport);
diff --git a/actionpack/test/template/javascript_helper_test.rb b/actionpack/test/template/javascript_helper_test.rb
index 6cd690e268..15e98654f0 100644
--- a/actionpack/test/template/javascript_helper_test.rb
+++ b/actionpack/test/template/javascript_helper_test.rb
@@ -57,6 +57,46 @@ class JavaScriptHelperTest < Test::Unit::TestCase
       form_remote_tag(:update => { :success => 'glass_of_beer', :failure => "glass_of_water" }, :url => { :action => :fast  })
   end
   
+  def test_on_callbacks
+    callbacks = [:uninitialized, :loading, :loaded, :interactive, :complete, :success, :failure]
+    callbacks.each do |callback|
+      assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater('glass_of_beer', 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, on#{callback.to_s.capitalize}:function(request){monkeys();}, parameters:Form.serialize(this)}); return false;">),
+        form_remote_tag(:update => "glass_of_beer", :url => { :action => :fast  }, callback=>"monkeys();")
+      assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater({success:'glass_of_beer'}, 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, on#{callback.to_s.capitalize}:function(request){monkeys();}, parameters:Form.serialize(this)}); return false;">),
+        form_remote_tag(:update => { :success => "glass_of_beer" }, :url => { :action => :fast  }, callback=>"monkeys();")
+      assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater({failure:'glass_of_beer'}, 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, on#{callback.to_s.capitalize}:function(request){monkeys();}, parameters:Form.serialize(this)}); return false;">),
+        form_remote_tag(:update => { :failure => "glass_of_beer" }, :url => { :action => :fast  }, callback=>"monkeys();")
+      assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater({success:'glass_of_beer',failure:'glass_of_water'}, 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, on#{callback.to_s.capitalize}:function(request){monkeys();}, parameters:Form.serialize(this)}); return false;">),
+        form_remote_tag(:update => { :success => "glass_of_beer", :failure => "glass_of_water" }, :url => { :action => :fast  }, callback=>"monkeys();")
+    end
+    
+    #HTTP status codes 200 up to 599 have callbacks
+    #these should work
+    100.upto(599) do |callback|
+      assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater('glass_of_beer', 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, on#{callback.to_s.capitalize}:function(request){monkeys();}, parameters:Form.serialize(this)}); return false;">),
+        form_remote_tag(:update => "glass_of_beer", :url => { :action => :fast  }, callback=>"monkeys();")
+    end
+    
+    #test 200 and 404
+    assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater('glass_of_beer', 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, on200:function(request){monkeys();}, on404:function(request){bananas();}, parameters:Form.serialize(this)}); return false;">),
+      form_remote_tag(:update => "glass_of_beer", :url => { :action => :fast  }, 200=>"monkeys();", 404=>"bananas();")
+    
+    #these shouldn't
+    1.upto(99) do |callback|
+      assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater('glass_of_beer', 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, parameters:Form.serialize(this)}); return false;">),
+        form_remote_tag(:update => "glass_of_beer", :url => { :action => :fast  }, callback=>"monkeys();")
+    end
+    600.upto(999) do |callback|
+      assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater('glass_of_beer', 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, parameters:Form.serialize(this)}); return false;">),
+        form_remote_tag(:update => "glass_of_beer", :url => { :action => :fast  }, callback=>"monkeys();")
+    end
+    
+    #test ultimate combo
+    assert_equal %(<form action=\"http://www.example.com/fast\" method=\"post\" onsubmit=\"new Ajax.Updater('glass_of_beer', 'http://www.example.com/fast', {asynchronous:true, evalScripts:true, on200:function(request){monkeys();}, on404:function(request){bananas();}, onComplete:function(request){c();}, onFailure:function(request){f();}, onLoading:function(request){c1()}, onSuccess:function(request){s()}, parameters:Form.serialize(this)}); return false;\">),
+      form_remote_tag(:update => "glass_of_beer", :url => { :action => :fast  }, :loading => "c1()", :success => "s()", :failure => "f();", :complete => "c();", 200=>"monkeys();", 404=>"bananas();")
+    
+  end
+  
   def test_submit_to_remote
     assert_equal %(<input name=\"More beer!\" onclick=\"new Ajax.Updater('empty_bottle', 'http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:Form.serialize(this.form)}); return false;\" type=\"button\" value=\"1000000\" />),
       submit_to_remote("More beer!", 1_000_000, :update => "empty_bottle")
diff --git a/railties/html/javascripts/prototype.js b/railties/html/javascripts/prototype.js
index 37635ccfab..5feddb6dda 100644
--- a/railties/html/javascripts/prototype.js
+++ b/railties/html/javascripts/prototype.js
@@ -268,7 +268,7 @@ Ajax.Request.prototype = (new Ajax.Base()).extend({
 
     if (event == 'Complete')
       (this.options['on' + this.transport.status]
-       || this.options['on' + this.responseIsSuccess() ? 'Success' : 'Failure']
+       || this.options['on' + (this.responseIsSuccess() ? 'Success' : 'Failure')]
        || Prototype.emptyFunction)(this.transport);
 
     (this.options['on' + event] || Prototype.emptyFunction)(this.transport);