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