From ba51aa0b1ba7488011388ed4cf33f72917da0729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 30 Mar 2011 17:22:05 +0200 Subject: Make action_method? public and change implicit rendering to override it instead. --- actionpack/lib/abstract_controller/base.rb | 33 ++++++++++++---------- .../lib/action_controller/metal/implicit_render.rb | 6 ++-- .../new_base/render_implicit_action_test.rb | 5 ++++ 3 files changed, 25 insertions(+), 19 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index c384fd0978..07ff5ad9f3 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -128,20 +128,23 @@ module AbstractController self.class.action_methods end - private + # Returns true if the name can be considered an action. This can + # be overridden in subclasses to modify the semantics of what + # can be considered an action. + # + # For instance, this is overriden by ActionController to add + # the implicit rendering feature. + # + # ==== Parameters + # * name - The name of an action to be tested + # + # ==== Returns + # * TrueClass, FalseClass + def action_method?(name) + self.class.action_methods.include?(name) + end - # Returns true if the name can be considered an action. This can - # be overridden in subclasses to modify the semantics of what - # can be considered an action. - # - # ==== Parameters - # * name - The name of an action to be tested - # - # ==== Returns - # * TrueClass, FalseClass - def action_method?(name) - self.class.action_methods.include?(name) - end + private # Call the action. Override this in a subclass to modify the # behavior around processing an action. This, and not #process, @@ -160,8 +163,8 @@ module AbstractController # If the action name was not found, but a method called "action_missing" # was found, #method_for_action will return "_handle_action_missing". # This method calls #action_missing with the current action name. - def _handle_action_missing - action_missing(@_action_name) + def _handle_action_missing(*args) + action_missing(@_action_name, *args) end # Takes an action name and returns the name of the method that will diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 4b301c0d90..678f4ca763 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -14,10 +14,8 @@ module ActionController render end - def method_for_action(action_name) - super || if template_exists?(action_name.to_s, _prefixes) - action_name.to_s - end + def action_method?(action_name) + super || template_exists?(action_name.to_s, _prefixes) end end end diff --git a/actionpack/test/controller/new_base/render_implicit_action_test.rb b/actionpack/test/controller/new_base/render_implicit_action_test.rb index 9f69d20329..667a9021be 100644 --- a/actionpack/test/controller/new_base/render_implicit_action_test.rb +++ b/actionpack/test/controller/new_base/render_implicit_action_test.rb @@ -24,5 +24,10 @@ module RenderImplicitAction assert_body "Hello hyphen-ated!" assert_status 200 end + + test "action_method? returns true for implicit actions" do + assert SimpleController.new.action_method?(:hello_world) + assert SimpleController.new.action_method?(:"hyphen-ated") + end end end -- cgit v1.2.3 From 58c3ec1b7b7ee073edf9c245de5d06426be60a25 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 30 Mar 2011 15:29:52 -0700 Subject: use assert_equal so we get normal error messages along with our custom failure message --- actionpack/lib/action_dispatch/testing/assertions/routing.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 11e8c63fa0..b760db42e2 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -46,7 +46,7 @@ module ActionDispatch expected_options.stringify_keys! msg = build_message(message, "The recognized options did not match , difference: ", request.path_parameters, expected_options, expected_options.diff(request.path_parameters)) - assert_block(msg) { request.path_parameters == expected_options } + assert_equal(expected_options, request.path_parameters, msg) end # Asserts that the provided options can be used to generate the provided path. This is the inverse of +assert_recognizes+. @@ -84,11 +84,11 @@ module ActionDispatch found_extras = options.reject {|k, v| ! extra_keys.include? k} msg = build_message(message, "found extras , not ", found_extras, extras) - assert_block(msg) { found_extras == extras } + assert_equal(extras, found_extras, msg) msg = build_message(message, "The generated path did not match ", generated_path, expected_path) - assert_block(msg) { expected_path == generated_path } + assert_equal(expected_path, generated_path, msg) end # Asserts that path and options match both ways; in other words, it verifies that path generates -- cgit v1.2.3 From 9766997f4ce26fe0d97d7b9eebf885ddb517c80c Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Wed, 30 Mar 2011 20:53:42 +0200 Subject: when using respond_with with an invalid resource and custom options, the default response status and error messages should be returned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_controller/metal/responder.rb | 3 ++- actionpack/test/controller/mime_responds_test.rb | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 4b45413cf8..82d11c8481 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -156,7 +156,8 @@ module ActionController #:nodoc: if get? display resource elsif has_errors? - display resource.errors, :status => :unprocessable_entity + # bypass the options merging of display + controller.render format => resource.errors, :status => :unprocessable_entity elsif post? display resource, :status => :created, :location => api_location elsif has_empty_resource_definition? diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 5debf96232..eead857927 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -953,6 +953,23 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal 201, @response.status end + def test_using_resource_with_status_and_location_with_invalid_resource + errors = { :name => :invalid } + Customer.any_instance.stubs(:errors).returns(errors) + + @request.accept = "text/xml" + + post :using_resource_with_status_and_location + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + + put :using_resource_with_status_and_location + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + end + def test_using_resource_with_responder get :using_resource_with_responder assert_equal "Resource name is david", @response.body -- cgit v1.2.3 From 48404a751d7cab1556c390a5915c90947d56b46e Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Thu, 31 Mar 2011 18:18:11 +0200 Subject: only try to display an api template in responders if the request is a get or there are no errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_controller/metal/responder.rb | 13 +++++++++---- actionpack/test/controller/mime_responds_test.rb | 21 +++++++++++++++++++++ .../using_invalid_resource_with_template.xml.erb | 1 + 3 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 82d11c8481..3862a68b44 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -131,7 +131,11 @@ module ActionController #:nodoc: # responds to :to_format and display it. # def to_format - default_render + if get? || !has_errors? + default_render + else + display_errors + end rescue ActionView::MissingTemplate => e api_behavior(e) end @@ -155,9 +159,6 @@ module ActionController #:nodoc: if get? display resource - elsif has_errors? - # bypass the options merging of display - controller.render format => resource.errors, :status => :unprocessable_entity elsif post? display resource, :status => :created, :location => api_location elsif has_empty_resource_definition? @@ -210,6 +211,10 @@ module ActionController #:nodoc: controller.render given_options.merge!(options).merge!(format => resource) end + def display_errors + controller.render format => resource.errors, :status => :unprocessable_entity + end + # Check whether the resource has errors. # def has_errors? diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index eead857927..4fde08b3f5 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -558,6 +558,10 @@ class RespondWithController < ActionController::Base respond_with(resource, :location => "http://test.host/", :status => :created) end + def using_invalid_resource_with_template + respond_with(resource) + end + def using_resource_with_responder responder = proc { |c, r, o| c.render :text => "Resource name is #{r.first.name}" } respond_with(resource, :responder => responder) @@ -970,6 +974,23 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal nil, @response.location end + def test_using_invalid_resource_with_template + errors = { :name => :invalid } + Customer.any_instance.stubs(:errors).returns(errors) + + @request.accept = "text/xml" + + post :using_invalid_resource_with_template + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + + put :using_invalid_resource_with_template + assert_equal errors.to_xml, @response.body + assert_equal 422, @response.status + assert_equal nil, @response.location + end + def test_using_resource_with_responder get :using_resource_with_responder assert_equal "Resource name is david", @response.body diff --git a/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb b/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb new file mode 100644 index 0000000000..bf5869ed22 --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_invalid_resource_with_template.xml.erb @@ -0,0 +1 @@ +I should not be displayed \ No newline at end of file -- cgit v1.2.3 From b45302d7676a5e38d82662f9068ee6d832ff2e3c Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Thu, 31 Mar 2011 18:25:29 +0200 Subject: pass respond_with options to controller render when using a template for api navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- .../lib/action_controller/metal/implicit_render.rb | 4 ++-- .../lib/action_controller/metal/mime_responds.rb | 4 ++-- actionpack/lib/action_controller/metal/responder.rb | 2 +- actionpack/test/controller/mime_responds_test.rb | 19 +++++++++++++++++++ .../respond_with/using_options_with_template.xml.erb | 1 + 5 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 678f4ca763..3ec0c4c6a4 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -10,8 +10,8 @@ module ActionController end end - def default_render - render + def default_render(*args) + render(*args) end def action_method?(action_name) diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index a2e06fe0a6..7a8fa7bd86 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -189,7 +189,7 @@ module ActionController #:nodoc: 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) - response.call + response.call(nil) end end @@ -259,7 +259,7 @@ module ActionController #:nodoc: # def retrieve_response_from_mimes(mimes=nil, &block) mimes ||= collect_mimes_from_class_level - collector = Collector.new(mimes) { default_render } + collector = Collector.new(mimes) { |options| default_render(options || {}) } block.call(collector) if block_given? if format = request.negotiate_mime(collector.order) diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 3862a68b44..3d009e3de3 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -187,7 +187,7 @@ module ActionController #:nodoc: # controller. # def default_render - @default_response.call + @default_response.call(options) end # Display is just a shortcut to render a resource with the current format. diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 4fde08b3f5..41f80d0784 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -562,6 +562,11 @@ class RespondWithController < ActionController::Base respond_with(resource) end + def using_options_with_template + @customer = resource + respond_with(@customer, :status => 123, :location => "http://test.host/") + end + def using_resource_with_responder responder = proc { |c, r, o| c.render :text => "Resource name is #{r.first.name}" } respond_with(resource, :responder => responder) @@ -991,6 +996,20 @@ class RespondWithControllerTest < ActionController::TestCase assert_equal nil, @response.location end + def test_using_options_with_template + @request.accept = "text/xml" + + post :using_options_with_template + assert_equal "david", @response.body + assert_equal 123, @response.status + assert_equal "http://test.host/", @response.location + + put :using_options_with_template + assert_equal "david", @response.body + assert_equal 123, @response.status + assert_equal "http://test.host/", @response.location + end + def test_using_resource_with_responder get :using_resource_with_responder assert_equal "Resource name is david", @response.body diff --git a/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb b/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb new file mode 100644 index 0000000000..b313017913 --- /dev/null +++ b/actionpack/test/fixtures/respond_with/using_options_with_template.xml.erb @@ -0,0 +1 @@ +<%= @customer.name %> \ No newline at end of file -- cgit v1.2.3 From 7a9dafd96cd4735b9134081f1d8103a6c62a6809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 31 Mar 2011 19:00:05 +0200 Subject: Improve docs. --- .../lib/action_controller/metal/responder.rb | 31 ++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 3d009e3de3..59a3621f72 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -77,6 +77,37 @@ module ActionController #:nodoc: # # respond_with(@project, :manager, @task) # + # === Custom options + # + # respond_with also allow you to pass options that are forwarded + # to the underlying render call. Those options are only applied success + # scenarios. For instance, you can do the following in the create method above: + # + # def create + # @project = Project.find(params[:project_id]) + # @task = @project.comments.build(params[:task]) + # flash[:notice] = 'Task was successfully created.' if @task.save + # respond_with(@project, @task, :status => 201) + # end + # + # This will return status 201 if the task was saved with success. If not, + # it will simply ignore the given options and return status 422 and the + # resource errors. To customize the failure scenario, you can pass a + # a block to respond_with: + # + # def create + # @project = Project.find(params[:project_id]) + # @task = @project.comments.build(params[:task]) + # respond_with(@project, @task, :status => 201) do |format| + # if @task.save + # flash[:notice] = 'Task was successfully created.' + # else + # format.html { render "some_special_template" } + # end + # end + # end + # + # Using respond_with with a block follows the same syntax as respond_to. class Responder attr_reader :controller, :request, :format, :resource, :resources, :options -- cgit v1.2.3 From 99da42c29944beed0cb9e892ae90bfeb2590c57e Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 2 Apr 2011 00:35:33 -0300 Subject: Gem::Specification#has_rdoc= is deprecated since rubygems 1.7.0 --- actionpack/actionpack.gemspec | 2 -- 1 file changed, 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 651f3b164a..6c55842eea 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -17,8 +17,6 @@ Gem::Specification.new do |s| s.require_path = 'lib' s.requirements << 'none' - s.has_rdoc = true - s.add_dependency('activesupport', version) s.add_dependency('activemodel', version) s.add_dependency('rack-cache', '~> 1.0.0') -- cgit v1.2.3