From a8931b472b859b44ccb9df410aedd1eb1627350b Mon Sep 17 00:00:00 2001
From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Date: Fri, 14 Oct 2016 11:07:49 +0200
Subject: [PATCH] Bug 17441: [QA Follow-up] Return value of SendAlerts
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

This patch makes the return value of SendAlerts more consistent.
It returns 1 on success, or undef || { error => 'msg' } on failure.
Needed to adjust one test in Letters.t too.
Adjusted one typo along the way (seleted).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested by running Letters.t.
Also tested SendAlerts from the interface with AutoEmailOpacUser and
memberentry (adding new patron).

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
(cherry picked from commit 17e2e6c994655aaa927b2579524022a4f24a8d1d)
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
(cherry picked from commit 18a472e627e8f8b7cec45ffe90f896374d90fb9d)
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
---
 C4/Letters.pm            | 19 +++++++++++++++----
 t/db_dependent/Letters.t |  2 +-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/C4/Letters.pm b/C4/Letters.pm
index 28de2d1288..1746342316 100644
--- a/C4/Letters.pm
+++ b/C4/Letters.pm
@@ -371,6 +371,9 @@ sub findrelatedto {
 
     send an alert to all borrowers having put an alert on a given subject.
 
+    Returns undef or { error => 'message } on failure.
+    Returns true on success.
+
 =cut
 
 sub SendAlerts {
@@ -440,7 +443,10 @@ sub SendAlerts {
                                     : 'text/plain; charset="utf-8"',
                 }
             );
-            sendmail(%mail) or carp $Mail::Sendmail::error;
+            unless( sendmail(%mail) ) {
+                carp $Mail::Sendmail::error;
+                return { error => $Mail::Sendmail::error };
+            }
         }
     }
     elsif ( $type eq 'claimacquisition' or $type eq 'claimissues' ) {
@@ -468,7 +474,7 @@ sub SendAlerts {
 
         if (!@$externalid){
             carp "No Order seleted";
-            return { error => "no_order_seleted" };
+            return { error => "no_order_selected" };
         }
 
         $strsth .= join( ",", @$externalid ) . ")";
@@ -552,7 +558,6 @@ sub SendAlerts {
                 . " Content="
                 . $letter->{content}
         ) if C4::Context->preference("LetterLog");
-        1;
     }
    # send an "account details" notice to a newly created user
     elsif ( $type eq 'members' ) {
@@ -586,8 +591,14 @@ sub SendAlerts {
                                 : 'text/plain; charset="utf-8"',
             }
         );
-        sendmail(%mail) or carp $Mail::Sendmail::error;
+        unless( sendmail(%mail) ) {
+            carp $Mail::Sendmail::error;
+            return { error => $Mail::Sendmail::error };
+        }
     }
+
+    # If we come here, return an OK status
+    return 1;
 }
 
 =head2 GetPreparedLetter( %params )
diff --git a/t/db_dependent/Letters.t b/t/db_dependent/Letters.t
index 6bb4c82cc2..9a6ff13e32 100644
--- a/t/db_dependent/Letters.t
+++ b/t/db_dependent/Letters.t
@@ -457,7 +457,7 @@ warning_is {
 $err2 = SendAlerts( 'issue', $serial->{serialid}, 'RLIST' ) }
     "Fake sendmail",
     "SendAlerts is using the mocked sendmail routine";
-is($err2, "", "Successfully sent serial notification");
+is($err2, 1, "Successfully sent serial notification");
 is($mail{'To'}, 'john.smith@test.de', "mailto correct in sent serial notification");
 is($mail{'Message'}, 'Silence in the library,'.$subscriptionid.',No. 0', 'Serial notification text constructed successfully');
 }
-- 
2.39.5