Skip to content

Commit

Permalink
Cleanup around partial rendering
Browse files Browse the repository at this point in the history 
Signed-off-by: Joshua Peek <josh@joshpeek.com>
  • Loading branch information
@ryanb @josh
ryanb authored and josh committed Aug 22, 2008
1 parent e6a66cb commit 1129a24
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 90 deletions.
20 changes: 3 additions & 17 deletions actionpack/lib/action_controller/base.rb
View file
Expand Up @@ -780,9 +780,6 @@ def append_view_path(path)
# 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
Expand Down Expand Up @@ -913,21 +910,10 @@ def render(options = nil, extra_options = {}, &block) #:doc:
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
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/rescue.rb
View file
Expand Up @@ -182,7 +182,7 @@ def rescue_action_locally(exception)
@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))
Expand Down
4 changes: 2 additions & 2 deletions actionpack/lib/action_controller/templates/rescues/diagnostics.erb
View file
Expand Up @@ -6,6 +6,6 @@
</h1>
<pre><%=h @exception.clean_message %></pre>

<%= 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") %>
4 changes: 2 additions & 2 deletions actionpack/lib/action_controller/templates/rescues/template_error.erb
View file
Expand Up @@ -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") %>
43 changes: 8 additions & 35 deletions actionpack/lib/action_view/base.rb
View file
Expand Up @@ -239,7 +239,7 @@ def render(options = {}, local_assigns = {}, &block) #: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)
Expand All @@ -262,13 +262,15 @@ def render(options = {}, local_assigns = {}, &block) #: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
Expand Down Expand Up @@ -345,35 +347,6 @@ def pick_template(template_path)
memoize :pick_template

private
# Renders the template present at <tt>template_path</tt>. The hash in <tt>local_assigns</tt>
# 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
Expand Down
83 changes: 50 additions & 33 deletions actionpack/lib/action_view/partials.rb
View file
@@ -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:
#
Expand All @@ -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
#
Expand All @@ -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:
Expand Down Expand Up @@ -139,36 +141,51 @@ module Partials
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
Expand All @@ -178,7 +195,7 @@ def render_partial_collection(partial_path, collection, partial_spacer_template
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)
Expand Down

3 comments on commit 1129a24

@johnreilly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure about this, but I think this commit causes the default generated rspec controller specs to fail.

Doing a “script/generate rspec_scaffold Bike name:string sku:string” generates a set of default controller specs like this:


  describe "responding to GET index" do
    it "should expose all bikes as @bikes" do
      Bike.should_receive(:find).with(:all).and_return([mock_bike])
      get :index
      assigns[:bikes].should == [mock_bike]
    end
  end

This spec fails with an “ActionView::TemplateError: Mock ‘Bike_1017’ received unexpected message :name with (no args)” coming from the view, when it attempts to display @bike.name.

Updating the spec to expect the fields to be called (e.g., mock_bike.should_receive(:name)) does allow the spec to pass, but this is no longer an isolated controller test.

I ran git bisect on edge rails, and the default specs start to fail as of this commit. I’m still new to all of this, so I don’t know why this commit breaks rspec, or if it should be considered a problem with rails or with rspec, but it does seem to be consistently reproducible.

Am I crazy? :)

-John

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on 1129a24 Aug 23, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds like an rspec issue to me, raise it on their list and suggest they get in contact with us with any questions.

@johnreilly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, thanks :)

-John

Please sign in to comment.