From 4a4ce67ef647df2553c081d38c8d89bfb484d7d7 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Wed, 21 Dec 2022 17:18:19 +0100 Subject: [PATCH] fix(dossier): explicitly send draft notification email --- app/controllers/users/dossiers_controller.rb | 2 ++ app/mailers/dossier_mailer.rb | 20 +++++++------ app/mailers/notification_mailer.rb | 2 +- app/models/dossier.rb | 15 +++++----- .../api/v2/graphql_controller_spec.rb | 8 +++--- spec/mailers/application_mailer_spec.rb | 2 +- spec/mailers/dossier_mailer_spec.rb | 2 +- spec/models/dossier_spec.rb | 28 ------------------- 8 files changed, 28 insertions(+), 51 deletions(-) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index 6b1d7bb85..bb0b63b0b 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/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 diff --git a/app/mailers/dossier_mailer.rb b/app/mailers/dossier_mailer.rb index 36f0e56f3..81d42cf4f 100644 --- a/app/mailers/dossier_mailer.rb +++ b/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) - I18n.with_locale(dossier.user_locale) do - @dossier = dossier - @service = dossier.procedure.service - @logo_url = attach_logo(dossier.procedure) - @subject = default_i18n_subject(libelle_demarche: dossier.procedure.libelle) + def notify_new_draft + @dossier = params[:dossier] + I18n.with_locale(@dossier.user_locale) do + @service = @dossier.procedure.service + @logo_url = attach_logo(@dossier.procedure) + @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 diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb index 140c18f54..706da2efd 100644 --- a/app/mailers/notification_mailer.rb +++ b/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 diff --git a/app/models/dossier.rb b/app/models/dossier.rb index c3d9812fa..7e538accc 100644 --- a/app/models/dossier.rb +++ b/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( diff --git a/spec/controllers/api/v2/graphql_controller_spec.rb b/spec/controllers/api/v2/graphql_controller_spec.rb index cd9e8a8ef..6110bf69a 100644 --- a/spec/controllers/api/v2/graphql_controller_spec.rb +++ b/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 diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb index f75be8e2c..8cde8e6fc 100644 --- a/spec/mailers/application_mailer_spec.rb +++ b/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 diff --git a/spec/mailers/dossier_mailer_spec.rb b/spec/mailers/dossier_mailer_spec.rb index 38a3cc92c..d4da5dff9 100644 --- a/spec/mailers/dossier_mailer_spec.rb +++ b/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) } diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 3d5f07201..1769f1d7d 100644 --- a/spec/models/dossier_spec.rb +++ b/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) }