Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tech: plus d'erreur lorsque l'email de transfert n'a plus de dossier associé #9935

Merged
merged 2 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions app/mailers/dossier_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
# Preview all emails at http://localhost:3000/rails/mailers/dossier_mailer
class DossierMailer < ApplicationMailer
class AbortDeliveryError < StandardError; end

helper ServiceHelper
helper MailerHelper
helper ProcedureHelper

layout 'mailers/layout'
default from: NO_REPLY_EMAIL
after_action :prevent_perform_deliveries, only: [:notify_new_draft, :notify_new_answer, :notify_pending_correction]

before_action :abort_perform_deliveries, only: [:notify_transfer]
after_action :prevent_perform_deliveries, only: [:notify_new_draft, :notify_new_answer, :notify_pending_correction, :notify_transfer]

# when we don't want to render the view
rescue_from AbortDeliveryError, with: -> {}

def notify_new_draft
@dossier = params[:dossier]
Expand Down Expand Up @@ -162,12 +169,13 @@ def notify_brouillon_not_submitted(dossier)
end
end

def notify_transfer(transfer)
I18n.with_locale(transfer.user_locale) do
def notify_transfer
@transfer = params[:dossier_transfer]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pour ma culture, pourquoi préférer cette écriture à notify_transfert(transfer) ?

Copy link
Member Author

@colinux colinux Jan 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

car sinon la méthode est exécutée immédiatement et on ne peut plus empêcher de passer par la vue. En quelque sorte les before/after action ont moins de "pouvoir" si on fait directement Mailer.action. C'est ce qui nous a pris pas mal de temps à comprendre .
bigup à @mfo d'avoir débuggué et compris cet aspect là


I18n.with_locale(@transfer.user_locale) do
@subject = default_i18n_subject()
@transfer = transfer

mail(to: transfer.email, subject: @subject)
mail(to: @transfer.email, subject: @subject)
end
end

Expand All @@ -186,6 +194,14 @@ def prevent_perform_deliveries
end
end

def abort_perform_deliveries
dossier_transfer = params[:dossier_transfer]

if dossier_transfer.dossiers.empty?
raise AbortDeliveryError.new
end
end

# This is an override of `default_i18n_subject` method
# https://api.rubyonrails.org/v5.0.0/classes/ActionMailer/Base.html#method-i-default_i18n_subject
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/dossier_transfer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,6 @@ def sender_email
private

def send_notification
DossierMailer.notify_transfer(self).deliver_later
DossierMailer.with(dossier_transfer: self).notify_transfer.deliver_later
end
end
4 changes: 2 additions & 2 deletions app/views/users/dossiers/_dossier_actions.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@

- if has_delete_action
- menu.with_item(class: 'danger') do
= link_to(dossier_path(dossier), role: 'menuitem', method: :delete, data: { disable: true, confirm: "En continuant, vous allez supprimer ce dossier ainsi que les informations qu’il contient. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?" }) do

- confirm = dossier.transfer.present? ? t("views.users.dossiers.dossier_action.delete_dossier_with_transfer_confirm") : t("views.users.dossiers.dossier_action.delete_dossier_confirm")
= link_to(dossier_path(dossier), role: 'menuitem', method: :delete, data: { disable: true, confirm: }) do
= dsfr_icon('fr-icon-delete-fill', :sm)
.dropdown-description
= t('views.users.dossiers.dossier_action.delete_dossier')
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,8 @@ en:
start_other_dossier: "Start another empty file"
clone: "Duplicate the file"
delete_dossier: "Delete the file"
delete_dossier_confirm: "If you continue, you will delete this file and the information it contains. Any deletion will result in the cancellation of the current procedure.\n\nConfirm deletion?"
delete_dossier_with_transfer_confirm: "If you continue, you will delete this file, the information it contains and its transfer request. Any deletion will result in the cancellation of the current procedure.\n\nConfirm deletion?"
transfer_dossier: "Transfer the file"
edit_draft: "Keep filling"
actions: "Actions"
Expand Down
2 changes: 2 additions & 0 deletions config/locales/fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@ fr:
start_other_dossier: "Commencer un autre dossier vide"
clone: "Dupliquer ce dossier"
delete_dossier: "Supprimer le dossier"
delete_dossier_confirm: "En continuant, vous allez supprimer ce dossier ainsi que les informations qu’il contient. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"
delete_dossier_with_transfer_confirm: "En continuant, vous allez supprimer ce dossier, les informations qu’il contient ainsi que sa demande de transfert. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Peut être rappeler vers qui est destiné le transfert. L'idée étant que voir le mail du destinataire rend plus compréhensible le transfert.

Suggested change
delete_dossier_with_transfer_confirm: "En continuant, vous allez supprimer ce dossier, les informations qu’il contient ainsi que sa demande de transfert. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"
delete_dossier_with_transfer_confirm: "En continuant, vous allez supprimer ce dossier, les informations qu’il contient ainsi que sa demande de transfert à %{email}. Toute suppression entraîne l’annulation de la démarche en cours.\n\nConfirmer la suppression ?"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bien vu je le rajouterai dans une autre PR vu que c'est déjà mergé

transfer_dossier: "Transférer le dossier"
edit_draft: "Continuer à remplir"
actions: "Actions"
Expand Down
15 changes: 9 additions & 6 deletions spec/controllers/manager/dossiers_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,26 @@
end

describe "POST #transfer" do
before do
allow(DossierMailer).to receive(:notify_transfer).and_call_original
subject do
post :transfer, params: { id: @dossier.id, email: }
end

context 'with valid email' do
let(:email) { "chouette.gars@laposte.net" }

it { expect(flash[:success]).to eq("Une invitation de transfert a été envoyée à chouette.gars@laposte.net") }
it { expect(DossierMailer).to have_received(:notify_transfer) }
it do
expect { subject }.to have_enqueued_mail(DossierMailer, :notify_transfer)
expect(flash[:success]).to eq("Une invitation de transfert a été envoyée à chouette.gars@laposte.net")
end
end

context 'with invalid email' do
let(:email) { "chouette" }

it { expect(flash[:alert]).to eq("L’adresse email est invalide") }
it { expect(DossierMailer).not_to have_received(:notify_transfer) }
it do
expect { subject }.not_to have_enqueued_mail
expect(flash[:alert]).to eq("L’adresse email est invalide")
end
end
end

Expand Down
13 changes: 12 additions & 1 deletion spec/mailers/dossier_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def notify_deletion_to_administration(deleted_dossier, to_email)
let(:dossier_transfer) { create(:dossier_transfer) }
let!(:dossier) { create(:dossier, user: user, transfer: dossier_transfer, procedure: procedure) }

subject { described_class.notify_transfer(dossier_transfer) }
subject { described_class.with(dossier_transfer: dossier_transfer).notify_transfer }

context 'when it is a transfer of one dossier' do
it { expect(subject.subject).to include("Vous avez une demande de transfert en attente.") }
Expand All @@ -294,5 +294,16 @@ def notify_deletion_to_administration(deleted_dossier, to_email)
it { expect(subject.subject).to include("Vous avez une demande de transfert en attente.") }
it { expect(subject.body).to include("Le support technique vous adresse une demande de transfert") }
end

context 'when dossiers have been dissociated from transfer' do
before do
dossier.update!(transfer: nil)
dossier_transfer.reload
end

it 'does not send an email' do
expect { subject.perform_now }.not_to raise_error
end
end
end
end