diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 1e446ad0befb9..398cdabca2846 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,7 @@ ## Rails 3.2.2 (unreleased) ## +* Default responder will now always use your overridden block in `respond_with` to render your response. *Prem Sichanugrist* + * check_box helper with :disabled => true will generate a disabled hidden field to conform with the HTML convention where disabled fields are not submitted with the form. This is a behavior change, previously the hidden tag had a value of the disabled checkbox. *Tadas Tamosauskas* diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index ca383be76b1ae..819bbb5463bf6 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -191,7 +191,8 @@ def clear_respond_to 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 @@ def respond_with(*resources, &block) 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 @@ def collect_mimes_from_class_level #: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 @@ def custom(mime_type, &block) end def response_for(mime) - @responses[mime] || @responses[Mime::ALL] || @default_response + @responses[mime] || @responses[Mime::ALL] end end end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 76a8c89e60845..7c4fb59c15a03 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -597,6 +597,19 @@ def index 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 @@ -968,6 +981,18 @@ def test_render_json_object_responds_to_str_still_produce_json 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 diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index f2362714d70bc..bbb4cc5ef3149 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -34,6 +34,16 @@ class BadCustomer < Customer 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