RT#34295: Error when attempting to create batch payments

2 files changed, 33 insertions, 9 deletions

diff --git a/FS/FS/cust_main/Billing_Batch.pm b/FS/FS/cust_main/Billing_Batch.pm
index c2416ab32..0cf2beeda 100644
--- a/FS/FS/cust_main/Billing_Batch.pm
+++ b/FS/FS/cust_main/Billing_Batch.pm
@@ -51,10 +51,22 @@ sub batch_card {
   return '' unless $amount > 0;
   
   my $invnum = delete $options{invnum};
-  my $payby = $options{payby} || $self->payby;  #still dubious
+
+  #false laziness with Billing_Realtime
+  my @cust_payby = qsearch({
+    'table'     => 'cust_payby',
+    'hashref'   => { 'custnum' => $self->custnum, },
+    'extra_sql' => " AND payby IN ( 'CARD', 'CHEK' ) ",
+    'order_by'  => 'ORDER BY weight ASC',
+  });
+
+  # batch can't try out every one like realtime, just use first one
+  my $cust_payby = $cust_payby[0] || $self; # somewhat dubious
+                                                   
+  my $payby = $options{payby} || $cust_payby->payby;
 
   if ($options{'realtime'}) {
-    return $self->realtime_bop( FS::payby->payby2bop($self->payby),
+    return $self->realtime_bop( FS::payby->payby2bop($payby),
                                 $amount,
                                 %options,
                               );
@@ -114,10 +126,10 @@ sub batch_card {
     'state'    => $options{state}    || $loc->state,
     'zip'      => $options{zip}      || $loc->zip,
     'country'  => $options{country}  || $loc->country,
-    'payby'    => $options{payby}    || $self->payby,
-    'payinfo'  => $options{payinfo}  || $self->payinfo,
-    'exp'      => $options{paydate}  || $self->paydate,
-    'payname'  => $options{payname}  || $self->payname,
+    'payby'    => $options{payby}    || $cust_payby->payby,
+    'payinfo'  => $options{payinfo}  || $cust_payby->payinfo,
+    'exp'      => $options{paydate}  || $cust_payby->paydate,
+    'payname'  => $options{payname}  || $cust_payby->payname,
     'amount'   => $amount,                         # consolidating
   } );
   
diff --git a/FS/FS/pay_batch.pm b/FS/FS/pay_batch.pm
index d7dd7bbe4..e299dd9c7 100644
--- a/FS/FS/pay_batch.pm
+++ b/FS/FS/pay_batch.pm
@@ -903,8 +903,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' && 
@@ -938,7 +936,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
@@ -948,6 +946,20 @@ sub prepare_for_export {
       }
       # else $balance >= $cust_pay_batch->amount
     }
+
+    # we might end up removing all cust_pay_batch above...
+    # probably the better way to handle this is to commit that removal,
+    # but no time to trace code & test that right now
+    #
+    # additionally, UI currently allows hand-deletion of all payments from a batch, meaning
+    # it's possible to try and process an empty batch...this is where we catch
+    # such an attempt, though it probably shouldn't be possible in the first place
+    return "Batch is empty" unless $self->cust_pay_batch;
+
+    #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
 
   '';