From 5c54db290f4e7cdd69b461d4c0b020a9cd364537 Mon Sep 17 00:00:00 2001 From: Marcus Ilgner Date: Tue, 8 Dec 2015 12:28:00 +0100 Subject: ActionMailer: support overriding template name in multipart Implicit rendering in multipart blocks now also uses the template name from the options hash instead of always using the action name. So you can now write mail(template_name: template_name) do |format| format.text format.html end --- actionmailer/CHANGELOG.md | 5 +++++ actionmailer/lib/action_mailer/base.rb | 11 ++++++++--- actionmailer/test/base_test.rb | 6 ++++++ actionmailer/test/mailers/base_mailer.rb | 7 +++++++ 4 files changed, 26 insertions(+), 3 deletions(-) (limited to 'actionmailer') diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 0ecb0235bc..b19d6b87c0 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,8 @@ +* Mails with multipart `format` blocks with implicit render now also check for + a template name in options hash instead of only using the action name. + + *Marcus Ilgner* + * `config.force_ssl = true` will set `config.action_mailer.default_url_options = { protocol: 'https' }` diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index cbbf480da8..9dd3f151ad 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -893,9 +893,7 @@ module ActionMailer def collect_responses(headers) if block_given? - collector = ActionMailer::Collector.new(lookup_context) { render(action_name) } - yield(collector) - collector.responses + collect_responses_from_block(headers, &Proc.new) elsif headers[:body] [{ body: headers.delete(:body), @@ -906,6 +904,13 @@ module ActionMailer end end + def collect_responses_from_block(headers) + templates_name = headers[:template_name] || action_name + collector = ActionMailer::Collector.new(lookup_context) { render(templates_name) } + yield(collector) + collector.responses + end + def collect_responses_from_templates(headers) templates_path = headers[:template_path] || self.class.mailer_name templates_name = headers[:template_name] || action_name diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 50f2c71737..fe5358f209 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -539,6 +539,12 @@ class BaseTest < ActiveSupport::TestCase assert_equal("TEXT Implicit Multipart", mail.text_part.body.decoded) end + test "you can specify a different template for multipart render" do + mail = BaseMailer.implicit_different_template_with_block('explicit_multipart_templates').deliver + assert_equal("HTML Explicit Multipart Templates", mail.html_part.body.decoded) + assert_equal("TEXT Explicit Multipart Templates", mail.text_part.body.decoded) + end + test "should raise if missing template in implicit render" do assert_raises ActionView::MissingTemplate do BaseMailer.implicit_different_template('missing_template').deliver_now diff --git a/actionmailer/test/mailers/base_mailer.rb b/actionmailer/test/mailers/base_mailer.rb index 8c2225ce60..f75bd48a9e 100644 --- a/actionmailer/test/mailers/base_mailer.rb +++ b/actionmailer/test/mailers/base_mailer.rb @@ -104,6 +104,13 @@ class BaseMailer < ActionMailer::Base mail(template_name: template_name) end + def implicit_different_template_with_block(template_name='') + mail(template_name: template_name) do |format| + format.text + format.html + end + end + def explicit_different_template(template_name='') mail do |format| format.text { render template: "#{mailer_name}/#{template_name}" } -- cgit v1.2.3 From d2d6f6b7dbbefea84fe847e9c0dc7313cc82700c Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sun, 14 Oct 2018 09:12:46 +0900 Subject: Fix tests on Mail 2.7.1 Up to `2.7.0`, encoding was chosen using `Mail::Encodings::TransferEncoding.negotiate`, and base64 encoding was used. In `2.7.1`, when `transfer_encoding` is not specified, the encoding of the message is respected. Related to: https://github.com/mikel/mail/commit/dead487e02f592d9058fd07deedcde39b569d18d However, what chosen for transfer encoding is not essential in these tests. To test more accurately, confirm that the decoded body instead. --- actionmailer/test/base_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionmailer') diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 7a1a505398..cbbee9fae8 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -152,9 +152,9 @@ class BaseTest < ActiveSupport::TestCase assert_equal(2, email.parts.length) assert_equal("multipart/mixed", email.mime_type) assert_equal("text/html", email.parts[0].mime_type) - assert_equal("Attachment with content", email.parts[0].body.encoded) + assert_equal("Attachment with content", email.parts[0].decoded) assert_equal("application/pdf", email.parts[1].mime_type) - assert_equal("VGhpcyBpcyB0ZXN0IEZpbGUgY29udGVudA==\r\n", email.parts[1].body.encoded) + assert_equal("This is test File content", email.parts[1].decoded) end test "adds the given :body as part" do @@ -162,9 +162,9 @@ class BaseTest < ActiveSupport::TestCase assert_equal(2, email.parts.length) assert_equal("multipart/mixed", email.mime_type) assert_equal("text/plain", email.parts[0].mime_type) - assert_equal("I'm the eggman", email.parts[0].body.encoded) + assert_equal("I'm the eggman", email.parts[0].decoded) assert_equal("application/pdf", email.parts[1].mime_type) - assert_equal("VGhpcyBpcyB0ZXN0IEZpbGUgY29udGVudA==\r\n", email.parts[1].body.encoded) + assert_equal("This is test File content", email.parts[1].decoded) end test "can embed an inline attachment" do -- cgit v1.2.3 From 1c11688b5624394c3792d1bb37599fd1e3452c9c Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Tue, 6 Nov 2018 14:17:23 -0500 Subject: Add CVE note to security guide and gemspecs [ci skip] --- actionmailer/actionmailer.gemspec | 3 +++ 1 file changed, 3 insertions(+) (limited to 'actionmailer') diff --git a/actionmailer/actionmailer.gemspec b/actionmailer/actionmailer.gemspec index f2fb160bdd..efcdcf019a 100644 --- a/actionmailer/actionmailer.gemspec +++ b/actionmailer/actionmailer.gemspec @@ -2,6 +2,9 @@ version = File.read(File.expand_path("../RAILS_VERSION", __dir__)).strip +# NOTE: There's no need to update dependencies for CVEs in minor +# releases when users can simply run `bundle update vulnerable_gem`. + Gem::Specification.new do |s| s.platform = Gem::Platform::RUBY s.name = "actionmailer" -- cgit v1.2.3 From e74fdbe00cd0f403d34f2bc83eb09e7a5bc56109 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Tue, 6 Nov 2018 18:05:40 -0500 Subject: Amend CVE note and security guide section wordings Reword first sentence of dep management and CVE section of security guide. Also, reword and move gemspec notes above deps. [ci skip] --- actionmailer/actionmailer.gemspec | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionmailer') diff --git a/actionmailer/actionmailer.gemspec b/actionmailer/actionmailer.gemspec index efcdcf019a..6f8d6d0699 100644 --- a/actionmailer/actionmailer.gemspec +++ b/actionmailer/actionmailer.gemspec @@ -2,9 +2,6 @@ version = File.read(File.expand_path("../RAILS_VERSION", __dir__)).strip -# NOTE: There's no need to update dependencies for CVEs in minor -# releases when users can simply run `bundle update vulnerable_gem`. - Gem::Specification.new do |s| s.platform = Gem::Platform::RUBY s.name = "actionmailer" @@ -29,6 +26,9 @@ Gem::Specification.new do |s| "changelog_uri" => "https://github.com/rails/rails/blob/v#{version}/actionmailer/CHANGELOG.md" } + # NOTE: Please read our dependency guidelines before updating versions: + # https://edgeguides.rubyonrails.org/security.html#dependency-management-and-cves + s.add_dependency "actionpack", version s.add_dependency "actionview", version s.add_dependency "activejob", version -- cgit v1.2.3 From e139a3ce1376e1b74e0996f6f0ae17e584d5acad Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Wed, 21 Nov 2018 23:10:18 +0100 Subject: Fix ActionMailer assertion not working for mail defining delivery_job: - If a Mail defines a custom delivery_job, all ActionMailer assertion helper (assert_emails, assert_enqueued_emails ...) wouldn't work. ```ruby MyMailer < ApplicationMailer self.delivery_job = MyJob end # This assertion will fail assert_emails(1) do MyMailer.my_mail.deliver_later end This PR leverage the new ActiveJob feature that accepts Procs for the `only` keyword and check if the delivery job is one of ActionMailer registered ones. --- actionmailer/CHANGELOG.md | 5 +++ actionmailer/lib/action_mailer/test_helper.rb | 19 +++++++--- actionmailer/test/test_helper_test.rb | 51 +++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 5 deletions(-) (limited to 'actionmailer') diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 35574a2c73..ec6f249974 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix ActionMailer assertions not working when a Mail defines + a custom delivery job class + + *Edouard Chin* + * Mails with multipart `format` blocks with implicit render now also check for a template name in options hash instead of only using the action name. diff --git a/actionmailer/lib/action_mailer/test_helper.rb b/actionmailer/lib/action_mailer/test_helper.rb index a4751916af..b3403e8331 100644 --- a/actionmailer/lib/action_mailer/test_helper.rb +++ b/actionmailer/lib/action_mailer/test_helper.rb @@ -34,7 +34,7 @@ module ActionMailer def assert_emails(number, &block) if block_given? original_count = ActionMailer::Base.deliveries.size - perform_enqueued_jobs(only: [ActionMailer::DeliveryJob, ActionMailer::Parameterized::DeliveryJob], &block) + perform_enqueued_jobs(only: ->(job) { delivery_job_filter(job) }, &block) new_count = ActionMailer::Base.deliveries.size assert_equal number, new_count - original_count, "#{number} emails expected, but #{new_count - original_count} were sent" else @@ -90,7 +90,7 @@ module ActionMailer # end # end def assert_enqueued_emails(number, &block) - assert_enqueued_jobs number, only: [ ActionMailer::DeliveryJob, ActionMailer::Parameterized::DeliveryJob ], &block + assert_enqueued_jobs(number, only: ->(job) { delivery_job_filter(job) }, &block) end # Asserts that a specific email has been enqueued, optionally @@ -125,10 +125,10 @@ module ActionMailer # end def assert_enqueued_email_with(mailer, method, args: nil, queue: "mailers", &block) if args.is_a? Hash - job = ActionMailer::Parameterized::DeliveryJob + job = mailer.parameterized_delivery_job args = [mailer.to_s, method.to_s, "deliver_now", args] else - job = ActionMailer::DeliveryJob + job = mailer.delivery_job args = [mailer.to_s, method.to_s, "deliver_now", *args] end @@ -151,7 +151,16 @@ module ActionMailer # end # end def assert_no_enqueued_emails(&block) - assert_no_enqueued_jobs only: [ ActionMailer::DeliveryJob, ActionMailer::Parameterized::DeliveryJob ], &block + assert_enqueued_emails 0, &block end + + private + + def delivery_job_filter(job) + job_class = job.is_a?(Hash) ? job.fetch(:job) : job.class + + Base.descendants.map(&:delivery_job).include?(job_class) || + Base.descendants.map(&:parameterized_delivery_job).include?(job_class) + end end end diff --git a/actionmailer/test/test_helper_test.rb b/actionmailer/test/test_helper_test.rb index d31170706b..c700db9408 100644 --- a/actionmailer/test/test_helper_test.rb +++ b/actionmailer/test/test_helper_test.rb @@ -24,6 +24,17 @@ class TestHelperMailer < ActionMailer::Base end end +class CustomDeliveryJob < ActionMailer::DeliveryJob +end + +class CustomParameterizedDeliveryJob < ActionMailer::Parameterized::DeliveryJob +end + +class CustomDeliveryMailer < TestHelperMailer + self.delivery_job = CustomDeliveryJob + self.parameterized_delivery_job = CustomParameterizedDeliveryJob +end + class TestHelperMailerTest < ActionMailer::TestCase include ActiveSupport::Testing::Stream @@ -69,6 +80,26 @@ class TestHelperMailerTest < ActionMailer::TestCase end end + def test_assert_emails_with_custom_delivery_job + assert_nothing_raised do + assert_emails(1) do + silence_stream($stdout) do + CustomDeliveryMailer.test.deliver_later + end + end + end + end + + def test_assert_emails_with_custom_parameterized_delivery_job + assert_nothing_raised do + assert_emails(1) do + silence_stream($stdout) do + CustomDeliveryMailer.with(foo: "bar").test_parameter_args.deliver_later + end + end + end + end + def test_assert_emails_with_enqueued_emails assert_nothing_raised do assert_emails 1 do @@ -201,6 +232,16 @@ class TestHelperMailerTest < ActionMailer::TestCase assert_match(/2 .* but 1/, error.message) end + def test_assert_enqueued_emails_with_custom_delivery_job + assert_nothing_raised do + assert_enqueued_emails(1) do + silence_stream($stdout) do + CustomDeliveryMailer.test.deliver_later + end + end + end + end + def test_assert_enqueued_emails_too_many_sent error = assert_raise ActiveSupport::TestCase::Assertion do assert_enqueued_emails 1 do @@ -252,6 +293,16 @@ class TestHelperMailerTest < ActionMailer::TestCase end end + def test_assert_enqueued_email_with_when_mailer_has_custom_delivery_job + assert_nothing_raised do + assert_enqueued_email_with CustomDeliveryMailer, :test do + silence_stream($stdout) do + CustomDeliveryMailer.test.deliver_later + end + end + end + end + def test_assert_enqueued_email_with_with_no_block assert_nothing_raised do silence_stream($stdout) do -- cgit v1.2.3 From 60339da5bcb76489576321fa12e665f176d8d692 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Thu, 1 Nov 2018 16:32:01 -0400 Subject: Deliver parameterized mail with DeliveryJob Deliver parameterized mail with `ActionMailer::DeliveryJob` and remove `ActionMailer::Parameterized::DeliveryJob`. --- actionmailer/CHANGELOG.md | 15 +++------ actionmailer/lib/action_mailer/base.rb | 1 - actionmailer/lib/action_mailer/delivery_job.rb | 5 +-- actionmailer/lib/action_mailer/message_delivery.rb | 2 +- actionmailer/lib/action_mailer/parameterized.rb | 8 +---- actionmailer/lib/action_mailer/test_helper.rb | 14 +++----- actionmailer/test/message_delivery_test.rb | 14 ++++---- actionmailer/test/parameterized_test.rb | 38 +++++++++++++++++----- actionmailer/test/test_helper_test.rb | 4 --- 9 files changed, 50 insertions(+), 51 deletions(-) (limited to 'actionmailer') diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index ec6f249974..9d2a1928c5 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,3 +1,7 @@ +* Deliver parameterized mail with `ActionMailer::DeliveryJob` and remove `ActionMailer::Parameterized::DeliveryJob`. + + *Gannon McGibbon* + * Fix ActionMailer assertions not working when a Mail defines a custom delivery job class @@ -8,17 +12,6 @@ *Marcus Ilgner* -* Allow ActionMailer classes to configure the parameterized delivery job - Example: - ``` - class MyMailer < ApplicationMailer - self.parameterized_delivery_job = MyCustomDeliveryJob - ... - end - ``` - - *Luke Pearce* - * `ActionDispatch::IntegrationTest` includes `ActionMailer::TestHelper` module by default. *Ricardo Díaz* diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 2bb1078bb8..5d41e5ff2a 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -462,7 +462,6 @@ module ActionMailer helper ActionMailer::MailHelper class_attribute :delivery_job, default: ::ActionMailer::DeliveryJob - class_attribute :parameterized_delivery_job, default: ::ActionMailer::Parameterized::DeliveryJob class_attribute :default_params, default: { mime_version: "1.0", charset: "UTF-8", diff --git a/actionmailer/lib/action_mailer/delivery_job.rb b/actionmailer/lib/action_mailer/delivery_job.rb index 40f26d8ad1..8af2097819 100644 --- a/actionmailer/lib/action_mailer/delivery_job.rb +++ b/actionmailer/lib/action_mailer/delivery_job.rb @@ -12,8 +12,9 @@ module ActionMailer rescue_from StandardError, with: :handle_exception_with_mailer_class - def perform(mailer, mail_method, delivery_method, *args) #:nodoc: - mailer.constantize.public_send(mail_method, *args).send(delivery_method) + def perform(mailer, mail_method, delivery_method, params, *args) #:nodoc: + mailer_class = params ? mailer.constantize.with(params) : mailer.constantize + mailer_class.public_send(mail_method, *args).send(delivery_method) end private diff --git a/actionmailer/lib/action_mailer/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb index a57980c322..6b02076422 100644 --- a/actionmailer/lib/action_mailer/message_delivery.rb +++ b/actionmailer/lib/action_mailer/message_delivery.rb @@ -135,7 +135,7 @@ module ActionMailer "#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_class.name, @action.to_s, delivery_method.to_s, *@args + args = @mailer_class.name, @action.to_s, delivery_method.to_s, nil, *@args job = @mailer_class.delivery_job job.set(options).perform_later(*args) end diff --git a/actionmailer/lib/action_mailer/parameterized.rb b/actionmailer/lib/action_mailer/parameterized.rb index 0fe417affe..0b0afd6ad4 100644 --- a/actionmailer/lib/action_mailer/parameterized.rb +++ b/actionmailer/lib/action_mailer/parameterized.rb @@ -140,16 +140,10 @@ module ActionMailer super else args = @mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args - job = @mailer_class.parameterized_delivery_job + job = @mailer_class.delivery_job job.set(options).perform_later(*args) end end end - - class DeliveryJob < ActionMailer::DeliveryJob # :nodoc: - def perform(mailer, mail_method, delivery_method, params, *args) - mailer.constantize.with(params).public_send(mail_method, *args).send(delivery_method) - end - end end end diff --git a/actionmailer/lib/action_mailer/test_helper.rb b/actionmailer/lib/action_mailer/test_helper.rb index b3403e8331..3eb61c2363 100644 --- a/actionmailer/lib/action_mailer/test_helper.rb +++ b/actionmailer/lib/action_mailer/test_helper.rb @@ -124,15 +124,12 @@ module ActionMailer # end # end def assert_enqueued_email_with(mailer, method, args: nil, queue: "mailers", &block) - if args.is_a? Hash - job = mailer.parameterized_delivery_job - args = [mailer.to_s, method.to_s, "deliver_now", args] + args = if args.is_a?(Hash) + [mailer.to_s, method.to_s, "deliver_now", args] else - job = mailer.delivery_job - args = [mailer.to_s, method.to_s, "deliver_now", *args] + [mailer.to_s, method.to_s, "deliver_now", nil, *args] end - - assert_enqueued_with(job: job, args: args, queue: queue, &block) + assert_enqueued_with(job: mailer.delivery_job, args: args, queue: queue, &block) end # Asserts that no emails are enqueued for later delivery. @@ -159,8 +156,7 @@ module ActionMailer def delivery_job_filter(job) job_class = job.is_a?(Hash) ? job.fetch(:job) : job.class - Base.descendants.map(&:delivery_job).include?(job_class) || - Base.descendants.map(&:parameterized_delivery_job).include?(job_class) + Base.descendants.map(&:delivery_job).include?(job_class) end end end diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb index f8dcb3f4ba..8d5c473faf 100644 --- a/actionmailer/test/message_delivery_test.rb +++ b/actionmailer/test/message_delivery_test.rb @@ -64,20 +64,20 @@ class MessageDeliveryTest < ActiveSupport::TestCase end test "should enqueue the email with :deliver_now delivery method" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do + assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do @mail.deliver_later end end test "should enqueue the email with :deliver_now! delivery method" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now!", 1, 2, 3]) do + assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now!", nil, 1, 2, 3]) do @mail.deliver_later! end end test "should enqueue a delivery with a delay" do travel_to Time.new(2004, 11, 24, 01, 04, 44) do - assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current + 10.minutes, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do + assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current + 10.minutes, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do @mail.deliver_later wait: 10.minutes end end @@ -85,13 +85,13 @@ class MessageDeliveryTest < ActiveSupport::TestCase test "should enqueue a delivery at a specific time" do later_time = Time.current + 1.hour - assert_performed_with(job: ActionMailer::DeliveryJob, at: later_time, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do + assert_performed_with(job: ActionMailer::DeliveryJob, at: later_time, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do @mail.deliver_later wait_until: later_time end end test "should enqueue the job on the correct queue" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3], queue: "test_queue") do + assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3], queue: "test_queue") do @mail.deliver_later end end @@ -100,7 +100,7 @@ class MessageDeliveryTest < ActiveSupport::TestCase old_delivery_job = DelayedMailer.delivery_job DelayedMailer.delivery_job = DummyJob - assert_performed_with(job: DummyJob, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3]) do + assert_performed_with(job: DummyJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do @mail.deliver_later end @@ -110,7 +110,7 @@ class MessageDeliveryTest < ActiveSupport::TestCase class DummyJob < ActionMailer::DeliveryJob; end test "can override the queue when enqueuing mail" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", 1, 2, 3], queue: "another_queue") do + assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3], queue: "another_queue") do @mail.deliver_later(queue: :another_queue) end end diff --git a/actionmailer/test/parameterized_test.rb b/actionmailer/test/parameterized_test.rb index d3b34fc3e3..3713dc3e3e 100644 --- a/actionmailer/test/parameterized_test.rb +++ b/actionmailer/test/parameterized_test.rb @@ -7,6 +7,9 @@ require "mailers/params_mailer" class ParameterizedTest < ActiveSupport::TestCase include ActiveJob::TestHelper + class DummyDeliveryJob < ActionMailer::DeliveryJob + end + setup do @previous_logger = ActiveJob::Base.logger ActiveJob::Base.logger = Logger.new(nil) @@ -35,7 +38,13 @@ class ParameterizedTest < ActiveSupport::TestCase end test "enqueue the email with params" do - assert_performed_with(job: ActionMailer::Parameterized::DeliveryJob, args: ["ParamsMailer", "invitation", "deliver_now", { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" } ]) do + args = [ + "ParamsMailer", + "invitation", + "deliver_now", + { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" }, + ] + assert_performed_with(job: ActionMailer::DeliveryJob, args: args) do @mail.deliver_later end end @@ -55,15 +64,26 @@ class ParameterizedTest < ActiveSupport::TestCase end test "should enqueue a parameterized request with the correct delivery job" do - old_delivery_job = ParamsMailer.parameterized_delivery_job - ParamsMailer.parameterized_delivery_job = ParameterizedDummyJob - - assert_performed_with(job: ParameterizedDummyJob, args: ["ParamsMailer", "invitation", "deliver_now", { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" } ]) do - @mail.deliver_later + args = [ + "ParamsMailer", + "invitation", + "deliver_now", + { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" }, + ] + + with_delivery_job DummyDeliveryJob do + assert_performed_with(job: DummyDeliveryJob, args: args) do + @mail.deliver_later + end end - - ParamsMailer.parameterized_delivery_job = old_delivery_job end - class ParameterizedDummyJob < ActionMailer::Parameterized::DeliveryJob; end + private + + def with_delivery_job(job) + old_delivery_job = ParamsMailer.delivery_job + ParamsMailer.delivery_job = job + ensure + ParamsMailer.delivery_job = old_delivery_job + end end diff --git a/actionmailer/test/test_helper_test.rb b/actionmailer/test/test_helper_test.rb index c700db9408..4e21ab0a69 100644 --- a/actionmailer/test/test_helper_test.rb +++ b/actionmailer/test/test_helper_test.rb @@ -27,12 +27,8 @@ end class CustomDeliveryJob < ActionMailer::DeliveryJob end -class CustomParameterizedDeliveryJob < ActionMailer::Parameterized::DeliveryJob -end - class CustomDeliveryMailer < TestHelperMailer self.delivery_job = CustomDeliveryJob - self.parameterized_delivery_job = CustomParameterizedDeliveryJob end class TestHelperMailerTest < ActionMailer::TestCase -- cgit v1.2.3 From ebf484a64018cbf84e6d6c39f4a22f2f2bfc9d51 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Fri, 30 Nov 2018 17:37:00 -0500 Subject: Add yield to with_delivery_job test helper Adds yield to parameterized mail test helper so assertions passed into with_delivery_job are actually ran. --- actionmailer/test/parameterized_test.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionmailer') diff --git a/actionmailer/test/parameterized_test.rb b/actionmailer/test/parameterized_test.rb index 3713dc3e3e..3369808515 100644 --- a/actionmailer/test/parameterized_test.rb +++ b/actionmailer/test/parameterized_test.rb @@ -83,6 +83,7 @@ class ParameterizedTest < ActiveSupport::TestCase def with_delivery_job(job) old_delivery_job = ParamsMailer.delivery_job ParamsMailer.delivery_job = job + yield ensure ParamsMailer.delivery_job = old_delivery_job end -- cgit v1.2.3 From f5050d998def98563f8fa4b381c09f563681f159 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Fri, 30 Nov 2018 17:33:35 -0500 Subject: Add MailDeliveryJob for unified mail delivery Add `MailDeliveryJob` for delivering both regular and parameterized mail. Deprecate using `DeliveryJob` and `Parameterized::DeliveryJob`. --- actionmailer/CHANGELOG.md | 2 +- actionmailer/lib/action_mailer.rb | 1 + actionmailer/lib/action_mailer/base.rb | 2 +- actionmailer/lib/action_mailer/delivery_job.rb | 13 +++- .../lib/action_mailer/mail_delivery_job.rb | 38 ++++++++++ actionmailer/lib/action_mailer/message_delivery.rb | 10 ++- actionmailer/lib/action_mailer/parameterized.rb | 18 ++++- actionmailer/lib/action_mailer/test_helper.rb | 4 +- actionmailer/test/legacy_delivery_job_test.rb | 86 ++++++++++++++++++++++ actionmailer/test/message_delivery_test.rb | 16 ++-- actionmailer/test/parameterized_test.rb | 10 ++- actionmailer/test/test_helper_test.rb | 2 +- 12 files changed, 179 insertions(+), 23 deletions(-) create mode 100644 actionmailer/lib/action_mailer/mail_delivery_job.rb create mode 100644 actionmailer/test/legacy_delivery_job_test.rb (limited to 'actionmailer') diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 9d2a1928c5..2de83e611b 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,4 +1,4 @@ -* Deliver parameterized mail with `ActionMailer::DeliveryJob` and remove `ActionMailer::Parameterized::DeliveryJob`. +* Add `MailDeliveryJob` for delivering both regular and parameterized mail. Deprecate using `DeliveryJob` and `Parameterized::DeliveryJob`. *Gannon McGibbon* diff --git a/actionmailer/lib/action_mailer.rb b/actionmailer/lib/action_mailer.rb index 69eae65d60..48f16180be 100644 --- a/actionmailer/lib/action_mailer.rb +++ b/actionmailer/lib/action_mailer.rb @@ -52,6 +52,7 @@ module ActionMailer autoload :TestHelper autoload :MessageDelivery autoload :DeliveryJob + autoload :MailDeliveryJob def self.eager_load! super diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 5d41e5ff2a..8ddc90b9df 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -461,7 +461,7 @@ module ActionMailer helper ActionMailer::MailHelper - class_attribute :delivery_job, default: ::ActionMailer::DeliveryJob + class_attribute :delivery_job, default: ::ActionMailer::MailDeliveryJob class_attribute :default_params, default: { mime_version: "1.0", charset: "UTF-8", diff --git a/actionmailer/lib/action_mailer/delivery_job.rb b/actionmailer/lib/action_mailer/delivery_job.rb index 8af2097819..f228006920 100644 --- a/actionmailer/lib/action_mailer/delivery_job.rb +++ b/actionmailer/lib/action_mailer/delivery_job.rb @@ -12,9 +12,16 @@ module ActionMailer rescue_from StandardError, with: :handle_exception_with_mailer_class - def perform(mailer, mail_method, delivery_method, params, *args) #:nodoc: - mailer_class = params ? mailer.constantize.with(params) : mailer.constantize - mailer_class.public_send(mail_method, *args).send(delivery_method) + before_perform do + ActiveSupport::Deprecation.warn <<~MSG.squish + Sending mail with DeliveryJob and Parameterized::DeliveryJob + is deprecated and will be removed in Rails 6.1. + Please use MailDeliveryJob instead. + MSG + end + + def perform(mailer, mail_method, delivery_method, *args) #:nodoc: + mailer.constantize.public_send(mail_method, *args).send(delivery_method) end private diff --git a/actionmailer/lib/action_mailer/mail_delivery_job.rb b/actionmailer/lib/action_mailer/mail_delivery_job.rb new file mode 100644 index 0000000000..93778edfce --- /dev/null +++ b/actionmailer/lib/action_mailer/mail_delivery_job.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "active_job" + +module ActionMailer + # The ActionMailer::NewDeliveryJob class is used when you + # want to send emails outside of the request-response cycle. It supports + # sending either parameterized or normal mail. + # + # Exceptions are rescued and handled by the mailer class. + class MailDeliveryJob < ActiveJob::Base # :nodoc: + queue_as { ActionMailer::Base.deliver_later_queue_name } + + rescue_from StandardError, with: :handle_exception_with_mailer_class + + def perform(mailer, mail_method, delivery_method, args:, params: nil) #:nodoc: + mailer_class = params ? mailer.constantize.with(params) : mailer.constantize + mailer_class.public_send(mail_method, *args).send(delivery_method) + end + + private + # "Deserialize" the mailer class name by hand in case another argument + # (like a Global ID reference) raised DeserializationError. + def mailer_class + if mailer = Array(@serialized_arguments).first || Array(arguments).first + mailer.constantize + end + end + + def handle_exception_with_mailer_class(exception) + if klass = mailer_class + klass.handle_exception exception + else + raise exception + end + end + end +end diff --git a/actionmailer/lib/action_mailer/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb index 6b02076422..1e5cab6d47 100644 --- a/actionmailer/lib/action_mailer/message_delivery.rb +++ b/actionmailer/lib/action_mailer/message_delivery.rb @@ -135,10 +135,18 @@ module ActionMailer "#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_class.name, @action.to_s, delivery_method.to_s, nil, *@args job = @mailer_class.delivery_job + args = arguments_for(job, delivery_method) job.set(options).perform_later(*args) end end + + def arguments_for(delivery_job, delivery_method) + if delivery_job <= MailDeliveryJob + [@mailer_class.name, @action.to_s, delivery_method.to_s, args: @args] + else + [@mailer_class.name, @action.to_s, delivery_method.to_s, *@args] + end + end end end diff --git a/actionmailer/lib/action_mailer/parameterized.rb b/actionmailer/lib/action_mailer/parameterized.rb index 0b0afd6ad4..999435919e 100644 --- a/actionmailer/lib/action_mailer/parameterized.rb +++ b/actionmailer/lib/action_mailer/parameterized.rb @@ -121,6 +121,12 @@ module ActionMailer end end + class DeliveryJob < ActionMailer::DeliveryJob # :nodoc: + def perform(mailer, mail_method, delivery_method, params, *args) + mailer.constantize.with(params).public_send(mail_method, *args).send(delivery_method) + end + end + class MessageDelivery < ActionMailer::MessageDelivery # :nodoc: def initialize(mailer_class, action, params, *args) super(mailer_class, action, *args) @@ -139,11 +145,19 @@ module ActionMailer if processed? super else - args = @mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args - job = @mailer_class.delivery_job + job = @mailer_class.delivery_job + args = arguments_for(job, delivery_method) job.set(options).perform_later(*args) end end + + def arguments_for(delivery_job, delivery_method) + if delivery_job <= MailDeliveryJob + [@mailer_class.name, @action.to_s, delivery_method.to_s, params: @params, args: @args] + else + [@mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args] + end + end end end end diff --git a/actionmailer/lib/action_mailer/test_helper.rb b/actionmailer/lib/action_mailer/test_helper.rb index 3eb61c2363..e222301dff 100644 --- a/actionmailer/lib/action_mailer/test_helper.rb +++ b/actionmailer/lib/action_mailer/test_helper.rb @@ -125,9 +125,9 @@ module ActionMailer # end def assert_enqueued_email_with(mailer, method, args: nil, queue: "mailers", &block) args = if args.is_a?(Hash) - [mailer.to_s, method.to_s, "deliver_now", args] + [mailer.to_s, method.to_s, "deliver_now", params: args, args: []] else - [mailer.to_s, method.to_s, "deliver_now", nil, *args] + [mailer.to_s, method.to_s, "deliver_now", args: Array(args)] end assert_enqueued_with(job: mailer.delivery_job, args: args, queue: queue, &block) end diff --git a/actionmailer/test/legacy_delivery_job_test.rb b/actionmailer/test/legacy_delivery_job_test.rb new file mode 100644 index 0000000000..112c842beb --- /dev/null +++ b/actionmailer/test/legacy_delivery_job_test.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "active_job" +require "mailers/params_mailer" +require "mailers/delayed_mailer" + +class LegacyDeliveryJobTest < ActiveSupport::TestCase + include ActiveJob::TestHelper + + class LegacyDeliveryJob < ActionMailer::DeliveryJob + end + + class LegacyParmeterizedDeliveryJob < ActionMailer::Parameterized::DeliveryJob + end + + setup do + @previous_logger = ActiveJob::Base.logger + ActiveJob::Base.logger = Logger.new(nil) + + @previous_delivery_method = ActionMailer::Base.delivery_method + ActionMailer::Base.delivery_method = :test + + @previous_deliver_later_queue_name = ActionMailer::Base.deliver_later_queue_name + ActionMailer::Base.deliver_later_queue_name = :test_queue + end + + teardown do + ActiveJob::Base.logger = @previous_logger + ParamsMailer.deliveries.clear + + ActionMailer::Base.delivery_method = @previous_delivery_method + ActionMailer::Base.deliver_later_queue_name = @previous_deliver_later_queue_name + end + + test "should send parameterized mail correctly" do + mail = ParamsMailer.with(inviter: "david@basecamp.com", invitee: "jason@basecamp.com").invitation + args = [ + "ParamsMailer", + "invitation", + "deliver_now", + { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" }, + ] + + with_delivery_job(LegacyParmeterizedDeliveryJob) do + assert_deprecated do + assert_performed_with(job: LegacyParmeterizedDeliveryJob, args: args) do + mail.deliver_later + end + end + end + end + + test "should send mail correctly" do + mail = DelayedMailer.test_message(1, 2, 3) + args = [ + "DelayedMailer", + "test_message", + "deliver_now", + 1, + 2, + 3, + ] + + with_delivery_job(LegacyDeliveryJob) do + assert_deprecated do + assert_performed_with(job: LegacyDeliveryJob, args: args) do + mail.deliver_later + end + end + end + end + + private + + def with_delivery_job(job) + old_params_delivery_job = ParamsMailer.delivery_job + old_regular_delivery_job = DelayedMailer.delivery_job + ParamsMailer.delivery_job = job + DelayedMailer.delivery_job = job + yield + ensure + ParamsMailer.delivery_job = old_params_delivery_job + DelayedMailer.delivery_job = old_regular_delivery_job + end +end diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb index 8d5c473faf..46260f6414 100644 --- a/actionmailer/test/message_delivery_test.rb +++ b/actionmailer/test/message_delivery_test.rb @@ -64,20 +64,20 @@ class MessageDeliveryTest < ActiveSupport::TestCase end test "should enqueue the email with :deliver_now delivery method" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do + assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do @mail.deliver_later end end test "should enqueue the email with :deliver_now! delivery method" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now!", nil, 1, 2, 3]) do + assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now!", args: [1, 2, 3]]) do @mail.deliver_later! end end test "should enqueue a delivery with a delay" do travel_to Time.new(2004, 11, 24, 01, 04, 44) do - assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current + 10.minutes, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do + assert_performed_with(job: ActionMailer::MailDeliveryJob, at: Time.current + 10.minutes, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do @mail.deliver_later wait: 10.minutes end end @@ -85,13 +85,13 @@ class MessageDeliveryTest < ActiveSupport::TestCase test "should enqueue a delivery at a specific time" do later_time = Time.current + 1.hour - assert_performed_with(job: ActionMailer::DeliveryJob, at: later_time, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do + assert_performed_with(job: ActionMailer::MailDeliveryJob, at: later_time, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do @mail.deliver_later wait_until: later_time end end test "should enqueue the job on the correct queue" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3], queue: "test_queue") do + assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]], queue: "test_queue") do @mail.deliver_later end end @@ -100,17 +100,17 @@ class MessageDeliveryTest < ActiveSupport::TestCase old_delivery_job = DelayedMailer.delivery_job DelayedMailer.delivery_job = DummyJob - assert_performed_with(job: DummyJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do + assert_performed_with(job: DummyJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do @mail.deliver_later end DelayedMailer.delivery_job = old_delivery_job end - class DummyJob < ActionMailer::DeliveryJob; end + class DummyJob < ActionMailer::MailDeliveryJob; end test "can override the queue when enqueuing mail" do - assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3], queue: "another_queue") do + assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]], queue: "another_queue") do @mail.deliver_later(queue: :another_queue) end end diff --git a/actionmailer/test/parameterized_test.rb b/actionmailer/test/parameterized_test.rb index 3369808515..7dd6156744 100644 --- a/actionmailer/test/parameterized_test.rb +++ b/actionmailer/test/parameterized_test.rb @@ -7,7 +7,7 @@ require "mailers/params_mailer" class ParameterizedTest < ActiveSupport::TestCase include ActiveJob::TestHelper - class DummyDeliveryJob < ActionMailer::DeliveryJob + class DummyDeliveryJob < ActionMailer::MailDeliveryJob end setup do @@ -42,9 +42,10 @@ class ParameterizedTest < ActiveSupport::TestCase "ParamsMailer", "invitation", "deliver_now", - { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" }, + params: { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" }, + args: [], ] - assert_performed_with(job: ActionMailer::DeliveryJob, args: args) do + assert_performed_with(job: ActionMailer::MailDeliveryJob, args: args) do @mail.deliver_later end end @@ -68,7 +69,8 @@ class ParameterizedTest < ActiveSupport::TestCase "ParamsMailer", "invitation", "deliver_now", - { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" }, + params: { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" }, + args: [], ] with_delivery_job DummyDeliveryJob do diff --git a/actionmailer/test/test_helper_test.rb b/actionmailer/test/test_helper_test.rb index 4e21ab0a69..60e2389aa8 100644 --- a/actionmailer/test/test_helper_test.rb +++ b/actionmailer/test/test_helper_test.rb @@ -24,7 +24,7 @@ class TestHelperMailer < ActionMailer::Base end end -class CustomDeliveryJob < ActionMailer::DeliveryJob +class CustomDeliveryJob < ActionMailer::MailDeliveryJob end class CustomDeliveryMailer < TestHelperMailer -- cgit v1.2.3