From e5d6af11f52aabe8c9e6e12ce43c3401ccf025a8 Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Wed, 2 Jan 2013 19:01:16 -0800 Subject: [PATCH] move credit card transaction results out of paybatch and into real fields, #18548 --- FS/FS/Schema.pm | 18 ++++++ FS/FS/TicketSystem/RT_Internal.pm | 2 +- FS/FS/cust_main/Billing_Realtime.pm | 69 +++++++++++----------- FS/FS/cust_pay.pm | 54 ++++++++++++++++- FS/FS/cust_pay_void.pm | 2 +- FS/FS/cust_refund.pm | 5 ++ FS/FS/payinfo_Mixin.pm | 28 +++++---- FS/FS/payinfo_transaction_Mixin.pm | 56 ++++++++++-------- FS/bin/freeside-void-payments | 13 +++- httemplate/edit/cust_refund.cgi | 20 +++---- httemplate/edit/process/cust_pay.cgi | 2 + httemplate/search/elements/cust_pay_or_refund.html | 10 ++++ .../search/elements/report_cust_pay_or_refund.html | 43 +++++++++++--- httemplate/view/cust_pay.html | 4 +- httemplate/view/cust_refund.html | 4 +- 15 files changed, 224 insertions(+), 106 deletions(-) diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm index b6fd3b67b..cbcd27b46 100644 --- a/FS/FS/Schema.pm +++ b/FS/FS/Schema.pm @@ -1560,7 +1560,14 @@ sub tables_hashref { 'depositor', 'varchar', 'NULL', $char_d, '', '', 'account', 'varchar', 'NULL', 20, '', '', 'teller', 'varchar', 'NULL', 20, '', '', + 'batchnum', 'int', 'NULL', '', '', '', #pay_batch foreign key + + # credit card/EFT fields (formerly in paybatch) + 'gatewaynum', 'int', 'NULL', '', '', '', # payment_gateway FK + 'processor', 'varchar', 'NULL', $char_d, '', '', # module name + 'auth', 'varchar','NULL',16, '', '', # CC auth number + 'order_number','varchar','NULL',$char_d, '', '', # transaction number ], 'primary_key' => 'paynum', #i guess not now, with cust_pay_pending, if we actually make it here, we _do_ want to record it# 'unique' => [ [ 'payunique' ] ], @@ -1591,6 +1598,12 @@ sub tables_hashref { 'teller', 'varchar', 'NULL', 20, '', '', 'batchnum', 'int', 'NULL', '', '', '', #pay_batch foreign key + # credit card/EFT fields (formerly in paybatch) + 'gatewaynum', 'int', 'NULL', '', '', '', # payment_gateway FK + 'processor', 'varchar', 'NULL', $char_d, '', '', # module name + 'auth', 'varchar','NULL',16, '', '', # CC auth number + 'order_number', 'varchar','NULL',$char_d, '', '', # transaction number + #void fields 'void_date', @date_type, '', '', 'reason', 'varchar', 'NULL', $char_d, '', '', @@ -1858,6 +1871,11 @@ sub tables_hashref { 'paymask', 'varchar', 'NULL', $char_d, '', '', 'paybatch', 'varchar', 'NULL', $char_d, '', '', 'closed', 'char', 'NULL', 1, '', '', + # credit card/EFT fields (formerly in paybatch) + 'gatewaynum', 'int', 'NULL', '', '', '', # payment_gateway FK + 'processor', 'varchar', 'NULL', $char_d, '', '', # module name + 'auth', 'varchar','NULL',16, '', '', # CC auth number + 'order_number', 'varchar','NULL',$char_d, '', '', # transaction number ], 'primary_key' => 'refundnum', 'unique' => [], diff --git a/FS/FS/TicketSystem/RT_Internal.pm b/FS/FS/TicketSystem/RT_Internal.pm index 01e2e2966..665c16692 100644 --- a/FS/FS/TicketSystem/RT_Internal.pm +++ b/FS/FS/TicketSystem/RT_Internal.pm @@ -589,7 +589,7 @@ sub _web_external_auth { # we failed to successfully create the user. abort abort abort. delete $session->{'CurrentUser'}; - die "can't auto-create RT user"; #an error message would be nice :/ + die "can't auto-create RT user: $msg"; #an error message would be nice :/ #$m->abort() unless $RT::WebFallbackToInternalAuth; #$m->comp( '/Elements/Login', %ARGS, # Error => loc( 'Cannot create user: [_1]', $msg ) ); diff --git a/FS/FS/cust_main/Billing_Realtime.pm b/FS/FS/cust_main/Billing_Realtime.pm index 80063debb..804969b16 100644 --- a/FS/FS/cust_main/Billing_Realtime.pm +++ b/FS/FS/cust_main/Billing_Realtime.pm @@ -757,19 +757,6 @@ sub fake_bop { return "Error: No error; test failure requested with fake_failure"; } - #my $paybatch = ''; - #if ( $payment_gateway->gatewaynum ) { # agent override - # $paybatch = $payment_gateway->gatewaynum. '-'; - #} - # - #$paybatch .= "$processor:". $transaction->authorization; - # - #$paybatch .= ':'. $transaction->order_number - # if $transaction->can('order_number') - # && length($transaction->order_number); - - my $paybatch = 'FakeProcessor:54:32'; - my $cust_pay = new FS::cust_pay ( { 'custnum' => $self->custnum, 'invnum' => $options{'invnum'}, @@ -778,9 +765,11 @@ sub fake_bop { 'payby' => $bop_method2payby{$options{method}}, #'payinfo' => $payinfo, 'payinfo' => '4111111111111111', - 'paybatch' => $paybatch, #'paydate' => $paydate, 'paydate' => '2012-05-01', + 'processor' => 'FakeProcessor', + 'auth' => '54', + 'order_number' => '32', } ); $cust_pay->payunique( $options{payunique} ) if length($options{payunique}); @@ -841,17 +830,8 @@ sub _realtime_bop_result { if ( $transaction->is_success() ) { - my $paybatch = ''; - if ( $payment_gateway->gatewaynum ) { # agent override - $paybatch = $payment_gateway->gatewaynum. '-'; - } - - $paybatch .= $payment_gateway->gateway_module. ":". - $transaction->authorization; - - $paybatch .= ':'. $transaction->order_number - if $transaction->can('order_number') - && length($transaction->order_number); + my $order_number = $transaction->order_number + if $transaction->can('order_number'); my $cust_pay = new FS::cust_pay ( { 'custnum' => $self->custnum, @@ -860,10 +840,14 @@ sub _realtime_bop_result { '_date' => '', 'payby' => $cust_pay_pending->payby, 'payinfo' => $options{'payinfo'}, - 'paybatch' => $paybatch, 'paydate' => $cust_pay_pending->paydate, 'pkgnum' => $cust_pay_pending->pkgnum, - 'discount_term' => $options{'discount_term'}, + 'discount_term' => $options{'discount_term'}, + 'gatewaynum' => ($payment_gateway->gatewaynum || ''), + 'processor' => $payment_gateway->gateway_module, + 'auth' => $transaction->authorization, + 'order_number' => $order_number || '', + } ); #doesn't hurt to know, even though the dup check is in cust_pay_pending now $cust_pay->payunique( $options{payunique} ) @@ -1363,6 +1347,7 @@ sub realtime_refund_bop { my( $processor, $login, $password, @bop_options, $namespace ) ; my( $auth, $order_number ) = ( '', '', '' ); + my $gatewaynum = ''; if ( $options{'paynum'} ) { @@ -1371,11 +1356,22 @@ sub realtime_refund_bop { or return "Unknown paynum $options{'paynum'}"; $amount ||= $cust_pay->paid; - $cust_pay->paybatch =~ /^((\d+)\-)?(\w+):\s*([\w\-\/ ]*)(:([\w\-]+))?$/ - or return "Can't parse paybatch for paynum $options{'paynum'}: ". - $cust_pay->paybatch; - my $gatewaynum = ''; - ( $gatewaynum, $processor, $auth, $order_number ) = ( $2, $3, $4, $6 ); + if ( $cust_pay->get('processor') ) { + ($gatewaynum, $processor, $auth, $order_number) = + ( + $cust_pay->gatewaynum, + $cust_pay->processor, + $cust_pay->auth, + $cust_pay->order_number, + ); + } else { + # this payment wasn't upgraded, which probably means this won't work, + # but try it anyway + $cust_pay->paybatch =~ /^((\d+)\-)?(\w+):\s*([\w\-\/ ]*)(:([\w\-]+))?$/ + or return "Can't parse paybatch for paynum $options{'paynum'}: ". + $cust_pay->paybatch; + ( $gatewaynum, $processor, $auth, $order_number ) = ( $2, $3, $4, $6 ); + } if ( $gatewaynum ) { #gateway for the payment to be refunded @@ -1605,9 +1601,7 @@ sub realtime_refund_bop { return "$processor error: ". $refund->error_message unless $refund->is_success(); - my $paybatch = "$processor:". $refund->authorization; - $paybatch .= ':'. $refund->order_number - if $refund->can('order_number') && $refund->order_number; + $order_number = $refund->order_number if $refund->can('order_number'); while ( $cust_pay && $cust_pay->unapplied < $amount ) { my @cust_bill_pay = $cust_pay->cust_bill_pay; @@ -1624,8 +1618,11 @@ sub realtime_refund_bop { '_date' => '', 'payby' => $bop_method2payby{$options{method}}, 'payinfo' => $payinfo, - 'paybatch' => $paybatch, 'reason' => $options{'reason'} || 'card or ACH refund', + 'gatewaynum' => $gatewaynum, # may be null + 'processor' => $processor, + 'auth' => $refund->authorization, + 'order_number' => $order_number, } ); my $error = $cust_refund->insert; if ( $error ) { diff --git a/FS/FS/cust_pay.pm b/FS/FS/cust_pay.pm index d28997ccd..4535aadb2 100644 --- a/FS/FS/cust_pay.pm +++ b/FS/FS/cust_pay.pm @@ -100,7 +100,7 @@ Masked payinfo (See L for how this works) =item paybatch -text field for tracking card processing or other batch grouping +obsolete text field for tracking card processing or other batch grouping =item payunique @@ -130,11 +130,32 @@ The deposit account number. The teller number. -=item pay_batch +=item batchnum The number of the batch this payment came from (see L), or null if it was processed through a realtime gateway or entered manually. +=item gatewaynum + +The number of the realtime or batch gateway L) this +payment was processed through. Null if it was entered manually or processed +by the "system default" gateway, which doesn't have a number. + +=item processor + +The name of the processor module (Business::OnlinePayment, ::BatchPayment, +or ::OnlineThirdPartyPayment subclass) used for this payment. Slightly +redundant with C. + +=item auth + +The authorization number returned by the credit card network. + +=item order_number + +The transaction ID returned by the gateway, if any. This is usually what +you would use to initiate a void or refund of the payment. + =back =head1 METHODS @@ -878,6 +899,8 @@ sub _upgrade_data { #class method warn "$me upgrading $class\n" if $DEBUG; + local $FS::payinfo_Mixin::ignore_masked_payinfo = 1; + ## # otaker/ivan upgrade ## @@ -1004,6 +1027,33 @@ sub _upgrade_data { #class method if $error; } + ### + # migrate gateway info from the misused 'paybatch' field + ### + + # not only cust_pay, but also voided and refunded payments + if (!FS::upgrade_journal->is_done('cust_pay__parse_paybatch')) { + # really inefficient, but again, only has to run once + foreach my $table (qw(cust_pay cust_pay_void cust_refund)) { + foreach my $object ( qsearch({ + table => $table, + extra_sql => "WHERE payby IN('CARD','CHEK') ". + "AND paybatch IS NOT NULL", + }) ) + { + my $parsed = $object->_parse_paybatch; + if (keys %$parsed) { + $object->set($_ => $parsed->{$_}) foreach keys %$parsed; + $object->set('paybatch', ''); + my $error = $object->replace; + warn "error parsing CARD/CHEK paybatch fields on $object #". + $object->get($object->primary_key).":\n $error\n" + if $error; + } + } #$object + } #$table + FS::upgrade_journal->set_done('cust_pay__parse_paybatch'); + } } =back diff --git a/FS/FS/cust_pay_void.pm b/FS/FS/cust_pay_void.pm index bebcfd4cc..42fc2966c 100644 --- a/FS/FS/cust_pay_void.pm +++ b/FS/FS/cust_pay_void.pm @@ -1,7 +1,7 @@ package FS::cust_pay_void; use strict; -use base qw( FS::otaker_Mixin FS::payinfo_Mixin FS::cust_main_Mixin +use base qw( FS::otaker_Mixin FS::payinfo_transaction_Mixin FS::cust_main_Mixin FS::Record ); use vars qw( @encrypted_fields $otaker_upgrade_kludge ); use Business::CreditCard; diff --git a/FS/FS/cust_refund.pm b/FS/FS/cust_refund.pm index 7df7a557a..45a170ba0 100644 --- a/FS/FS/cust_refund.pm +++ b/FS/FS/cust_refund.pm @@ -87,6 +87,11 @@ order taker (see L books closed flag, empty or `Y' +=item gatewaynum, processor, auth, order_number + +Same as for L, but specifically the result of realtime +authorization of the refund. + =back =head1 METHODS diff --git a/FS/FS/payinfo_Mixin.pm b/FS/FS/payinfo_Mixin.pm index 7b713efd3..9879a3abd 100644 --- a/FS/FS/payinfo_Mixin.pm +++ b/FS/FS/payinfo_Mixin.pm @@ -5,6 +5,8 @@ use Business::CreditCard; use FS::payby; use FS::Record qw(qsearch); +use vars qw($ignore_masked_payinfo); + =head1 NAME FS::payinfo_Mixin - Mixin class for records in tables that contain payinfo. @@ -207,17 +209,21 @@ sub payinfo_check { if ( $self->payby eq 'CARD' && ! $self->is_encrypted($self->payinfo) ) { my $payinfo = $self->payinfo; - $payinfo =~ s/\D//g; - $self->payinfo($payinfo); - if ( $self->payinfo ) { - $self->payinfo =~ /^(\d{13,16}|\d{8,9})$/ - or return "Illegal (mistyped?) credit card number (payinfo)"; - $self->payinfo($1); - validate($self->payinfo) or return "Illegal credit card number"; - return "Unknown card type" if $self->payinfo !~ /^99\d{14}$/ #token - && cardtype($self->payinfo) eq "Unknown"; + if ( $ignore_masked_payinfo and $self->mask_payinfo eq $self->payinfo ) { + # allow it } else { - $self->payinfo('N/A'); #??? + $payinfo =~ s/\D//g; + $self->payinfo($payinfo); + if ( $self->payinfo ) { + $self->payinfo =~ /^(\d{13,16}|\d{8,9})$/ + or return "Illegal (mistyped?) credit card number (payinfo)"; + $self->payinfo($1); + validate($self->payinfo) or return "Illegal credit card number"; + return "Unknown card type" if $self->payinfo !~ /^99\d{14}$/ #token + && cardtype($self->payinfo) eq "Unknown"; + } else { + $self->payinfo('N/A'); #??? + } } } else { if ( $self->is_encrypted($self->payinfo) ) { @@ -230,8 +236,6 @@ sub payinfo_check { } } - ''; - } =item payby_payinfo_pretty diff --git a/FS/FS/payinfo_transaction_Mixin.pm b/FS/FS/payinfo_transaction_Mixin.pm index 19419de1c..093891e93 100644 --- a/FS/FS/payinfo_transaction_Mixin.pm +++ b/FS/FS/payinfo_transaction_Mixin.pm @@ -23,7 +23,8 @@ use vars qw(@ISA); =head1 DESCRIPTION This is a mixin class for records that represent transactions: that contain -payinfo and paybatch. Currently FS::cust_pay and FS::cust_refund +payinfo and realtime result fields (gatewaynum, processor, authorization, +order_number). Currently FS::cust_pay, FS::cust_refund, and FS::cust_pay_void. =head1 METHODS @@ -55,32 +56,8 @@ sub payby_name { } } -=item gatewaynum +# We keep _parse_paybatch just because the upgrade needs it. -Returns a gatewaynum for the processing gateway. - -=item processor - -Returns a name for the processing gateway. - -=item authorization - -Returns a name for the processing gateway. - -=item order_number - -Returns a name for the processing gateway. - -=cut - -sub gatewaynum { shift->_parse_paybatch->{'gatewaynum'}; } -sub processor { shift->_parse_paybatch->{'processor'}; } -sub authorization { shift->_parse_paybatch->{'authorization'}; } -sub order_number { shift->_parse_paybatch->{'order_number'}; } - -#sucks that this stuff is in paybatch like this in the first place, -#but at least other code can start to use new field names -#(code nicked from FS::cust_main::realtime_refund_bop) sub _parse_paybatch { my $self = shift; @@ -112,6 +89,33 @@ sub _parse_paybatch { } +# because we can't actually name the field 'authorization' (reserved word) +sub authorization { + my $self = shift; + $self->auth(@_); +} + +=item payinfo_check + +Checks the validity of the realtime payment fields (gatewaynum, processor, +auth, and order_number) as well as payby and payinfo + +=cut + +sub payinfo_check { + my $self = shift; + + # All of these can be null, so in principle this could go in payinfo_Mixin. + + $self->SUPER::payinfo_check() + || $self->ut_numbern('gatewaynum') + # not ut_foreign_keyn, it causes upgrades to fail + || $self->ut_alphan('processor') + || $self->ut_textn('auth') + || $self->ut_textn('order_number') + || ''; +} + =back =head1 SEE ALSO diff --git a/FS/bin/freeside-void-payments b/FS/bin/freeside-void-payments index 8c1f3dbdf..49b74d388 100755 --- a/FS/bin/freeside-void-payments +++ b/FS/bin/freeside-void-payments @@ -90,8 +90,11 @@ my $notfound = 0; my $canceled = 0; print "Voiding ".scalar(@auths)." transactions:\n" if $opt{'v'}; foreach my $authnum (@auths) { - my $paybatch = $gatewaynum . $processor . ':' . $authnum; - my $cust_pay = qsearchs('cust_pay', { paybatch => $paybatch } ); + my $cust_pay = qsearchs('cust_pay', { + gatewaynum => $gatewaynum, + processor => $processor, + authorization => $authnum, + }); my $error; my $cancel_error; if($cust_pay) { @@ -103,7 +106,11 @@ foreach my $authnum (@auths) { } } else { - my $cpv = qsearchs('cust_pay_void', { paybatch => $paybatch }); + my $cpv = qsearchs('cust_pay_void', { + gatewaynum => $gatewaynum, + processor => $processor, + authorization => $authnum, + }); if($cpv) { $error = 'already voided '.time2str('%Y-%m-%d', $cpv->void_date) . ' by ' . $cpv->otaker; diff --git a/httemplate/edit/cust_refund.cgi b/httemplate/edit/cust_refund.cgi index 1ef69fdae..656d5ebb5 100755 --- a/httemplate/edit/cust_refund.cgi +++ b/httemplate/edit/cust_refund.cgi @@ -60,29 +60,25 @@ % } -% -% #false laziness w/FS/FS/cust_main::realtime_refund_bop -% if ( $cust_pay->paybatch =~ /^(\w+):(\w+)(:(\w+))?$/ ) { -% my ( $processor, $auth, $order_number ) = ( $1, $2, $4 ); -% - - - Processor<% $processor %> + Processor + <% $cust_pay->processor %> % if ( length($auth) ) { - Authorization<% $auth %> + Authorization + <% $cust_pay->auth %> % } -% if ( length($order_number) ) { +% if ( length($cust_pay->order_number) ) { - Order number<% $order_number %> + Order number + <% $cust_pay->order_number %> % } -% } +% } #if $cust_pay % } diff --git a/httemplate/edit/process/cust_pay.cgi b/httemplate/edit/process/cust_pay.cgi index ce0ec3212..a002fa181 100755 --- a/httemplate/edit/process/cust_pay.cgi +++ b/httemplate/edit/process/cust_pay.cgi @@ -57,6 +57,8 @@ my $new = new FS::cust_pay ( { bank depositor account teller ) #} fields('cust_pay') + # gatewaynum, processor, auth, order_number + # are for realtime payments only, and can't be entered manually } ); my @rights = ('Post payment'); diff --git a/httemplate/search/elements/cust_pay_or_refund.html b/httemplate/search/elements/cust_pay_or_refund.html index b0524913a..eeef0c0e1 100755 --- a/httemplate/search/elements/cust_pay_or_refund.html +++ b/httemplate/search/elements/cust_pay_or_refund.html @@ -330,6 +330,16 @@ if ( $cgi->param('magic') ) { push @search, "$table.payinfo = '$1'"; } + if ( $cgi->param('ccpay') =~ /^([\w-:]+)$/ ) { + # I think that's all the characters we need to allow. + # To avoid confusion, this parameter searches both auth and order_number. + push @search, "($table.auth LIKE '$1%') OR ($table.order_number LIKE '$1%')"; + push @fields, 'auth', 'order_number'; + push @header, 'Auth #', 'Transaction #'; + $align .= 'rr'; + + } + if ( $cgi->param('usernum') =~ /^(\d+)$/ ) { push @search, "$table.usernum = $1"; } diff --git a/httemplate/search/elements/report_cust_pay_or_refund.html b/httemplate/search/elements/report_cust_pay_or_refund.html index a2b90b47d..7a1216bb6 100644 --- a/httemplate/search/elements/report_cust_pay_or_refund.html +++ b/httemplate/search/elements/report_cust_pay_or_refund.html @@ -50,23 +50,48 @@ Examples: - <% mt('Check #:') |h %> + <% mt('Check #:') |h %> + + + + + + + + <% mt('Transaction #') |h %> + + - + diff --git a/httemplate/view/cust_pay.html b/httemplate/view/cust_pay.html index f9c8bc19c..76a24884a 100644 --- a/httemplate/view/cust_pay.html +++ b/httemplate/view/cust_pay.html @@ -77,7 +77,7 @@ <% $cust_pay->payby_name %> #<% $cust_pay->paymask %> -% if ( $cust_pay->payby =~ /^(CARD|CHEK|LECB)$/ && $cust_pay->paybatch ) { +% if ( $cust_pay->payby =~ /^(CARD|CHEK|LECB)$/ && $cust_pay->processor ) { <% mt('Processor') |h %> @@ -86,7 +86,7 @@ <% mt('Authorization #') |h %> - <% $cust_pay->authorization %> + <% $cust_pay->auth %> % if ( $cust_pay->order_number ) { diff --git a/httemplate/view/cust_refund.html b/httemplate/view/cust_refund.html index 996b4c05a..319761506 100644 --- a/httemplate/view/cust_refund.html +++ b/httemplate/view/cust_refund.html @@ -62,7 +62,7 @@ <% $cust_refund->payby_name %><% $cust_refund->paymask ? ' #'.$cust_refund->paymask : '' %> -% if ( $cust_refund->payby =~ /^(CARD|CHEK|LECB)$/ && $cust_refund->paybatch ) { +% if ( $cust_refund->payby =~ /^(CARD|CHEK|LECB)$/ && $cust_refund->processor ) { <% mt('Processor') |h %> @@ -71,7 +71,7 @@ <% mt('Authorization #') |h %> - <% $cust_refund->authorization %> + <% $cust_refund->auth %> % if ( $cust_refund->order_number ) { -- 2.11.0