From 7660286379d7232c8bf38ea44560b058bf9df125 Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Tue, 22 Sep 2020 18:25:14 +0900 Subject: [PATCH 1/4] Make .delay method work with ActionMailer::Parameterized::Mailer This PR unifies the DelayedJob behavior of ActionMailer::Base and ActionMailer::Parameterized::Mailer Now, the following syntax works equivalently: ```ruby # works currently MyMailer.delay.my_method # this PR makes the following work MyMailer.with(foo: 1, bar: 2).delay.my_method ``` Note that ActionMailer::Parameterized::Mailer does not inherit ActionMailer::Base (moreover, the `.with()` method returns an object instance, hence we use `include` rather than `extend`) --- lib/delayed_job.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/delayed_job.rb b/lib/delayed_job.rb index 511bf7b88..5146ed215 100644 --- a/lib/delayed_job.rb +++ b/lib/delayed_job.rb @@ -16,6 +16,7 @@ ActiveSupport.on_load(:action_mailer) do require 'delayed/performable_mailer' ActionMailer::Base.extend(Delayed::DelayMail) + ActionMailer::Parameterized::Mailer.include(Delayed::DelayMail) end module Delayed From fc0e9e219225e40cd8f78bc4d79f859e62c26c8c Mon Sep 17 00:00:00 2001 From: shields Date: Tue, 22 Sep 2020 18:38:38 +0900 Subject: [PATCH 2/4] Add specs --- lib/delayed_job.rb | 2 +- spec/performable_mailer_spec.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/delayed_job.rb b/lib/delayed_job.rb index 5146ed215..d38f2edbc 100644 --- a/lib/delayed_job.rb +++ b/lib/delayed_job.rb @@ -16,7 +16,7 @@ ActiveSupport.on_load(:action_mailer) do require 'delayed/performable_mailer' ActionMailer::Base.extend(Delayed::DelayMail) - ActionMailer::Parameterized::Mailer.include(Delayed::DelayMail) + ActionMailer::Parameterized::Mailer.include(Delayed::DelayMail) if defined?(ActionMailer::Parameterized::Mailer) end module Delayed diff --git a/spec/performable_mailer_spec.rb b/spec/performable_mailer_spec.rb index 520d96596..0cc9a4b6e 100644 --- a/spec/performable_mailer_spec.rb +++ b/spec/performable_mailer_spec.rb @@ -40,3 +40,29 @@ def signup(email) end end end + +if defined?(ActionMailer::Parameterized::Mailer) + describe ActionMailer::Parameterized::Mailer do + describe 'delay' do + it 'enqueues a PerformableEmail job' do + expect do + job = MyMailer.with(foo: 1, bar: 2).delay.signup('john@example.com') + expect(job.payload_object.class).to eq(Delayed::PerformableMailer) + expect(job.payload_object.object.class).to eq(ActionMailer::Parameterized::Mailer) + expect(job.payload_object.object.instance_variable_get('@mailer')).to eq(MyMailer) + expect(job.payload_object.object.instance_variable_get('@params')).to eq(foo: 1, bar: 2) + expect(job.payload_object.method_name).to eq(:signup) + expect(job.payload_object.args).to eq(['john@example.com']) + end.to change { Delayed::Job.count }.by(1) + end + end + + describe 'delay on a mail object' do + it 'raises an exception' do + expect do + MyMailer.with(foo: 1, bar: 2).signup('john@example.com').delay + end.to raise_error(RuntimeError) + end + end + end +end From 4878fcd255ed11ea6ea81b80fe9c85d716721bb7 Mon Sep 17 00:00:00 2001 From: shields Date: Tue, 22 Sep 2020 19:10:27 +0900 Subject: [PATCH 3/4] Add to and improve README for Mailers --- README.md | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 3bf63336e..37998eb05 100644 --- a/README.md +++ b/README.md @@ -168,9 +168,10 @@ end If you ever want to call a `handle_asynchronously`'d method without Delayed Job, for instance while debugging something at the console, just add `_without_delay` to the method name. For instance, if your original method was `foo`, then call `foo_without_delay`. -Rails 3 Mailers -=============== -Due to how mailers are implemented in Rails 3, we had to do a little work around to get delayed_job to work. +Rails Mailers +============= +Delayed Job uses special syntax for Rails Mailers. +Do not call the `.deliver` method when using `.delay`. ```ruby # without delayed_job @@ -179,12 +180,16 @@ Notifier.signup(@user).deliver # with delayed_job Notifier.delay.signup(@user) -# with delayed_job running at a specific time +# delayed_job running at a specific time Notifier.delay(run_at: 5.minutes.from_now).signup(@user) + +# when using parameters, the .with method must be called before the .delay method +Notifier.with(foo: 1, bar: 2).delay.signup(@user) ``` -Remove the `.deliver` method to make it work. It's not ideal, but it's the best -we could do for now. +You may also wish to consider using +[Active Job with Action Mailer](https://edgeguides.rubyonrails.org/active_job_basics.html#action-mailer) +which provides convenient `.deliver_later` syntax that forwards to Delayed Job under-the-hood. Named Queues ============ From 75bec31fb407b7444e943ad40312f464754af5df Mon Sep 17 00:00:00 2001 From: shields Date: Tue, 22 Sep 2020 19:28:34 +0900 Subject: [PATCH 4/4] Fix rubocop --- spec/performable_mailer_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/performable_mailer_spec.rb b/spec/performable_mailer_spec.rb index 0cc9a4b6e..39796eea2 100644 --- a/spec/performable_mailer_spec.rb +++ b/spec/performable_mailer_spec.rb @@ -46,11 +46,11 @@ def signup(email) describe 'delay' do it 'enqueues a PerformableEmail job' do expect do - job = MyMailer.with(foo: 1, bar: 2).delay.signup('john@example.com') + job = MyMailer.with(:foo => 1, :bar => 2).delay.signup('john@example.com') expect(job.payload_object.class).to eq(Delayed::PerformableMailer) expect(job.payload_object.object.class).to eq(ActionMailer::Parameterized::Mailer) expect(job.payload_object.object.instance_variable_get('@mailer')).to eq(MyMailer) - expect(job.payload_object.object.instance_variable_get('@params')).to eq(foo: 1, bar: 2) + expect(job.payload_object.object.instance_variable_get('@params')).to eq(:foo => 1, :bar => 2) expect(job.payload_object.method_name).to eq(:signup) expect(job.payload_object.args).to eq(['john@example.com']) end.to change { Delayed::Job.count }.by(1) @@ -60,7 +60,7 @@ def signup(email) describe 'delay on a mail object' do it 'raises an exception' do expect do - MyMailer.with(foo: 1, bar: 2).signup('john@example.com').delay + MyMailer.with(:foo => 1, :bar => 2).signup('john@example.com').delay end.to raise_error(RuntimeError) end end