From 1129a24caff9f1804c2bff6569c0cbd8598dfa86 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Thu, 21 Aug 2008 21:02:10 -0500 Subject: Cleanup around partial rendering Signed-off-by: Joshua Peek --- actionpack/lib/action_controller/base.rb | 20 +----- actionpack/lib/action_controller/rescue.rb | 2 +- .../templates/rescues/diagnostics.erb | 4 +- .../templates/rescues/template_error.erb | 4 +- actionpack/lib/action_view/base.rb | 43 +++-------- actionpack/lib/action_view/partials.rb | 83 +++++++++++++--------- 6 files changed, 66 insertions(+), 90 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 082f09ff16..5887e1e3b4 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -780,9 +780,6 @@ module ActionController #:nodoc: # render :file => "/path/to/some/template.erb", :layout => true, :status => 404 # render :file => "c:/path/to/some/template.erb", :layout => true, :status => 404 # - # # Renders a template relative to the template root and chooses the proper file extension - # render :file => "some/template", :use_full_path => true - # # === Rendering text # # Rendering of text is usually used for tests or for rendering prepared content, such as a cache. By default, text @@ -913,21 +910,10 @@ module ActionController #:nodoc: response.content_type ||= Mime::JSON render_for_text(json, options[:status]) - elsif partial = options[:partial] - partial = default_template_name if partial == true + elsif options[:partial] + options[:partial] = default_template_name if options[:partial] == true add_variables_to_assigns - - if collection = options[:collection] - render_for_text( - @template.send!(:render_partial_collection, partial, collection, - options[:spacer_template], options[:locals], options[:as]), options[:status] - ) - else - render_for_text( - @template.send!(:render_partial, partial, - options[:object], options[:locals]), options[:status] - ) - end + render_for_text(@template.render(options), options[:status]) elsif options[:update] add_variables_to_assigns diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 482ac7d7a4..a1a9d68a35 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -182,7 +182,7 @@ module ActionController #:nodoc: @template.instance_variable_set("@rescues_path", File.dirname(rescues_path("stub"))) @template.send!(:assign_variables_from_controller) - @template.instance_variable_set("@contents", @template.render(:file => template_path_for_local_rescue(exception), :use_full_path => false)) + @template.instance_variable_set("@contents", @template.render(:file => template_path_for_local_rescue(exception))) response.content_type = Mime::HTML render_for_file(rescues_path("layout"), response_code_for_rescue(exception)) diff --git a/actionpack/lib/action_controller/templates/rescues/diagnostics.erb b/actionpack/lib/action_controller/templates/rescues/diagnostics.erb index 385c6c1b09..b5483eccae 100644 --- a/actionpack/lib/action_controller/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_controller/templates/rescues/diagnostics.erb @@ -6,6 +6,6 @@ 
<%=h @exception.clean_message %>
-<%= render(:file => @rescues_path + "/_trace.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_trace.erb") %> -<%= render(:file => @rescues_path + "/_request_and_response.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_request_and_response.erb") %> diff --git a/actionpack/lib/action_controller/templates/rescues/template_error.erb b/actionpack/lib/action_controller/templates/rescues/template_error.erb index 4aecc68d18..76fa3df89d 100644 --- a/actionpack/lib/action_controller/templates/rescues/template_error.erb +++ b/actionpack/lib/action_controller/templates/rescues/template_error.erb @@ -15,7 +15,7 @@ <% @real_exception = @exception @exception = @exception.original_exception || @exception %> -<%= render(:file => @rescues_path + "/_trace.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_trace.erb") %> <% @exception = @real_exception %> -<%= render(:file => @rescues_path + "/_request_and_response.erb", :use_full_path => false) %> +<%= render(:file => @rescues_path + "/_request_and_response.erb") %> diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 46bacbcbc1..cdbb71bdaa 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -239,7 +239,7 @@ module ActionView #:nodoc: local_assigns ||= {} if options.is_a?(String) - render_file(options, nil, local_assigns) + render(:file => options, :locals => local_assigns) elsif options == :update update_page(&block) elsif options.is_a?(Hash) @@ -262,13 +262,15 @@ module ActionView #:nodoc: end end elsif options[:file] - render_file(options[:file], nil, options[:locals]) - elsif options[:partial] && options.has_key?(:collection) - render_partial_collection(options[:partial], options[:collection], options[:spacer_template], options[:locals], options[:as]) + if options[:use_full_path] + ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller) + end + + pick_template(options[:file]).render_template(self, options[:locals]) elsif options[:partial] - render_partial(options[:partial], options[:object], options[:locals]) + render_partial(options) elsif options[:inline] - render_inline(options[:inline], options[:locals], options[:type]) + InlineTemplate.new(options[:inline], options[:type]).render(self, options[:locals]) end end end @@ -345,35 +347,6 @@ module ActionView #:nodoc: memoize :pick_template private - # Renders the template present at template_path. The hash in local_assigns - # is made available as local variables. - def render_file(template_path, use_full_path = nil, local_assigns = {}) #:nodoc: - unless use_full_path == nil - ActiveSupport::Deprecation.warn("use_full_path option has been deprecated and has no affect.", caller) - end - - if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) && - template_path.is_a?(String) && !template_path.include?("/") - raise ActionViewError, <<-END_ERROR - Due to changes in ActionMailer, you need to provide the mailer_name along with the template name. - - render "user_mailer/signup" - render :file => "user_mailer/signup" - - If you are rendering a subtemplate, you must now use controller-like partial syntax: - - render :partial => 'signup' # no mailer_name necessary - END_ERROR - end - - template = pick_template(template_path) - template.render_template(self, local_assigns) - end - - def render_inline(text, local_assigns = {}, type = nil) - InlineTemplate.new(text, type).render(self, local_assigns) - end - # Evaluate the local assigns and pushes them to the view. def evaluate_assigns unless @assigns_added diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index 074ba5a2b5..7f19532bc1 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -1,14 +1,15 @@ module ActionView - # There's also a convenience method for rendering sub templates within the current controller that depends on a single object - # (we call this kind of sub templates for partials). It relies on the fact that partials should follow the naming convention of being - # prefixed with an underscore -- as to separate them from regular templates that could be rendered on their own. + # There's also a convenience method for rendering sub templates within the current controller that depends on a + # single object (we call this kind of sub templates for partials). It relies on the fact that partials should + # follow the naming convention of being prefixed with an underscore -- as to separate them from regular + # templates that could be rendered on their own. # # In a template for Advertiser#account: # # <%= render :partial => "account" %> # - # This would render "advertiser/_account.erb" and pass the instance variable @account in as a local variable +account+ to - # the template for display. + # This would render "advertiser/_account.erb" and pass the instance variable @account in as a local variable + # +account+ to the template for display. # # In another template for Advertiser#buy, we could have: # @@ -18,24 +19,24 @@ module ActionView # <%= render :partial => "ad", :locals => { :ad => ad } %> # <% end %> # - # This would first render "advertiser/_account.erb" with @buyer passed in as the local variable +account+, then render - # "advertiser/_ad.erb" and pass the local variable +ad+ to the template for display. + # This would first render "advertiser/_account.erb" with @buyer passed in as the local variable +account+, then + # render "advertiser/_ad.erb" and pass the local variable +ad+ to the template for display. # # == Rendering a collection of partials # - # The example of partial use describes a familiar pattern where a template needs to iterate over an array and render a sub - # template for each of the elements. This pattern has been implemented as a single method that accepts an array and renders - # a partial by the same name as the elements contained within. So the three-lined example in "Using partials" can be rewritten - # with a single line: + # The example of partial use describes a familiar pattern where a template needs to iterate over an array and + # render a sub template for each of the elements. This pattern has been implemented as a single method that + # accepts an array and renders a partial by the same name as the elements contained within. So the three-lined + # example in "Using partials" can be rewritten with a single line: # # <%= render :partial => "ad", :collection => @advertisements %> # - # This will render "advertiser/_ad.erb" and pass the local variable +ad+ to the template for display. An iteration counter - # will automatically be made available to the template with a name of the form +partial_name_counter+. In the case of the - # example above, the template would be fed +ad_counter+. + # This will render "advertiser/_ad.erb" and pass the local variable +ad+ to the template for display. An + # iteration counter will automatically be made available to the template with a name of the form + # +partial_name_counter+. In the case of the example above, the template would be fed +ad_counter+. # - # NOTE: Due to backwards compatibility concerns, the collection can't be one of hashes. Normally you'd also just keep domain objects, - # like Active Records, in there. + # NOTE: Due to backwards compatibility concerns, the collection can't be one of hashes. Normally you'd also + # just keep domain objects, like Active Records, in there. # # == Rendering shared partials # @@ -47,8 +48,9 @@ module ActionView # # == Rendering partials with layouts # - # Partials can have their own layouts applied to them. These layouts are different than the ones that are specified globally - # for the entire action, but they work in a similar fashion. Imagine a list with two types of users: + # Partials can have their own layouts applied to them. These layouts are different than the ones that are + # specified globally for the entire action, but they work in a similar fashion. Imagine a list with two types + # of users: # # <%# app/views/users/index.html.erb &> # Here's the administrator: @@ -139,36 +141,51 @@ module ActionView extend ActiveSupport::Memoizable private - def render_partial(partial_path, object_assigns = nil, local_assigns = {}) #:nodoc: - local_assigns ||= {} + def render_partial(options = {}) #:nodoc: + local_assigns = options[:locals] || {} - case partial_path + case partial_path = options[:partial] when String, Symbol, NilClass - pick_template(find_partial_path(partial_path)).render_partial(self, object_assigns, local_assigns) + if options.has_key?(:collection) + render_partial_collection(options) + else + pick_template(find_partial_path(partial_path)).render_partial(self, options[:object], local_assigns) + end when ActionView::Helpers::FormBuilder builder_partial_path = partial_path.class.to_s.demodulize.underscore.sub(/_builder$/, '') - render_partial(builder_partial_path, object_assigns, (local_assigns || {}).merge(builder_partial_path.to_sym => partial_path)) + render_partial( + :partial => builder_partial_path, + :object => options[:object], + :locals => local_assigns.merge(builder_partial_path.to_sym => partial_path) + ) when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope if partial_path.any? - collection = partial_path - render_partial_collection(nil, collection, nil, local_assigns) + render_partial(:collection => partial_path, :locals => local_assigns) else "" end else - render_partial(ActionController::RecordIdentifier.partial_path(partial_path, controller.class.controller_path), partial_path, local_assigns) + object = partial_path + render_partial( + :partial => ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path), + :object => object, + :locals => local_assigns + ) end end - def render_partial_collection(partial_path, collection, partial_spacer_template = nil, local_assigns = {}, as = nil) #:nodoc: - return nil if collection.blank? + def render_partial_collection(options = {}) #:nodoc: + return nil if options[:collection].blank? - local_assigns = local_assigns ? local_assigns.clone : {} - spacer = partial_spacer_template ? render(:partial => partial_spacer_template) : '' + partial = options[:partial] + spacer = options[:spacer_template] ? render(:partial => options[:spacer_template]) : '' + local_assigns = options[:locals] ? options[:locals].clone : {} + as = options[:as] index = 0 - collection.map do |object| - _partial_path ||= partial_path || ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path) + options[:collection].map do |object| + _partial_path ||= partial || + ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path) path = find_partial_path(_partial_path) template = pick_template(path) local_assigns[template.counter_name] = index @@ -178,7 +195,7 @@ module ActionView end.join(spacer) end - def find_partial_path(partial_path) + def find_partial_path(partial_path) #:nodoc: if partial_path.include?('/') File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}") elsif respond_to?(:controller) -- cgit v1.2.3 From 893fb5bb639b0938f6762ef1165b05abae255986 Mon Sep 17 00:00:00 2001 From: Adrian Mugnolo Date: Fri, 22 Aug 2008 03:03:26 +0100 Subject: Add ActiveResource::Base.find(:last). [#754 state:resolved] Signed-off-by: Pratik Naik --- activeresource/CHANGELOG | 2 + activeresource/lib/active_resource/base.rb | 137 +++++++++++++++-------------- activeresource/test/base_test.rb | 98 +++++++++++---------- 3 files changed, 125 insertions(+), 112 deletions(-) diff --git a/activeresource/CHANGELOG b/activeresource/CHANGELOG index b62aa91b45..012f9028a0 100644 --- a/activeresource/CHANGELOG +++ b/activeresource/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Add ActiveResource::Base.find(:last). [#754 state:resolved] (Adrian Mugnolo) + * Fixed problems with the logger used if the logging string included %'s [#840 state:resolved] (Jamis Buck) * Fixed Base#exists? to check status code as integer [#299 state:resolved] (Wes Oldenbeuving) diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 492ab27bef..7e4a75aa70 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -13,43 +13,43 @@ module ActiveResource # to Ruby objects, Active Resource only needs a class name that corresponds to the resource name (e.g., the class # Person maps to the resources people, very similarly to Active Record) and a +site+ value, which holds the # URI of the resources. - # + # # class Person < ActiveResource::Base # self.site = "http://api.people.com:3000/" # end - # + # # Now the Person class is mapped to RESTful resources located at http://api.people.com:3000/people/, and - # you can now use Active Resource's lifecycles methods to manipulate resources. In the case where you already have + # you can now use Active Resource's lifecycles methods to manipulate resources. In the case where you already have # an existing model with the same name as the desired RESTful resource you can set the +element_name+ value. # # class PersonResource < ActiveResource::Base # self.site = "http://api.people.com:3000/" # self.element_name = "person" # end - # - # + # + # # == Lifecycle methods # # Active Resource exposes methods for creating, finding, updating, and deleting resources # from REST web services. - # + # # ryan = Person.new(:first => 'Ryan', :last => 'Daigle') # ryan.save # => true # ryan.id # => 2 # Person.exists?(ryan.id) # => true # ryan.exists? # => true - # + # # ryan = Person.find(1) # # Resource holding our newly created Person object - # + # # ryan.first = 'Rizzle' # ryan.save # => true - # + # # ryan.destroy # => true # # As you can see, these are very similar to Active Record's lifecycle methods for database records. # You can read more about each of these methods in their respective documentation. - # + # # === Custom REST methods # # Since simple CRUD/lifecycle methods can't accomplish every task, Active Resource also supports @@ -71,14 +71,14 @@ module ActiveResource # # # DELETE to 'fire' a person, i.e. DELETE /people/1/fire.xml. # Person.find(1).delete(:fire) - # + # # For more information on using custom REST methods, see the # ActiveResource::CustomMethods documentation. # # == Validations # # You can validate resources client side by overriding validation methods in the base class. - # + # # class Person < ActiveResource::Base # self.site = "http://api.people.com:3000/" # protected @@ -86,19 +86,19 @@ module ActiveResource # errors.add("last", "has invalid characters") unless last =~ /[a-zA-Z]*/ # end # end - # + # # See the ActiveResource::Validations documentation for more information. # # == Authentication - # + # # Many REST APIs will require authentication, usually in the form of basic # HTTP authentication. Authentication can be specified by: # * putting the credentials in the URL for the +site+ variable. - # + # # class Person < ActiveResource::Base # self.site = "http://ryan:password@api.people.com:3000/" # end - # + # # * defining +user+ and/or +password+ variables # # class Person < ActiveResource::Base @@ -107,29 +107,29 @@ module ActiveResource # self.password = "password" # end # - # For obvious security reasons, it is probably best if such services are available + # For obvious security reasons, it is probably best if such services are available # over HTTPS. - # - # Note: Some values cannot be provided in the URL passed to site. e.g. email addresses + # + # Note: Some values cannot be provided in the URL passed to site. e.g. email addresses # as usernames. In those situations you should use the separate user and password option. # == Errors & Validation # # Error handling and validation is handled in much the same manner as you're used to seeing in # Active Record. Both the response code in the HTTP response and the body of the response are used to # indicate that an error occurred. - # + # # === Resource errors - # + # # When a GET is requested for a resource that does not exist, the HTTP 404 (Resource Not Found) # response code will be returned from the server which will raise an ActiveResource::ResourceNotFound # exception. - # + # # # GET http://api.people.com:3000/people/999.xml # ryan = Person.find(999) # 404, raises ActiveResource::ResourceNotFound - # + # # 404 is just one of the HTTP error response codes that Active Resource will handle with its own exception. The # following HTTP response codes will also result in these exceptions: - # + # # * 200..399 - Valid response, no exception # * 404 - ActiveResource::ResourceNotFound # * 409 - ActiveResource::ResourceConflict @@ -149,17 +149,17 @@ module ActiveResource # end # # === Validation errors - # + # # Active Resource supports validations on resources and will return errors if any these validations fail - # (e.g., "First name can not be blank" and so on). These types of errors are denoted in the response by + # (e.g., "First name can not be blank" and so on). These types of errors are denoted in the response by # a response code of 422 and an XML representation of the validation errors. The save operation will # then fail (with a false return value) and the validation errors can be accessed on the resource in question. - # + # # ryan = Person.find(1) # ryan.first # => '' # ryan.save # => false # - # # When + # # When # # PUT http://api.people.com:3000/people/1.xml # # is requested with invalid values, the response is: # # @@ -169,7 +169,7 @@ module ActiveResource # # ryan.errors.invalid?(:first) # => true # ryan.errors.full_messages # => ['First cannot be empty'] - # + # # Learn more about Active Resource's validation features in the ActiveResource::Validations documentation. # # === Timeouts @@ -280,7 +280,7 @@ module ActiveResource # # Default format is :xml. def format=(mime_type_reference_or_format) - format = mime_type_reference_or_format.is_a?(Symbol) ? + format = mime_type_reference_or_format.is_a?(Symbol) ? ActiveResource::Formats[mime_type_reference_or_format] : mime_type_reference_or_format write_inheritable_attribute("format", format) @@ -332,7 +332,7 @@ module ActiveResource attr_accessor_with_default(:collection_name) { element_name.pluralize } #:nodoc: attr_accessor_with_default(:primary_key, 'id') #:nodoc: - + # Gets the prefix for a resource's nested URL (e.g., prefix/collectionname/1.xml) # This method is regenerated at runtime based on what the prefix is set to. def prefix(options={}) @@ -381,21 +381,21 @@ module ActiveResource # +query_options+ - A hash to add items to the query string for the request. # # ==== Examples - # Post.element_path(1) + # Post.element_path(1) # # => /posts/1.xml # - # Comment.element_path(1, :post_id => 5) + # Comment.element_path(1, :post_id => 5) # # => /posts/5/comments/1.xml # - # Comment.element_path(1, :post_id => 5, :active => 1) + # Comment.element_path(1, :post_id => 5, :active => 1) # # => /posts/5/comments/1.xml?active=1 # - # Comment.element_path(1, {:post_id => 5}, {:active => 1}) + # Comment.element_path(1, {:post_id => 5}, {:active => 1}) # # => /posts/5/comments/1.xml?active=1 # def element_path(id, prefix_options = {}, query_options = nil) prefix_options, query_options = split_options(prefix_options) if query_options.nil? - "#{prefix(prefix_options)}#{collection_name}/#{id}.#{format.extension}#{query_string(query_options)}" + "#{prefix(prefix_options)}#{collection_name}/#{id}.#{format.extension}#{query_string(query_options)}" end # Gets the collection path for the REST resources. If the +query_options+ parameter is omitted, Rails @@ -410,13 +410,13 @@ module ActiveResource # Post.collection_path # # => /posts.xml # - # Comment.collection_path(:post_id => 5) + # Comment.collection_path(:post_id => 5) # # => /posts/5/comments.xml # - # Comment.collection_path(:post_id => 5, :active => 1) + # Comment.collection_path(:post_id => 5, :active => 1) # # => /posts/5/comments.xml?active=1 # - # Comment.collection_path({:post_id => 5}, {:active => 1}) + # Comment.collection_path({:post_id => 5}, {:active => 1}) # # => /posts/5/comments.xml?active=1 # def collection_path(prefix_options = {}, query_options = nil) @@ -451,50 +451,54 @@ module ActiveResource # that_guy.valid? # => false # that_guy.new? # => true def create(attributes = {}) - returning(self.new(attributes)) { |res| res.save } + returning(self.new(attributes)) { |res| res.save } end # Core method for finding resources. Used similarly to Active Record's +find+ method. # # ==== Arguments - # The first argument is considered to be the scope of the query. That is, how many + # The first argument is considered to be the scope of the query. That is, how many # resources are returned from the request. It can be one of the following. # # * :one - Returns a single resource. # * :first - Returns the first resource found. + # * :last - Returns the last resource found. # * :all - Returns every resource that matches the request. - # + # # ==== Options # # * :from - Sets the path or custom method that resources will be fetched from. # * :params - Sets query and prefix (nested URL) parameters. # # ==== Examples - # Person.find(1) + # Person.find(1) # # => GET /people/1.xml # - # Person.find(:all) + # Person.find(:all) # # => GET /people.xml # - # Person.find(:all, :params => { :title => "CEO" }) + # Person.find(:all, :params => { :title => "CEO" }) # # => GET /people.xml?title=CEO # - # Person.find(:first, :from => :managers) + # Person.find(:first, :from => :managers) + # # => GET /people/managers.xml + # + # Person.find(:last, :from => :managers) # # => GET /people/managers.xml # - # Person.find(:all, :from => "/companies/1/people.xml") + # Person.find(:all, :from => "/companies/1/people.xml") # # => GET /companies/1/people.xml # - # Person.find(:one, :from => :leader) + # Person.find(:one, :from => :leader) # # => GET /people/leader.xml # # Person.find(:all, :from => :developers, :params => { :language => 'ruby' }) # # => GET /people/developers.xml?language=ruby # - # Person.find(:one, :from => "/companies/1/manager.xml") + # Person.find(:one, :from => "/companies/1/manager.xml") # # => GET /companies/1/manager.xml # - # StreetAddress.find(1, :params => { :person_id => 1 }) + # StreetAddress.find(1, :params => { :person_id => 1 }) # # => GET /people/1/street_addresses/1.xml def find(*arguments) scope = arguments.slice!(0) @@ -503,6 +507,7 @@ module ActiveResource case scope when :all then find_every(options) when :first then find_every(options).first + when :last then find_every(options).last when :one then find_one(options) else find_single(scope, options) end @@ -560,7 +565,7 @@ module ActiveResource instantiate_collection( (connection.get(path, headers) || []), prefix_options ) end end - + # Find a single resource from a one-off URL def find_one(options) case from = options[:from] @@ -578,7 +583,7 @@ module ActiveResource path = element_path(scope, prefix_options, query_options) instantiate_record(connection.get(path, headers), prefix_options) end - + def instantiate_collection(collection, prefix_options = {}) collection.collect! { |record| instantiate_record(record, prefix_options) } end @@ -602,10 +607,10 @@ module ActiveResource # Builds the query string for the request. def query_string(options) - "?#{options.to_query}" unless options.nil? || options.empty? + "?#{options.to_query}" unless options.nil? || options.empty? end - # split an option hash into two hashes, one containing the prefix options, + # split an option hash into two hashes, one containing the prefix options, # and the other containing the leftovers. def split_options(options = {}) prefix_options, query_options = {}, {} @@ -654,7 +659,7 @@ module ActiveResource # ryan = Person.find(1) # ryan.address = StreetAddress.find(1, :person_id => ryan.id) # ryan.hash = {:not => "an ARes instance"} - # + # # not_ryan = ryan.clone # not_ryan.new? # => true # not_ryan.address # => NoMethodError @@ -706,7 +711,7 @@ module ActiveResource id && id.to_s end - # Test for equality. Resource are equal if and only if +other+ is the same object or + # Test for equality. Resource are equal if and only if +other+ is the same object or # is an instance of the same class, is not new?, and has the same +id+. # # ==== Examples @@ -742,7 +747,7 @@ module ActiveResource def hash id.hash end - + # Duplicate the current resource without saving it. # # ==== Examples @@ -762,7 +767,7 @@ module ActiveResource end end - # A method to save (+POST+) or update (+PUT+) a resource. It delegates to +create+ if a new object, + # A method to save (+POST+) or update (+PUT+) a resource. It delegates to +create+ if a new object, # +update+ if it is existing. If the response to the save includes a body, it will be assumed that this body # is XML for the final object as it looked after the save (which would include attributes like +created_at+ # that weren't part of the original submit). @@ -786,7 +791,7 @@ module ActiveResource # my_person = Person.find(my_id) # my_person.destroy # Person.find(my_id) # 404 (Resource Not Found) - # + # # new_person = Person.create(:name => 'James') # new_id = new_person.id # => 7 # new_person.destroy @@ -825,7 +830,7 @@ module ActiveResource # * :indent - Set the indent level for the XML output (default is +2+). # * :dasherize - Boolean option to determine whether or not element names should # replace underscores with dashes (default is false). - # * :skip_instruct - Toggle skipping the +instruct!+ call on the XML builder + # * :skip_instruct - Toggle skipping the +instruct!+ call on the XML builder # that generates the XML declaration (default is false). # # ==== Examples @@ -849,7 +854,7 @@ module ActiveResource # ==== Examples # my_branch = Branch.find(:first) # my_branch.name # => "Wislon Raod" - # + # # # Another client fixes the typo... # # my_branch.name # => "Wislon Raod" @@ -897,7 +902,7 @@ module ActiveResource end self end - + # For checking respond_to? without searching the attributes (which is faster). alias_method :respond_to_without_attributes?, :respond_to? @@ -909,7 +914,7 @@ module ActiveResource if attributes.nil? return super elsif attributes.has_key?(method_name) - return true + return true elsif ['?','='].include?(method_name.last) && attributes.has_key?(method_name.first(-1)) return true end @@ -917,7 +922,7 @@ module ActiveResource # would return true for generated readers, even if the attribute wasn't present super end - + protected def connection(refresh = false) @@ -938,7 +943,7 @@ module ActiveResource load_attributes_from_response(response) end end - + def load_attributes_from_response(response) if response['Content-Length'] != "0" && response.body.strip.size > 0 load(self.class.format.decode(response.body)) @@ -963,7 +968,7 @@ module ActiveResource def find_or_create_resource_for_collection(name) find_or_create_resource_for(name.to_s.singularize) end - + # Tries to find a resource in a non empty list of nested modules # Raises a NameError if it was not found in any of the given nested modules def find_resource_in_modules(resource_name, module_names) diff --git a/activeresource/test/base_test.rb b/activeresource/test/base_test.rb index 4addd52636..0d997b2a2f 100644 --- a/activeresource/test/base_test.rb +++ b/activeresource/test/base_test.rb @@ -8,7 +8,7 @@ class BaseTest < Test::Unit::TestCase def setup @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') - @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') + @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') @addy = { :id => 1, :street => '12345 Street' }.to_xml(:root => 'address') @default_request_headers = { 'Content-Type' => 'application/xml' } @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") @@ -50,7 +50,7 @@ class BaseTest < Test::Unit::TestCase ActiveResource::HttpMock.respond_to do |mock| mock.get "/people/1.xml", {}, @matz mock.get "/people/2.xml", {}, @david - mock.get "/people/Greg.xml", {}, @greg + mock.get "/people/Greg.xml", {}, @greg mock.get "/people/4.xml", {'key' => 'value'}, nil, 404 mock.put "/people/1.xml", {}, nil, 204 mock.delete "/people/1.xml", {}, nil, 200 @@ -62,7 +62,7 @@ class BaseTest < Test::Unit::TestCase mock.get "/people/1/addresses/1.xml", {}, @addy mock.get "/people/1/addresses/2.xml", {}, nil, 404 mock.get "/people/2/addresses/1.xml", {}, nil, 404 - mock.get "/people/Greg/addresses/1.xml", {}, @addy + mock.get "/people/Greg/addresses/1.xml", {}, @addy mock.put "/people/1/addresses/1.xml", {}, nil, 204 mock.delete "/people/1/addresses/1.xml", {}, nil, 200 mock.post "/people/1/addresses.xml", {}, nil, 201, 'Location' => '/people/1/addresses/5' @@ -101,13 +101,13 @@ class BaseTest < Test::Unit::TestCase assert_equal 'http://foo:bar@beast.caboo.se', Forum.site.to_s assert_equal 'http://foo:bar@beast.caboo.se/forums/:forum_id', Topic.site.to_s end - + def test_site_variable_can_be_reset - actor = Class.new(ActiveResource::Base) + actor = Class.new(ActiveResource::Base) assert_nil actor.site actor.site = 'http://localhost:31337' actor.site = nil - assert_nil actor.site + assert_nil actor.site end def test_should_accept_setting_user @@ -194,18 +194,18 @@ class BaseTest < Test::Unit::TestCase actor.site = 'http://nomad' assert_equal actor.site, jester.site assert jester.site.frozen? - - # Subclasses are always equal to superclass site when not overridden + + # Subclasses are always equal to superclass site when not overridden fruit = Class.new(ActiveResource::Base) apple = Class.new(fruit) - + fruit.site = 'http://market' assert_equal fruit.site, apple.site, 'subclass did not adopt changes from parent class' - + fruit.site = 'http://supermarket' assert_equal fruit.site, apple.site, 'subclass did not adopt changes from parent class' end - + def test_user_reader_uses_superclass_user_until_written # Superclass is Object so returns nil. assert_nil ActiveResource::Base.user @@ -317,14 +317,14 @@ class BaseTest < Test::Unit::TestCase end def test_updating_baseclass_site_object_wipes_descendent_cached_connection_objects - # Subclasses are always equal to superclass site when not overridden + # Subclasses are always equal to superclass site when not overridden fruit = Class.new(ActiveResource::Base) apple = Class.new(fruit) - + fruit.site = 'http://market' assert_equal fruit.connection.site, apple.connection.site first_connection = apple.connection.object_id - + fruit.site = 'http://supermarket' assert_equal fruit.connection.site, apple.connection.site second_connection = apple.connection.object_id @@ -393,34 +393,34 @@ class BaseTest < Test::Unit::TestCase assert_equal '/people.xml?gender=', Person.collection_path(:gender => nil) assert_equal '/people.xml?gender=male', Person.collection_path('gender' => 'male') - + # Use includes? because ordering of param hash is not guaranteed assert Person.collection_path(:gender => 'male', :student => true).include?('/people.xml?') assert Person.collection_path(:gender => 'male', :student => true).include?('gender=male') assert Person.collection_path(:gender => 'male', :student => true).include?('student=true') assert_equal '/people.xml?name%5B%5D=bob&name%5B%5D=your+uncle%2Bme&name%5B%5D=&name%5B%5D=false', Person.collection_path(:name => ['bob', 'your uncle+me', nil, false]) - + assert_equal '/people.xml?struct%5Ba%5D%5B%5D=2&struct%5Ba%5D%5B%5D=1&struct%5Bb%5D=fred', Person.collection_path(:struct => {:a => [2,1], 'b' => 'fred'}) end def test_custom_element_path assert_equal '/people/1/addresses/1.xml', StreetAddress.element_path(1, :person_id => 1) assert_equal '/people/1/addresses/1.xml', StreetAddress.element_path(1, 'person_id' => 1) - assert_equal '/people/Greg/addresses/1.xml', StreetAddress.element_path(1, 'person_id' => 'Greg') + assert_equal '/people/Greg/addresses/1.xml', StreetAddress.element_path(1, 'person_id' => 'Greg') end - + def test_custom_element_path_with_redefined_to_param Person.module_eval do alias_method :original_to_param_element_path, :to_param - def to_param + def to_param name end end # Class method. assert_equal '/people/Greg.xml', Person.element_path('Greg') - + # Protected Instance method. assert_equal '/people/Greg.xml', Person.find('Greg').send(:element_path) @@ -470,21 +470,21 @@ class BaseTest < Test::Unit::TestCase assert_equal "/", Person.prefix assert_equal Set.new, Person.send!(:prefix_parameters) end - + def test_set_prefix SetterTrap.rollback_sets(Person) do |person_class| person_class.prefix = "the_prefix" assert_equal "the_prefix", person_class.prefix end end - + def test_set_prefix_with_inline_keys SetterTrap.rollback_sets(Person) do |person_class| person_class.prefix = "the_prefix:the_param" assert_equal "the_prefixthe_param_value", person_class.prefix(:the_param => "the_param_value") end end - + def test_set_prefix_with_default_value SetterTrap.rollback_sets(Person) do |person_class| person_class.set_prefix @@ -504,7 +504,7 @@ class BaseTest < Test::Unit::TestCase assert_equal "Matz", matz.name assert matz.name? end - + def test_respond_to matz = Person.find(1) assert matz.respond_to?(:name) @@ -533,6 +533,12 @@ class BaseTest < Test::Unit::TestCase assert_equal "Matz", matz.name end + def test_find_last + david = Person.find(:last) + assert_kind_of Person, david + assert_equal 'David', david.name + end + def test_custom_header Person.headers['key'] = 'value' assert_raises(ActiveResource::ResourceNotFound) { Person.find(4) } @@ -547,7 +553,7 @@ class BaseTest < Test::Unit::TestCase def test_find_all_by_from ActiveResource::HttpMock.respond_to { |m| m.get "/companies/1/people.xml", {}, @people_david } - + people = Person.find(:all, :from => "/companies/1/people.xml") assert_equal 1, people.size assert_equal "David", people.first.name @@ -555,7 +561,7 @@ class BaseTest < Test::Unit::TestCase def test_find_all_by_from_with_options ActiveResource::HttpMock.respond_to { |m| m.get "/companies/1/people.xml", {}, @people_david } - + people = Person.find(:all, :from => "/companies/1/people.xml") assert_equal 1, people.size assert_equal "David", people.first.name @@ -563,7 +569,7 @@ class BaseTest < Test::Unit::TestCase def test_find_all_by_symbol_from ActiveResource::HttpMock.respond_to { |m| m.get "/people/managers.xml", {}, @people_david } - + people = Person.find(:all, :from => :managers) assert_equal 1, people.size assert_equal "David", people.first.name @@ -593,7 +599,7 @@ class BaseTest < Test::Unit::TestCase p = Person.new resp = {'Location' => '/foo/bar/1'} assert_equal '1', p.send!(:id_from_response, resp) - + resp['Location'] << '.xml' assert_equal '1', p.send!(:id_from_response, resp) end @@ -610,16 +616,16 @@ class BaseTest < Test::Unit::TestCase ryan = Person.new(:id => 1, :name => 'Ryan', :address => address) assert_equal address.prefix_options, ryan.address.prefix_options end - + def test_reload_works_with_prefix_options address = StreetAddress.find(1, :params => { :person_id => 1 }) assert_equal address, address.reload end - + def test_reload_with_redefined_to_param Person.module_eval do alias_method :original_to_param_reload, :to_param - def to_param + def to_param name end end @@ -634,13 +640,13 @@ class BaseTest < Test::Unit::TestCase alias_method :reload_to_param, :to_param alias_method :to_param, :original_to_param_reload end - end - - def test_reload_works_without_prefix_options + end + + def test_reload_works_without_prefix_options person = Person.find(:first) assert_equal person, person.reload end - + def test_create rick = Person.create(:name => 'Rick') @@ -650,11 +656,11 @@ class BaseTest < Test::Unit::TestCase # test additional attribute returned on create assert_equal 25, rick.age - + # Test that save exceptions get bubbled up too ActiveResource::HttpMock.respond_to do |mock| mock.post "/people.xml", {}, nil, 409 - end + end assert_raises(ActiveResource::ResourceConflict) { Person.create(:name => 'Rick') } end @@ -716,7 +722,7 @@ class BaseTest < Test::Unit::TestCase assert_equal "54321 Lane", addy.street addy.save end - + def test_update_conflict ActiveResource::HttpMock.respond_to do |mock| mock.get "/people/2.xml", {}, @david @@ -748,7 +754,7 @@ class BaseTest < Test::Unit::TestCase end assert_raises(ActiveResource::ResourceNotFound) { Person.find(1) } end - + def test_delete_with_custom_prefix assert StreetAddress.delete(1, :person_id => 1) ActiveResource::HttpMock.respond_to do |mock| @@ -778,23 +784,23 @@ class BaseTest < Test::Unit::TestCase assert !StreetAddress.new({:id => 1, :person_id => 2}).exists? assert !StreetAddress.new({:id => 2, :person_id => 1}).exists? end - + def test_exists_with_redefined_to_param Person.module_eval do alias_method :original_to_param_exists, :to_param - def to_param + def to_param name end end # Class method. - assert Person.exists?('Greg') + assert Person.exists?('Greg') # Instance method. - assert Person.find('Greg').exists? + assert Person.find('Greg').exists? # Nested class method. - assert StreetAddress.exists?(1, :params => { :person_id => Person.find('Greg').to_param }) + assert StreetAddress.exists?(1, :params => { :person_id => Person.find('Greg').to_param }) # Nested instance method. assert StreetAddress.find(1, :params => { :person_id => Person.find('Greg').to_param }).exists? @@ -806,8 +812,8 @@ class BaseTest < Test::Unit::TestCase alias_method :exists_to_param, :to_param alias_method :to_param, :original_to_param_exists end - end - + end + def test_to_xml matz = Person.find(1) xml = matz.to_xml -- cgit v1.2.3 From ba516b40f5867a68588c2fa5b6710dbfb97a12c6 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 21 Aug 2008 21:24:08 -0500 Subject: Tidy up pick partial template logic --- actionpack/lib/action_view/partials.rb | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/actionpack/lib/action_view/partials.rb b/actionpack/lib/action_view/partials.rb index 7f19532bc1..a91417293a 100644 --- a/actionpack/lib/action_view/partials.rb +++ b/actionpack/lib/action_view/partials.rb @@ -149,21 +149,14 @@ module ActionView if options.has_key?(:collection) render_partial_collection(options) else - pick_template(find_partial_path(partial_path)).render_partial(self, options[:object], local_assigns) + _pick_partial_template(partial_path).render_partial(self, options[:object], local_assigns) end when ActionView::Helpers::FormBuilder builder_partial_path = partial_path.class.to_s.demodulize.underscore.sub(/_builder$/, '') - render_partial( - :partial => builder_partial_path, - :object => options[:object], - :locals => local_assigns.merge(builder_partial_path.to_sym => partial_path) - ) + local_assigns.merge!(builder_partial_path.to_sym => partial_path) + render_partial(:partial => builder_partial_path, :object => options[:object], :locals => local_assigns) when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope - if partial_path.any? - render_partial(:collection => partial_path, :locals => local_assigns) - else - "" - end + partial_path.any? ? render_partial(:collection => partial_path, :locals => local_assigns) : "" else object = partial_path render_partial( @@ -186,8 +179,7 @@ module ActionView options[:collection].map do |object| _partial_path ||= partial || ActionController::RecordIdentifier.partial_path(object, controller.class.controller_path) - path = find_partial_path(_partial_path) - template = pick_template(path) + template = _pick_partial_template(_partial_path) local_assigns[template.counter_name] = index result = template.render_partial(self, object, local_assigns, as) index += 1 @@ -195,15 +187,17 @@ module ActionView end.join(spacer) end - def find_partial_path(partial_path) #:nodoc: + def _pick_partial_template(partial_path) #:nodoc: if partial_path.include?('/') - File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}") + path = File.join(File.dirname(partial_path), "_#{File.basename(partial_path)}") elsif respond_to?(:controller) - "#{controller.class.controller_path}/_#{partial_path}" + path = "#{controller.class.controller_path}/_#{partial_path}" else - "_#{partial_path}" + path = "_#{partial_path}" end + + pick_template(path) end - memoize :find_partial_path + memoize :_pick_partial_template end end -- cgit v1.2.3 From 0096f5586987b720ca24c09103c9371f64ed26e5 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Thu, 21 Aug 2008 21:34:03 -0500 Subject: Removed template_public? because it will always be true since the default template is never a partial --- actionpack/lib/action_controller/base.rb | 6 +----- actionpack/lib/action_view/base.rb | 5 ----- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 5887e1e3b4..91023cd774 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -1189,7 +1189,7 @@ module ActionController #:nodoc: elsif respond_to? :method_missing method_missing action_name default_render unless performed? - elsif template_exists? && template_public? + elsif template_exists? default_render else raise UnknownAction, "No action responded to #{action_name}. Actions: #{action_methods.sort.to_sentence}", caller @@ -1264,10 +1264,6 @@ module ActionController #:nodoc: @template.file_exists?(template_name) end - def template_public?(template_name = default_template_name) - @template.file_public?(template_name) - end - def template_exempt_from_layout?(template_name = default_template_name) template_name = @template.pick_template(template_name).to_s if @template @@exempt_from_layout.any? { |ext| template_name =~ ext } diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index cdbb71bdaa..a85e698c1f 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -275,11 +275,6 @@ module ActionView #:nodoc: end end - # Returns true is the file may be rendered implicitly. - def file_public?(template_path)#:nodoc: - template_path.split('/').last[0,1] != '_' - end - # The format to be used when choosing between multiple templates with # the same name but differing formats. See +Request#template_format+ # for more details. -- cgit v1.2.3