refactor(mail.delay): use simplier implementation using after_action to prevent email delivery with delay

app/jobs/notify_new_answer_with_delay_job.rb

@@ -1,10 +0,0 @@
-class NotifyNewAnswerWithDelayJob < ApplicationJob
-  queue_as :mailers # use default queue for email, same priority
-
-  discard_on ActiveRecord::RecordNotFound
-
-  def perform(dossier, body, commentaire)
-    return if commentaire.discarded?
-    DossierMailer.notify_new_answer(dossier, body).deliver_now
-  end
-end

app/mailers/dossier_mailer.rb

@@ -6,6 +6,7 @@ class DossierMailer < ApplicationMailer
 
   layout 'mailers/layout'
   default from: NO_REPLY_EMAIL
+  after_action :prevent_perform_deliveries, only: [:notify_new_answer]
 
   def notify_new_draft(dossier)
     I18n.with_locale(dossier.user_locale) do
@@ -20,12 +21,13 @@ class DossierMailer < ApplicationMailer
     end
   end
 
-  def notify_new_answer(dossier, body = nil)
+  def notify_new_answer
+    dossier = params[:commentaire].dossier
     I18n.with_locale(dossier.user_locale) do
       @dossier = dossier
       @service = dossier.procedure.service
       @logo_url = attach_logo(dossier.procedure)
-      @body = body
+      @body = commentaire.body
       @subject = default_i18n_subject(dossier_id: dossier.id, libelle_demarche: dossier.procedure.libelle)
 
       mail(to: dossier.user_email_for(:notification), subject: @subject) do |format|
@@ -168,6 +170,10 @@ class DossierMailer < ApplicationMailer
 
   protected
 
+  def prevent_perform_deliveries
+    mail.perform_deliveries = false if params[:commentaire].discarded?
+  end
+
   # This is an override of `default_i18n_subject` method
   # https://api.rubyonrails.org/v5.0.0/classes/ActionMailer/Base.html#method-i-default_i18n_subject
   #

app/models/commentaire.rb

@@ -99,18 +99,16 @@ class Commentaire < ApplicationRecord
     #   the notification system will properly
     # - Otherwise, a instructeur posted a commentaire, we need to notify the user
     if sent_by_instructeur?
-      notify_user_with_delay
+      notify_user(wait: 5.minutes)
     elsif sent_by_expert?
       notify_user
     end
   end
 
-  def notify_user_with_delay
+  def notify_user(job_options)
-    NotifyNewAnswerWithDelayJob.set(wait: 5.minutes).perform_later(dossier, body, self)
+    DossierMailer.with(commentaire: self)
-  end
+      .notify_new_answer
-
+      .deliver_later(job_options)
-  def notify_user
-    DossierMailer.notify_new_answer(dossier, body).deliver_later
   end
 
   def messagerie_available?

spec/jobs/notify_new_answer_with_delay_job_spec.rb

@@ -1,22 +0,0 @@
-describe NotifyNewAnswerWithDelayJob, type: :job do
-  let(:dossier) { double }
-  let(:body) { "bim un body" }
-
-  context 'when commentaire not discarded?' do
-    let(:commentaire) { create(:commentaire) }
-
-    it 'call DossierMailer.notify_new_answer' do
-      expect(DossierMailer).to receive(:notify_new_answer).with(dossier, body).and_return(double(deliver_now: true))
-      NotifyNewAnswerWithDelayJob.perform_now(dossier, body, commentaire)
-    end
-  end
-
-  context 'when commentaire is discarded?' do
-    let(:commentaire) { create(:commentaire, discarded_at: 2.hours.ago) }
-
-    it 'skips DossierMailer.notify_new_anser' do
-      expect(DossierMailer).not_to receive(:notify_new_answer).with(dossier, body)
-      NotifyNewAnswerWithDelayJob.perform_now(dossier, body, commentaire)
-    end
-  end
-end

spec/mailers/dossier_mailer_spec.rb

@@ -28,8 +28,8 @@ RSpec.describe DossierMailer, type: :mailer do
 
   describe '.notify_new_answer with dossier brouillon' do
     let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
-
+    let(:commentaire) { create(:commentaire, dossier: dossier) }
-    subject { described_class.notify_new_answer(dossier) }
+    subject { described_class.with(commentaire: commentaire).notify_new_answer }
 
     it { expect(subject.subject).to include("Nouveau message") }
     it { expect(subject.subject).to include(dossier.id.to_s) }
@@ -40,8 +40,8 @@ RSpec.describe DossierMailer, type: :mailer do
 
   describe '.notify_new_answer with dossier en construction' do
     let(:dossier) { create(:dossier, state: "en_construction", procedure: build(:simple_procedure)) }
-
+    let(:commentaire) { create(:commentaire, dossier: dossier) }
-    subject { described_class.notify_new_answer(dossier) }
+    subject { described_class.with(commentaire: commentaire).notify_new_answer }
 
     it { expect(subject.subject).to include("Nouveau message") }
     it { expect(subject.subject).to include(dossier.id.to_s) }
@@ -50,6 +50,15 @@ RSpec.describe DossierMailer, type: :mailer do
     it_behaves_like 'a dossier notification'
   end
 
+  describe '.notify_new_answer with commentaire discarded' do
+    let(:dossier) { create(:dossier, procedure: build(:simple_procedure)) }
+    let(:commentaire) { create(:commentaire, dossier: dossier, discarded_at: 2.minutes.ago) }
+
+    subject { described_class.with(commentaire: commentaire).notify_new_answer }
+
+    it { expect(subject.perform_deliveries).to be_falsy }
+  end
+
   describe '.notify_deletion_to_user' do
     let(:deleted_dossier) { build(:deleted_dossier) }
 

spec/models/commentaire_spec.rb

@@ -87,7 +87,7 @@ describe Commentaire do
       let(:commentaire) { CommentaireService.build(instructeur, dossier, body: "Mon commentaire") }
 
       it "calls notify_user with delay so instructeur can destroy his comment in case of failure" do
-        expect(commentaire).to receive(:notify_user_with_delay)
+        expect(commentaire).to receive(:notify_user).with(wait: 5.minutes)
         commentaire.save
       end
     end
@@ -96,7 +96,7 @@ describe Commentaire do
       let(:commentaire) { CommentaireService.build(expert, dossier, body: "Mon commentaire") }
 
       it "calls notify_user" do
-        expect(commentaire).to receive(:notify_user)
+        expect(commentaire).to receive(:notify_user).with(no_args)
         commentaire.save
       end
     end
@@ -105,7 +105,7 @@ describe Commentaire do
       let(:commentaire) { CommentaireService.build_with_email(CONTACT_EMAIL, dossier, body: "Mon commentaire") }
 
       it "does not call notify_user" do
-        expect(commentaire).not_to receive(:notify_user)
+        expect(commentaire).not_to receive(:notify_user).with(no_args)
         commentaire.save
       end
     end