[#1972] Avoid passing unsaved templates to ActionMailer

So that all mail arguments can be serialized

app/controllers/new_gestionnaire/dossiers_controller.rb

@@ -91,25 +91,22 @@ module NewGestionnaire
       case params[:process_action]
       when "refuser"
         dossier.refuse!
-        notice = "Dossier considéré comme refusé."
+        dossier.save
-        template = procedure.refused_mail_template
+        flash.notice = "Dossier considéré comme refusé."
+        NotificationMailer.send_refused_notification(dossier).deliver_now!
       when "classer_sans_suite"
         dossier.sans_suite!
-        notice = "Dossier considéré comme sans suite."
+        dossier.save
-        template = procedure.without_continuation_mail_template
+        flash.notice = "Dossier considéré comme sans suite."
+        NotificationMailer.send_without_continuation_notification(dossier).deliver_now!
       when "accepter"
         dossier.accepte!
         dossier.attestation = dossier.build_attestation
-        notice = "Dossier traité avec succès."
+        dossier.save
-        template = procedure.closed_mail_template
+        flash.notice = "Dossier traité avec succès."
+        NotificationMailer.send_closed_notification(dossier).deliver_now!
       end
 
-      dossier.save
-
-      flash.notice = notice
-
-      NotificationMailer.send_notification(dossier, template).deliver_now!
-
       redirect_to gestionnaire_dossier_path(procedure, dossier)
     end
 

app/controllers/new_user/dossiers_controller.rb

@@ -71,7 +71,7 @@ module NewUser
         render :modifier
       elsif @dossier.brouillon?
         @dossier.en_construction!
-        NotificationMailer.send_notification(@dossier, @dossier.procedure.initiated_mail_template).deliver_now!
+        NotificationMailer.send_initiated_notification(@dossier).deliver_now!
         redirect_to merci_dossier_path(@dossier)
       elsif owns_dossier?
         redirect_to users_dossier_recapitulatif_path(@dossier)

app/controllers/users/description_controller.rb

@@ -55,7 +55,7 @@ class Users::DescriptionController < UsersController
       if dossier.brouillon?
         dossier.en_construction!
         # TODO move to model
-        NotificationMailer.send_notification(dossier, procedure.initiated_mail_template).deliver_now!
+        NotificationMailer.send_initiated_notification(dossier).deliver_now!
       end
       flash.notice = 'Félicitations, votre demande a bien été enregistrée.'
       redirect_to url_for(controller: :recapitulatif, action: :show, dossier_id: dossier.id)

app/mailers/notification_mailer.rb

@@ -8,15 +8,6 @@ class NotificationMailer < ApplicationMailer
     send_notification(dossier, dossier.procedure.received_mail_template)
   end
 
-  def send_notification(dossier, mail_template)
-    vars_mailer(dossier)
-
-    @subject = mail_template.subject_for_dossier dossier
-    @body = mail_template.body_for_dossier dossier
-
-    mail(subject: @subject) { |format| format.html { @body } }
-  end
-
   def send_draft_notification(dossier)
     vars_mailer(dossier)
 
@@ -25,12 +16,41 @@ class NotificationMailer < ApplicationMailer
     mail(subject: @subject)
   end
 
+  def send_initiated_notification(dossier)
+    send_notification(dossier, dossier.procedure.initiated_mail_template)
+  end
+
+  def send_received_notification(dossier)
+    send_notification(dossier, dossier.procedure.received_mail_template)
+  end
+
+  def send_closed_notification(dossier)
+    send_notification(dossier, dossier.procedure.closed_mail_template)
+  end
+
+  def send_refused_notification(dossier)
+    send_notification(dossier, dossier.procedure.refused_mail_template)
+  end
+
+  def send_without_continuation_notification(dossier)
+    send_notification(dossier, dossier.procedure.without_continuation_mail_template)
+  end
+
   def new_answer(dossier)
     send_mail dossier, "Nouveau message pour votre dossier demarches-simplifiees.fr nº #{dossier.id}"
   end
 
   private
 
+  def send_notification(dossier, mail_template)
+    vars_mailer(dossier)
+
+    @subject = mail_template.subject_for_dossier dossier
+    @body = mail_template.body_for_dossier dossier
+
+    mail(subject: @subject) { |format| format.html { @body } }
+  end
+
   def create_commentaire_for_notification
     Commentaire.create(
       dossier: @dossier,

spec/controllers/new_gestionnaire/dossiers_controller_spec.rb

@@ -151,8 +151,8 @@ describe NewGestionnaire::DossiersController, type: :controller do
       end
 
       it 'Notification email is sent' do
-        expect(NotificationMailer).to receive(:send_notification)
+        expect(NotificationMailer).to receive(:send_refused_notification)
-          .with(dossier, kind_of(Mails::RefusedMail)).and_return(NotificationMailer)
+          .with(dossier).and_return(NotificationMailer)
         expect(NotificationMailer).to receive(:deliver_now!)
 
         subject
@@ -177,8 +177,8 @@ describe NewGestionnaire::DossiersController, type: :controller do
       end
 
       it 'Notification email is sent' do
-        expect(NotificationMailer).to receive(:send_notification)
+        expect(NotificationMailer).to receive(:send_without_continuation_notification)
-          .with(dossier, kind_of(Mails::WithoutContinuationMail)).and_return(NotificationMailer)
+          .with(dossier).and_return(NotificationMailer)
         expect(NotificationMailer).to receive(:deliver_now!)
 
         subject
@@ -192,8 +192,8 @@ describe NewGestionnaire::DossiersController, type: :controller do
         dossier.en_instruction!
         sign_in gestionnaire
 
-        expect(NotificationMailer).to receive(:send_notification)
+        expect(NotificationMailer).to receive(:send_closed_notification)
-          .with(dossier, kind_of(Mails::ClosedMail))
+          .with(dossier)
           .and_return(NotificationMailer)
 
         expect(NotificationMailer).to receive(:deliver_now!)

spec/controllers/new_user/dossiers_controller_spec.rb

@@ -223,12 +223,12 @@ describe NewUser::DossiersController, type: :controller do
       delivery = double
       expect(delivery).to receive(:deliver_now!).with(no_args)
 
-      expect(NotificationMailer).to receive(:send_notification)
+      expect(NotificationMailer).to receive(:send_initiated_notification)
         .and_return(delivery)
 
       subject
 
-      expect(NotificationMailer).not_to receive(:send_notification)
+      expect(NotificationMailer).not_to receive(:send_initiated_notification)
 
       subject
     end
@@ -246,7 +246,7 @@ describe NewUser::DossiersController, type: :controller do
       it { expect(flash.alert).to eq(['nop']) }
 
       it 'does not send an email' do
-        expect(NotificationMailer).not_to receive(:send_notification)
+        expect(NotificationMailer).not_to receive(:send_received_notification)
 
         subject
       end

spec/controllers/users/description_controller_shared_example.rb

@@ -132,9 +132,12 @@ shared_examples 'description_controller_spec' do
             after { Timecop.return }
 
             it 'sets the state of the dossier before sending the mail' do
-              expect_any_instance_of(Mails::InitiatedMail)
+              sender = double("notification sender")
-                .to receive(:subject_for_dossier)
+              allow(sender).to receive(:deliver_now!)
+              expect(NotificationMailer)
+                .to receive(:send_initiated_notification)
                 .with(have_attributes(en_construction_at: DateTime.now))
+                .and_return(sender)
 
               submit_dossier
             end

spec/mailers/notification_mailer_spec.rb

@@ -18,7 +18,19 @@ RSpec.describe NotificationMailer, type: :mailer do
   describe '.send_notification' do
     let(:email_template) { instance_double('email_template', subject_for_dossier: 'subject', body_for_dossier: 'body') }
 
-    subject { described_class.send_notification(dossier, email_template) }
+    subject do
+      klass = Class.new(described_class) do
+        # We’re testing the (private) method `NotificationMailer#send_notification`.
+        #
+        # The standard trick to test a private method would be to `send(:send_notification`, but doesn’t work here,
+        # because ActionMailer does some magic to expose public instace methods as class methods.
+        # So, we use inheritance instead to make the private method public for testing purposes.
+        def send_notification(dossier, template)
+          super
+        end
+      end
+      klass.send_notification(dossier, email_template)
+    end
 
     it { expect(subject.subject).to eq(email_template.subject_for_dossier) }
     it { expect(subject.body).to eq(email_template.body_for_dossier) }

spec/mailers/previews/notification_mailer_preview.rb

@@ -1,6 +1,6 @@
 class NotificationMailerPreview < ActionMailer::Preview
   def send_notification
-    NotificationMailer.send_notification(Dossier.last, Dossier.last.procedure.initiated_mail_template)
+    NotificationMailer.send_initiated_notification(Dossier.last)
   end
 
   def send_draft_notification