From b1b3d6b3c0db38909b6f569e08877d2678587d22 Mon Sep 17 00:00:00 2001 From: Jonathan Prykop Date: Mon, 22 Feb 2016 17:14:07 -0600 Subject: [PATCH] RT#39586 Manual check refunds cannot be unapplied [source_paynum field, reason bug fixes, link text] --- FS/FS/Schema.pm | 5 +++++ FS/FS/cust_credit.pm | 2 +- FS/FS/cust_main/Billing_Realtime.pm | 17 +++++++++++---- FS/FS/cust_pay.pm | 4 ++-- FS/FS/cust_refund.pm | 24 ++++++++++++++-------- httemplate/edit/process/cust_refund.cgi | 3 +-- .../view/cust_main/payment_history/credit.html | 12 +++++------ .../view/cust_main/payment_history/payment.html | 10 ++++----- 8 files changed, 47 insertions(+), 30 deletions(-) diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm index 66575fa13..ff6a9241a 100644 --- a/FS/FS/Schema.pm +++ b/FS/FS/Schema.pm @@ -3061,6 +3061,7 @@ sub tables_hashref { 'paymask', 'varchar', 'NULL', $char_d, '', '', 'paybatch', 'varchar', 'NULL', $char_d, '', '', 'closed', 'char', 'NULL', 1, '', '', + 'source_paynum', 'int', 'NULL', '', '', '', # link to cust_payby, to prevent unapply of gateway-generated refunds # credit card/EFT fields (formerly in paybatch) 'gatewaynum', 'int', 'NULL', '', '', '', # payment_gateway FK 'processor', 'varchar', 'NULL', $char_d, '', '', # module name @@ -3083,6 +3084,10 @@ sub tables_hashref { { columns => [ 'gatewaynum' ], table => 'payment_gateway', }, + { columns => [ 'source_paynum' ], + table => 'cust_pay', + references => [ 'paynum' ], + }, ], }, diff --git a/FS/FS/cust_credit.pm b/FS/FS/cust_credit.pm index a598b37a3..4be4b177a 100644 --- a/FS/FS/cust_credit.pm +++ b/FS/FS/cust_credit.pm @@ -1045,7 +1045,7 @@ sub refund_to_unapply { 'table' => 'cust_credit_refund', 'hashref' => { 'crednum' => $self->crednum }, 'addl_from' => 'LEFT JOIN cust_refund USING (refundnum)', - 'extra_sql' => "AND (cust_refund.closed = '' OR cust_refund.closed IS NULL)", + 'extra_sql' => "AND cust_refund.closed IS NULL AND cust_refund.source_paynum IS NULL", }); } diff --git a/FS/FS/cust_main/Billing_Realtime.pm b/FS/FS/cust_main/Billing_Realtime.pm index 3396ec4a0..747f4af5d 100644 --- a/FS/FS/cust_main/Billing_Realtime.pm +++ b/FS/FS/cust_main/Billing_Realtime.pm @@ -1323,14 +1323,14 @@ L for supported gateways. Available methods are: I, I and I -Available options are: I, I, I, I +Available options are: I, I, I, I Most gateways require a reference to an original payment transaction to refund, so you probably need to specify a I. I defaults to the original amount of the payment if not specified. -I specifies a reason for the refund. +I specified an existing refund reason for the refund I specifies the expiration date for a credit card overriding the value from the customer record or the payment record. Specified as yyyy-mm-dd @@ -1373,6 +1373,8 @@ sub realtime_refund_bop { warn " $_ => $options{$_}\n" foreach keys %options; } + return "No reason specified" unless $options{'reasonnum'} =~ /^\d+$/; + my %content = (); ### @@ -1531,7 +1533,12 @@ sub realtime_refund_bop { if $conf->exists('business-onlinepayment-test_transaction'); $void->submit(); if ( $void->is_success ) { - my $error = $cust_pay->void($options{'reason'}); + # specified as a refund reason, but now we want a payment void reason + # extract just the reason text, let cust_pay::void handle new_or_existing + my $reason = qsearchs('reason',{ 'reasonnum' => $options{'reasonnum'} }); + my $error; + $error = 'Reason could not be loaded' unless $reason; + $error = $cust_pay->void($reason->reason) unless $error; if ( $error ) { # gah, even with transactions. my $e = 'WARNING: Card/ACH voided but database not updated - '. @@ -1652,11 +1659,12 @@ sub realtime_refund_bop { my $cust_refund = new FS::cust_refund ( { 'custnum' => $self->custnum, 'paynum' => $options{'paynum'}, + 'source_paynum' => $options{'paynum'}, 'refund' => $amount, '_date' => '', 'payby' => $bop_method2payby{$options{method}}, 'payinfo' => $payinfo, - 'reason' => $options{'reason'} || 'card or ACH refund', + 'reasonnum' => $options{'reasonnum'}, 'gatewaynum' => $gatewaynum, # may be null 'processor' => $processor, 'auth' => $refund->authorization, @@ -1665,6 +1673,7 @@ sub realtime_refund_bop { my $error = $cust_refund->insert; if ( $error ) { $cust_refund->paynum(''); #try again with no specific paynum + $cust_refund->source_paynum(''); my $error2 = $cust_refund->insert; if ( $error2 ) { # gah, even with transactions. diff --git a/FS/FS/cust_pay.pm b/FS/FS/cust_pay.pm index af76b8990..620f6c629 100644 --- a/FS/FS/cust_pay.pm +++ b/FS/FS/cust_pay.pm @@ -444,7 +444,7 @@ sub void { unless (ref($reason) || !$reason) { $reason = FS::reason->new_or_existing( - 'class' => 'X', + 'class' => 'P', 'type' => 'Void payment', 'reason' => $reason ); @@ -920,7 +920,7 @@ sub refund_to_unapply { 'table' => 'cust_pay_refund', 'hashref' => { 'paynum' => $self->paynum }, 'addl_from' => 'LEFT JOIN cust_refund USING (refundnum)', - 'extra_sql' => "AND (cust_refund.closed = '' OR cust_refund.closed IS NULL)", + 'extra_sql' => "AND cust_refund.closed IS NULL AND cust_refund.source_paynum IS NULL", }); } diff --git a/FS/FS/cust_refund.pm b/FS/FS/cust_refund.pm index efbdceeb0..ced954036 100644 --- a/FS/FS/cust_refund.pm +++ b/FS/FS/cust_refund.pm @@ -143,16 +143,23 @@ sub insert { local $FS::UID::AutoCommit = 0; my $dbh = dbh; - unless ($self->reasonnum) { - my $result = $self->reason( $self->getfield('reason'), - exists($options{ 'reason_type' }) - ? ('reason_type' => $options{ 'reason_type' }) - : (), - ); - unless($result) { + if (!$self->reasonnum) { + my $reason_text = $self->get('reason') + or return "reason text or existing reason required"; + my $reason_type = $options{'reason_type'} + or return "reason type required"; + + local $@; + my $reason = FS::reason->new_or_existing( + reason => $reason_text, + type => $reason_type, + class => 'F', + ); + if ($@) { $dbh->rollback if $oldAutoCommit; - return "failed to set reason for $me"; #: ". $dbh->errstr; + return "failed to set refund reason: $@"; } + $self->set('reasonnum', $reason->reasonnum); } $self->setfield('reason', ''); @@ -303,6 +310,7 @@ sub check { || $self->ut_numbern('_date') || $self->ut_textn('paybatch') || $self->ut_enum('closed', [ '', 'Y' ]) + || $self->ut_foreign_keyn('source_paynum', 'cust_pay', 'paynum') ; return $error if $error; diff --git a/httemplate/edit/process/cust_refund.cgi b/httemplate/edit/process/cust_refund.cgi index 6ad468b6c..8977ced20 100755 --- a/httemplate/edit/process/cust_refund.cgi +++ b/httemplate/edit/process/cust_refund.cgi @@ -47,12 +47,11 @@ if ( $error ) { my $refund = "$1$2"; $cgi->param('paynum') =~ /^(\d*)$/ or die "Illegal paynum!"; my $paynum = $1; - my $reason = $cgi->param('reason'); my $paydate = $cgi->param('exp_year'). '-'. $cgi->param('exp_month'). '-01'; $options{'paydate'} = $paydate if $paydate =~ /^\d{2,4}-\d{1,2}-01$/; $error = $cust_main->realtime_refund_bop( $bop, 'amount' => $refund, 'paynum' => $paynum, - 'reason' => $reason, + 'reasonnum' => scalar($cgi->param('reasonnum')), %options ); } else { my %hash = map { diff --git a/httemplate/view/cust_main/payment_history/credit.html b/httemplate/view/cust_main/payment_history/credit.html index 85911a03f..da8ca2e9a 100644 --- a/httemplate/view/cust_main/payment_history/credit.html +++ b/httemplate/view/cust_main/payment_history/credit.html @@ -45,7 +45,7 @@ if ( scalar(@cust_credit_bill) == 0 if ( $opt{total_unapplied_refunds} > 0 ) { $apply.= ' ('. include( '/elements/popup_link.html', - 'label' => emt('apply to refund'), + 'label' => emt('apply refund'), 'action' => "${p}edit/cust_credit_refund.cgi?". $cust_credit->crednum, 'actionlabel' => emt('Apply credit to refund'), @@ -100,7 +100,7 @@ if ( scalar(@cust_credit_bill) == 0 if ( $opt{total_unapplied_refunds} > 0 ) { $apply.= ' ('. include( '/elements/popup_link.html', - 'label' => emt('apply to refund'), + 'label' => emt('apply refund'), 'action' => "${p}edit/cust_credit_refund.cgi?". $cust_credit->crednum, 'actionlabel' => emt('Apply credit to refund'), @@ -141,20 +141,18 @@ $void = ' ('. my $unapply = ''; if ($opt{'Unapply credit'} && !$cust_credit->closed) { - my $refund_to_unapply = $cust_credit->refund_to_unapply; - my $usepre = $refund_to_unapply && @cust_credit_bill; $unapply = areyousure_link("${p}misc/unapply-cust_credit.cgi?".$cust_credit->crednum, emt('Are you sure you want to unapply this credit from invoices?'), emt('Keep this credit, but dissociate it from the invoices it is currently applied against'), - emt('unapply') . ($usepre ? ' ' . emt('invoices') : '') + emt('unapply') ) if @cust_credit_bill; $unapply .= areyousure_link("${p}misc/unapply-cust_credit_refund.cgi?".$cust_credit->crednum, emt('Are you sure you want to unapply this credit from refunds?'), emt('Keep this credit, but dissociate it from the refunds it is currently applied to'), - emt('unapply') . ($usepre ? ' ' . emt('refunds') : '') + emt('unapply refunds') ) - if $refund_to_unapply; + if $cust_credit->refund_to_unapply; } my $reason = $cust_credit->reason; diff --git a/httemplate/view/cust_main/payment_history/payment.html b/httemplate/view/cust_main/payment_history/payment.html index 16b91c2f2..8faed3ca7 100644 --- a/httemplate/view/cust_main/payment_history/payment.html +++ b/httemplate/view/cust_main/payment_history/payment.html @@ -103,7 +103,7 @@ if ($unapplied > 0) { if ( $opt{total_unapplied_refunds} > 0 ) { $apply.= ' ('. include( '/elements/popup_link.html', - 'label' => emt('apply to refund'), + 'label' => emt('apply refund'), 'action' => "${p}edit/cust_pay_refund.cgi?". $cust_pay->paynum, 'actionlabel' => emt('Apply payment to refund'), @@ -198,20 +198,18 @@ $void = ' ('. my $unapply = ''; if ($opt{'Unapply payment'} && !$cust_pay->closed) { - my $refund_to_unapply = $cust_pay->refund_to_unapply; - my $usepre = $refund_to_unapply && @cust_bill_pay; $unapply = areyousure_link("${p}misc/unapply-cust_pay.cgi?".$cust_pay->paynum, emt('Are you sure you want to unapply this payment from invoices?'), emt('Keep this payment, but dissociate it from the invoices it is currently applied against'), - emt('unapply') . ($usepre ? ' ' . emt('invoices') : '') + emt('unapply') ) if @cust_bill_pay; $unapply .= areyousure_link("${p}misc/unapply-cust_pay_refund.cgi?".$cust_pay->paynum, emt('Are you sure you want to unapply this payment from refunds?'), emt('Keep this payment, but dissociate it from the refunds it is currently applied to'), - emt('unapply') . ($usepre ? ' ' . emt('refunds') : '') + emt('unapply refunds') ) - if $refund_to_unapply; + if $cust_pay->refund_to_unapply; } -- 2.11.0