From 9f1407b6d7915455cd0baf8266e642b5c7069b50 Mon Sep 17 00:00:00 2001
From: Pierre de La Morinerie <kemenaran@gmail.com>
Date: Mon, 18 May 2020 16:24:08 +0200
Subject: [PATCH] expiration: fix the mailer arguments

The mailers expect serializable arguments, but were given
ActiveRecord::Relation objects instead. This made the mailers throw an
exception.

But how was that possible ? This code is tested, and the tests were
green.

Well, the specs spy on the mailer implementation, in order to check that
the mailers methods were properly called. Fair enough.

But if the specs mock the mailer code (instead of calling the original
implementation), we may not notice that the original implementation
rejects our method parameters.

Here this is the case: once we actually call the original implementation
the tests start to fail, because some arguments are not converted from
an ActiveRecord::Relation to a serializable array.

This is fixed by ensuring that the mailer code is executed (and doesn't
throw an exception).
---
 .../expired_dossiers_deletion_service.rb         |  4 ++--
 .../expired_dossiers_deletion_service_spec.rb    | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/app/services/expired_dossiers_deletion_service.rb b/app/services/expired_dossiers_deletion_service.rb
index 9e1aec31b..6e0bb1f0d 100644
--- a/app/services/expired_dossiers_deletion_service.rb
+++ b/app/services/expired_dossiers_deletion_service.rb
@@ -117,14 +117,14 @@ class ExpiredDossiersDeletionService
       .group_by(&:user)
       .each do |(user, dossiers)|
         DossierMailer.notify_automatic_deletion_to_user(
-          DeletedDossier.where(dossier_id: dossiers.map(&:id)),
+          DeletedDossier.where(dossier_id: dossiers.map(&:id)).to_a,
           user.email
         ).deliver_later
       end
 
     self.group_by_fonctionnaire_email(dossiers_to_remove).each do |(email, dossiers)|
       DossierMailer.notify_automatic_deletion_to_administration(
-        DeletedDossier.where(dossier_id: dossiers.map(&:id)),
+        DeletedDossier.where(dossier_id: dossiers.map(&:id)).to_a,
         email
       ).deliver_later
 
diff --git a/spec/services/expired_dossiers_deletion_service_spec.rb b/spec/services/expired_dossiers_deletion_service_spec.rb
index 29d89f8ce..6417a0b74 100644
--- a/spec/services/expired_dossiers_deletion_service_spec.rb
+++ b/spec/services/expired_dossiers_deletion_service_spec.rb
@@ -19,8 +19,8 @@ describe ExpiredDossiersDeletionService do
       let!(:valid_brouillon) { create(:dossier, procedure: procedure, created_at: date_not_expired) }
 
       before do
-        allow(DossierMailer).to receive(:notify_brouillon_near_deletion).and_return(double(deliver_later: nil))
-        allow(DossierMailer).to receive(:notify_brouillon_deletion).and_return(double(deliver_later: nil))
+        allow(DossierMailer).to receive(:notify_brouillon_near_deletion).and_call_original
+        allow(DossierMailer).to receive(:notify_brouillon_deletion).and_call_original
 
         ExpiredDossiersDeletionService.process_expired_dossiers_brouillon
       end
@@ -207,8 +207,8 @@ describe ExpiredDossiersDeletionService do
     after  { Timecop.return }
 
     before do
-      allow(DossierMailer).to receive(:notify_automatic_deletion_to_user).and_return(double(deliver_later: nil))
-      allow(DossierMailer).to receive(:notify_automatic_deletion_to_administration).and_return(double(deliver_later: nil))
+      allow(DossierMailer).to receive(:notify_automatic_deletion_to_user).and_call_original
+      allow(DossierMailer).to receive(:notify_automatic_deletion_to_administration).and_call_original
     end
 
     context 'with a single dossier' do
@@ -275,8 +275,8 @@ describe ExpiredDossiersDeletionService do
     after  { Timecop.return }
 
     before do
-      allow(DossierMailer).to receive(:notify_near_deletion_to_user).and_return(double(deliver_later: nil))
-      allow(DossierMailer).to receive(:notify_near_deletion_to_administration).and_return(double(deliver_later: nil))
+      allow(DossierMailer).to receive(:notify_near_deletion_to_user).and_call_original
+      allow(DossierMailer).to receive(:notify_near_deletion_to_administration).and_call_original
     end
 
     context 'with a single dossier' do
@@ -344,8 +344,8 @@ describe ExpiredDossiersDeletionService do
     after  { Timecop.return }
 
     before do
-      allow(DossierMailer).to receive(:notify_automatic_deletion_to_user).and_return(double(deliver_later: nil))
-      allow(DossierMailer).to receive(:notify_automatic_deletion_to_administration).and_return(double(deliver_later: nil))
+      allow(DossierMailer).to receive(:notify_automatic_deletion_to_user).and_call_original
+      allow(DossierMailer).to receive(:notify_automatic_deletion_to_administration).and_call_original
     end
 
     context 'with a single dossier' do