diff options
Diffstat (limited to 'actionmailer')
| -rw-r--r-- | actionmailer/CHANGELOG.md | 23 | ||||
| -rw-r--r-- | actionmailer/lib/action_mailer/base.rb | 8 | ||||
| -rw-r--r-- | actionmailer/lib/action_mailer/message_delivery.rb | 16 | ||||
| -rw-r--r-- | actionmailer/lib/rails/generators/mailer/templates/application_mailer.rb | 2 | ||||
| -rw-r--r-- | actionmailer/test/message_delivery_test.rb | 8 |
5 files changed, 47 insertions, 10 deletions
diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 1109912b04..76d99f31c7 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,21 @@ +* 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. @@ -23,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* @@ -33,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/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/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/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 |