Refactor NotificationMailer

app/controllers/users/dossiers_controller.rb

@@ -143,7 +143,7 @@ module Users
 
       if passage_en_construction? && errors.blank?
         @dossier.passer_en_construction!
-        NotificationMailer.send_initiated_notification(@dossier).deliver_later
+        NotificationMailer.send_en_construction_notification(@dossier).deliver_later
         @dossier.groupe_instructeur.instructeurs.with_instant_email_dossier_notifications.each do |instructeur|
           DossierMailer.notify_new_dossier_depose_to_instructeur(@dossier, instructeur.email).deliver_later
         end

app/mailers/notification_mailer.rb

@@ -8,7 +8,8 @@
 class NotificationMailer < ApplicationMailer
   include ActionView::Helpers::SanitizeHelper
 
-  before_action :prevent_delivery_to_deleted_users
+  before_action :set_dossier
+  after_action :create_commentaire_for_notification
 
   helper ServiceHelper
   helper MailerHelper
@@ -16,52 +17,54 @@ class NotificationMailer < ApplicationMailer
   layout 'mailers/notifications_layout'
   default from: NO_REPLY_EMAIL
 
-  def send_dossier_received(dossier)
-    send_notification(dossier, dossier.procedure.received_mail_template)
+  def send_notification
+    @service = @dossier.procedure.service
+    @logo_url = attach_logo(@dossier.procedure)
+    @rendered_template = sanitize(@body)
+
+    mail(subject: @subject, to: @email, template_name: 'send_notification')
   end
 
-  def send_initiated_notification(dossier)
-    send_notification(dossier, dossier.procedure.initiated_mail_template)
+  def self.send_en_construction_notification(dossier)
+    with(dossier: dossier, state: Dossier.states.fetch(:en_construction)).send_notification
   end
 
-  def send_closed_notification(dossier)
-    send_notification(dossier, dossier.procedure.closed_mail_template)
+  def self.send_en_instruction_notification(dossier)
+    with(dossier: dossier, state: Dossier.states.fetch(:en_instruction)).send_notification
   end
 
-  def send_refused_notification(dossier)
-    send_notification(dossier, dossier.procedure.refused_mail_template)
+  def self.send_accepte_notification(dossier)
+    with(dossier: dossier, state: Dossier.states.fetch(:accepte)).send_notification
   end
 
-  def send_without_continuation_notification(dossier)
-    send_notification(dossier, dossier.procedure.without_continuation_mail_template)
+  def self.send_refuse_notification(dossier)
+    with(dossier: dossier, state: Dossier.states.fetch(:refuse)).send_notification
+  end
+
+  def self.send_sans_suite_notification(dossier)
+    with(dossier: dossier, state: Dossier.states.fetch(:sans_suite)).send_notification
   end
 
   private
 
-  def prevent_delivery_to_deleted_users
-    !@dossier.user_deleted?
+  def set_dossier
+    @dossier = params[:dossier]
+
+    if @dossier.user_deleted?
+      mail.perform_deliveries = false
+    else
+      mail_template = @dossier.procedure.mail_template_for(params[:state])
+
+      @email = @dossier.user_email_for(:notification)
+      @subject = mail_template.subject_for_dossier(@dossier)
+      @body = mail_template.body_for_dossier(@dossier)
+      @actions = mail_template.actions_for_dossier(@dossier)
+    end
   end
 
-  def send_notification(dossier, mail_template)
-    email = dossier.user_email_for(:notification)
-
-    subject = mail_template.subject_for_dossier(dossier)
-    body = mail_template.body_for_dossier(dossier)
-
-    create_commentaire_for_notification(dossier, subject, body)
-
-    @dossier = dossier
-    @service = dossier.procedure.service
-    @logo_url = attach_logo(dossier.procedure)
-    @rendered_template = sanitize(body)
-    @actions = mail_template.actions_for_dossier(dossier)
-
-    mail(subject: subject, to: email, template_name: 'send_notification')
-  end
-
-  def create_commentaire_for_notification(dossier, subject, body)
-    params = { body: ["[#{subject}]", body].join("<br><br>") }
-    commentaire = CommentaireService.build_with_email(CONTACT_EMAIL, dossier, params)
+  def create_commentaire_for_notification
+    body = ["[#{@subject}]", @body].join("<br><br>")
+    commentaire = CommentaireService.build_with_email(CONTACT_EMAIL, @dossier, body: body)
     commentaire.save!
   end
 end

app/models/dossier.rb

@@ -344,7 +344,7 @@ class Dossier < ApplicationRecord
   before_save :build_default_champs, if: Proc.new { revision_id_was.nil? }
   before_save :update_search_terms
 
-  after_save :send_dossier_received
+  after_save :send_dossier_en_instruction
   after_save :send_web_hook
   after_create_commit :send_draft_notification_email
 
@@ -690,7 +690,7 @@ class Dossier < ApplicationRecord
 
     save!
     remove_titres_identite!
-    NotificationMailer.send_closed_notification(self).deliver_later
+    NotificationMailer.send_accepte_notification(self).deliver_later
     send_dossier_decision_to_experts(self)
     log_dossier_operation(instructeur, :accepter, self)
   end
@@ -705,7 +705,7 @@ class Dossier < ApplicationRecord
 
     save!
     remove_titres_identite!
-    NotificationMailer.send_closed_notification(self).deliver_later
+    NotificationMailer.send_accepte_notification(self).deliver_later
     log_automatic_dossier_operation(:accepter, self)
   end
 
@@ -718,7 +718,7 @@ class Dossier < ApplicationRecord
 
     save!
     remove_titres_identite!
-    NotificationMailer.send_refused_notification(self).deliver_later
+    NotificationMailer.send_refuse_notification(self).deliver_later
     send_dossier_decision_to_experts(self)
     log_dossier_operation(instructeur, :refuser, self)
   end
@@ -732,7 +732,7 @@ class Dossier < ApplicationRecord
 
     save!
     remove_titres_identite!
-    NotificationMailer.send_without_continuation_notification(self).deliver_later
+    NotificationMailer.send_sans_suite_notification(self).deliver_later
     send_dossier_decision_to_experts(self)
     log_dossier_operation(instructeur, :classer_sans_suite, self)
   end
@@ -949,9 +949,9 @@ class Dossier < ApplicationRecord
     end
   end
 
-  def send_dossier_received
+  def send_dossier_en_instruction
     if saved_change_to_state? && en_instruction? && !procedure.declarative_accepte?
-      NotificationMailer.send_dossier_received(self).deliver_later
+      NotificationMailer.send_en_instruction_notification(self).deliver_later
     end
   end
 

app/models/procedure.rb

@@ -500,6 +500,23 @@ class Procedure < ApplicationRecord
     without_continuation_mail || Mails::WithoutContinuationMail.default_for_procedure(self)
   end
 
+  def mail_template_for(state)
+    case state
+    when Dossier.states.fetch(:en_construction)
+      initiated_mail_template
+    when Dossier.states.fetch(:en_instruction)
+      received_mail_template
+    when Dossier.states.fetch(:accepte)
+      closed_mail_template
+    when Dossier.states.fetch(:refuse)
+      refused_mail_template
+    when Dossier.states.fetch(:sans_suite)
+      without_continuation_mail_template
+    else
+      raise "Unknown dossier state: #{state}"
+    end
+  end
+
   def self.default_sort
     {
       'table' => 'self',

spec/controllers/instructeurs/dossiers_controller_spec.rb

@@ -210,7 +210,7 @@ describe Instructeurs::DossiersController, type: :controller do
         end
 
         it 'Notification email is sent' do
-          expect(NotificationMailer).to receive(:send_refused_notification)
+          expect(NotificationMailer).to receive(:send_refuse_notification)
             .with(dossier).and_return(NotificationMailer)
           expect(NotificationMailer).to receive(:deliver_later)
 
@@ -250,7 +250,7 @@ describe Instructeurs::DossiersController, type: :controller do
         end
 
         it 'Notification email is sent' do
-          expect(NotificationMailer).to receive(:send_without_continuation_notification)
+          expect(NotificationMailer).to receive(:send_sans_suite_notification)
             .with(dossier).and_return(NotificationMailer)
           expect(NotificationMailer).to receive(:deliver_later)
 
@@ -280,7 +280,7 @@ describe Instructeurs::DossiersController, type: :controller do
         dossier.passer_en_instruction!(instructeur)
         sign_in(instructeur.user)
 
-        expect(NotificationMailer).to receive(:send_closed_notification)
+        expect(NotificationMailer).to receive(:send_accepte_notification)
           .with(dossier)
           .and_return(NotificationMailer)
 

spec/controllers/users/dossiers_controller_spec.rb

@@ -476,12 +476,12 @@ describe Users::DossiersController, type: :controller do
       delivery = double
       expect(delivery).to receive(:deliver_later).with(no_args)
 
-      expect(NotificationMailer).to receive(:send_initiated_notification)
+      expect(NotificationMailer).to receive(:send_en_construction_notification)
         .and_return(delivery)
 
       subject
 
-      expect(NotificationMailer).not_to receive(:send_initiated_notification)
+      expect(NotificationMailer).not_to receive(:send_en_construction_notification)
 
       subject
     end
@@ -499,7 +499,7 @@ describe Users::DossiersController, type: :controller do
       it { expect(flash.alert).to eq(['nop']) }
 
       it 'does not send an email' do
-        expect(NotificationMailer).not_to receive(:send_initiated_notification)
+        expect(NotificationMailer).not_to receive(:send_en_construction_notification)
 
         subject
       end
@@ -687,7 +687,7 @@ describe Users::DossiersController, type: :controller do
       end
 
       it 'does not send an email' do
-        expect(NotificationMailer).not_to receive(:send_initiated_notification)
+        expect(NotificationMailer).not_to receive(:send_en_construction_notification)
 
         subject
       end

spec/mailers/notification_mailer_spec.rb

@@ -1,19 +1,21 @@
 RSpec.describe NotificationMailer, type: :mailer do
+  let(:administrateur) { create(:administrateur) }
   let(:user) { create(:user) }
   let(:procedure) { create(:simple_procedure) }
-  let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_service, user: user, procedure: procedure) }
 
-  describe '.send_dossier_received' do
+  describe 'send_en_instruction_notification' do
+    let(:dossier) { create(:dossier, :en_construction, :with_individual, :with_service, user: user, procedure: procedure) }
     let(:email_template) { create(:received_mail, subject: 'Email subject', body: 'Your dossier was processed. Thanks.') }
 
     before do
       dossier.procedure.received_mail = email_template
     end
 
-    subject(:mail) { described_class.send_dossier_received(dossier) }
+    subject(:mail) { described_class.send_en_instruction_notification(dossier) }
 
     it 'creates a commentaire in the messagerie' do
       expect { subject.deliver_now }.to change { Commentaire.count }.by(1)
+      expect(subject.perform_deliveries).to be_truthy
 
       commentaire = Commentaire.last
       expect(commentaire.body).to include(email_template.subject_for_dossier(dossier), email_template.body_for_dossier(dossier))
@@ -59,4 +61,27 @@ RSpec.describe NotificationMailer, type: :mailer do
       expect(subject.from.first).to eq(Mail::Address.new(NO_REPLY_EMAIL).address)
     end
   end
+
+  describe 'send_accepte_notification' do
+    let(:dossier) { create(:dossier, :en_instruction, :with_individual, :with_service, user: user, procedure: procedure) }
+    let(:email_template) { create(:closed_mail, subject: 'Email subject', body: 'Your dossier was accepted. Thanks.') }
+
+    before do
+      dossier.procedure.closed_mail = email_template
+    end
+
+    subject(:mail) { described_class.send_accepte_notification(dossier) }
+
+    context 'when dossier user is deleted' do
+      before do
+        dossier.user.delete_and_keep_track_dossiers(administrateur)
+        dossier.reload
+      end
+
+      it 'should not send notification' do
+        expect { subject.deliver_now }.not_to change { Commentaire.count }
+        expect(subject.perform_deliveries).to be_falsey
+      end
+    end
+  end
 end

spec/mailers/previews/notification_mailer_preview.rb

@@ -1,23 +1,36 @@
 class NotificationMailerPreview < ActionMailer::Preview
-  def send_initiated_notification
-    p = Procedure.where(id: Mails::InitiatedMail.where("body like ?", "%<img%").pluck(:procedure_id).uniq).order("RANDOM()").first
-    NotificationMailer.send_initiated_notification(p.dossiers.last)
+  def send_en_construction_notification
+    NotificationMailer.send_en_construction_notification(dossier_with_image)
   end
 
-  def send_dossier_received
-    NotificationMailer.send_dossier_received(Dossier.last)
+  def send_en_instruction_notification
+    NotificationMailer.send_en_instruction_notification(dossier)
   end
 
-  def send_closed_notification
-    NotificationMailer.send_closed_notification(Dossier.last)
+  def send_accepte_notification
+    NotificationMailer.send_accepte_notification(dossier)
   end
 
-  def send_refused_notification
-    dossier = Dossier.last.tap { |d| d.assign_attributes(motivation: 'Le montant demandé dépasse le plafond autorisé') }
-    NotificationMailer.send_refused_notification(dossier)
+  def send_refuse_notification
+    NotificationMailer.send_refuse_notification(dossier_with_motivation)
   end
 
-  def send_without_continuation_notification
-    NotificationMailer.send_without_continuation_notification(Dossier.last)
+  def send_sans_suite_notification
+    NotificationMailer.send_sans_suite_notification(dossier)
+  end
+
+  private
+
+  def dossier
+    Dossier.last
+  end
+
+  def dossier_with_image
+    procedure = Procedure.where(id: Mails::InitiatedMail.where("body like ?", "%<img%").pluck(:procedure_id).uniq).order("RANDOM()").first
+    procedure.dossiers.last
+  end
+
+  def dossier_with_motivation
+    Dossier.last.tap { |d| d.assign_attributes(motivation: 'Le montant demandé dépasse le plafond autorisé') }
   end
 end

spec/models/dossier_spec.rb

@@ -512,17 +512,17 @@ describe Dossier do
     let(:instructeur) { create(:instructeur) }
 
     before do
-      allow(NotificationMailer).to receive(:send_dossier_received).and_return(double(deliver_later: nil))
+      allow(NotificationMailer).to receive(:send_en_instruction_notification).and_return(double(deliver_later: nil))
     end
 
     it "sends an email when the dossier becomes en_instruction" do
       dossier.passer_en_instruction!(instructeur)
-      expect(NotificationMailer).to have_received(:send_dossier_received).with(dossier)
+      expect(NotificationMailer).to have_received(:send_en_instruction_notification).with(dossier)
     end
 
     it "does not an email when the dossier becomes accepte" do
       dossier.accepte!
-      expect(NotificationMailer).to_not have_received(:send_dossier_received)
+      expect(NotificationMailer).to_not have_received(:send_en_instruction_notification)
     end
   end
 
@@ -935,7 +935,7 @@ describe Dossier do
     let(:attestation) { Attestation.new }
 
     before do
-      allow(NotificationMailer).to receive(:send_closed_notification).and_return(double(deliver_later: true))
+      allow(NotificationMailer).to receive(:send_accepte_notification).and_return(double(deliver_later: true))
       allow(dossier).to receive(:build_attestation).and_return(attestation)
 
       Timecop.freeze(now)
@@ -957,7 +957,7 @@ describe Dossier do
     it { expect(operation_serialized['operation']).to eq('accepter') }
     it { expect(operation_serialized['dossier_id']).to eq(dossier.id) }
     it { expect(operation_serialized['executed_at']).to eq(last_operation.executed_at.iso8601) }
-    it { expect(NotificationMailer).to have_received(:send_closed_notification).with(dossier) }
+    it { expect(NotificationMailer).to have_received(:send_accepte_notification).with(dossier) }
     it { expect(dossier.attestation).to eq(attestation) }
   end
 
@@ -968,7 +968,7 @@ describe Dossier do
     let(:attestation) { Attestation.new }
 
     before do
-      allow(NotificationMailer).to receive(:send_closed_notification).and_return(double(deliver_later: true))
+      allow(NotificationMailer).to receive(:send_accepte_notification).and_return(double(deliver_later: true))
       allow(dossier).to receive(:build_attestation).and_return(attestation)
 
       Timecop.freeze(now)
@@ -984,7 +984,7 @@ describe Dossier do
     it { expect(dossier.state).to eq('accepte') }
     it { expect(last_operation.operation).to eq('accepter') }
     it { expect(last_operation.automatic_operation?).to be_truthy }
-    it { expect(NotificationMailer).to have_received(:send_closed_notification).with(dossier) }
+    it { expect(NotificationMailer).to have_received(:send_accepte_notification).with(dossier) }
     it { expect(dossier.attestation).to eq(attestation) }
   end