Merge pull request #1973 from betagouv/frederic/fix-1972-async-mail

[Fix #1972] Envoyer les mails en asynchrone

Gemfile

@@ -42,6 +42,7 @@ gem 'unicode_utils'
 
 # Gestion des comptes utilisateurs
 gem 'devise'
+gem 'devise-async'
 gem 'openid_connect'
 gem 'omniauth-github'
 

Gemfile.lock

@@ -175,6 +175,9 @@ GEM
       railties (>= 4.1.0, < 6.0)
       responders
       warden (~> 1.2.3)
+    devise-async (1.0.0)
+      activejob (>= 5.0)
+      devise (>= 4.0)
     diff-lcs (1.3)
     domain_name (0.5.20170404)
       unf (>= 0.0.5, < 1.0.0)
@@ -803,6 +806,7 @@ DEPENDENCIES
   delayed_job_active_record
   delayed_job_web!
   devise
+  devise-async
   dotenv-rails
   draper
   factory_bot

app/controllers/admin/gestionnaires_controller.rb

@@ -50,7 +50,7 @@ class Admin::GestionnairesController < AdminController
       @gestionnaire.invite!
 
       if User.exists?(email: @gestionnaire.email)
-        GestionnaireMailer.user_to_gestionnaire(@gestionnaire.email).deliver_now!
+        GestionnaireMailer.user_to_gestionnaire(@gestionnaire.email).deliver_later
       else
         User.create(email: email, password: password)
       end

app/controllers/invites_controller.rb

@@ -13,9 +13,9 @@ class InvitesController < ApplicationController
 
     if invite.valid?
       if invite.user.present?
-        InviteMailer.invite_user(invite).deliver_now!
+        InviteMailer.invite_user(invite).deliver_later
       else
-        InviteMailer.invite_guest(invite).deliver_now!
+        InviteMailer.invite_guest(invite).deliver_later
       end
 
       flash.notice = "Invitation envoyée (#{invite.email})"

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_later
       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_later
       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_later
       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_later
         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_later
       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/jobs/find_dubious_procedures_job.rb

@@ -24,6 +24,6 @@ class FindDubiousProceduresJob < ApplicationJob
       .group_by(&:procedure_id)
       .map { |_procedure_id, tdcs| [tdcs[0].procedure, tdcs] }
 
-    AdministrationMailer.dubious_procedures(dubious_procedures_and_tdcs).deliver_now
+    AdministrationMailer.dubious_procedures(dubious_procedures_and_tdcs).deliver_later
   end
 end

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,

app/models/administrateur.rb

@@ -2,7 +2,7 @@ class Administrateur < ApplicationRecord
   include CredentialsSyncableConcern
   include EmailSanitizableConcern
 
-  devise :database_authenticatable, :registerable,
+  devise :database_authenticatable, :registerable, :async,
     :recoverable, :rememberable, :trackable, :validatable
 
   has_and_belongs_to_many :gestionnaires
@@ -51,7 +51,7 @@ class Administrateur < ApplicationRecord
 
     reset_password_token = set_reset_password_token
 
-    AdministrationMailer.invite_admin(self, reset_password_token).deliver_now!
+    AdministrationMailer.invite_admin(self, reset_password_token).deliver_later
 
     reset_password_token
   end

app/models/administration.rb

@@ -1,7 +1,7 @@
 class Administration < ApplicationRecord
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable and :omniauthable
-  devise :database_authenticatable, :rememberable, :trackable, :validatable, :omniauthable, omniauth_providers: [:github]
+  devise :database_authenticatable, :rememberable, :trackable, :validatable, :omniauthable, :async, omniauth_providers: [:github]
 
   def self.from_omniauth(params)
     find_by(email: params["info"]["email"])
@@ -17,7 +17,7 @@ class Administration < ApplicationRecord
     })
 
     if administrateur.save
-      AdministrationMailer.new_admin_email(administrateur, self).deliver_now!
+      AdministrationMailer.new_admin_email(administrateur, self).deliver_later
       administrateur.invite!
       User.create({
         email: email,

app/models/avis.rb

@@ -34,7 +34,7 @@ class Avis < ApplicationRecord
   private
 
   def notify_gestionnaire
-    AvisMailer.avis_invitation(self).deliver_now
+    AvisMailer.avis_invitation(self).deliver_later
   end
 
   def try_to_assign_gestionnaire

app/models/commentaire.rb

@@ -42,7 +42,7 @@ class Commentaire < ApplicationRecord
   end
 
   def notify_user
-    NotificationMailer.new_answer(dossier).deliver_now!
+    NotificationMailer.new_answer(dossier).deliver_later
   end
 
   def is_virus_free?

app/models/dossier.rb

@@ -322,7 +322,7 @@ class Dossier < ApplicationRecord
 
   def send_draft_notification_email
     if brouillon?
-      NotificationMailer.send_draft_notification(self).deliver_now!
+      NotificationMailer.send_draft_notification(self).deliver_later
     end
   end
 

app/models/gestionnaire.rb

@@ -2,7 +2,7 @@ class Gestionnaire < ApplicationRecord
   include CredentialsSyncableConcern
   include EmailSanitizableConcern
 
-  devise :database_authenticatable, :registerable,
+  devise :database_authenticatable, :registerable, :async,
     :recoverable, :rememberable, :trackable, :validatable
 
   has_and_belongs_to_many :administrateurs
@@ -147,7 +147,7 @@ class Gestionnaire < ApplicationRecord
   def invite!
     reset_password_token = set_reset_password_token
 
-    GestionnaireMailer.invite_gestionnaire(self, reset_password_token).deliver_now!
+    GestionnaireMailer.invite_gestionnaire(self, reset_password_token).deliver_later
   end
 
   private

app/models/user.rb

@@ -9,7 +9,7 @@ class User < ApplicationRecord
 
   # Include default devise modules. Others available are:
   # :confirmable, :lockable, :timeoutable and :omniauthable
-  devise :database_authenticatable, :registerable,
+  devise :database_authenticatable, :registerable, :async,
     :recoverable, :rememberable, :trackable, :validatable, :confirmable
 
   has_many :dossiers, dependent: :destroy

spec/controllers/admin/gestionnaires_controller_spec.rb

@@ -71,7 +71,7 @@ describe Admin::GestionnairesController, type: :controller do
       describe 'Email Notification' do
         it {
           expect(GestionnaireMailer).not_to receive(:new_gestionnaire)
-          expect(GestionnaireMailer).not_to receive(:deliver_now!)
+          expect(GestionnaireMailer).not_to receive(:deliver_later)
           subject
         }
       end
@@ -87,7 +87,7 @@ describe Admin::GestionnairesController, type: :controller do
 
       it 'Notification email is not send' do
         expect(GestionnaireMailer).not_to receive(:new_gestionnaire)
-        expect(GestionnaireMailer).not_to receive(:deliver_now!)
+        expect(GestionnaireMailer).not_to receive(:deliver_later)
       end
     end
 
@@ -104,7 +104,7 @@ describe Admin::GestionnairesController, type: :controller do
       describe 'Email notification' do
         it 'is not sent when email already exists' do
           expect(GestionnaireMailer).not_to receive(:new_gestionnaire)
-          expect(GestionnaireMailer).not_to receive(:deliver_now!)
+          expect(GestionnaireMailer).not_to receive(:deliver_later)
 
           subject
         end

spec/controllers/invites_controller_spec.rb

@@ -131,7 +131,7 @@ describe InvitesController, type: :controller do
           context 'when user does not exist' do
             it 'send email' do
               expect(InviteMailer).to receive(:invite_guest).and_return(InviteMailer)
-              expect(InviteMailer).to receive(:deliver_now!)
+              expect(InviteMailer).to receive(:deliver_later)
 
               subject
             end
@@ -144,7 +144,7 @@ describe InvitesController, type: :controller do
 
             it 'send email' do
               expect(InviteMailer).to receive(:invite_user).and_return(InviteMailer)
-              expect(InviteMailer).to receive(:deliver_now!)
+              expect(InviteMailer).to receive(:deliver_later)
 
               subject
             end

spec/controllers/manager/administrateurs_controller_spec.rb

@@ -18,9 +18,9 @@ describe Manager::AdministrateursController, type: :controller do
 
       it 'alert new mail are send' do
         expect(AdministrationMailer).to receive(:new_admin_email).and_return(AdministrationMailer)
-        expect(AdministrationMailer).to receive(:deliver_now!)
+        expect(AdministrationMailer).to receive(:deliver_later)
         expect(AdministrationMailer).to receive(:invite_admin).and_return(AdministrationMailer)
-        expect(AdministrationMailer).to receive(:deliver_now!)
+        expect(AdministrationMailer).to receive(:deliver_later)
         subject
       end
     end

spec/controllers/new_gestionnaire/dossiers_controller_spec.rb

@@ -151,9 +151,9 @@ 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!)
+        expect(NotificationMailer).to receive(:deliver_later)
 
         subject
       end
@@ -177,9 +177,9 @@ 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!)
+        expect(NotificationMailer).to receive(:deliver_later)
 
         subject
       end
@@ -192,11 +192,11 @@ 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!)
+        expect(NotificationMailer).to receive(:deliver_later)
       end
 
       subject { post :terminer, params: { process_action: "accepter", procedure_id: procedure.id, dossier_id: dossier.id } }

spec/controllers/new_user/dossiers_controller_spec.rb

@@ -221,14 +221,14 @@ describe NewUser::DossiersController, type: :controller do
 
     it 'sends an email only on the first #update' do
       delivery = double
-      expect(delivery).to receive(:deliver_now!).with(no_args)
+      expect(delivery).to receive(:deliver_later).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_later)
+              expect(NotificationMailer)
+                .to receive(:send_initiated_notification)
                 .with(have_attributes(en_construction_at: DateTime.now))
+                .and_return(sender)
 
               submit_dossier
             end

spec/controllers/users/dossiers/commentaires_controller_spec.rb

@@ -24,7 +24,7 @@ describe Users::Dossiers::CommentairesController, type: :controller do
 
       it 'should notify user' do
         expect(NotificationMailer).to receive(:new_answer).and_return(NotificationMailer)
-        expect(NotificationMailer).to receive(:deliver_now!)
+        expect(NotificationMailer).to receive(:deliver_later)
 
         subject
       end

spec/controllers/users/registrations_controller_spec.rb

@@ -15,8 +15,9 @@ describe Users::RegistrationsController, type: :controller do
 
     context 'when user is correct' do
       it 'sends confirmation instruction' do
-        expect(DeviseUserMailer).to receive(:confirmation_instructions).and_return(DeviseUserMailer)
+        message = double()
-        expect(DeviseUserMailer).to receive(:deliver)
+        expect(DeviseUserMailer).to receive(:confirmation_instructions).and_return(message)
+        expect(message).to receive(:deliver_later)
 
         subject
       end

spec/features/new_gestionnaire/gestionnaire_spec.rb

@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 feature 'The gestionnaire part' do
+  include ActiveJob::TestHelper
+
   let(:password) { 'secret_password' }
   let!(:gestionnaire) { create(:gestionnaire, password: password) }
 
@@ -56,6 +58,7 @@ feature 'The gestionnaire part' do
 
   scenario 'A gestionnaire can use avis' do
     ActionMailer::Base.deliveries = []
+    ActiveJob::Base.queue_adapter = :test
 
     log_in(gestionnaire.email, password)
 
@@ -66,7 +69,10 @@ feature 'The gestionnaire part' do
     expect(page).to have_current_path(avis_gestionnaire_dossier_path(procedure, dossier))
 
     expert_email = 'expert@tps.com'
-    ask_confidential_avis(expert_email, 'a good introduction')
+
+    perform_enqueued_jobs do
+      ask_confidential_avis(expert_email, 'a good introduction')
+    end
 
     log_out
 

spec/jobs/find_dubious_procedures_job_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 RSpec.describe FindDubiousProceduresJob, type: :job do
   describe 'perform' do
-    let(:mailer_double) { double('mailer', deliver_now: true) }
+    let(:mailer_double) { double('mailer', deliver_later: true) }
     let(:procedure) { create(:procedure) }
     let(:allowed_tdc) { create(:type_de_champ, libelle: 'fournir') }
 

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

spec/models/avis_spec.rb

@@ -90,7 +90,7 @@ RSpec.describe Avis, type: :model do
   describe '#notify_gestionnaire' do
     context 'when an avis is created' do
       before do
-        avis_invitation_double = double('avis_invitation', deliver_now: true)
+        avis_invitation_double = double('avis_invitation', deliver_later: true)
         allow(AvisMailer).to receive(:avis_invitation).and_return(avis_invitation_double)
         Avis.create(claimant: claimant, email: 'email@l.com')
       end

spec/models/dossier_spec.rb

@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe Dossier do
+  include ActiveJob::TestHelper
+
   let(:user) { create(:user) }
 
   describe "without_followers scope" do
@@ -619,7 +621,12 @@ describe Dossier do
     end
 
     it "send an email when the dossier is created for the very first time" do
-      expect { Dossier.create(procedure: procedure, state: "brouillon", user: user) }.to change(ActionMailer::Base.deliveries, :size).from(0).to(1)
+      ActiveJob::Base.queue_adapter = :test
+      expect do
+        perform_enqueued_jobs do
+          Dossier.create(procedure: procedure, state: "brouillon", user: user)
+        end
+      end.to change(ActionMailer::Base.deliveries, :size).from(0).to(1)
 
       mail = ActionMailer::Base.deliveries.last
       expect(mail.subject).to eq("Retrouvez votre brouillon pour la démarche : #{procedure.libelle}")