mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Fix override API response bug in respond_with
Default responder was only using the given respond block when user requested for HTML format, or JSON/XML format with valid resource. This fix the responder so that it will use the given block regardless of the validity of the resource. Note that in this case you'll have to check for object's validity by yourself in the controller. Fixes #4796
This commit is contained in:
parent
d709b124d1
commit
3def1c8edb
4 changed files with 60 additions and 8 deletions
|
@ -1,5 +1,7 @@
|
|||
## Rails 4.0.0 (unreleased) ##
|
||||
|
||||
* Default responder will now always use your overridden block in `respond_with` to render your response. *Prem Sichanugrist*
|
||||
|
||||
* Allow `value_method` and `text_method` arguments from `collection_select` and
|
||||
`options_from_collection_for_select` to receive an object that responds to `:call`,
|
||||
such as a `proc`, to evaluate the option in the current element context. This works
|
||||
|
|
|
@ -191,7 +191,8 @@ module ActionController #:nodoc:
|
|||
def respond_to(*mimes, &block)
|
||||
raise ArgumentError, "respond_to takes either types or a block, never both" if mimes.any? && block_given?
|
||||
|
||||
if response = retrieve_response_from_mimes(mimes, &block)
|
||||
if collector = retrieve_collector_from_mimes(mimes, &block)
|
||||
response = collector.response_for(negotiated_format(collector)) || collector.default_response
|
||||
response.call(nil)
|
||||
end
|
||||
end
|
||||
|
@ -232,10 +233,19 @@ module ActionController #:nodoc:
|
|||
raise "In order to use respond_with, first you need to declare the formats your " <<
|
||||
"controller responds to in the class level" if self.class.mimes_for_respond_to.empty?
|
||||
|
||||
if response = retrieve_response_from_mimes(&block)
|
||||
if collector = retrieve_collector_from_mimes(&block)
|
||||
options = resources.size == 1 ? {} : resources.extract_options!
|
||||
options.merge!(:default_response => response)
|
||||
(options.delete(:responder) || self.class.responder).call(self, resources, options)
|
||||
|
||||
if defined_response = collector.response_for(negotiated_format(collector))
|
||||
if action = options.delete(:action)
|
||||
render :action => action
|
||||
else
|
||||
defined_response.call(nil)
|
||||
end
|
||||
else
|
||||
options.merge!(:default_response => collector.default_response)
|
||||
(options.delete(:responder) || self.class.responder).call(self, resources, options)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -263,24 +273,29 @@ module ActionController #:nodoc:
|
|||
# Collects mimes and return the response for the negotiated format. Returns
|
||||
# nil if :not_acceptable was sent to the client.
|
||||
#
|
||||
def retrieve_response_from_mimes(mimes=nil, &block) #:nodoc:
|
||||
def retrieve_collector_from_mimes(mimes=nil, &block) #:nodoc:
|
||||
mimes ||= collect_mimes_from_class_level
|
||||
collector = Collector.new(mimes) { |options| default_render(options || {}) }
|
||||
block.call(collector) if block_given?
|
||||
|
||||
if format = request.negotiate_mime(collector.order)
|
||||
if format = negotiated_format(collector)
|
||||
self.content_type ||= format.to_s
|
||||
lookup_context.freeze_formats([format.to_sym])
|
||||
collector.response_for(format)
|
||||
collector
|
||||
else
|
||||
head :not_acceptable
|
||||
nil
|
||||
end
|
||||
end
|
||||
|
||||
def negotiated_format(collector)
|
||||
request.negotiate_mime(collector.order)
|
||||
end
|
||||
|
||||
class Collector #:nodoc:
|
||||
include AbstractController::Collector
|
||||
attr_accessor :order
|
||||
attr_reader :default_response
|
||||
|
||||
def initialize(mimes, &block)
|
||||
@order, @responses, @default_response = [], {}, block
|
||||
|
@ -303,7 +318,7 @@ module ActionController #:nodoc:
|
|||
end
|
||||
|
||||
def response_for(mime)
|
||||
@responses[mime] || @responses[Mime::ALL] || @default_response
|
||||
@responses[mime] || @responses[Mime::ALL]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -593,6 +593,19 @@ class RenderJsonRespondWithController < RespondWithController
|
|||
format.json { render :json => RenderJsonTestException.new('boom') }
|
||||
end
|
||||
end
|
||||
|
||||
def create
|
||||
resource = ValidatedCustomer.new(params[:name], 1)
|
||||
respond_with(resource) do |format|
|
||||
format.json do
|
||||
if resource.errors.empty?
|
||||
render :json => { :valid => true }
|
||||
else
|
||||
render :json => { :valid => false }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
class EmptyRespondWithController < ActionController::Base
|
||||
|
@ -964,6 +977,18 @@ class RespondWithControllerTest < ActionController::TestCase
|
|||
assert_match(/"error":"RenderJsonTestException"/, @response.body)
|
||||
end
|
||||
|
||||
def test_api_response_with_valid_resource_respect_override_block
|
||||
@controller = RenderJsonRespondWithController.new
|
||||
post :create, :name => "sikachu", :format => :json
|
||||
assert_equal '{"valid":true}', @response.body
|
||||
end
|
||||
|
||||
def test_api_response_with_invalid_resource_respect_override_block
|
||||
@controller = RenderJsonRespondWithController.new
|
||||
post :create, :name => "david", :format => :json
|
||||
assert_equal '{"valid":false}', @response.body
|
||||
end
|
||||
|
||||
def test_no_double_render_is_raised
|
||||
@request.accept = "text/html"
|
||||
assert_raise ActionView::MissingTemplate do
|
||||
|
|
|
@ -34,6 +34,16 @@ end
|
|||
class GoodCustomer < Customer
|
||||
end
|
||||
|
||||
class ValidatedCustomer < Customer
|
||||
def errors
|
||||
if name =~ /Sikachu/i
|
||||
[]
|
||||
else
|
||||
[{:name => "is invalid"}]
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
module Quiz
|
||||
class Question < Struct.new(:name, :id)
|
||||
extend ActiveModel::Naming
|
||||
|
|
Loading…
Reference in a new issue