[freeside-commits] branch FREESIDE_3_BRANCH updated. 8a90342b21d4ae2e132bdbc12b9bb6523e8847e4
Mark Wells
mark at 420.am
Tue Aug 30 16:37:55 PDT 2016
The branch, FREESIDE_3_BRANCH has been updated
via 8a90342b21d4ae2e132bdbc12b9bb6523e8847e4 (commit)
via 5efca2378f5eb82e5c807cbcc66c72d91fd267c3 (commit)
from b44c17c7d9686e3a0227f15a567fe39de508160f (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 8a90342b21d4ae2e132bdbc12b9bb6523e8847e4
Author: Mark Wells <mark at freeside.biz>
Date: Wed Aug 24 17:53:40 2016 -0700
but keep failed verification payments linked to their customers, #57135
diff --git a/FS/FS/cust_main/Billing_Realtime.pm b/FS/FS/cust_main/Billing_Realtime.pm
index dc0eea6..5d35fc2 100644
--- a/FS/FS/cust_main/Billing_Realtime.pm
+++ b/FS/FS/cust_main/Billing_Realtime.pm
@@ -1862,10 +1862,19 @@ sub realtime_verify_bop {
if defined($options{payunique}) && length($options{payunique});
IMMEDIATE: {
- # open a separate handle for creating/updating the cust_pay_pending record
+ # open a separate handle for creating/updating the cust_pay_pending
+ # record
local $FS::UID::dbh = myconnect();
local $FS::UID::AutoCommit = 1;
+ # if this is an existing customer (and we can tell now because
+ # this is a fresh transaction), it's safe to assign their custnum
+ # to the cust_pay_pending record, and then the verification attempt
+ # will remain linked to them even if it fails.
+ if ( FS::cust_main->by_key($self->custnum) ) {
+ $cust_pay_pending->set('custnum', $self->custnum);
+ }
+
warn "inserting cust_pay_pending record for customer ". $self->custnum. "\n"
if $DEBUG > 1;
@@ -2092,12 +2101,14 @@ sub realtime_verify_bop {
# the cust_main)
###
- $cust_pay_pending->set('custnum', $self->custnum);
- my $set_custnum_err = $cust_pay_pending->replace;
- if ($set_custnum_err) {
- $log->error($set_custnum_err);
- $error ||= $set_custnum_err;
- # but if there was a real verification error also, return that one
+ if (!$cust_pay_pending->custnum) {
+ $cust_pay_pending->set('custnum', $self->custnum);
+ my $set_custnum_err = $cust_pay_pending->replace;
+ if ($set_custnum_err) {
+ $log->error($set_custnum_err);
+ $error ||= $set_custnum_err;
+ # but if there was a real verification error also, return that one
+ }
}
###
commit 5efca2378f5eb82e5c807cbcc66c72d91fd267c3
Author: Mark Wells <mark at freeside.biz>
Date: Wed Aug 24 17:17:03 2016 -0700
de-transactionize cust_pay_pending updates during card verification, #57135
diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 980cd21..b7ec7df 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -1713,7 +1713,7 @@ sub tables_hashref {
'cust_pay_pending' => {
'columns' => [
'paypendingnum','serial', '', '', '', '',
- 'custnum', 'int', '', '', '', '',
+ 'custnum', 'int', 'NULL', '', '', '',
'paid', @money_type, '', '',
'_date', @date_type, '', '',
'payby', 'char', '', 4, '', '', #CARD/BILL/COMP, should
diff --git a/FS/FS/cust_main/Billing_Realtime.pm b/FS/FS/cust_main/Billing_Realtime.pm
index 7c194e1..dc0eea6 100644
--- a/FS/FS/cust_main/Billing_Realtime.pm
+++ b/FS/FS/cust_main/Billing_Realtime.pm
@@ -5,7 +5,7 @@ use vars qw( $conf $DEBUG $me );
use vars qw( $realtime_bop_decline_quiet ); #ugh
use Data::Dumper;
use Business::CreditCard 0.35;
-use FS::UID qw( dbh );
+use FS::UID qw( dbh myconnect );
use FS::Record qw( qsearch qsearchs );
use FS::Misc qw( send_email );
use FS::payby;
@@ -1731,6 +1731,7 @@ sub realtime_verify_bop {
my $self = shift;
local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
+ my $log = FS::Log->new('FS::cust_main::Billing_Realtime::realtime_verify_bop');
my %options = ();
if (ref($_[0]) eq 'HASH') {
@@ -1836,29 +1837,14 @@ sub realtime_verify_bop {
# run transaction(s)
###
- warn "claiming mutex on customer ". $self->custnum. "\n" if $DEBUG > 1;
- $self->select_for_update; #mutex ... just until we get our pending record in
- warn "obtained mutex on customer ". $self->custnum. "\n" if $DEBUG > 1;
-
- #the checks here are intended to catch concurrent payments
- #double-form-submission prevention is taken care of in cust_pay_pending::check
-
- #also check and make sure there aren't *other* pending payments for this cust
-
- my @pending = qsearch('cust_pay_pending', {
- 'custnum' => $self->custnum,
- 'status' => { op=>'!=', value=>'done' }
- });
-
- return "A payment is already being processed for this customer (".
- join(', ', map 'paypendingnum '. $_->paypendingnum, @pending ).
- "); verification transaction aborted."
- if scalar(@pending);
-
- #okay, good to go, if we're a duplicate, cust_pay_pending will kick us out
+ my $error;
+ my $transaction; #need this back so we can do _tokenize_card
+ # don't mutex the customer here, because they might be uncommitted. and
+ # this is only verification. it doesn't matter if they have other
+ # unfinished verifications.
my $cust_pay_pending = new FS::cust_pay_pending {
- 'custnum' => $self->custnum,
+ 'custnum_pending' => 1,
'paid' => '1.00',
'_date' => '',
'payby' => $bop_method2payby{'CC'},
@@ -1875,221 +1861,243 @@ sub realtime_verify_bop {
$cust_pay_pending->payunique( $options{payunique} )
if defined($options{payunique}) && length($options{payunique});
- warn "inserting cust_pay_pending record for customer ". $self->custnum. "\n"
- if $DEBUG > 1;
- my $cpp_new_err = $cust_pay_pending->insert; #mutex lost when this is inserted
- return $cpp_new_err if $cpp_new_err;
+ IMMEDIATE: {
+ # open a separate handle for creating/updating the cust_pay_pending record
+ local $FS::UID::dbh = myconnect();
+ local $FS::UID::AutoCommit = 1;
- warn "inserted cust_pay_pending record for customer ". $self->custnum. "\n"
- if $DEBUG > 1;
- warn Dumper($cust_pay_pending) if $DEBUG > 2;
+ warn "inserting cust_pay_pending record for customer ". $self->custnum. "\n"
+ if $DEBUG > 1;
- my $transaction = new $namespace( $payment_gateway->gateway_module,
- $self->_bop_options(\%options),
- );
+ # if this fails, just return; everything else will still allow the
+ # cust_pay_pending to have its custnum set later
+ my $cpp_new_err = $cust_pay_pending->insert;
+ return $cpp_new_err if $cpp_new_err;
- $transaction->content(
- 'type' => 'CC',
- $self->_bop_auth(\%options),
- 'action' => 'Authorization Only',
- 'description' => $options{'description'},
- 'amount' => '1.00',
- #'invoice_number' => $options{'invnum'},
- 'customer_id' => $self->custnum,
- %$bop_content,
- 'reference' => $cust_pay_pending->paypendingnum, #for now
- 'callback_url' => $payment_gateway->gateway_callback_url,
- 'cancel_url' => $payment_gateway->gateway_cancel_url,
- 'email' => $email,
- %content, #after
- );
+ warn "inserted cust_pay_pending record for customer ". $self->custnum. "\n"
+ if $DEBUG > 1;
+ warn Dumper($cust_pay_pending) if $DEBUG > 2;
- $cust_pay_pending->status('pending');
- my $cpp_pending_err = $cust_pay_pending->replace;
- return $cpp_pending_err if $cpp_pending_err;
+ $transaction = new $namespace( $payment_gateway->gateway_module,
+ $self->_bop_options(\%options),
+ );
- warn Dumper($transaction) if $DEBUG > 2;
+ $transaction->content(
+ 'type' => 'CC',
+ $self->_bop_auth(\%options),
+ 'action' => 'Authorization Only',
+ 'description' => $options{'description'},
+ 'amount' => '1.00',
+ #'invoice_number' => $options{'invnum'},
+ 'customer_id' => $self->custnum,
+ %$bop_content,
+ 'reference' => $cust_pay_pending->paypendingnum, #for now
+ 'callback_url' => $payment_gateway->gateway_callback_url,
+ 'cancel_url' => $payment_gateway->gateway_cancel_url,
+ 'email' => $email,
+ %content, #after
+ );
- unless ( $BOP_TESTING ) {
- $transaction->test_transaction(1)
- if $conf->exists('business-onlinepayment-test_transaction');
- $transaction->submit();
- } else {
- if ( $BOP_TESTING_SUCCESS ) {
- $transaction->is_success(1);
- $transaction->authorization('fake auth');
+ $cust_pay_pending->status('pending');
+ my $cpp_pending_err = $cust_pay_pending->replace;
+ return $cpp_pending_err if $cpp_pending_err;
+
+ warn Dumper($transaction) if $DEBUG > 2;
+
+ unless ( $BOP_TESTING ) {
+ $transaction->test_transaction(1)
+ if $conf->exists('business-onlinepayment-test_transaction');
+ $transaction->submit();
} else {
- $transaction->is_success(0);
- $transaction->error_message('fake failure');
+ if ( $BOP_TESTING_SUCCESS ) {
+ $transaction->is_success(1);
+ $transaction->authorization('fake auth');
+ } else {
+ $transaction->is_success(0);
+ $transaction->error_message('fake failure');
+ }
}
- }
- my $log = FS::Log->new('FS::cust_main::Billing_Realtime::realtime_verify_bop');
+ if ( $transaction->is_success() ) {
- if ( $transaction->is_success() ) {
+ $cust_pay_pending->status('authorized');
+ my $cpp_authorized_err = $cust_pay_pending->replace;
+ return $cpp_authorized_err if $cpp_authorized_err;
- $cust_pay_pending->status('authorized');
- my $cpp_authorized_err = $cust_pay_pending->replace;
- return $cpp_authorized_err if $cpp_authorized_err;
+ my $auth = $transaction->authorization;
+ my $ordernum = $transaction->can('order_number')
+ ? $transaction->order_number
+ : '';
- my $auth = $transaction->authorization;
- my $ordernum = $transaction->can('order_number')
- ? $transaction->order_number
- : '';
+ my $reverse = new $namespace( $payment_gateway->gateway_module,
+ $self->_bop_options(\%options),
+ );
- my $reverse = new $namespace( $payment_gateway->gateway_module,
- $self->_bop_options(\%options),
- );
+ $reverse->content( 'action' => 'Reverse Authorization',
+ $self->_bop_auth(\%options),
- $reverse->content( 'action' => 'Reverse Authorization',
- $self->_bop_auth(\%options),
+ # B:OP
+ 'amount' => '1.00',
+ 'authorization' => $transaction->authorization,
+ 'order_number' => $ordernum,
- # B:OP
- 'amount' => '1.00',
- 'authorization' => $transaction->authorization,
- 'order_number' => $ordernum,
+ # vsecure
+ 'result_code' => $transaction->result_code,
+ 'txn_date' => $transaction->txn_date,
- # vsecure
- 'result_code' => $transaction->result_code,
- 'txn_date' => $transaction->txn_date,
+ %content,
+ );
+ $reverse->test_transaction(1)
+ if $conf->exists('business-onlinepayment-test_transaction');
+ $reverse->submit();
- %content,
- );
- $reverse->test_transaction(1)
- if $conf->exists('business-onlinepayment-test_transaction');
- $reverse->submit();
+ if ( $reverse->is_success ) {
- if ( $reverse->is_success ) {
+ $cust_pay_pending->status('done');
+ $cust_pay_pending->statustext('reversed');
+ my $cpp_reversed_err = $cust_pay_pending->replace;
+ return $cpp_reversed_err if $cpp_reversed_err;
- $cust_pay_pending->status('done');
- $cust_pay_pending->statustext('reversed');
- my $cpp_authorized_err = $cust_pay_pending->replace;
- return $cpp_authorized_err if $cpp_authorized_err;
+ } else {
- } else {
+ my $e = "Authorization successful but reversal failed, custnum #".
+ $self->custnum. ': '. $reverse->result_code.
+ ": ". $reverse->error_message;
+ $log->warning($e);
+ warn $e;
+ return $e;
- my $e = "Authorization successful but reversal failed, custnum #".
- $self->custnum. ': '. $reverse->result_code.
- ": ". $reverse->error_message;
- $log->warning($e);
- warn $e;
- return $e;
+ }
- }
+ ### Address Verification ###
+ #
+ # Single-letter codes vary by cardtype.
+ #
+ # Erring on the side of accepting cards if avs is not available,
+ # only rejecting if avs occurred and there's been an explicit mismatch
+ #
+ # Charts below taken from vSecure documentation,
+ # shows codes for Amex/Dscv/MC/Visa
+ #
+ # ACCEPTABLE AVS RESPONSES:
+ # Both Address and 5-digit postal code match Y A Y Y
+ # Both address and 9-digit postal code match Y A X Y
+ # United Kingdom – Address and postal code match _ _ _ F
+ # International transaction – Address and postal code match _ _ _ D/M
+ #
+ # ACCEPTABLE, BUT ISSUE A WARNING:
+ # Ineligible transaction; or message contains a content error _ _ _ E
+ # System unavailable; retry R U R R
+ # Information unavailable U W U U
+ # Issuer does not support AVS S U S S
+ # AVS is not applicable _ _ _ S
+ # Incompatible formats – Not verified _ _ _ C
+ # Incompatible formats – Address not verified; postal code matches _ _ _ P
+ # International transaction – address not verified _ G _ G/I
+ #
+ # UNACCEPTABLE AVS RESPONSES:
+ # Only Address matches A Y A A
+ # Only 5-digit postal code matches Z Z Z Z
+ # Only 9-digit postal code matches Z Z W W
+ # Neither address nor postal code matches N N N N
+
+ if (my $avscode = uc($transaction->avs_code)) {
+
+ # map codes to accept/warn/reject
+ my $avs = {
+ 'American Express card' => {
+ 'A' => 'r',
+ 'N' => 'r',
+ 'R' => 'w',
+ 'S' => 'w',
+ 'U' => 'w',
+ 'Y' => 'a',
+ 'Z' => 'r',
+ },
+ 'Discover card' => {
+ 'A' => 'a',
+ 'G' => 'w',
+ 'N' => 'r',
+ 'U' => 'w',
+ 'W' => 'w',
+ 'Y' => 'r',
+ 'Z' => 'r',
+ },
+ 'MasterCard' => {
+ 'A' => 'r',
+ 'N' => 'r',
+ 'R' => 'w',
+ 'S' => 'w',
+ 'U' => 'w',
+ 'W' => 'r',
+ 'X' => 'a',
+ 'Y' => 'a',
+ 'Z' => 'r',
+ },
+ 'VISA card' => {
+ 'A' => 'r',
+ 'C' => 'w',
+ 'D' => 'a',
+ 'E' => 'w',
+ 'F' => 'a',
+ 'G' => 'w',
+ 'I' => 'w',
+ 'M' => 'a',
+ 'N' => 'r',
+ 'P' => 'w',
+ 'R' => 'w',
+ 'S' => 'w',
+ 'U' => 'w',
+ 'W' => 'r',
+ 'Y' => 'a',
+ 'Z' => 'r',
+ },
+ };
+ my $cardtype = cardtype($content{card_number});
+ if ($avs->{$cardtype}) {
+ my $avsact = $avs->{$cardtype}->{$avscode};
+ my $warning = '';
+ if ($avsact eq 'r') {
+ return "AVS code verification failed, cardtype $cardtype, code $avscode";
+ } elsif ($avsact eq 'w') {
+ $warning = "AVS did not occur, cardtype $cardtype, code $avscode";
+ } elsif (!$avsact) {
+ $warning = "AVS code unknown, cardtype $cardtype, code $avscode";
+ } # else $avsact eq 'a'
+ if ($warning) {
+ $log->warning($warning);
+ warn $warning;
+ }
+ } # else $cardtype avs handling not implemented
+ } # else !$transaction->avs_code
+
+ } else { # is not success
+
+ # status is 'done' not 'declined', as in _realtime_bop_result
+ $cust_pay_pending->status('done');
+ $error = $transaction->error_message || 'Unknown error';
+ $cust_pay_pending->statustext($error);
+ # could also record failure_status here,
+ # but it's not supported by B::OP::vSecureProcessing...
+ # need a B::OP module with (reverse) auth only to test it with
+ my $cpp_declined_err = $cust_pay_pending->replace;
+ return $cpp_declined_err if $cpp_declined_err;
- ### Address Verification ###
- #
- # Single-letter codes vary by cardtype.
- #
- # Erring on the side of accepting cards if avs is not available,
- # only rejecting if avs occurred and there's been an explicit mismatch
- #
- # Charts below taken from vSecure documentation,
- # shows codes for Amex/Dscv/MC/Visa
- #
- # ACCEPTABLE AVS RESPONSES:
- # Both Address and 5-digit postal code match Y A Y Y
- # Both address and 9-digit postal code match Y A X Y
- # United Kingdom – Address and postal code match _ _ _ F
- # International transaction – Address and postal code match _ _ _ D/M
- #
- # ACCEPTABLE, BUT ISSUE A WARNING:
- # Ineligible transaction; or message contains a content error _ _ _ E
- # System unavailable; retry R U R R
- # Information unavailable U W U U
- # Issuer does not support AVS S U S S
- # AVS is not applicable _ _ _ S
- # Incompatible formats – Not verified _ _ _ C
- # Incompatible formats – Address not verified; postal code matches _ _ _ P
- # International transaction – address not verified _ G _ G/I
- #
- # UNACCEPTABLE AVS RESPONSES:
- # Only Address matches A Y A A
- # Only 5-digit postal code matches Z Z Z Z
- # Only 9-digit postal code matches Z Z W W
- # Neither address nor postal code matches N N N N
-
- if (my $avscode = uc($transaction->avs_code)) {
-
- # map codes to accept/warn/reject
- my $avs = {
- 'American Express card' => {
- 'A' => 'r',
- 'N' => 'r',
- 'R' => 'w',
- 'S' => 'w',
- 'U' => 'w',
- 'Y' => 'a',
- 'Z' => 'r',
- },
- 'Discover card' => {
- 'A' => 'a',
- 'G' => 'w',
- 'N' => 'r',
- 'U' => 'w',
- 'W' => 'w',
- 'Y' => 'r',
- 'Z' => 'r',
- },
- 'MasterCard' => {
- 'A' => 'r',
- 'N' => 'r',
- 'R' => 'w',
- 'S' => 'w',
- 'U' => 'w',
- 'W' => 'r',
- 'X' => 'a',
- 'Y' => 'a',
- 'Z' => 'r',
- },
- 'VISA card' => {
- 'A' => 'r',
- 'C' => 'w',
- 'D' => 'a',
- 'E' => 'w',
- 'F' => 'a',
- 'G' => 'w',
- 'I' => 'w',
- 'M' => 'a',
- 'N' => 'r',
- 'P' => 'w',
- 'R' => 'w',
- 'S' => 'w',
- 'U' => 'w',
- 'W' => 'r',
- 'Y' => 'a',
- 'Z' => 'r',
- },
- };
- my $cardtype = cardtype($content{card_number});
- if ($avs->{$cardtype}) {
- my $avsact = $avs->{$cardtype}->{$avscode};
- my $warning = '';
- if ($avsact eq 'r') {
- return "AVS code verification failed, cardtype $cardtype, code $avscode";
- } elsif ($avsact eq 'w') {
- $warning = "AVS did not occur, cardtype $cardtype, code $avscode";
- } elsif (!$avsact) {
- $warning = "AVS code unknown, cardtype $cardtype, code $avscode";
- } # else $avsact eq 'a'
- if ($warning) {
- $log->warning($warning);
- warn $warning;
- }
- } # else $cardtype avs handling not implemented
- } # else !$transaction->avs_code
+ }
- } else { # is not success
+ } # end of IMMEDIATE; we now have our $error and $transaction
- # status is 'done' not 'declined', as in _realtime_bop_result
- $cust_pay_pending->status('done');
- $cust_pay_pending->statustext( $transaction->error_message || 'Unknown error' );
- # could also record failure_status here,
- # but it's not supported by B::OP::vSecureProcessing...
- # need a B::OP module with (reverse) auth only to test it with
- my $cpp_declined_err = $cust_pay_pending->replace;
- return $cpp_declined_err if $cpp_declined_err;
+ ###
+ # Save the custnum (as part of the main transaction, so it can reference
+ # the cust_main)
+ ###
+ $cust_pay_pending->set('custnum', $self->custnum);
+ my $set_custnum_err = $cust_pay_pending->replace;
+ if ($set_custnum_err) {
+ $log->error($set_custnum_err);
+ $error ||= $set_custnum_err;
+ # but if there was a real verification error also, return that one
}
###
@@ -2114,7 +2122,9 @@ sub realtime_verify_bop {
# result handling
###
- $transaction->is_success() ? '' : $transaction->error_message();
+ # $error contains the transaction error_message, if is_success was false.
+
+ return $error;
}
diff --git a/FS/FS/cust_pay_pending.pm b/FS/FS/cust_pay_pending.pm
index a775553..4874802 100644
--- a/FS/FS/cust_pay_pending.pm
+++ b/FS/FS/cust_pay_pending.pm
@@ -213,7 +213,7 @@ sub check {
my $error =
$self->ut_numbern('paypendingnum')
- || $self->ut_foreign_key('custnum', 'cust_main', 'custnum')
+ || $self->ut_foreign_keyn('custnum', 'cust_main', 'custnum')
|| $self->ut_money('paid')
|| $self->ut_numbern('_date')
|| $self->ut_textn('payunique')
@@ -232,6 +232,10 @@ sub check {
;
return $error if $error;
+ if (!$self->custnum and !$self->get('custnum_pending')) {
+ return 'custnum required';
+ }
+
$self->_date(time) unless $self->_date;
# UNIQUE index should catch this too, without race conditions, but this
-----------------------------------------------------------------------
Summary of changes:
FS/FS/Schema.pm | 2 +-
FS/FS/cust_main/Billing_Realtime.pm | 445 ++++++++++++++++++-----------------
FS/FS/cust_pay_pending.pm | 6 +-
3 files changed, 239 insertions(+), 214 deletions(-)
More information about the freeside-commits
mailing list