[freeside-commits] branch FREESIDE_3_BRANCH updated. e2505a120a7d182d86d589a7b0cb881840021ddc

Mark Wells mark at 420.am
Tue Feb 2 16:37:16 PST 2016


The branch, FREESIDE_3_BRANCH has been updated
       via  e2505a120a7d182d86d589a7b0cb881840021ddc (commit)
       via  fdc589c5d08623f437df749c5e43360248ccce11 (commit)
      from  159e3b1170a3011738e0937ea1b155488ee5a83c (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 e2505a120a7d182d86d589a7b0cb881840021ddc
Author: Mark Wells <mark at freeside.biz>
Date:   Tue Feb 2 16:33:32 2016 -0800

    fix reasonnum upgrade, fallout from #39398

diff --git a/FS/FS/reason_Mixin.pm b/FS/FS/reason_Mixin.pm
index a397541..9c436ab 100644
--- a/FS/FS/reason_Mixin.pm
+++ b/FS/FS/reason_Mixin.pm
@@ -6,18 +6,22 @@ use FS::Record qw( qsearch qsearchs dbdef );
 use FS::access_user;
 use FS::UID qw( dbh );
 use FS::reason;
+use FS::reason_type;
 
 our $DEBUG = 0;
 our $me = '[FS::reason_Mixin]';
 
 =item reason
 
-Returns the text of the associated reason (see L<FS::reason>) for this credit.
+Returns the text of the associated reason (see L<FS::reason>) for this credit /
+voided payment / voided invoice. This can no longer be used to set the
+(deprecated) free-text "reason" field; see L<FS::reason/new_or_existing>.
 
 =cut
 
 sub reason {
-  my ($self, $value, %options) = @_;
+  my $self = shift;
+
   my $reason_text;
   if ( $self->reasonnum ) {
     my $reason = FS::reason->by_key($self->reasonnum);
@@ -32,65 +36,136 @@ sub reason {
   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;
-  my $table = $class->table;
-
-  if (defined dbdef->table($table)->column('reason')) {
-
-    warn "$me Checking for unmigrated reasons\n" if $DEBUG;
-
-    my @cust_refunds = qsearch({ 'table'     => $table,
-                                 'hashref'   => {},
-                                 'extra_sql' => 'WHERE reason IS NOT NULL',
-                              });
-
-    if (scalar(grep { $_->getfield('reason') =~ /\S/ } @cust_refunds)) {
-      warn "$me Found unmigrated reasons\n" if $DEBUG;
-      my $hashref = { 'class' => 'F', 'type' => 'Legacy' };
-      my $reason_type = qsearchs( 'reason_type', $hashref );
-      unless ($reason_type) {
-        $reason_type  = new FS::reason_type( $hashref );
-        my $error   = $reason_type->insert();
-        die "$class had error inserting FS::reason_type into database: $error\n"
-          if $error;
-      }
-
-      $hashref = { 'reason_type' => $reason_type->typenum,
-                   'reason' => '(none)'
-                 };
-      my $noreason = qsearchs( 'reason', $hashref );
-      unless ($noreason) {
-        $hashref->{'disabled'} = 'Y';
-        $noreason = new FS::reason( $hashref );
-        my $error  = $noreason->insert();
-        die "can't insert legacy reason '(none)' into database: $error\n"
-          if $error;
-      }
-
-      foreach my $cust_refund ( @cust_refunds ) {
-        my $reason = $cust_refund->getfield('reason');
-        warn "Contemplating reason $reason\n" if $DEBUG > 1;
-        if ($reason =~ /\S/) {
-          $cust_refund->reason($reason, 'reason_type' => $reason_type->typenum)
-            or die "can't insert legacy reason $reason into database\n";
-        }else{
-          $cust_refund->reasonnum($noreason->reasonnum);
+# Note that any new tables that get reasonnum fields do NOT need to be
+# added here unless they have previously had a free-text "reason" field.
+
+sub _upgrade_reasonnum {    # class method
+    my $class = shift;
+    my $table = $class->table;
+
+    my $reason_class;
+    if ( $table =~ /^cust_bill/ ) { # also includes cust_bill_pkg
+      $reason_class = 'I';
+    } elsif ( $table =~ /^cust_pay/ ) {
+      $reason_class = 'P';
+    } elsif ( $table eq 'cust_refund' ) {
+      $reason_class = 'F';
+    } elsif ( $table =~ /^cust_credit/ ) {
+      $reason_class = 'R';
+    } else {
+      die "don't know the reason class to use for upgrading $table";
+    }
+
+    for my $fieldname (qw(reason void_reason)) {
+
+        if ( $table =~ /^cust_credit/ and $fieldname eq 'void_reason' ) {
+            $reason_class = 'X';
+        }
+
+        if (   defined dbdef->table($table)->column($fieldname)
+            && defined dbdef->table($table)->column( $fieldname . 'num' ) )
+        {
+
+            warn "$me Checking for unmigrated reasons\n" if $DEBUG;
+
+            my @legacy_reason_records = qsearch(
+                {
+                    'table'     => $table,
+                    'hashref'   => {},
+                    'extra_sql' => 'WHERE ' . $fieldname . ' IS NOT NULL',
+                }
+            );
+
+            if ( @legacy_reason_records ) {
+
+                warn "$me Found unmigrated reasons\n" if $DEBUG;
+
+                my $reason_type =
+                  $class->_upgrade_get_legacy_reason_type( $reason_class );
+                # XXX "noreason" does not actually work, because we limited to
+                # "reason is not null" above. Records where the reason string
+                # is null will end up with a reasonnum of null also.
+                my $noreason = $class->_upgrade_get_no_reason( $reason_type );
+
+                foreach my $record_to_upgrade (@legacy_reason_records) {
+                    my $reason = $record_to_upgrade->getfield($fieldname);
+                    warn "Contemplating reason $reason\n" if $DEBUG > 1;
+                    if ( $reason =~ /\S/ ) {
+                        my $reason =
+                          $class->_upgrade_get_reason( $reason, $reason_type );
+                        $record_to_upgrade->set( $fieldname . 'num',
+                            $reason->reasonnum );
+                    }
+                    else {
+                        $record_to_upgrade->set( $fieldname . 'num',
+                            $noreason->reasonnum );
+                    }
+
+                    $record_to_upgrade->setfield( $fieldname, '' );
+                    my $error = $record_to_upgrade->replace;
+
+                    my $primary_key = $record_to_upgrade->primary_key;
+                    warn "*** WARNING: error replacing $fieldname in $class "
+                      . $record_to_upgrade->get($primary_key)
+                      . ": $error ***\n"
+                      if $error;
+                }
+            }
         }
+    }
+}
 
-        $cust_refund->setfield('reason', '');
-        my $error = $cust_refund->replace;
+# internal methods for upgrade
 
-        warn "*** WARNING: error replacing reason in $class ".
-             $cust_refund->refundnum. ": $error ***\n"
-          if $error;
-      }
+# _upgrade_get_legacy_reason_type is class method supposed to be used only
+# within the reason_Mixin class which will either find or create a reason_type
+sub _upgrade_get_legacy_reason_type {
+ 
+    my $class = shift;
+    my $reason_class = shift;
+    my $reason_type_params = { 'class' => $reason_class, 'type' => 'Legacy' };
+    my $reason_type = qsearchs( 'reason_type', $reason_type_params );
+    unless ($reason_type) {
+        $reason_type = new FS::reason_type($reason_type_params);
+        my $error = $reason_type->insert();
+        die "$class had error inserting FS::reason_type into database: $error\n"
+           if $error;
     }
-  }
+    return $reason_type;
+}
+
+# _upgrade_get_no_reason is class method supposed to be used only within the
+# reason_Mixin class which will either find or create a default (no reason)
+# reason
+sub _upgrade_get_no_reason {
+
+    my $class       = shift;
+    my $reason_type = shift;
+    return $class->_upgrade_get_reason( '(none)', $reason_type );
+}
+
+# _upgrade_get_reason is class method supposed to be used only within the
+# reason_Mixin class which will either find or create a reason
+sub _upgrade_get_reason {
+
+    my $class       = shift;
+    my $reason_text = shift;
+    my $reason_type = shift;
+
+    my $reason_params = {
+        'reason_type' => $reason_type->typenum,
+        'reason'      => $reason_text
+    };
+    my $reason = qsearchs( 'reason', $reason_params );
+    unless ($reason) {
+        $reason_params->{'disabled'} = 'Y';
+        $reason = new FS::reason($reason_params);
+        my $error = $reason->insert();
+        die "can't insert legacy reason '$reason_text' into database: $error\n"
+           if $error;
+     }
+    return $reason;
 }
 
 1;

commit fdc589c5d08623f437df749c5e43360248ccce11
Author: Mark Wells <mark at freeside.biz>
Date:   Tue Feb 2 15:51:58 2016 -0800

    fix recording of BOP refunds, fallout from #39398

diff --git a/FS/FS/cust_main/Billing_Realtime.pm b/FS/FS/cust_main/Billing_Realtime.pm
index f33c454..61acb1d 100644
--- a/FS/FS/cust_main/Billing_Realtime.pm
+++ b/FS/FS/cust_main/Billing_Realtime.pm
@@ -1311,14 +1311,14 @@ L<http://420.am/business-onlinepayment> for supported gateways.
 
 Available methods are: I<CC>, I<ECHECK> and I<LEC>
 
-Available options are: I<amount>, I<reason>, I<paynum>, I<paydate>
+Available options are: I<amount>, I<reasonnum>, I<paynum>, I<paydate>
 
 Most gateways require a reference to an original payment transaction to refund,
 so you probably need to specify a I<paynum>.
 
 I<amount> defaults to the original amount of the payment if not specified.
 
-I<reason> specifies a reason for the refund.
+I<reasonnum> specifies a reason for the refund.
 
 I<paydate> specifies the expiration date for a credit card overriding the
 value from the customer record or the payment record. Specified as yyyy-mm-dd
@@ -1356,6 +1356,27 @@ sub realtime_refund_bop {
     $options{method} = $method;
   }
 
+  my ($reason, $reason_text);
+  if ( $options{'reasonnum'} ) {
+    # do this here, because we need the plain text reason string in case we
+    # void the payment
+    $reason = FS::reason->by_key($options{'reasonnum'});
+    $reason_text = $reason->reason;
+  } else {
+    # support old 'reason' string parameter in case it's still used,
+    # or else set a default
+    $reason_text = $options{'reason'} || 'card or ACH refund';
+    local $@;
+    $reason = FS::reason->new_or_existing(
+      reason  => $reason_text,
+      type    => 'Refund reason',
+      class   => 'F',
+    );
+    if ($@) {
+      return "failed to add refund reason: $@";
+    }
+  }
+
   if ( $DEBUG ) {
     warn "$me realtime_refund_bop (new): $options{method} refund\n";
     warn "  $_ => $options{$_}\n" foreach keys %options;
@@ -1523,7 +1544,7 @@ sub realtime_refund_bop {
       if $conf->exists('business-onlinepayment-test_transaction');
     $void->submit();
     if ( $void->is_success ) {
-      my $error = $cust_pay->void($options{'reason'});
+      my $error = $cust_pay->void($reason_text);
       if ( $error ) {
         # gah, even with transactions.
         my $e = 'WARNING: Card/ACH voided but database not updated - '.
@@ -1648,7 +1669,7 @@ sub realtime_refund_bop {
     '_date'    => '',
     'payby'    => $bop_method2payby{$options{method}},
     'payinfo'  => $payinfo,
-    'reason'   => $options{'reason'} || 'card or ACH refund',
+    'reasonnum'   => $reason->reasonnum,
     'gatewaynum'    => $gatewaynum, # may be null
     'processor'     => $processor,
     'auth'          => $refund->authorization,
diff --git a/FS/FS/cust_refund.pm b/FS/FS/cust_refund.pm
index 15335a4..74728a6 100644
--- a/FS/FS/cust_refund.pm
+++ b/FS/FS/cust_refund.pm
@@ -145,19 +145,21 @@ sub insert {
   my $dbh = dbh;
 
   unless ($self->reasonnum) {
-    my $result = $self->reason( $self->getfield('reason'),
-                                exists($options{ 'reason_type' })
-                                  ? ('reason_type' => $options{ 'reason_type' })
-                                  : (),
-                              );
-    unless($result) {
-      $dbh->rollback if $oldAutoCommit;
-      return "failed to set reason for $me"; #: ". $dbh->errstr;
+    local $@;
+    if ( $self->get('reason') ) {
+      my $reason = FS::reason->new_or_existing(
+        reason  => $self->get('reason'),
+        class   => 'F',
+        type    => 'Refund reason',
+      );
+      if ($@) {
+        return "failed to add refund reason: $@";
+      }
+      $self->set('reasonnum', $reason->get('reasonnum'));
+      $self->set('reason', '');
     }
   }
 
-  $self->setfield('reason', '');
-
   if ( $self->crednum ) {
     my $cust_credit = qsearchs('cust_credit', { 'crednum' => $self->crednum } )
       or do {
diff --git a/httemplate/edit/process/cust_refund.cgi b/httemplate/edit/process/cust_refund.cgi
index 6ad468b..764f2de 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' => $reasonnum,
                                                   %options );
 } else {
   my %hash = map {

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/cust_main/Billing_Realtime.pm     |   29 ++++-
 FS/FS/cust_refund.pm                    |   22 ++--
 FS/FS/reason_Mixin.pm                   |  187 ++++++++++++++++++++++---------
 httemplate/edit/process/cust_refund.cgi |    3 +-
 4 files changed, 169 insertions(+), 72 deletions(-)




More information about the freeside-commits mailing list