Deprecate the old mailer API that was not deprecated yet.

13 files changed, 80 insertions, 116 deletions

diff --git a/actionmailer/lib/action_mailer/adv_attr_accessor.rb b/actionmailer/lib/action_mailer/adv_attr_accessor.rb
index be6b1feca9..c1aa8021ce 100644
--- a/actionmailer/lib/action_mailer/adv_attr_accessor.rb
+++ b/actionmailer/lib/action_mailer/adv_attr_accessor.rb
@@ -1,26 +1,28 @@
 module ActionMailer
   module AdvAttrAccessor #:nodoc:
-    def adv_attr_accessor(*names)
-      names.each do |name|
-        ivar = "@#{name}"
+    def adv_attr_accessor(name, deprecation=nil)
+      ivar = "@#{name}"
+      deprecation ||= "Please pass :#{name} as hash key to mail() instead"
 
-        class_eval <<-ACCESSORS, __FILE__, __LINE__ + 1
-          def #{name}=(value)
-            #{ivar} = value
-          end
+      class_eval <<-ACCESSORS, __FILE__, __LINE__ + 1
+        def #{name}=(value)
+          ActiveSupport::Deprecation.warn "#{name}= is deprecated. #{deprecation}"
+          #{ivar} = value
+        end
 
-          def #{name}(*args)
-            raise ArgumentError, "expected 0 or 1 parameters" unless args.length <= 1
-            if args.empty?
-              #{ivar} if instance_variable_names.include?(#{ivar.inspect})
-            else
-              #{ivar} = args.first
-            end
+        def #{name}(*args)
+          raise ArgumentError, "expected 0 or 1 parameters" unless args.length <= 1
+          if args.empty?
+            ActiveSupport::Deprecation.warn "#{name}() is deprecated and will be removed in future versions."
+            #{ivar} if instance_variable_names.include?(#{ivar.inspect})
+          else
+            ActiveSupport::Deprecation.warn "#{name}(value) is deprecated. #{deprecation}"
+            #{ivar} = args.first
           end
-        ACCESSORS
+        end
+      ACCESSORS
 
-        self.protected_instance_variables << ivar if self.respond_to?(:protected_instance_variables)
-      end
+      self.protected_instance_variables << ivar if self.respond_to?(:protected_instance_variables)
     end
   end
 end
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index efa50d0839..31e1b26afd 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -341,8 +341,10 @@ module ActionMailer #:nodoc:
     include AbstractController::Translation
     include AbstractController::AssetPaths
 
-    helper  ActionMailer::MailHelper
+    cattr_reader :protected_instance_variables
+    @@protected_instance_variables = []
 
+    helper  ActionMailer::MailHelper
     include ActionMailer::OldApi
 
     delegate :register_observer, :to => Mail
@@ -445,6 +447,10 @@ module ActionMailer #:nodoc:
       super
     end
 
+    def mailer_name
+      self.class.mailer_name
+    end
+
     # Allows you to pass random and unusual headers to the new +Mail::Message+ object
     # which will add them to itself.
     #
diff --git a/actionmailer/lib/action_mailer/old_api.rb b/actionmailer/lib/action_mailer/old_api.rb
index 2a6289c22d..b8c15df263 100644
--- a/actionmailer/lib/action_mailer/old_api.rb
+++ b/actionmailer/lib/action_mailer/old_api.rb
@@ -8,9 +8,7 @@ module ActionMailer
 
     included do
       extend ActionMailer::AdvAttrAccessor
-
-      @@protected_instance_variables = %w(@parts)
-      cattr_reader :protected_instance_variables
+      self.protected_instance_variables.concat %w(@parts @mail_was_called)
 
       # Specify the BCC addresses for the message
       adv_attr_accessor :bcc
@@ -42,11 +40,11 @@ module ActionMailer
 
       # The recipient addresses for the message, either as a string (for a single
       # address) or an array (for multiple addresses).
-      adv_attr_accessor :recipients
+      adv_attr_accessor :recipients, "Please pass :to as hash key to mail() instead"
 
       # The date on which the message was sent. If not set (the default), the
       # header will be set by the delivery agent.
-      adv_attr_accessor :sent_on
+      adv_attr_accessor :sent_on, "Please pass :date as hash key to mail() instead"
 
       # Specify the subject of the message.
       adv_attr_accessor :subject
@@ -54,20 +52,12 @@ module ActionMailer
       # Specify the template name to use for current message. This is the "base"
       # template name, without the extension or directory, and may be used to
       # have multiple mailer methods share the same template.
-      adv_attr_accessor :template
-
-      # Override the mailer name, which defaults to an inflected version of the
-      # mailer's class name. If you want to use a template in a non-standard
-      # location, you can use this to specify that location.
-      adv_attr_accessor :mailer_name
+      adv_attr_accessor :template, "Please pass :template_name or :template_path as hash key to mail() instead"
 
       # Define the body of the message. This is either a Hash (in which case it
       # specifies the variables to pass to the template when it is rendered),
       # or a string, in which case it specifies the actual text of the message.
       adv_attr_accessor :body
-
-      # Alias controller_path to mailer_name so render :partial in views work.
-      alias :controller_path :mailer_name
     end
 
     def process(method_name, *args)
@@ -84,6 +74,8 @@ module ActionMailer
     # part itself is yielded to the block so that other properties (charset,
     # body, headers, etc.) can be set on it.
     def part(params)
+      ActiveSupport::Deprecation.warn "part() is deprecated and will be removed in future versions. " <<
+        "Please pass a block to mail() instead."
       params = {:content_type => params} if String === params
 
       if custom_headers = params.delete(:headers)
@@ -99,6 +91,8 @@ module ActionMailer
     # Add an attachment to a multipart message. This is simply a part with the
     # content-disposition set to "attachment".
     def attachment(params, &block)
+      ActiveSupport::Deprecation.warn "attachment() is deprecated and will be removed in future versions. " <<
+        "Please use the attachments[] API instead."
       params = { :content_type => params } if String === params
 
       params[:content] ||= params.delete(:data) || params.delete(:body)
@@ -148,11 +142,11 @@ module ActionMailer
     def create_mail
       m = @_message
 
-      set_fields!({:subject => subject, :to => recipients, :from => from,
-                   :bcc => bcc, :cc => cc, :reply_to => reply_to}, charset)
+      set_fields!({:subject => @subject, :to => @recipients, :from => @from,
+                   :bcc => @bcc, :cc => @cc, :reply_to => @reply_to}, @charset)
 
-      m.mime_version = mime_version    unless mime_version.nil?
-      m.date         = sent_on.to_time rescue sent_on if sent_on
+      m.mime_version = @mime_version    if @mime_version
+      m.date         = @sent_on.to_time rescue @sent_on if @sent_on
 
       @headers.each { |k, v| m[k] = v }
 
@@ -191,6 +185,8 @@ module ActionMailer
       @implicit_parts_order ||= self.class.default[:parts_order].try(:dup)
       @mime_version         ||= self.class.default[:mime_version].try(:dup)
 
+      @cc, @bcc, @reply_to, @subject, @from, @recipients = nil, nil, nil, nil, nil, nil
+
       @mailer_name   ||= self.class.mailer_name.dup
       @template      ||= method_name
       @mail_was_called = false
diff --git a/actionmailer/test/abstract_unit.rb b/actionmailer/test/abstract_unit.rb
index b430dffdf8..0dce0ac15d 100644
--- a/actionmailer/test/abstract_unit.rb
+++ b/actionmailer/test/abstract_unit.rb
@@ -78,3 +78,5 @@ end
 def restore_delivery_method
   ActionMailer::Base.delivery_method = @old_delivery_method
 end
+
+ActiveSupport::Deprecation.silenced = true
\ No newline at end of file
diff --git a/actionmailer/test/old_base/asset_host_test.rb b/actionmailer/test/asset_host_test.rb
index cc13c8a4d7..069860ff06 100644
--- a/actionmailer/test/old_base/asset_host_test.rb
+++ b/actionmailer/test/asset_host_test.rb
@@ -3,9 +3,9 @@ require 'action_controller'
 
 class AssetHostMailer < ActionMailer::Base
   def email_with_asset
-    recipients 'test@localhost'
-    subject    "testing email containing asset path while asset_host is set"
-    from       "tester@example.com"
+    mail :to => 'test@localhost',
+      :subject => 'testing email containing asset path while asset_host is set',
+      :from => 'tester@example.com'
   end
 end
 
diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb
index c11081072d..7ed9d4a5c0 100644
--- a/actionmailer/test/base_test.rb
+++ b/actionmailer/test/base_test.rb
@@ -7,9 +7,6 @@ require 'mailers/proc_mailer'
 require 'mailers/asset_mailer'
 
 class BaseTest < ActiveSupport::TestCase
-  # TODO Add some tests for implicity layout render and url helpers
-  # so we can get rid of old base tests altogether with old base.
-
   def teardown
     ActionMailer::Base.asset_host = nil
     ActionMailer::Base.assets_dir = nil
diff --git a/actionmailer/test/old_base/mail_layout_test.rb b/actionmailer/test/mail_layout_test.rb
index 2c2daa0f28..def8da81b8 100644
--- a/actionmailer/test/old_base/mail_layout_test.rb
+++ b/actionmailer/test/mail_layout_test.rb
@@ -1,53 +1,45 @@
 require 'abstract_unit'
 
 class AutoLayoutMailer < ActionMailer::Base
+  default :to => 'test@localhost',
+    :subject => "You have a mail",
+    :from => "tester@example.com"
 
   def hello
-    recipients 'test@localhost'
-    subject    "You have a mail"
-    from       "tester@example.com"
+    mail()
   end
 
   def spam
-    recipients 'test@localhost'
-    subject    "You have a mail"
-    from       "tester@example.com"
-
     @world = "Earth"
-    body render(:inline => "Hello, <%= @world %>", :layout => 'spam')
+    mail(:body => render(:inline => "Hello, <%= @world %>", :layout => 'spam'))
   end
 
   def nolayout
-    recipients 'test@localhost'
-    subject    "You have a mail"
-    from       "tester@example.com"
-
     @world = "Earth"
-    body render(:inline => "Hello, <%= @world %>", :layout => false)
+    mail(:body => render(:inline => "Hello, <%= @world %>", :layout => false))
   end
 
   def multipart(type = nil)
-    recipients 'test@localhost'
-    subject    "You have a mail"
-    from       "tester@example.com"
-
-    content_type(type) if type
+    mail(:content_type => type) do |format|
+      format.text { render }
+      format.html { render }
+    end
   end
 end
 
 class ExplicitLayoutMailer < ActionMailer::Base
   layout 'spam', :except => [:logout]
 
+  default :to => 'test@localhost',
+    :subject => "You have a mail",
+    :from => "tester@example.com"
+
   def signup
-    recipients 'test@localhost'
-    subject    "You have a mail"
-    from       "tester@example.com"
+    mail()
   end
 
   def logout
-    recipients 'test@localhost'
-    subject    "You have a mail"
-    from       "tester@example.com"
+    mail()
   end
 end
 
@@ -91,19 +83,6 @@ class LayoutMailerTest < Test::Unit::TestCase
     assert_equal "Hello from layout text/html multipart", mail.parts.last.body.to_s
   end
 
-  def test_should_fix_multipart_layout
-    mail = AutoLayoutMailer.multipart("text/plain")
-    assert_equal "multipart/alternative", mail.mime_type
-    assert_equal 2, mail.parts.size
-
-    assert_equal 'text/plain', mail.parts.first.mime_type
-    assert_equal "text/plain layout - text/plain multipart", mail.parts.first.body.to_s
-
-    assert_equal 'text/html', mail.parts.last.mime_type
-    assert_equal "Hello from layout text/html multipart", mail.parts.last.body.to_s
-  end
-
-
   def test_should_pickup_layout_given_to_render
     mail = AutoLayoutMailer.spam
     assert_equal "Spammer layout Hello, Earth", mail.body.to_s.strip
diff --git a/actionmailer/test/mailers/base_mailer.rb b/actionmailer/test/mailers/base_mailer.rb
index e89a5820cc..9416bc718e 100644
--- a/actionmailer/test/mailers/base_mailer.rb
+++ b/actionmailer/test/mailers/base_mailer.rb
@@ -113,6 +113,6 @@ class BaseMailer < ActionMailer::Base
   end
 
   def email_with_translations
-    body render("email_with_translations.html")
+    mail :body => render("email_with_translations.html")
   end
 end
diff --git a/actionmailer/test/old_base/adv_attr_test.rb b/actionmailer/test/old_base/adv_attr_test.rb
index f22d733bc5..c5a6b6d88b 100644
--- a/actionmailer/test/old_base/adv_attr_test.rb
+++ b/actionmailer/test/old_base/adv_attr_test.rb
@@ -11,9 +11,14 @@ class AdvAttrTest < ActiveSupport::TestCase
   end
 
   def setup
+    ActiveSupport::Deprecation.silenced = true
     @person = Person.new
   end
 
+  def teardown
+    ActiveSupport::Deprecation.silenced = false
+  end
+
   def test_adv_attr
     assert_nil @person.name
     @person.name 'Bob'
diff --git a/actionmailer/test/old_base/mail_render_test.rb b/actionmailer/test/old_base/mail_render_test.rb
index 9b1455da33..3a1d3184f4 100644
--- a/actionmailer/test/old_base/mail_render_test.rb
+++ b/actionmailer/test/old_base/mail_render_test.rb
@@ -19,18 +19,6 @@ class RenderMailer < ActionMailer::Base
     body       render(:file => "templates/signed_up")
   end
 
-  def rxml_template
-    recipients 'test@localhost'
-    subject    "rendering rxml template"
-    from       "tester@example.com"
-  end
-
-  def included_subtemplate
-    recipients 'test@localhost'
-    subject    "Including another template in the one being rendered"
-    from       "tester@example.com"
-  end
-
   def no_instance_variable
     recipients 'test@localhost'
     subject    "No Instance Variable"
@@ -41,11 +29,6 @@ class RenderMailer < ActionMailer::Base
     end
   end
 
-  def initialize_defaults(method_name)
-    super
-    mailer_name "test_mailer"
-  end
-
   def multipart_alternative
     recipients 'test@localhost'
     subject    'multipart/alternative'
@@ -97,11 +80,13 @@ class RenderHelperTest < Test::Unit::TestCase
     set_delivery_method :test
     ActionMailer::Base.perform_deliveries = true
     ActionMailer::Base.deliveries.clear
+    ActiveSupport::Deprecation.silenced = true
 
     @recipient = 'test@localhost'
   end
 
   def teardown
+    ActiveSupport::Deprecation.silenced = false
     restore_delivery_method
   end
 
@@ -115,16 +100,6 @@ class RenderHelperTest < Test::Unit::TestCase
     assert_equal "Hello there,\n\nMr. test@localhost", mail.body.to_s.strip
   end
 
-  def test_rxml_template
-    mail = RenderMailer.rxml_template.deliver
-    assert_equal %(<?xml version="1.0" encoding="UTF-8"?>\n<test/>), mail.body.to_s.strip
-  end
-
-  def test_included_subtemplate
-    mail = RenderMailer.included_subtemplate.deliver
-    assert_equal "Hey Ho, let's go!", mail.body.to_s.strip
-  end
-
   def test_no_instance_variable
     mail = RenderMailer.no_instance_variable.deliver
     assert_equal "Look, subject.nil? is true!", mail.body.to_s.strip
@@ -134,6 +109,7 @@ end
 class FirstSecondHelperTest < Test::Unit::TestCase
   def setup
     set_delivery_method :test
+    ActiveSupport::Deprecation.silenced = true
     ActionMailer::Base.perform_deliveries = true
     ActionMailer::Base.deliveries.clear
 
@@ -141,6 +117,7 @@ class FirstSecondHelperTest < Test::Unit::TestCase
   end
 
   def teardown
+    ActiveSupport::Deprecation.silenced = false
     restore_delivery_method
   end
 
diff --git a/actionmailer/test/old_base/mail_service_test.rb b/actionmailer/test/old_base/mail_service_test.rb
index 49647c9612..93921978b6 100644
--- a/actionmailer/test/old_base/mail_service_test.rb
+++ b/actionmailer/test/old_base/mail_service_test.rb
@@ -334,6 +334,7 @@ class ActionMailerTest < Test::Unit::TestCase
     ActionMailer::Base.perform_deliveries = true
     ActionMailer::Base.raise_delivery_errors = true
     ActionMailer::Base.deliveries.clear
+    ActiveSupport::Deprecation.silenced = true
 
     @original_logger = TestMailer.logger
     @recipient = 'test@localhost'
@@ -343,6 +344,7 @@ class ActionMailerTest < Test::Unit::TestCase
 
   def teardown
     TestMailer.logger = @original_logger
+    ActiveSupport::Deprecation.silenced = false
     restore_delivery_method
   end
 
diff --git a/actionmailer/test/test_helper_test.rb b/actionmailer/test/test_helper_test.rb
index 5a101e852f..dd62164176 100644
--- a/actionmailer/test/test_helper_test.rb
+++ b/actionmailer/test/test_helper_test.rb
@@ -2,11 +2,10 @@ require 'abstract_unit'
 
 class TestHelperMailer < ActionMailer::Base
   def test
-    recipients "test@example.com"
-    from       "tester@example.com"
-
     @world = "Earth"
-    render(:inline => "Hello, <%= @world %>")
+    mail :body => render(:inline => "Hello, <%= @world %>"),
+      :to => "test@example.com",
+      :from => "tester@example.com"
   end
 end
 
diff --git a/actionmailer/test/old_base/url_test.rb b/actionmailer/test/url_test.rb
index 573186dbee..9b2b38d9e8 100644
--- a/actionmailer/test/old_base/url_test.rb
+++ b/actionmailer/test/url_test.rb
@@ -18,13 +18,10 @@ class UrlTestMailer < ActionMailer::Base
   end
 
   def signed_up_with_url(recipient)
-    @recipients   = recipient
-    @subject      = "[Signed up] Welcome #{recipient}"
-    @from         = "system@loudthinking.com"
-    @sent_on      = Time.local(2004, 12, 12)
-
     @recipient   = recipient
     @welcome_url = url_for :host => "example.com", :controller => "welcome", :action => "greeting"
+    mail(:to => recipient, :subject => "[Signed up] Welcome #{recipient}",
+      :from => "system@loudthinking.com", :date => Time.local(2004, 12, 12))
   end
 end
 
@@ -47,6 +44,7 @@ class ActionMailerUrlTest < ActionMailer::TestCase
     set_delivery_method :test
     ActionMailer::Base.perform_deliveries = true
     ActionMailer::Base.deliveries.clear
+    ActiveSupport::Deprecation.silenced = false
 
     @recipient = 'test@localhost'
   end
@@ -71,6 +69,7 @@ class ActionMailerUrlTest < ActionMailer::TestCase
     expected.body    = "Hello there,\n\nMr. #{@recipient}. Please see our greeting at http://example.com/welcome/greeting http://www.basecamphq.com/welcome\n\n<img alt=\"Somelogo\" src=\"/images/somelogo.png\" />"
     expected.from    = "system@loudthinking.com"
     expected.date    = Time.local(2004, 12, 12)
+    expected.content_type = "text/html"
 
     created = nil
     assert_nothing_raised { created = UrlTestMailer.signed_up_with_url(@recipient) }