diff options
Diffstat (limited to 'actionmailer')
| -rw-r--r-- | actionmailer/CHANGELOG.md | 29 | ||||
| -rw-r--r-- | actionmailer/lib/action_mailer/base.rb | 8 | ||||
| -rw-r--r-- | actionmailer/lib/action_mailer/delivery_methods.rb | 2 | ||||
| -rw-r--r-- | actionmailer/lib/action_mailer/message_delivery.rb | 16 | ||||
| -rw-r--r-- | actionmailer/lib/action_mailer/test_case.rb | 4 | ||||
| -rw-r--r-- | actionmailer/lib/rails/generators/mailer/templates/application_mailer.rb | 2 | ||||
| -rw-r--r-- | actionmailer/test/delivery_methods_test.rb | 2 | ||||
| -rw-r--r-- | actionmailer/test/message_delivery_test.rb | 8 |
8 files changed, 57 insertions, 14 deletions
diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 51d85b95f2..76d99f31c7 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,27 @@ +* Disallow calling `#deliver_later` after making local modifications to + the message which would be lost when the delivery job is enqueued. + + Prevents a common, hard-to-find bug like: + + message = Notifier.welcome(user, foo) + message.message_id = my_generated_message_id + message.deliver_later + + The message_id is silently lost! *Only the mailer arguments are passed + to the delivery job.* + + This raises an exception now. Make modifications to the message within + the mailer method instead, or use a custom Active Job to manage delivery + instead of using #deliver_later. + + *Jeremy Daer* + +* Removes `-t` from default Sendmail arguments to match the underlying + `Mail::Sendmail` setting. + + *Clayton Liggitt* + + ## Rails 5.0.0.beta3 (February 24, 2016) ## * Add support for fragment caching in Action Mailer views. @@ -17,8 +41,7 @@ ## Rails 5.0.0.beta1 (December 18, 2015) ## -* `config.force_ssl = true` will set - `config.action_mailer.default_url_options = { protocol: 'https' }`. +* `config.action_mailer.default_url_options[:protocol]` is now set to `https` if `config.force_ssl` is set to `true`. *Andrew Kampjes* @@ -27,7 +50,7 @@ *Chris McGrath* -* `assert_emails` in block form use the given number as expected value. +* `assert_emails` in block form, uses the given number as expected value. This makes the error message much easier to understand. *Yuji Yaginuma* diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index a223cf82a1..4bb7842297 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -86,7 +86,7 @@ module ActionMailer # Like Action Controller, each mailer class has a corresponding view directory in which each # method of the class looks for a template with its name. # - # To define a template to be used with a mailing, create an <tt>.erb</tt> file with the same + # To define a template to be used with a mailer, create an <tt>.erb</tt> file with the same # name as the method in your mailer model. For example, in the mailer defined above, the template at # <tt>app/views/notifier_mailer/welcome.text.erb</tt> would be used to generate the email. # @@ -144,7 +144,7 @@ module ActionMailer # mail.deliver_now # generates and sends the email now # # The <tt>ActionMailer::MessageDelivery</tt> class is a wrapper around a delegate that will call - # your method to generate the mail. If you want direct access to delegator, or <tt>Mail::Message</tt>, + # your method to generate the mail. If you want direct access to the delegator, or <tt>Mail::Message</tt>, # you can call the <tt>message</tt> method on the <tt>ActionMailer::MessageDelivery</tt> object. # # NotifierMailer.welcome(User.first).message # => a Mail::Message object @@ -163,7 +163,7 @@ module ActionMailer # # Multipart messages can also be used implicitly because Action Mailer will automatically detect and use # multipart templates, where each template is named after the name of the action, followed by the content - # type. Each such detected template will be added as a separate part to the message. + # type. Each such detected template will be added to the message, as a separate part. # # For example, if the following templates exist: # * signup_notification.text.erb @@ -288,7 +288,7 @@ module ActionMailer # end # # Note that the proc is evaluated right at the start of the mail message generation, so if you - # set something in the default using a proc, and then set the same thing inside of your + # set something in the default hash using a proc, and then set the same thing inside of your # mailer method, it will get overwritten by the mailer method. # # It is also possible to set these default options that will be used in all mailers through diff --git a/actionmailer/lib/action_mailer/delivery_methods.rb b/actionmailer/lib/action_mailer/delivery_methods.rb index 4758b55a2a..571c8e7d2a 100644 --- a/actionmailer/lib/action_mailer/delivery_methods.rb +++ b/actionmailer/lib/action_mailer/delivery_methods.rb @@ -36,7 +36,7 @@ module ActionMailer add_delivery_method :sendmail, Mail::Sendmail, location: '/usr/sbin/sendmail', - arguments: '-i -t' + arguments: '-i' add_delivery_method :test, Mail::TestMailer end diff --git a/actionmailer/lib/action_mailer/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb index 5fcb5a0c88..d638057d72 100644 --- a/actionmailer/lib/action_mailer/message_delivery.rb +++ b/actionmailer/lib/action_mailer/message_delivery.rb @@ -18,6 +18,7 @@ module ActionMailer @mailer = mailer @mail_method = mail_method @args = args + @obj = nil end def __getobj__ #:nodoc: @@ -91,8 +92,19 @@ module ActionMailer private def enqueue_delivery(delivery_method, options={}) - args = @mailer.name, @mail_method.to_s, delivery_method.to_s, *@args - ActionMailer::DeliveryJob.set(options).perform_later(*args) + if @obj + raise "You've accessed the message before asking to deliver it " \ + "later, so you may have made local changes that would be " \ + "silently lost if we enqueued a job to deliver it. Why? Only " \ + "the mailer method *arguments* are passed with the delivery job! " \ + "Do not access the message in any way if you mean to deliver it " \ + "later. Workarounds: 1. don't touch the message before calling " \ + "#deliver_later, 2. only touch the message *within your mailer " \ + "method*, or 3. use a custom Active Job instead of #deliver_later." + else + args = @mailer.name, @mail_method.to_s, delivery_method.to_s, *@args + ActionMailer::DeliveryJob.set(options).perform_later(*args) + end end end end diff --git a/actionmailer/lib/action_mailer/test_case.rb b/actionmailer/lib/action_mailer/test_case.rb index d83719e57d..65e7347ae4 100644 --- a/actionmailer/lib/action_mailer/test_case.rb +++ b/actionmailer/lib/action_mailer/test_case.rb @@ -15,11 +15,11 @@ module ActionMailer extend ActiveSupport::Concern included do - teardown :clear_test_deliviers + teardown :clear_test_deliveries end private - def clear_test_deliviers + def clear_test_deliveries if ActionMailer::Base.delivery_method == :test ActionMailer::Base.deliveries.clear end diff --git a/actionmailer/lib/rails/generators/mailer/templates/application_mailer.rb b/actionmailer/lib/rails/generators/mailer/templates/application_mailer.rb index f23e575fe5..00fb9bd48f 100644 --- a/actionmailer/lib/rails/generators/mailer/templates/application_mailer.rb +++ b/actionmailer/lib/rails/generators/mailer/templates/application_mailer.rb @@ -1,4 +1,4 @@ -<% module_namespacing do %> +<% module_namespacing do -%> class ApplicationMailer < ActionMailer::Base default from: 'from@example.com' layout 'mailer' diff --git a/actionmailer/test/delivery_methods_test.rb b/actionmailer/test/delivery_methods_test.rb index 2786fe0d07..bcbd036f26 100644 --- a/actionmailer/test/delivery_methods_test.rb +++ b/actionmailer/test/delivery_methods_test.rb @@ -39,7 +39,7 @@ class DefaultsDeliveryMethodsTest < ActiveSupport::TestCase test "default sendmail settings" do settings = { location: '/usr/sbin/sendmail', - arguments: '-i -t' + arguments: '-i' } assert_equal settings, ActionMailer::Base.sendmail_settings end diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb index b834cdd08c..f06d69369f 100644 --- a/actionmailer/test/message_delivery_test.rb +++ b/actionmailer/test/message_delivery_test.rb @@ -93,4 +93,12 @@ class MessageDeliveryTest < ActiveSupport::TestCase @mail.deliver_later(queue: :another_queue) end end + + test 'deliver_later after accessing the message is disallowed' do + @mail.message # Load the message, which calls the mailer method. + + assert_raise RuntimeError do + @mail.deliver_later + end + end end |