Bug 17441: [QA Follow-up] Return value of SendAlerts

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>

diff --git a/C4/Letters.pm b/C4/Letters.pm
index de63de1..613300f 100644 (file)
--- a/C4/Letters.pm
+++ b/C4/Letters.pm
@@ -372,6 +372,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 {
@@ -441,7 +444,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' ) {
@@ -469,7 +475,7 @@ sub SendAlerts {
 
         if (!@$externalid){
             carp "No Order seleted";
-            return { error => "no_order_seleted" };
+            return { error => "no_order_selected" };
         }
 
         $strsth .= join( ",", @$externalid ) . ")";
@@ -555,7 +561,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' ) {
@@ -589,8 +594,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 39d11c7..0b7d125 100644 (file)
--- a/t/db_dependent/Letters.t
+++ b/t/db_dependent/Letters.t
@@ -467,7 +467,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');
 }