Bug 3350 - fail on queued emails w/ no address
authorJoe Atzberger <joe.atzberger@liblime.com>
Mon, 22 Jun 2009 19:40:56 +0000 (14:40 -0500)
committerGalen Charlton <galen.charlton@liblime.com>
Wed, 24 Jun 2009 13:43:45 +0000 (08:43 -0500)
If a patron has no email address, we need to avoid stockpiling all
their messages indefinitely.  Otherwise they get mailbombed when
their email IS added.

Note that overdues should not be affected, since the overdues job
checks whether the patron email exists before sending the message
(falling back to the admin).  The other messaging features are the
targets affected by this patch.

Signed-off-by: Galen Charlton <galen.charlton@liblime.com>
C4/Letters.pm

index 3d83c88..f3ba145 100644 (file)
@@ -607,7 +607,7 @@ sub SendQueuedMessages (;$) {
         if ( lc( $message->{'message_transport_type'} ) eq 'email' ) {
             _send_message_by_email( $message );
         }
-        if ( lc( $message->{'message_transport_type'} ) eq 'sms' ) {
+        elsif ( lc( $message->{'message_transport_type'} ) eq 'sms' ) {
             _send_message_by_sms( $message );
         }
     }
@@ -761,24 +761,35 @@ ENDSQL
 }
 
 sub _send_message_by_email ($) {
-    my $message = shift or return undef;
-
-    my $member = C4::Members::GetMember( $message->{'borrowernumber'} );
-    return unless $message->{'to_address'} or $member->{'email'};
+    my $message = shift or return;
+
+    my $to_address = $message->{to_address};
+    unless ($to_address) {
+        my $member = C4::Members::GetMember( $message->{'borrowernumber'} );
+        unless ($member) {
+            warn "FAIL: No 'to_address' and INVALID borrowernumber ($message->{borrowernumber})";
+            _set_message_status( { message_id => $message->{'message_id'},
+                                   status     => 'failed' } );
+            return;
+        }
+        unless ($to_address = $member->{email}) {   # assigment, not comparison
+            # warn "FAIL: No 'to_address' and no email for " . ($member->{surname} ||'') . ", borrowernumber ($message->{borrowernumber})";
+            # warning too verbose for this more common case?
+            _set_message_status( { message_id => $message->{'message_id'},
+                                   status     => 'failed' } );
+            return;
+        }
+    }
 
        my $content = encode('utf8', $message->{'content'});
     my %sendmail_params = (
-        To   => $message->{'to_address'}   || $member->{'email'},
+        To   => $to_address,
         From => $message->{'from_address'} || C4::Context->preference('KohaAdminEmailAddress'),
         Subject => $message->{'subject'},
-               charset => 'utf8',
+        charset => 'utf8',
         Message => $content,
+        'content-type' => $message->{'content_type'} || 'text/plain; charset="UTF-8"',
     );
-    if ($message->{'content_type'}) {
-        $sendmail_params{'content-type'} = $message->{'content_type'};
-    }else{
-        $sendmail_params{'content-type'} = 'text/plain; charset="UTF-8"';
-    }
     
     my $success = sendmail( %sendmail_params );