separate reason classes for voiding different transaction types, #38532

5 files changed, 46 insertions, 26 deletions

diff --git a/FS/FS/reason_Mixin.pm b/FS/FS/reason_Mixin.pm
index af9aa50e6..95fe3e9f8 100644
--- a/FS/FS/reason_Mixin.pm
+++ b/FS/FS/reason_Mixin.pm
@@ -14,7 +14,8 @@ our $me = '[FS::reason_Mixin]';
 =item reason
 
 Returns the text of the associated reason (see L<FS::reason>) for this credit /
-voided payment / voided invoice.
+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
 
@@ -35,15 +36,33 @@ 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.
+# 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 eq 'cust_bill' or $table eq 'cust_bill_pkg' ) {
+      $reason_class = 'I';
+    } elsif ( $table eq 'cust_pay' ) {
+      $reason_class = 'P';
+    } elsif ( $table eq 'cust_refund' ) {
+      $reason_class = 'F';
+    } elsif ( $table eq '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 eq 'cust_credit' and $fieldname eq 'void_reason' ) {
+            $reason_class = 'X';
+        }
+
         if (   defined dbdef->table($table)->column($fieldname)
             && defined dbdef->table($table)->column( $fieldname . 'num' ) )
         {
diff --git a/FS/FS/reason_type.pm b/FS/FS/reason_type.pm
index 4042972b4..a727ccd0a 100644
--- a/FS/FS/reason_type.pm
+++ b/FS/FS/reason_type.pm
@@ -6,12 +6,14 @@ use FS::Record qw( qsearch qsearchs );
 
 @ISA = qw(FS::Record);
 
-our %class_name = (  
+tie our %class_name, Tie::IxHash, (  
   'C' => 'cancel',
   'R' => 'credit',
   'S' => 'suspend',
   'F' => 'refund',
-  'X' => 'void', # credit/invoice/payment
+  'X' => 'void credit',
+  'I' => 'void invoice',
+  'P' => 'void payment',
 );
 
 our %class_purpose = (  
@@ -19,7 +21,19 @@ our %class_purpose = (
   'R' => 'explain why a customer was credited',
   'S' => 'explain why a customer package was suspended',
   'F' => 'explain why a customer was refunded',
-  'X' => 'explain why a transaction was voided',
+  'X' => 'explain why a credit was voided',
+  'I' => 'explain why an invoice was voided',
+  'P' => 'explain why a payment was voided',
+);
+
+our %class_add_access_right = (
+  'C' => 'Add on-the-fly cancel reason',
+  'R' => 'Add on-the-fly credit reason',
+  'S' => 'Add on-the-fly suspend reason',
+  'F' => 'Add on-the-fly refund reason',
+  'X' => 'Add on-the-fly void reason',
+  'I' => 'Add on-the-fly void reason',
+  'P' => 'Add on-the-fly void reason',
 );
 
 =head1 NAME
@@ -50,7 +64,7 @@ inherits from FS::Record.  The following fields are currently supported:
 
 =item typenum - primary key
 
-=item class - currently 'C', 'R', 'S', 'F' or 'X' for cancel, credit, suspend, refund or void credit 
+=item class - one of the keys of %class_name
 
 =item type - name of the type of reason
 
diff --git a/httemplate/elements/tr-select-reason.html b/httemplate/elements/tr-select-reason.html
index 37a34baba..93949ba8c 100755
--- a/httemplate/elements/tr-select-reason.html
+++ b/httemplate/elements/tr-select-reason.html
@@ -6,8 +6,7 @@ Example:
 
     #required 
     'field'         => 'reasonnum',
-    'reason_class'  => 'C', # currently 'C', 'R', 'F', 'S' or 'X'
-                           # for cancel, credit, refund, suspend or void credit
+    'reason_class'  => 'C', # one of those in %FS::reason_type::class_name
 
     #recommended
     'cgi' => $cgi, #easiest way for things to be properly "sticky" on errors
@@ -189,20 +188,8 @@ if ( $opt{'cgi'} ) {
 my $id = $opt{'id'} || $name;
 $id =~ s/\./_/g; # for edit/part_event
 
-my $add_access_right;
-if ($class eq 'C') {
-  $add_access_right = 'Add on-the-fly cancel reason';
-} elsif ($class eq 'S') {
-  $add_access_right = 'Add on-the-fly suspend reason';
-} elsif ($class eq 'R') {
-  $add_access_right = 'Add on-the-fly credit reason';
-} elsif ($class eq 'F') {
-  $add_access_right = 'Add on-the-fly refund reason';
-} elsif ($class eq 'X') {
-  $add_access_right = 'Add on-the-fly void reason';
-} else {
-  die "illegal class: $class";
-}
+my $add_access_right = $FS::reason_type::class_add_access_right{$class}
+  or die "unknown class: $class";
 
 my @reasons = qsearch({
   'table'           => 'reason',
diff --git a/httemplate/misc/void-cust_bill.cgi b/httemplate/misc/void-cust_bill.cgi
index 213cf9566..e4e4705d7 100755
--- a/httemplate/misc/void-cust_bill.cgi
+++ b/httemplate/misc/void-cust_bill.cgi
@@ -14,7 +14,7 @@
 <% ntable("#cccccc", 2) %>
 <& /elements/tr-select-reason.html,
              'field'          => 'reasonnum',
-             'reason_class'   => 'X',
+             'reason_class'   => 'I',
              'cgi'            => $cgi
 &>
 
diff --git a/httemplate/misc/void-cust_pay.cgi b/httemplate/misc/void-cust_pay.cgi
index 784bb9a94..dc0ff4d35 100755
--- a/httemplate/misc/void-cust_pay.cgi
+++ b/httemplate/misc/void-cust_pay.cgi
@@ -18,7 +18,7 @@
 <TABLE BGCOLOR="#cccccc" BORDER="0" CELLSPACING="2" STYLE="margin-left:auto; margin-right:auto">
 <& /elements/tr-select-reason.html,
              'field'          => 'reasonnum',
-             'reason_class'   => 'X',
+             'reason_class'   => 'P',
              'cgi'            => $cgi
 &>
 </TABLE>