Permalink
Browse files

Refactor ActionView::Path

  * Decouple from ActionController and ActionMailer
  * Bring back localization support.
  * Prepare to decouple templates from the filesystem.
  * Prepare to decouple localization from ActionView
  * Fix ActionMailer to take advantage of ActionView::Path
  • Loading branch information...
Yehuda Katz + Carl Lerche authored and wycats committed Apr 23, 2009
1 parent b2d6fda commit 0a132c2fe13fb2b8d5dade9cf6abd70601376287
Showing with 165 additions and 114 deletions.
  1. +26 −23 actionmailer/lib/action_mailer/base.rb
  2. +1 −0 actionmailer/lib/action_mailer/vendor/tmail-1.2.3/tmail/mail.rb
  3. 0 actionmailer/test/fixtures/auto_layout_mailer/{multipart.text.html.erb → multipart.html.erb}
  4. 0 actionmailer/test/fixtures/auto_layout_mailer/{multipart.text.plain.erb → multipart.text.erb}
  5. 0 actionmailer/test/fixtures/test_mailer/{_subtemplate.text.plain.erb → _subtemplate.text.erb}
  6. 0 .../test_mailer/{custom_templating_extension.text.html.haml → custom_templating_extension.html.haml}
  7. 0 ...test_mailer/{custom_templating_extension.text.plain.haml → custom_templating_extension.text.haml}
  8. 0 .../test_mailer/{implicitly_multipart_example.text.html.erb → implicitly_multipart_example.html.erb}
  9. 0 ...est_mailer/{implicitly_multipart_example.text.html.erb~ → implicitly_multipart_example.html.erb~}
  10. 0 ...test_mailer/{implicitly_multipart_example.text.plain.erb → implicitly_multipart_example.text.erb}
  11. 0 .../test_mailer/{implicitly_multipart_example.text.yaml.erb → implicitly_multipart_example.yaml.erb}
  12. 0 ...r/test/fixtures/test_mailer/{included_subtemplate.text.plain.erb → included_subtemplate.text.erb}
  13. +0 −2 actionmailer/test/fixtures/test_mailer/rxml_template.builder
  14. +7 −7 actionmailer/test/mail_service_test.rb
  15. +2 −2 actionpack/lib/action_controller/abstract/layouts.rb
  16. +1 −1 actionpack/lib/action_controller/abstract/renderer.rb
  17. +1 −1 actionpack/lib/action_controller/base/layout.rb
  18. +6 −1 actionpack/lib/action_controller/base/render.rb
  19. +3 −3 actionpack/lib/action_view/paths.rb
  20. +4 −3 actionpack/lib/action_view/render/partials.rb
  21. +2 −2 actionpack/lib/action_view/render/rendering.rb
  22. +4 −0 actionpack/lib/action_view/template/handlers.rb
  23. +97 −54 actionpack/lib/action_view/template/path.rb
  24. +1 −1 actionpack/lib/action_view/template/template.rb
  25. +2 −2 actionpack/test/abstract_controller/abstract_controller_test.rb
  26. +8 −12 actionpack/test/template/render_test.rb
@@ -465,48 +465,48 @@ def initialize(method_name=nil, *parameters) #:nodoc:
def create!(method_name, *parameters) #:nodoc:
initialize_defaults(method_name)
__send__(method_name, *parameters)
-
+
# If an explicit, textual body has not been set, we check assumptions.
unless String === @body
# First, we look to see if there are any likely templates that match,
# which include the content-type in their file name (i.e.,
# "the_template_file.text.html.erb", etc.). Only do this if parts
# have not already been specified manually.
- if @parts.empty?
- Dir.glob("#{template_path}/#{@template}.*").each do |path|
- template = template_root.find_by_parts("#{mailer_name}/#{File.basename(path)}")
-
- # Skip unless template has a multipart format
- next unless template && template.multipart?
-
+ # if @parts.empty?
+ template_root.find_all_by_parts(@template, {}, template_path).each do |template|
@parts << Part.new(
- :content_type => template.content_type,
+ :content_type => Mime::Type.lookup_by_extension(template.content_type || "text").to_s,
:disposition => "inline",
:charset => charset,
:body => render_template(template, @body)
)
end
- unless @parts.empty?
+
+ if @parts.size > 1
@content_type = "multipart/alternative" if @content_type !~ /^multipart/
@parts = sort_parts(@parts, @implicit_parts_order)
end
- end
-
+ # end
+
# Then, if there were such templates, we check to see if we ought to
# also render a "normal" template (without the content type). If a
# normal template exists (or if there were no implicit parts) we render
# it.
- template_exists = @parts.empty?
- template_exists ||= template_root.find_by_parts("#{mailer_name}/#{@template}")
- @body = render_message(@template, @body) if template_exists
+ # ====
+ # TODO: Revisit this
+ # template_exists = @parts.empty?
+ # template_exists ||= template_root.find_by_parts("#{mailer_name}/#{@template}")
+ # @body = render_message(@template, @body) if template_exists
# Finally, if there are other message parts and a textual body exists,
# we shift it onto the front of the parts and set the body to nil (so
# that create_mail doesn't try to render it in addition to the parts).
- if !@parts.empty? && String === @body
- @parts.unshift Part.new(:charset => charset, :body => @body)
- @body = nil
- end
+ # ====
+ # TODO: Revisit this
+ # if !@parts.empty? && String === @body
+ # @parts.unshift Part.new(:charset => charset, :body => @body)
+ # @body = nil
+ # end
end
# If this is a multipart e-mail add the mime_version if it is not
@@ -580,7 +580,7 @@ def render(opts)
if file
prefix = mailer_name unless file =~ /\//
- template = view_paths.find_by_parts(file, formats, prefix)
+ template = view_paths.find_by_parts(file, {:formats => formats}, prefix)
end
layout = _pick_layout(layout,
@@ -611,7 +611,7 @@ def template_root=(root)
end
def template_path
- "#{template_root}/#{mailer_name}"
+ "#{mailer_name}"
end
def initialize_template_class(assigns)
@@ -622,7 +622,7 @@ def initialize_template_class(assigns)
def sort_parts(parts, order = [])
order = order.collect { |s| s.downcase }
-
+
parts = parts.sort do |a, b|
a_ct = a.content_type.downcase
b_ct = b.content_type.downcase
@@ -663,10 +663,13 @@ def create_mail
headers.each { |k, v| m[k] = v }
real_content_type, ctype_attrs = parse_content_type
-
+
if @parts.empty?
m.set_content_type(real_content_type, nil, ctype_attrs)
m.body = normalize_new_lines(body)
+ elsif @parts.size == 1 && @parts.first.parts.empty?
+ m.set_content_type(real_content_type, nil, ctype_attrs)
+ m.body = normalize_new_lines(@parts.first.body)
else
if String === body
part = TMail::Mail.new
@@ -518,6 +518,7 @@ def content_type_is_text?
def parse_body( f = nil )
return if @body_parsed
+
if f
parse_body_0 f
else
@@ -1,2 +0,0 @@
-xml.instruct!
-xml.test
@@ -338,6 +338,7 @@ def test_nested_parts
def test_nested_parts_with_body
created = nil
+ TestMailer.create_nested_multipart_with_body(@recipient)
assert_nothing_raised { created = TestMailer.create_nested_multipart_with_body(@recipient)}
assert_equal 1,created.parts.size
assert_equal 2,created.parts.first.parts.size
@@ -351,8 +352,8 @@ def test_nested_parts_with_body
def test_attachment_with_custom_header
created = nil
- assert_nothing_raised { created = TestMailer.create_attachment_with_custom_header(@recipient)}
- assert_equal "<test@test.com>", created.parts[1].header['content-id'].to_s
+ assert_nothing_raised { created = TestMailer.create_attachment_with_custom_header(@recipient) }
+ assert created.parts.any? { |p| p.header['content-id'].to_s == "<test@test.com>" }
end
def test_signed_up
@@ -824,7 +825,7 @@ def test_implicitly_multipart_messages
assert_equal 3, mail.parts.length
assert_equal "1.0", mail.mime_version
assert_equal "multipart/alternative", mail.content_type
- assert_equal "text/yaml", mail.parts[0].content_type
+ assert_equal "application/x-yaml", mail.parts[0].content_type
assert_equal "utf-8", mail.parts[0].sub_header("content-type", "charset")
assert_equal "text/plain", mail.parts[1].content_type
assert_equal "utf-8", mail.parts[1].sub_header("content-type", "charset")
@@ -835,11 +836,11 @@ def test_implicitly_multipart_messages
def test_implicitly_multipart_messages_with_custom_order
assert ActionView::Template.template_handler_extensions.include?("bak"), "bak extension was not registered"
- mail = TestMailer.create_implicitly_multipart_example(@recipient, nil, ["text/yaml", "text/plain"])
+ mail = TestMailer.create_implicitly_multipart_example(@recipient, nil, ["application/x-yaml", "text/plain"])
assert_equal 3, mail.parts.length
assert_equal "text/html", mail.parts[0].content_type
assert_equal "text/plain", mail.parts[1].content_type
- assert_equal "text/yaml", mail.parts[2].content_type
+ assert_equal "application/x-yaml", mail.parts[2].content_type
end
def test_implicitly_multipart_messages_with_charset
@@ -938,8 +939,7 @@ def test_deliver_with_mail_object
def test_multipart_with_template_path_with_dots
mail = FunkyPathMailer.create_multipart_with_template_path_with_dots(@recipient)
assert_equal 2, mail.parts.length
- assert_equal 'text/plain', mail.parts[0].content_type
- assert_equal 'utf-8', mail.parts[0].charset
+ assert mail.parts.any? {|part| part.content_type == "text/plain" && part.charset == "utf-8"}
end
def test_custom_content_type_attributes
@@ -38,7 +38,7 @@ def _write_layout_method
else
self.class_eval %{
def _layout
- if view_paths.find_by_parts?("#{_implied_layout_name}", formats, "layouts")
+ if view_paths.find_by_parts?("#{_implied_layout_name}", {:formats => formats}, "layouts")
"#{_implied_layout_name}"
else
super
@@ -62,7 +62,7 @@ def _layout_for_name(name)
raise ArgumentError, "String, false, or nil expected; you passed #{name.inspect}"
end
- name && view_paths.find_by_parts(name, formats, "layouts")
+ name && view_paths.find_by_parts(name, {:formats => formats}, "layouts")
end
def _default_layout(require_layout = false)
@@ -29,7 +29,7 @@ def render(options = {})
def render_to_body(options = {})
name = options[:_template_name] || action_name
- template = options[:_template] || view_paths.find_by_parts(name.to_s, formats, options[:_prefix])
+ template = options[:_template] || view_paths.find_by_parts(name.to_s, {:formats => formats}, options[:_prefix])
_render_template(template, options)
end
@@ -182,7 +182,7 @@ def default_layout(*args)
def memoized_find_layout(layout, formats) #:nodoc:
return layout if layout.nil? || layout.respond_to?(:render)
prefix = layout.to_s =~ /layouts\// ? nil : "layouts"
- view_paths.find_by_parts(layout.to_s, formats, prefix)
+ view_paths.find_by_parts(layout.to_s, {:formats => formats}, prefix)
end
def find_layout(*args)
@@ -374,8 +374,13 @@ def render_for_name(name, layout, options)
render_for_file(name.sub(/^\//, ''), [layout, true], options)
end
end
-
+
+ # ==== Arguments
+ # parts<Array[String, Array[Symbol*], String, Boolean]>::
+ # Example: ["show", [:html, :xml], "users", false]
def render_for_parts(parts, layout, options = {})
+ parts[1] = {:formats => parts[1], :locales => [I18n.locale]}
+
tmp = view_paths.find_by_parts(*parts)
layout = _pick_layout(*layout) unless tmp.exempt_from_layout?
@@ -33,18 +33,18 @@ def unshift(*objs)
super(*objs.map { |obj| self.class.type_cast(obj) })
end
- def find_by_parts(path, extension = nil, prefix = nil, partial = false)
+ def find_by_parts(path, details = {}, prefix = nil, partial = false)
template_path = path.sub(/^\//, '')
each do |load_path|
- if template = load_path.find_by_parts(template_path, extension, prefix, partial)
+ if template = load_path.find_by_parts(template_path, details, prefix, partial)
return template
end
end
Template.new(path, self)
rescue ActionView::MissingTemplate => e
- extension ||= []
+ extension = details[:formats] || []
raise ActionView::MissingTemplate.new(self, "#{prefix}/#{path}.{#{extension.join(",")}}")
end
@@ -187,6 +187,7 @@ def _render_partial(options = {}) #:nodoc:
path = ActionController::RecordIdentifier.partial_path(object, controller_path)
end
_, _, prefix, object = parts = partial_parts(path, options)
+ parts[1] = {:formats => parts[1]}
template = find_by_parts(*parts)
_render_partial_object(template, options, (object unless object == true))
end
@@ -225,7 +226,7 @@ def _render_partial_with_block(layout, block, options)
def _render_partial_with_layout(layout, options)
if layout
prefix = controller && !layout.include?("/") ? controller.controller_path : nil
- layout = find_by_parts(layout, formats, prefix, true)
+ layout = find_by_parts(layout, {:formats => formats}, prefix, true)
end
content = _render_partial(options)
return _render_content_with_layout(content, layout, options[:locals])
@@ -253,7 +254,7 @@ def _render_partial_with_block(layout, block, options)
def _render_partial_with_layout(layout, options)
if layout
prefix = controller && !layout.include?("/") ? controller.controller_path : nil
- layout = find_by_parts(layout, formats, prefix, true)
+ layout = find_by_parts(layout, {:formats => formats}, prefix, true)
end
content = _render_partial(options)
return _render_content_with_layout(content, layout, options[:locals])
@@ -321,7 +322,7 @@ def _render_partial_collection(collection, options = {}, passed_template = nil)
def _pick_partial_template(partial_path) #:nodoc:
prefix = controller_path unless partial_path.include?('/')
- find_by_parts(partial_path, formats, prefix, true)
+ find_by_parts(partial_path, {:formats => formats}, prefix, true)
end
memoize :_pick_partial_template
end
@@ -23,10 +23,10 @@ def render(options = {}, local_assigns = {}, &block) #:nodoc:
return _render_partial_with_layout(layout, options) if options.key?(:partial)
return _render_partial_with_block(layout, block, options) if block_given?
- layout = find_by_parts(layout, formats) if layout
+ layout = find_by_parts(layout, {:formats => formats}) if layout
if file = options[:file]
- template = find_by_parts(file, formats)
+ template = find_by_parts(file, {:formats => formats})
_render_template_with_layout(template, layout, :locals => options[:locals])
elsif inline = options[:inline]
_render_inline(inline, layout, options)
@@ -16,6 +16,10 @@ def self.extended(base)
@@template_handlers = {}
@@default_template_handlers = nil
+
+ def self.extensions
+ @@template_handlers.keys
+ end
# Register a class that knows how to handle template files with the given
# extension. This can be used to implement new template types.
Oops, something went wrong.

4 comments on commit 0a132c2

@mislav

This comment has been minimized.

Show comment Hide comment
@mislav

mislav Apr 23, 2009

Member

Yehuda and Carl, sitting in the tree,
R-E-F-A-C-T-O-R-I-N-G

Member

mislav replied Apr 23, 2009

Yehuda and Carl, sitting in the tree,
R-E-F-A-C-T-O-R-I-N-G

@fesplugas

This comment has been minimized.

Show comment Hide comment
@fesplugas

fesplugas Apr 23, 2009

Contributor

Not a diff parsing error. There's an extra 't' at the end of that hash.

Contributor

fesplugas replied Apr 23, 2009

Not a diff parsing error. There's an extra 't' at the end of that hash.

@ryana

This comment has been minimized.

Show comment Hide comment
@ryana

ryana Apr 23, 2009

Contributor

lol at mislav.

Contributor

ryana replied Apr 23, 2009

lol at mislav.

@carllerche

This comment has been minimized.

Show comment Hide comment
@carllerche

carllerche Apr 23, 2009

Contributor

Woops, there was an extra 't' at the end of that hash

Contributor

carllerche replied Apr 23, 2009

Woops, there was an extra 't' at the end of that hash

Please sign in to comment.