From: Jonathan Prykop Date: Tue, 22 Dec 2015 06:28:53 +0000 (-0600) Subject: RT#34295: Error when attempting to create batch payments [better handling of empty... X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=commitdiff_plain;h=1259a17db297fa2352619b29f2c5bd34e313cd64 RT#34295: Error when attempting to create batch payments [better handling of empty batches] --- diff --git a/FS/FS/cust_main/Billing_Batch.pm b/FS/FS/cust_main/Billing_Batch.pm index cdaf2938f..f91c5fbdc 100644 --- a/FS/FS/cust_main/Billing_Batch.pm +++ b/FS/FS/cust_main/Billing_Batch.pm @@ -23,8 +23,6 @@ Options may include: B: the amount to be paid; defaults to the customer's balance minus any payments in transit. -B: the payment method; defaults to cust_main.payby - B: runs this as a realtime payment instead of adding it to a batch. Deprecated. @@ -34,8 +32,9 @@ B, B, B, B, B, B: sets the billing address for the payment; defaults to the customer's billing location. -B, B, B: sets the payment account, expiration -date, and name; defaults to those fields in cust_main. +B, B, B, B: sets the payment method, +payment account, expiration date, and name; defaults to those fields +in cust_main. =cut @@ -58,6 +57,13 @@ sub batch_card { my $invnum = delete $options{invnum}; + #pay fields should all come from either cust_payby or options, not both + # in theory, could just pass payby, and use it to select cust_payby, + # but nothing currently needs that, so not implementing it now + die "Incomplete payment details" + if ($options{payby} || $options{payinfo} || $options{paydate} || $options{payname}) + && !($options{payby} && $options{payinfo} && $options{paydate} && $options{payname}); + #false laziness with Billing_Realtime my @cust_payby = qsearch({ 'table' => 'cust_payby', @@ -67,7 +73,10 @@ sub batch_card { }); # batch can't try out every one like realtime, just use first one - my $cust_payby = $cust_payby[0] || $self; # somewhat dubious + my $cust_payby = $cust_payby[0]; + + die "No customer payment info found" + unless $options{payinfo} || $cust_payby; my $payby = $options{payby} || $cust_payby->payby; diff --git a/FS/FS/pay_batch.pm b/FS/FS/pay_batch.pm index e299dd9c7..35c79f50b 100644 --- a/FS/FS/pay_batch.pm +++ b/FS/FS/pay_batch.pm @@ -888,7 +888,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. @@ -947,15 +948,6 @@ 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; @@ -973,6 +965,10 @@ module, in which case the configuration options are in 'batchconfig-FORMAT'. Alternatively, GATEWAY can be an L object set to a L module. +Returns the text of the batch. If batch contains no cust_pay_batch entries +(or has them all removed by L) then the batch will be +resolved and a blank string will be returned. All other errors are fatal. + =cut sub export_batch { @@ -1008,6 +1004,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"; @@ -1052,6 +1054,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) then nothing will be transported (or written to +the override file) and the batch will be resolved. + =cut sub export_to_gateway { @@ -1072,6 +1078,13 @@ sub export_to_gateway { my $processor = $gateway->batch_processor(%proc_opt); my @items = map { $_->request_item } $self->cust_pay_batch; + unless (@items) { + # if it's empty, just resolve the batch + $self->set_status('R'); + $dbh->commit or die $dbh->errstr if $oldAutoCommit; + return ''; + } + my $batch = Business::BatchPayment->create(Batch => batch_id => $self->batchnum, items => \@items diff --git a/FS/bin/freeside-eftca-upload b/FS/bin/freeside-eftca-upload index b66765af3..107aa19ce 100755 --- a/FS/bin/freeside-eftca-upload +++ b/FS/bin/freeside-eftca-upload @@ -47,6 +47,10 @@ foreach my $pay_batch (@batches) { my $filename = time2str('%Y%m%d', time) . '-' . sprintf('%06d.csv',$batchnum); print STDERR "Exporting batch $batchnum to $filename...\n" if $opt_v; my $text = $pay_batch->export_batch(format => 'eft_canada'); + unless ($text) { + print STDERR "Batch is empty, resolving..." if $opt_v; + next; + } open OUT, ">$tmpdir/$filename"; print OUT $text; close OUT; diff --git a/FS/bin/freeside-paymentech-upload b/FS/bin/freeside-paymentech-upload index 5ae147d07..799e6c42c 100755 --- a/FS/bin/freeside-paymentech-upload +++ b/FS/bin/freeside-paymentech-upload @@ -68,6 +68,10 @@ foreach my $pay_batch (@batches) { my $filename = sprintf('%06d',$batchnum) . '-' .time2str('%Y%m%d%H%M%S', time); print STDERR "Exporting batch $batchnum to $filename...\n" if $opt_v; my $text = $pay_batch->export_batch(format => 'paymentech'); + unless ($text) { + print STDERR "Batch is empty, resolving..." if $opt_v; + next; + } $text =~ s!FILEID!$filename! or log_and_die("couldn't find FILEID tag\n"); open OUT, ">$tmpdir/$filename.xml"; @@ -80,6 +84,7 @@ foreach my $pay_batch (@batches) { log_and_die("failed to create zip file\n") if (! -f "$tmpdir/$filename.zip" ); push @filenames, $filename; } +log_and_die("All batches empty\n") if !@filenames; my $host = ($opt_t ? 'orbitalbatchvar.paymentech.net' : 'orbitalbatch.paymentech.net'); diff --git a/FS/bin/freeside-rbc-upload b/FS/bin/freeside-rbc-upload index 52501028c..3fff32a67 100755 --- a/FS/bin/freeside-rbc-upload +++ b/FS/bin/freeside-rbc-upload @@ -70,6 +70,10 @@ foreach my $pay_batch (@batches) { debug "Exporting batch $batchnum to $filename\n"; my $text = $pay_batch->export_batch(format => 'RBC'); + unless ($text) { + print STDERR "Batch is empty, resolving..." if $opt_v; + next; + } write_file("$tmpdir/$filename", $text); debug "Uploading $filename..."; diff --git a/httemplate/misc/download-batch.cgi b/httemplate/misc/download-batch.cgi index f3a31eb3b..c4bc37e93 100644 --- a/httemplate/misc/download-batch.cgi +++ b/httemplate/misc/download-batch.cgi @@ -1,4 +1,4 @@ -<% $pay_batch->export_batch(%opt) %><%init> +<% $exporttext %><%init> #http_header('Content-Type' => 'text/comma-separated-values' ); #IE chokes http_header('Content-Type' => 'text/plain' ); # not necessarily correct... @@ -23,4 +23,15 @@ elsif ( $cgi->param('format') =~ /^([\w\- ]+)$/ ) { my $pay_batch = qsearchs('pay_batch', { batchnum => $batchnum } ); die "Batch not found: '$batchnum'" if !$pay_batch; +my $exporttext = $pay_batch->export_batch(%opt); +unless ($exporttext) { + http_header('Content-Type' => 'text/html' ); + $exporttext = < +alert('Batch was empty, and has been resolved'); +window.top.location.href = '${p}search/pay_batch.cgi?magic=_date;open=1;intransit=1;resolved=1'; + +EOF +} +