[freeside-commits] branch FREESIDE_4_BRANCH updated. 1667ab631f9fbd0f4ceeb6ec000b0f4ff3ddb51a
Jonathan Prykop
jonathan at 420.am
Tue Jan 5 19:20:20 PST 2016
The branch, FREESIDE_4_BRANCH has been updated
via 1667ab631f9fbd0f4ceeb6ec000b0f4ff3ddb51a (commit)
via 9088495eea6c836c7086dec1e76839ed8e93e0be (commit)
from f8b24f482b7e48ac731115b877fbbdc49aa54c76 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 1667ab631f9fbd0f4ceeb6ec000b0f4ff3ddb51a
Author: Jonathan Prykop <jonathan at freeside.biz>
Date: Tue Dec 22 00:28:53 2015 -0600
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 cdaf293..f91c5fb 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<amount>: the amount to be paid; defaults to the customer's balance minus
any payments in transit.
-B<payby>: the payment method; defaults to cust_main.payby
-
B<realtime>: runs this as a realtime payment instead of adding it to a
batch. Deprecated.
@@ -34,8 +32,9 @@ B<address1>, B<address2>, B<city>, B<state>, B<zip>, B<country>: sets
the billing address for the payment; defaults to the customer's billing
location.
-B<payinfo>, B<paydate>, B<payname>: sets the payment account, expiration
-date, and name; defaults to those fields in cust_main.
+B<payby>, B<payinfo>, B<paydate>, B<payname>: 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 e299dd9..35c79f5 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<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 {
@@ -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</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 {
@@ -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 b66765a..107aa19 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 5ae147d..799e6c4 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>FILEID</fileID>!<fileID>$filename</fileID>!
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 5250102..3fff32a 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 f3a31eb..c4bc37e 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 = <<EOF;
+<SCRIPT>
+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';
+</SCRIPT>
+EOF
+}
+
</%init>
commit 9088495eea6c836c7086dec1e76839ed8e93e0be
Author: Jonathan Prykop <jonathan at freeside.biz>
Date: Wed Oct 21 21:19:16 2015 -0500
RT#34295: Error when attempting to create batch payments
diff --git a/FS/FS/cust_main/Billing_Batch.pm b/FS/FS/cust_main/Billing_Batch.pm
index 2f68d7f..cdaf293 100644
--- a/FS/FS/cust_main/Billing_Batch.pm
+++ b/FS/FS/cust_main/Billing_Batch.pm
@@ -57,10 +57,22 @@ sub batch_card {
}
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,
);
@@ -120,10 +132,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 d7dd7bb..e299dd9 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
'';
-----------------------------------------------------------------------
Summary of changes:
FS/FS/cust_main/Billing_Batch.pm | 41 +++++++++++++++++++++++++++---------
FS/FS/pay_batch.pm | 33 +++++++++++++++++++++++++----
FS/bin/freeside-eftca-upload | 4 ++++
FS/bin/freeside-paymentech-upload | 5 +++++
FS/bin/freeside-rbc-upload | 4 ++++
httemplate/misc/download-batch.cgi | 13 +++++++++++-
6 files changed, 85 insertions(+), 15 deletions(-)
More information about the freeside-commits
mailing list