Merge pull request #8321 from tchak/fix-explicitly-send-draft-notification-email

fix(dossier): explicitly send draft notification email

app/controllers/users/dossiers_controller.rb

@@ -306,6 +306,7 @@ module Users
       dossier.build_default_individual
       dossier.save!
       dossier.prefill!(PrefillParams.new(dossier, retrieve_and_delete_stored_query_params).to_a)
+      DossierMailer.with(dossier:).notify_new_draft.deliver_later
 
       if dossier.procedure.for_individual
         redirect_to identite_dossier_path(dossier)
@@ -335,6 +336,7 @@ module Users
 
     def clone
       cloned_dossier = @dossier.clone
+      DossierMailer.with(dossier: cloned_dossier).notify_new_draft.deliver_later
       flash.notice = t('users.dossiers.cloned_success')
       redirect_to brouillon_dossier_path(cloned_dossier)
     rescue ActiveRecord::RecordInvalid => e

app/mailers/dossier_mailer.rb

@@ -6,16 +6,16 @@ class DossierMailer < ApplicationMailer
 
   layout 'mailers/layout'
   default from: NO_REPLY_EMAIL
-  after_action :prevent_perform_deliveries, only: [:notify_new_answer]
+  after_action :prevent_perform_deliveries, only: [:notify_new_draft, :notify_new_answer]
 
-  def notify_new_draft(dossier)
+  def notify_new_draft
-    I18n.with_locale(dossier.user_locale) do
+    @dossier = params[:dossier]
-      @dossier = dossier
+    I18n.with_locale(@dossier.user_locale) do
-      @service = dossier.procedure.service
+      @service = @dossier.procedure.service
-      @logo_url = attach_logo(dossier.procedure)
+      @logo_url = attach_logo(@dossier.procedure)
-      @subject = default_i18n_subject(libelle_demarche: dossier.procedure.libelle)
+      @subject = default_i18n_subject(libelle_demarche: @dossier.procedure.libelle)
 
-      mail(to: dossier.user_email_for(:notification), subject: @subject) do |format|
+      mail(to: @dossier.user_email_for(:notification), subject: @subject) do |format|
         format.html { render layout: 'mailers/notifications_layout' }
       end
     end
@@ -170,7 +170,9 @@ class DossierMailer < ApplicationMailer
   protected
 
   def prevent_perform_deliveries
-    mail.perform_deliveries = false if params[:commentaire].discarded?
+    if params[:commentaire]&.discarded? || params[:dossier]&.skip_user_notification_email?
+      mail.perform_deliveries = false
+    end
   end
 
   # This is an override of `default_i18n_subject` method

app/mailers/notification_mailer.rb

@@ -53,7 +53,7 @@ class NotificationMailer < ApplicationMailer
   def set_dossier
     @dossier = params[:dossier]
 
-    if @dossier.user_deleted?
+    if @dossier.skip_user_notification_email?
       mail.perform_deliveries = false
     else
       I18n.with_locale(@dossier.user_locale) do

app/models/dossier.rb

@@ -433,7 +433,6 @@ class Dossier < ApplicationRecord
   before_save :update_search_terms
 
   after_save :send_web_hook
-  after_create_commit :send_draft_notification_email
 
   validates :user, presence: true, if: -> { deleted_user_email_never_send.nil? }
   validates :individual, presence: true, if: -> { revision.procedure.for_individual? }
@@ -1244,6 +1243,14 @@ class Dossier < ApplicationRecord
     champs_public.joins(:type_de_champ).where(types_de_champ: { stable_id: stable_ids })
   end
 
+  def skip_user_notification_email?
+    return true if brouillon? && procedure.declarative?
+    return true if for_procedure_preview?
+    return true if user_deleted?
+
+    false
+  end
+
   private
 
   def create_missing_traitemets
@@ -1301,12 +1308,6 @@ class Dossier < ApplicationRecord
     end
   end
 
-  def send_draft_notification_email
-    if brouillon? && !procedure.declarative? && !for_procedure_preview?
-      DossierMailer.notify_new_draft(self).deliver_later
-    end
-  end
-
   def send_web_hook
     if saved_change_to_state? && !brouillon? && procedure.web_hook_url.present?
       WebHookJob.perform_later(

spec/controllers/api/v2/graphql_controller_spec.rb

@@ -1005,7 +1005,7 @@ describe API::V2::GraphqlController do
             })
 
             perform_enqueued_jobs except: [APIEntreprise::ServiceJob]
-            expect(ActionMailer::Base.deliveries.size).to eq(4)
+            expect(ActionMailer::Base.deliveries.size).to eq(1)
           end
         end
 
@@ -1048,7 +1048,7 @@ describe API::V2::GraphqlController do
             })
 
             perform_enqueued_jobs except: [APIEntreprise::ServiceJob]
-            expect(ActionMailer::Base.deliveries.size).to eq(3)
+            expect(ActionMailer::Base.deliveries.size).to eq(0)
           end
         end
       end
@@ -1091,7 +1091,7 @@ describe API::V2::GraphqlController do
             })
 
             perform_enqueued_jobs except: [APIEntreprise::ServiceJob]
-            expect(ActionMailer::Base.deliveries.size).to eq(4)
+            expect(ActionMailer::Base.deliveries.size).to eq(1)
           end
         end
       end
@@ -1134,7 +1134,7 @@ describe API::V2::GraphqlController do
             })
 
             perform_enqueued_jobs except: [APIEntreprise::ServiceJob]
-            expect(ActionMailer::Base.deliveries.size).to eq(3)
+            expect(ActionMailer::Base.deliveries.size).to eq(0)
           end
         end
       end

spec/mailers/application_mailer_spec.rb

@@ -1,7 +1,7 @@
 RSpec.describe ApplicationMailer, type: :mailer do
   describe 'dealing with invalid emails' do
     let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) }
-    subject { DossierMailer.notify_new_draft(dossier) }
+    subject { DossierMailer.with(dossier:).notify_new_draft }
 
     describe 'invalid emails are not sent' do
       before do

spec/mailers/dossier_mailer_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe DossierMailer, type: :mailer do
   describe '.notify_new_draft' do
     let(:dossier) { create(:dossier, procedure: create(:simple_procedure, :with_auto_archive)) }
 
-    subject { described_class.notify_new_draft(dossier) }
+    subject { described_class.with(dossier:).notify_new_draft }
 
     it { expect(subject.subject).to include("brouillon") }
     it { expect(subject.subject).to include(dossier.procedure.libelle) }

spec/models/dossier_spec.rb

@@ -650,34 +650,6 @@ describe Dossier do
     end
   end
 
-  describe "#send_draft_notification_email" do
-    include Rails.application.routes.url_helpers
-
-    let(:procedure) { create(:procedure) }
-    let(:user) { create(:user) }
-
-    it "send an email when the dossier is created for the very first time" do
-      dossier = nil
-      expect do
-        perform_enqueued_jobs do
-          dossier = create(:dossier, procedure: procedure, state: Dossier.states.fetch(: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} »")
-      expect(mail.html_part.body).to include(dossier_url(dossier))
-    end
-
-    it "does not send an email when the dossier is created with a non brouillon state" do
-      expect { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_construction), user: user) }.not_to change(ActionMailer::Base.deliveries, :size)
-      expect { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:en_instruction), user: user) }.not_to change(ActionMailer::Base.deliveries, :size)
-      expect { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:accepte), user: user) }.not_to change(ActionMailer::Base.deliveries, :size)
-      expect { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:refuse), user: user) }.not_to change(ActionMailer::Base.deliveries, :size)
-      expect { create(:dossier, procedure: procedure, state: Dossier.states.fetch(:sans_suite), user: user) }.not_to change(ActionMailer::Base.deliveries, :size)
-    end
-  end
-
   describe "#unspecified_attestation_champs" do
     let(:procedure) { create(:procedure, attestation_template: attestation_template, types_de_champ_public: types_de_champ, types_de_champ_private: types_de_champ_private) }
     let(:dossier) { create(:dossier, :en_instruction, procedure: procedure) }