From 2073798304acbd9402f73e0dee7507a7a4d22ceb Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Thu, 22 Jan 2015 17:29:48 -0800 Subject: [PATCH] unsnarl creation of credit/refund reasons, partial fallout from #31702 --- FS/FS/cust_credit.pm | 23 +++++++---- FS/FS/reason.pm | 26 +++++++++---- FS/FS/reason_Mixin.pm | 53 +++++++------------------- httemplate/edit/cust_refund.cgi | 2 +- httemplate/edit/process/cust_refund.cgi | 9 ++++- httemplate/view/cust_main/payment_history.html | 2 +- 6 files changed, 57 insertions(+), 58 deletions(-) diff --git a/FS/FS/cust_credit.pm b/FS/FS/cust_credit.pm index 9eb624e48..df2a6cc19 100644 --- a/FS/FS/cust_credit.pm +++ b/FS/FS/cust_credit.pm @@ -172,16 +172,23 @@ sub insert { my $cust_main = qsearchs( 'cust_main', { 'custnum' => $self->custnum } ); my $old_balance = $cust_main->balance; - 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 => 'R', + ); + if ($@) { $dbh->rollback if $oldAutoCommit; - return "failed to set reason for $me"; #: ". $dbh->errstr; + return "failed to set credit reason: $@"; } + $self->set('reasonnum', $reason->reasonnum); } $self->setfield('reason', ''); diff --git a/FS/FS/reason.pm b/FS/FS/reason.pm index f28989a9b..864804d26 100644 --- a/FS/FS/reason.pm +++ b/FS/FS/reason.pm @@ -152,7 +152,8 @@ sub reasontype { Fetches the reason matching these parameters if there is one. If not, inserts one. Will also insert the reason type if necessary. CLASS must -be one of 'C' (cancel reasons), 'R' (credit reasons), or 'S' (suspend reasons). +be one of 'C' (cancel reasons), 'R' (credit reasons), 'S' (suspend reasons), +or 'F' (refund reasons). This will die if anything fails. @@ -163,14 +164,25 @@ sub new_or_existing { my %opt = @_; my $error = ''; - my %hash = ('class' => $opt{'class'}, 'type' => $opt{'type'}); - my $reason_type = qsearchs('reason_type', \%hash) - || FS::reason_type->new(\%hash); + my $reason_type; + if ( ref $opt{type} eq 'FS::reason_type' ) { + $reason_type = $opt{type}; + } elsif ( $opt{type} =~ /^\d+$/ ) { + $reason_type = FS::reason_type->by_key($opt{type}); + if (!$reason_type) { + die "reason_type #$opt{type} not found\n"; + } + } else { + my %hash = ('class' => $opt{'class'}, 'type' => $opt{'type'}); + my $reason_type = qsearchs('reason_type', \%hash) + || FS::reason_type->new(\%hash); - $error = $reason_type->insert unless $reason_type->typenum; - die "error inserting reason type: $error\n" if $error; + $error = $reason_type->insert unless $reason_type->typenum; + die "error inserting reason type: $error\n" if $error; + } - %hash = ('reason_type' => $reason_type->typenum, 'reason' => $opt{'reason'}); + my %hash = ('reason_type' => $reason_type->typenum, + 'reason' => $opt{'reason'}); my $reason = qsearchs('reason', \%hash) || FS::reason->new(\%hash); diff --git a/FS/FS/reason_Mixin.pm b/FS/FS/reason_Mixin.pm index fdf796219..a3975419c 100644 --- a/FS/FS/reason_Mixin.pm +++ b/FS/FS/reason_Mixin.pm @@ -5,6 +5,7 @@ use Carp qw( croak ); #confess ); use FS::Record qw( qsearch qsearchs dbdef ); use FS::access_user; use FS::UID qw( dbh ); +use FS::reason; our $DEBUG = 0; our $me = '[FS::reason_Mixin]'; @@ -17,49 +18,23 @@ Returns the text of the associated reason (see L) for this credit. sub reason { my ($self, $value, %options) = @_; - my $dbh = dbh; - my $reason; - my $typenum = $options{'reason_type'}; - - my $oldAutoCommit = $FS::UID::AutoCommit; # this should already be in - local $FS::UID::AutoCommit = 0; # a transaction if it matters - - if ( defined( $value ) ) { - my $hashref = { 'reason' => $value }; - $hashref->{'reason_type'} = $typenum if $typenum; - my $addl_from = "LEFT JOIN reason_type ON ( reason_type = typenum ) "; - my $extra_sql = " AND reason_type.class='F'"; - - $reason = qsearchs( { 'table' => 'reason', - 'hashref' => $hashref, - 'addl_from' => $addl_from, - 'extra_sql' => $extra_sql, - } ); - - if (!$reason && $typenum) { - $reason = new FS::reason( { 'reason_type' => $typenum, - 'reason' => $value, - 'disabled' => 'Y', - } ); - my $error = $reason->insert; - if ( $error ) { - warn "error inserting reason: $error\n"; - $reason = undef; - } - } - - $self->reasonnum($reason ? $reason->reasonnum : '') ; - warn "$me reason used in set mode with non-existant reason -- clearing" - unless $reason; + my $reason_text; + if ( $self->reasonnum ) { + my $reason = FS::reason->by_key($self->reasonnum); + $reason_text = $reason->reason; + } else { # in case one of these somehow still exists + $reason_text = $self->get('reason'); + } + if ( $self->get('addlinfo') ) { + $reason_text .= ' ' . $self->get('addlinfo'); } - $reason = qsearchs( 'reason', { 'reasonnum' => $self->reasonnum } ); - - $dbh->commit or die $dbh->errstr if $oldAutoCommit; - ( $reason ? $reason->reason : '' ). - ( $self->addlinfo ? ' '.$self->addlinfo : '' ); + return $reason_text; } +# it was a mistake to allow setting the reason this way; use +# FS::reason->new_or_existing + # Used by FS::Upgrade to migrate reason text fields to reasonnum. sub _upgrade_reasonnum { # class method my $class = shift; diff --git a/httemplate/edit/cust_refund.cgi b/httemplate/edit/cust_refund.cgi index 9f7ac8dee..f9095fd7a 100755 --- a/httemplate/edit/cust_refund.cgi +++ b/httemplate/edit/cust_refund.cgi @@ -109,7 +109,7 @@ <& /elements/tr-select-reason.html, 'field' => 'reasonnum', 'reason_class' => 'F', - 'control_button' => "document.getElementById('confirm_refund_button')", + 'control_button' => "confirm_refund_button", 'cgi' => $cgi, &> diff --git a/httemplate/edit/process/cust_refund.cgi b/httemplate/edit/process/cust_refund.cgi index bde40727a..599c8b8e2 100755 --- a/httemplate/edit/process/cust_refund.cgi +++ b/httemplate/edit/process/cust_refund.cgi @@ -41,8 +41,13 @@ push @rights, 'Refund Echeck payment' if $payby eq 'CHEK'; die "access denied" unless $FS::CurrentUser::CurrentUser->access_right(\@rights); -my $error = ''; -if ( $payby =~ /^(CARD|CHEK)$/ ) { +$cgi->param('reasonnum') =~ /^(-?\d+)$/ or die "Illegal reasonnum"; +my ($reasonnum, $error) = $m->comp('/misc/process/elements/reason'); +$cgi->param('reasonnum', $reasonnum) unless $error; + +if ( $error ) { + # do nothing +} elsif ( $payby =~ /^(CARD|CHEK)$/ ) { my %options = (); my $bop = $FS::payby::payby2bop{$1}; $cgi->param('refund') =~ /^(\d*)(\.\d{2})?$/ diff --git a/httemplate/view/cust_main/payment_history.html b/httemplate/view/cust_main/payment_history.html index fcc4470a0..a8f2f861a 100644 --- a/httemplate/view/cust_main/payment_history.html +++ b/httemplate/view/cust_main/payment_history.html @@ -97,7 +97,7 @@ 'action' => "${p}edit/cust_refund.cgi?popup=1;payby=BILL", 'cust_main' => $cust_main, 'actionlabel' => emt('Enter check refund'), - 'width' => 392, + 'width' => 440, &> % } -- 2.11.0