Handle some TODOs and deprecations.

actionmailer/lib/action_mailer/adv_attr_accessor.rb

@@ -1,8 +1,6 @@
 module ActionMailer
   module AdvAttrAccessor #:nodoc:
     def adv_attr_accessor(*names)
-      
-      # TODO: ActiveSupport::Deprecation.warn()
       names.each do |name|
         ivar = "@#{name}"
 

actionmailer/lib/action_mailer/base.rb

@@ -302,7 +302,6 @@ module ActionMailer #:nodoc:
         @mailer_name ||= name.underscore
       end
       attr_writer :mailer_name
-
       alias :controller_path :mailer_name
 
       # Receives a raw email, parses it into an email object, decodes it,
@@ -324,23 +323,21 @@ module ActionMailer #:nodoc:
         end
       end
 
-      def template_root
-        self.view_paths && self.view_paths.first
-      end
-
-      # Should template root overwrite the whole view_paths?
-      def template_root=(root)
-        self.view_paths = ActionView::Base.process_view_paths(root)
-      end
-
       # TODO The delivery should happen inside the instrument block
       def delivered_email(mail)
-        ActiveSupport::Notifications.instrument("action_mailer.deliver", :mailer => self.name) do |payload|
+        ActiveSupport::Notifications.instrument("action_mailer.deliver") do |payload|
           self.set_payload_for_mail(payload, mail)
         end
       end
 
+      def respond_to?(method, *args) #:nodoc:
+        super || action_methods.include?(method.to_s)
+      end
+
+    protected
+
       def set_payload_for_mail(payload, mail) #:nodoc:
+        payload[:mailer]     = self.name
         payload[:message_id] = mail.message_id
         payload[:subject]    = mail.subject
         payload[:to]         = mail.to
@@ -351,13 +348,7 @@ module ActionMailer #:nodoc:
         payload[:mail]       = mail.encoded
       end
 
-      def respond_to?(method, *args)
-        super || action_methods.include?(method.to_s)
-      end
-
-    protected
-
-      def method_missing(method, *args)
+      def method_missing(method, *args) #:nodoc:
         if action_methods.include?(method.to_s)
           new(method, *args).message
         else
@@ -459,7 +450,7 @@ module ActionMailer #:nodoc:
           :content_type => self.class.default_content_type.dup
         }
       else
-        self.class.template_root.find_all(action_name, {}, self.class.mailer_name).each do |template|
+        each_template do |template|
           responses << {
             :body => render_to_body(:_template => template),
             :content_type => template.mime_type.to_s
@@ -470,6 +461,16 @@ module ActionMailer #:nodoc:
       [responses, sort_order]
     end
 
+    def each_template(&block) #:nodoc:
+      self.class.view_paths.each do |load_paths|
+        templates = load_paths.find_all(action_name, {}, self.class.mailer_name)
+        unless templates.empty?
+          templates.each(&block)
+          return
+        end
+      end
+    end
+
     def create_parts_from_responses(m, responses, charset) #:nodoc:
       if responses.size == 1 && !m.has_attachments?
         m.body = responses[0][:body]

actionmailer/lib/action_mailer/deprecated_api.rb

@@ -25,11 +25,20 @@ module ActionMailer
         mail
       end
 
-      def respond_to?(method_symbol, include_private = false) #:nodoc:
+      def template_root
+        self.view_paths && self.view_paths.first
+      end
+
+      def template_root=(root)
+        ActiveSupport::Deprecation.warn "template_root= is deprecated, use view_paths.unshift instead", caller[0,2]
+        self.view_paths = ActionView::Base.process_view_paths(root)
+      end
+
+      def respond_to?(method_symbol, include_private = false)
         matches_dynamic_method?(method_symbol) || super
       end
 
-      def method_missing(method_symbol, *parameters) #:nodoc:
+      def method_missing(method_symbol, *parameters)
         if match = matches_dynamic_method?(method_symbol)
           case match[1]
             when 'create'
@@ -49,7 +58,7 @@ module ActionMailer
 
     private
 
-      def matches_dynamic_method?(method_name) #:nodoc:
+      def matches_dynamic_method?(method_name)
         method_name = method_name.to_s
         /^(create|deliver)_([_a-z]\w*)/.match(method_name) || /^(new)$/.match(method_name)
       end
@@ -70,10 +79,8 @@ module ActionMailer
       if options[:body]
         ActiveSupport::Deprecation.warn(':body in render deprecated. Please use instance ' <<
                                         'variables as assigns instead', caller[0,1])
-
         body options.delete(:body)
       end
-
       super
     end
 
@@ -93,13 +100,11 @@ module ActionMailer
 
   private
     
-
-    def create_parts #:nodoc:
+    def create_parts
       if @body.is_a?(Hash) && !@body.empty?
         ActiveSupport::Deprecation.warn "Giving a hash to body is deprecated, please use instance variables instead", caller[0,2]
         @body.each { |k, v| instance_variable_set(:"@#{k}", v) }
       end
-
       super
     end
 

actionmailer/lib/action_mailer/old_api.rb

@@ -1,5 +1,5 @@
 module ActionMailer
-  module OldApi
+  module OldApi #:nodoc:
     extend ActiveSupport::Concern
 
     included do
@@ -144,7 +144,7 @@ module ActionMailer
         :content_disposition => content_disposition }.merge(params)
     end
     
-    def create_mail #:nodoc:
+    def create_mail 
       m = @_message
 
       quote_fields!({:subject => subject, :to => recipients, :from => from,
@@ -184,7 +184,7 @@ module ActionMailer
     # Set up the default values for the various instance variables of this
     # mailer. Subclasses may override this method to provide different
     # defaults.
-    def initialize_defaults(method_name) #:nodoc:
+    def initialize_defaults(method_name) 
       @charset              ||= self.class.default_charset.dup
       @content_type         ||= self.class.default_content_type.dup
       @implicit_parts_order ||= self.class.default_implicit_parts_order.dup
@@ -200,7 +200,7 @@ module ActionMailer
       @body ||= {}
     end
 
-    def create_parts #:nodoc:
+    def create_parts 
       if String === @body
         self.response_body = @body
       end
@@ -208,7 +208,7 @@ module ActionMailer
       if String === response_body
         @parts.unshift create_inline_part(response_body)
       else
-        self.class.template_root.find_all(@template, {}, @mailer_name).each do |template|
+        self.class.view_paths.first.find_all(@template, {}, @mailer_name).each do |template|
           @parts << create_inline_part(render_to_body(:_template => template), template.mime_type)
         end
 
@@ -222,7 +222,7 @@ module ActionMailer
       end
     end
 
-    def create_inline_part(body, mime_type=nil) #:nodoc:
+    def create_inline_part(body, mime_type=nil) 
       ct = mime_type || "text/plain"
       main_type, sub_type = split_content_type(ct.to_s)
 
@@ -233,11 +233,11 @@ module ActionMailer
       )
     end
 
-    def split_content_type(ct) #:nodoc:
+    def split_content_type(ct) 
       ct.to_s.split("/")
     end
 
-    def parse_content_type(defaults=nil) #:nodoc:
+    def parse_content_type(defaults=nil) 
       if @content_type.blank?
         [ nil, {} ]
       else

actionmailer/lib/action_mailer/test_helper.rb

@@ -58,7 +58,6 @@ module ActionMailer
   end
 end
 
-# TODO: Deprecate this
 module Test
   module Unit
     class TestCase

actionmailer/test/mail_service_test.rb

@@ -1085,13 +1085,15 @@ EOF
   end
 end
 
-class InheritableTemplateRootTest < Test::Unit::TestCase
+class InheritableTemplateRootTest < ActiveSupport::TestCase
   def test_attr
     expected = File.expand_path("#{File.dirname(__FILE__)}/fixtures/path.with.dots")
     assert_equal expected, FunkyPathMailer.template_root.to_s
 
     sub = Class.new(FunkyPathMailer)
-    sub.template_root = 'test/path'
+    assert_deprecated do
+      sub.template_root = 'test/path'
+    end
 
     assert_equal File.expand_path('test/path'), sub.template_root.to_s
     assert_equal expected, FunkyPathMailer.template_root.to_s