prevent B:BP batches from being marked in-transit if uploading the batch fails, ...

[freeside.git] / FS / FS / pay_batch.pm

diff --git a/FS/FS/pay_batch.pm b/FS/FS/pay_batch.pm
index 2a522b4..4aeb331 100644 (file)
--- a/FS/FS/pay_batch.pm
+++ b/FS/FS/pay_batch.pm
@@ -9,11 +9,12 @@ use List::Util qw(sum);
 use Time::Local;
 use Text::CSV_XS;
 use Date::Parse qw(str2time);
-use Business::CreditCard qw(cardtype);
-use FS::Misc qw(send_email); # for error notification
+use Business::CreditCard qw( 0.35 cardtype );
 use FS::Record qw( dbh qsearch qsearchs );
 use FS::Conf;
 use FS::cust_pay;
+use FS::Log;
+use Try::Tiny;
 
 =head1 NAME
 
@@ -567,8 +568,8 @@ sub import_from_gateway {
   );
 
   my @item_errors;
-  my $mail_on_error = $conf->config('batch-errors_to');
-  if ( $mail_on_error ) {
+  my $errors_not_fatal = $conf->config('batch-errors_not_fatal');
+  if ( $errors_not_fatal ) {
     # construct error trap
     $proc_opt{'on_parse_error'} = sub {
       my ($self, $line, $error) = @_;
@@ -614,7 +615,8 @@ sub import_from_gateway {
       my $error;
 
       my $paybatch = $gateway->gatewaynum .  '-' .  $gateway->gateway_module .
-        ':' . $item->authorization .  ':' . $item->order_number;
+        ':' . ($item->authorization || '') .
+        ':' . ($item->order_number || '');
 
       if ( $batch->incoming ) {
         # This is a one-way batch.
@@ -801,15 +803,10 @@ sub import_from_gateway {
       "Errors during batch import: ".scalar(@item_errors),
       @item_errors
     );
-    if ( $mail_on_error ) {
-      my $subject = "Batch import errors"; #?
-      my $body = "Import from gateway ".$gateway->label."\n".$error_text;
-      send_email(
-        to      => $mail_on_error,
-        from    => $conf->invoice_from_full(),
-        subject => $subject,
-        body    => $body,
-      );
+    if ( $errors_not_fatal ) {
+      my $message = "Import from gateway ".$gateway->label." errors: ".$error_text;
+      my $log = FS::Log->new('FS::pay_batch::import_from_gateway');
+      $log->error($message);
     } else {
       # Bail out.
       $dbh->rollback if $oldAutoCommit;
@@ -893,7 +890,8 @@ Prepare the batch to be exported.  This will:
   increment expiration dates that are in the past.
 - If this is the first download for this batch, adjust payment amounts to 
   not be greater than the customer's current balance.  If the customer's 
-  balance is zero, the entry will be removed.
+  balance is zero, the entry will be removed (caution: all cust_pay_batch
+  entries might be removed!)
 
 Use this within a transaction.
 
@@ -908,8 +906,6 @@ sub prepare_for_export {
   my $status = $self->status;
   if ($status eq 'O') {
     $first_download = 1;
-    my $error = $self->set_status('I');
-    return "error updating pay_batch status: $error\n" if $error;
   } elsif ($status eq 'I' && $curuser->access_right('Reprocess batches')) {
     $first_download = 0;
   } elsif ($status eq 'R' && 
@@ -943,7 +939,7 @@ sub prepare_for_export {
 
       my $balance = $cust_pay_batch->cust_main->balance;
       if ($balance <= 0) { # then don't charge this customer
-        my $error = $cust_pay_batch->delete;
+        my $error = $cust_pay_batch->unbatch_and_delete;
         return $error if $error;
       } elsif ($balance < $cust_pay_batch->amount) {
         # reduce the charge to the remaining balance
@@ -953,6 +949,11 @@ sub prepare_for_export {
       }
       # else $balance >= $cust_pay_batch->amount
     }
+
+    #need to do this after unbatch_and_delete
+    my $error = $self->set_status('I');
+    return "error updating pay_batch status: $error\n" if $error;
+
   } #if $first_download
 
   '';
@@ -966,6 +967,10 @@ module, in which case the configuration options are in 'batchconfig-FORMAT'.
 Alternatively, GATEWAY can be an L<FS::payment_gateway> object set to a
 L<Business::BatchPayment> module.
 
+Returns the text of the batch.  If batch contains no cust_pay_batch entries
+(or has them all removed by L</prepare_for_export>) then the batch will be 
+resolved and a blank string will be returned.  All other errors are fatal.
+
 =cut
 
 sub export_batch {
@@ -1001,6 +1006,12 @@ sub export_batch {
   my $batchcount = 0;
 
   my @cust_pay_batch = $self->cust_pay_batch;
+  unless (@cust_pay_batch) {
+    # if it's empty, just resolve the batch
+    $self->set_status('R');
+    $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+    return '';
+  }
 
   my $delim = exists($info->{'delimiter'}) ? $info->{'delimiter'} : "\n";
 
@@ -1045,6 +1056,10 @@ that gateway via Business::BatchPayment. OPTIONS may include:
 
 - file: override the default transport and write to this file (name or handle)
 
+If batch contains no cust_pay_batch entries (or has them all removed by 
+L</prepare_for_export>) then nothing will be transported (or written to 
+the override file) and the batch will be resolved.
+
 =cut
 
 sub export_to_gateway {
@@ -1065,17 +1080,29 @@ sub export_to_gateway {
   my $processor = $gateway->batch_processor(%proc_opt);
 
   my @items = map { $_->request_item } $self->cust_pay_batch;
-  my $batch = Business::BatchPayment->create(Batch =>
-    batch_id  => $self->batchnum,
-    items     => \@items
-  );
-  $processor->submit($batch);
-
-  if ($batch->processor_id) {
-    $self->set('processor_id',$batch->processor_id);
-    $self->replace;
+  unless (@items) {
+    # if it's empty, just resolve the batch
+    $self->set_status('R');
+    $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+    return '';
   }
 
+  try {
+    my $batch = Business::BatchPayment->create(Batch =>
+      batch_id  => $self->batchnum,
+      items     => \@items
+    );
+    $processor->submit($batch);
+
+    if ($batch->processor_id) {
+      $self->set('processor_id',$batch->processor_id);
+      $self->replace;
+    }
+  } catch {
+    $dbh->rollback if $oldAutoCommit;
+    die $_;
+  };
+
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   '';
 }