From 7034272354ad41dd4d1c90138a79e7f14ebc2bed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 1 Aug 2009 16:47:44 +0200 Subject: [PATCH] Add destroyed? to ActiveRecord, include tests for polymorphic urls for destroyed objects and refactor mime responds tests and documentation. --- .../action_controller/metal/mime_responds.rb | 31 +++++++ .../activerecord/polymorphic_routes_test.rb | 24 ++++- .../test/controller/mime_responds_test.rb | 92 +++++-------------- actionpack/test/controller/render_test.rb | 12 ++- actionpack/test/controller/render_xml_test.rb | 4 +- actionpack/test/lib/controller/fake_models.rb | 22 +++++ activerecord/lib/active_record/base.rb | 7 ++ activerecord/test/cases/base_test.rb | 17 ++++ 8 files changed, 133 insertions(+), 76 deletions(-) diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 837496e4777e4..02a88437e3921 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -330,6 +330,37 @@ def respond_to(*mimes, &block) # In this case, since @person.destroyed? returns true, polymorphic urls will # redirect to the collection url, instead of the resource url. # + # === Nested resources + # + # respond_with also works with nested resources, you just need to pass them + # as you do in form_for and polymorphic_url. Consider the project has many + # tasks example. The create action for TasksController would be like: + # + # def create + # @project = Project.find(params[:project_id]) + # @task = @project.comments.build(params[:task]) + # @task.save + # respond_with([@project, @task]) + # end + # + # Namespaced and singleton resources requires a symbol to be given, as in + # polymorphic urls. If a project has one manager with has many tasks, it + # should be invoked as: + # + # respond_with([@project, :manager, @task]) + # + # Be sure to check polymorphic_url documentation. The only occasion you will + # need to give clear input to respond_with is in DELETE verbs for singleton + # resources. In such cases, the collection url does not exist, so you need + # to supply the destination url after delete: + # + # def destroy + # @project = Project.find(params[:project_id]) + # @manager = @project.manager + # @manager.destroy + # respond_with([@project, @manager], :location => root_url) + # end + # def respond_with(resource, options={}, &block) respond_to(&block) rescue ActionView::MissingTemplate => e diff --git a/actionpack/test/activerecord/polymorphic_routes_test.rb b/actionpack/test/activerecord/polymorphic_routes_test.rb index 2036d1eeb5c5f..f001b0dab8bc8 100644 --- a/actionpack/test/activerecord/polymorphic_routes_test.rb +++ b/actionpack/test/activerecord/polymorphic_routes_test.rb @@ -52,6 +52,13 @@ def test_with_new_record end end + def test_with_destroyed_record + with_test_routes do + @project.destroy + assert_equal "http://example.com/projects", polymorphic_url(@project) + end + end + def test_with_record_and_action with_test_routes do assert_equal "http://example.com/projects/new", polymorphic_url(@project, :action => 'new') @@ -129,6 +136,14 @@ def test_with_nested_unsaved end end + def test_with_nested_destroyed + with_test_routes do + @project.save + @task.destroy + assert_equal "http://example.com/projects/#{@project.id}/tasks", polymorphic_url([@project, @task]) + end + end + def test_new_with_array_and_namespace with_admin_test_routes do assert_equal "http://example.com/admin/projects/new", polymorphic_url([:admin, @project], :action => 'new') @@ -257,6 +272,13 @@ def test_with_irregular_plural_new_record assert_equal "http://example.com/taxes", polymorphic_url(@tax) end end + + def test_with_irregular_plural_destroyed_record + with_test_routes do + @tax.destroy + assert_equal "http://example.com/taxes", polymorphic_url(@tax) + end + end def test_with_irregular_plural_record_and_action with_test_routes do @@ -395,4 +417,4 @@ def with_admin_and_site_test_routes(options = {}) end end -end \ No newline at end of file +end diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index e9c8a3c10f629..faa84e1971c15 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -1,4 +1,5 @@ require 'abstract_unit' +require 'controller/fake_models' class RespondToController < ActionController::Base layout :set_layout @@ -471,44 +472,6 @@ def test_format_with_custom_response_type_and_request_headers end end -class RespondResource - undef_method :to_json - - def self.model_name - @_model_name ||= ActiveModel::Name.new("resource") - end - - def to_param - 13 - end - - def to_xml - "XML" - end - - def to_js - "JS" - end - - def errors - [] - end - - def destroyed? - false - end -end - -class ParentResource - def self.model_name - @_model_name ||= ActiveModel::Name.new("parent") - end - - def to_param - 11 - end -end - class RespondWithController < ActionController::Base respond_to :html, :json respond_to :xml, :except => :using_defaults @@ -531,17 +494,17 @@ def default_overwritten end def using_resource - respond_with(RespondResource.new) + respond_with(Customer.new("david", 13)) end def using_resource_with_options - respond_with(RespondResource.new, :status => :unprocessable_entity) do |format| + respond_with(Customer.new("david", 13), :status => :unprocessable_entity) do |format| format.js end end def using_resource_with_parent - respond_with([ParentResource.new, RespondResource.new]) + respond_with([Quiz::Store.new("developer?", 11), Customer.new("david", 13)]) end protected @@ -550,18 +513,6 @@ def _render_js(js, options) self.content_type ||= Mime::JS self.response_body = js.respond_to?(:to_js) ? js.to_js : js end - - def resources_url - request.host + "/resources" - end - - def resource_url(resource) - request.host + "/resources/#{resource.to_param}" - end - - def parent_resource_url(parent, resource) - request.host + "/parents/#{parent.to_param}/resources/#{resource.to_param}" - end end class InheritedRespondWithController < RespondWithController @@ -569,7 +520,7 @@ class InheritedRespondWithController < RespondWithController respond_to :xml, :json def index - respond_with(RespondResource.new) do |format| + respond_with(Customer.new("david", 13)) do |format| format.json { render :text => "JSON" } end end @@ -582,6 +533,11 @@ def setup super ActionController::Base.use_accept_header = true @request.host = "www.example.com" + + ActionController::Routing::Routes.draw do |map| + map.resources :customers + map.resources :quiz_stores, :has_many => :customers + end end def teardown @@ -645,11 +601,11 @@ def test_using_resource_for_post_with_html post :using_resource assert_equal "text/html", @response.content_type assert_equal 302, @response.status - assert_equal "www.example.com/resources/13", @response.location + assert_equal "http://www.example.com/customers/13", @response.location assert @response.redirect? errors = { :name => :invalid } - RespondResource.any_instance.stubs(:errors).returns(errors) + Customer.any_instance.stubs(:errors).returns(errors) post :using_resource assert_equal "text/html", @response.content_type assert_equal 200, @response.status @@ -664,10 +620,10 @@ def test_using_resource_for_post_with_xml assert_equal "application/xml", @response.content_type assert_equal 201, @response.status assert_equal "XML", @response.body - assert_equal "www.example.com/resources/13", @response.location + assert_equal "http://www.example.com/customers/13", @response.location errors = { :name => :invalid } - RespondResource.any_instance.stubs(:errors).returns(errors) + Customer.any_instance.stubs(:errors).returns(errors) post :using_resource assert_equal "application/xml", @response.content_type assert_equal 422, @response.status @@ -679,11 +635,11 @@ def test_using_resource_for_put_with_html put :using_resource assert_equal "text/html", @response.content_type assert_equal 302, @response.status - assert_equal "www.example.com/resources/13", @response.location + assert_equal "http://www.example.com/customers/13", @response.location assert @response.redirect? errors = { :name => :invalid } - RespondResource.any_instance.stubs(:errors).returns(errors) + Customer.any_instance.stubs(:errors).returns(errors) put :using_resource assert_equal "text/html", @response.content_type assert_equal 200, @response.status @@ -698,10 +654,10 @@ def test_using_resource_for_put_with_xml assert_equal "application/xml", @response.content_type assert_equal 200, @response.status assert_equal " ", @response.body - assert_equal "www.example.com/resources/13", @response.location + assert_equal "http://www.example.com/customers/13", @response.location errors = { :name => :invalid } - RespondResource.any_instance.stubs(:errors).returns(errors) + Customer.any_instance.stubs(:errors).returns(errors) put :using_resource assert_equal "application/xml", @response.content_type assert_equal 422, @response.status @@ -710,21 +666,21 @@ def test_using_resource_for_put_with_xml end def test_using_resource_for_delete_with_html - RespondResource.any_instance.stubs(:destroyed?).returns(true) + Customer.any_instance.stubs(:destroyed?).returns(true) delete :using_resource assert_equal "text/html", @response.content_type assert_equal 302, @response.status - assert_equal "www.example.com/resources", @response.location + assert_equal "http://www.example.com/customers", @response.location end def test_using_resource_for_delete_with_xml - RespondResource.any_instance.stubs(:destroyed?).returns(true) + Customer.any_instance.stubs(:destroyed?).returns(true) @request.accept = "application/xml" delete :using_resource assert_equal "application/xml", @response.content_type assert_equal 200, @response.status assert_equal " ", @response.body - assert_equal "www.example.com/resources", @response.location + assert_equal "http://www.example.com/customers", @response.location end def test_using_resource_with_options @@ -756,10 +712,10 @@ def test_using_resource_with_parent_for_post assert_equal "application/xml", @response.content_type assert_equal 201, @response.status assert_equal "XML", @response.body - assert_equal "www.example.com/parents/11/resources/13", @response.location + assert_equal "http://www.example.com/quiz_stores/11/customers/13", @response.location errors = { :name => :invalid } - RespondResource.any_instance.stubs(:errors).returns(errors) + Customer.any_instance.stubs(:errors).returns(errors) post :using_resource assert_equal "application/xml", @response.content_type assert_equal 422, @response.status diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index f2cd6dbb85e68..9546fdb50d680 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -459,7 +459,7 @@ def head_with_location_header end def head_with_location_object - head :location => Customer.new("david") + head :location => Customer.new("david", 1) end def head_with_symbolic_status @@ -622,9 +622,6 @@ def rescue_action(e) end private - def customer_url(customer) - request.host + "/customers/1" - end def determine_layout case action_name @@ -1093,9 +1090,14 @@ def test_head_with_location_header end def test_head_with_location_object + ActionController::Routing::Routes.draw do |map| + map.resources :customers + map.connect ':controller/:action/:id' + end + get :head_with_location_object assert @response.body.blank? - assert_equal "www.nextangle.com/customers/1", @response.headers["Location"] + assert_equal "http://www.nextangle.com/customers/1", @response.headers["Location"] assert_response :ok end diff --git a/actionpack/test/controller/render_xml_test.rb b/actionpack/test/controller/render_xml_test.rb index 052b4f0b52435..139f55d8bd646 100644 --- a/actionpack/test/controller/render_xml_test.rb +++ b/actionpack/test/controller/render_xml_test.rb @@ -11,7 +11,7 @@ def render_with_location def render_with_object_location customer = Customer.new("Some guy", 1) - render :xml => "", :location => customer_url(customer), :status => :created + render :xml => "", :location => customer, :status => :created end def render_with_to_xml @@ -78,4 +78,4 @@ def test_should_use_implicit_content_type get :implicit_content_type, :format => 'atom' assert_equal Mime::ATOM, @response.content_type end -end \ No newline at end of file +end diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index c6726432ec92d..0faf8f3f9a504 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -4,9 +4,27 @@ class Customer < Struct.new(:name, :id) extend ActiveModel::Naming include ActiveModel::Conversion + undef_method :to_json + def to_param id.to_s end + + def to_xml + "XML" + end + + def to_js + "JS" + end + + def errors + [] + end + + def destroyed? + false + end end class BadCustomer < Customer @@ -24,4 +42,8 @@ def to_param id.to_s end end + + class Store < Question + end end + diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index e358564eadef6..531a698f77c1f 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -2492,6 +2492,11 @@ def new_record? @new_record || false end + # Returns true if this object has been destroyed, otherwise returns false. + def destroyed? + @destroyed || false + end + # :call-seq: # save(perform_validation = true) # @@ -2542,6 +2547,7 @@ def save! # options, use #destroy. def delete self.class.delete(id) unless new_record? + @destroyed = true freeze end @@ -2556,6 +2562,7 @@ def destroy ) end + @destroyed = true freeze end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 82eba81549704..16364141df8d9 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1271,6 +1271,23 @@ def test_new_record_returns_boolean assert_equal Topic.find(1).new_record?, false end + def test_destroyed_returns_boolean + developer = Developer.new + assert_equal developer.destroyed?, false + developer.destroy + assert_equal developer.destroyed?, true + + developer = Developer.first + assert_equal developer.destroyed?, false + developer.destroy + assert_equal developer.destroyed?, true + + developer = Developer.last + assert_equal developer.destroyed?, false + developer.delete + assert_equal developer.destroyed?, true + end + def test_clone topic = Topic.find(1) cloned_topic = nil