Merge pull request #5298 from sikachu/master-fix-responder

Always passing a respond block from to responder
rails · Mar 6, 2012 · f97e3ed · f97e3ed
2 parents 9beb0b6 + 284041c
commit f97e3ed
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 12 deletions.
diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb
@@ -235,16 +235,8 @@ def respond_with(*resources, &block)
 
       if collector = retrieve_collector_from_mimes(&block)
         options = resources.size == 1 ? {} : resources.extract_options!
-
-        if defined_response = collector.response
-          if action = options.delete(:action)
-            render :action => action
-          else
-            defined_response.call
-          end
-        else
-          (options.delete(:responder) || self.class.responder).call(self, resources, options)
-        end
+        options[:default_response] = collector.response
+        (options.delete(:responder) || self.class.responder).call(self, resources, options)
       end
     end
 

diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb
@@ -130,6 +130,7 @@ def initialize(controller, resources, options={})
       @resources = resources
       @options = options
       @action = options.delete(:action)
+      @default_response = options.delete(:default_response)
     end
 
     delegate :head, :render, :redirect_to,   :to => :controller
@@ -172,7 +173,7 @@ def to_js
     # responds to :to_format and display it.
     #
     def to_format
-      if get? || !has_errors?
+      if get? || !has_errors? || response_overridden?
         default_render
       else
         display_errors
@@ -226,7 +227,11 @@ def resource_location
     # controller.
     #
     def default_render
-      controller.default_render(options)
+      if @default_response
+        @default_response.call(options)
+      else
+        controller.default_render(options)
+      end
     end
 
     # Display is just a shortcut to render a resource with the current format.
@@ -274,5 +279,9 @@ def resource_errors
     def json_resource_errors
       {:errors => resource.errors}
     end
+
+    def response_overridden?
+      @default_response.present?
+    end
   end
 end
diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb
@@ -1183,3 +1183,39 @@ def test_format_with_inherited_layouts
     assert_equal '<html><div id="super_iphone">Super iPhone</div></html>', @response.body
   end
 end
+
+class FlashResponder < ActionController::Responder
+  def initialize(controller, resources, options={})
+    super
+  end
+
+  def to_html
+    controller.flash[:notice] = 'Success'
+    super
+  end
+end
+
+class FlashResponderController < ActionController::Base
+  self.responder = FlashResponder
+  respond_to :html
+
+  def index
+    respond_with Object.new do |format|
+      format.html { render :text => 'HTML' }
+    end
+  end
+end
+
+class FlashResponderControllerTest < ActionController::TestCase
+  tests FlashResponderController
+
+  def test_respond_with_block_executed
+    get :index
+    assert_equal 'HTML', @response.body
+  end
+
+  def test_flash_responder_executed
+    get :index
+    assert_equal 'Success', flash[:notice]
+  end
+end