Merge branch 'master' of git.freeside.biz:/home/git/freeside

15 files changed, 134 insertions, 143 deletions

diff --git a/FS/FS/cust_pay_void.pm b/FS/FS/cust_pay_void.pm
index 8fd539616..72ada2534 100644
--- a/FS/FS/cust_pay_void.pm
+++ b/FS/FS/cust_pay_void.pm
@@ -234,6 +234,8 @@ Returns the text of the associated void reason (see L<FS::reason>) for this.
 sub _upgrade_data {  # class method
   my ($class, %opts) = @_;
 
+  local $FS::payinfo_Mixin::ignore_masked_payinfo = 1;
+
   $class->_upgrade_reasonnum(%opts);
 
   my $sql = "SELECT usernum FROM access_user WHERE username = ( SELECT history_user FROM h_cust_pay_void WHERE paynum = ? AND history_action = 'insert' ORDER BY history_date LIMIT 1 ) ";
diff --git a/FS/FS/reason_Mixin.pm b/FS/FS/reason_Mixin.pm
index af9aa50e6..9c436ab1e 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 =~ /^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' ) )
         {
@@ -58,25 +77,23 @@ sub _upgrade_reasonnum {    # class method
                 }
             );
 
-            if (
-                scalar(
-                    grep { $_->getfield($fieldname) =~ /\S/ }
-                      @legacy_reason_records
-                )
-              )
-            {
+            if ( @legacy_reason_records ) {
+
                 warn "$me Found unmigrated reasons\n" if $DEBUG;
 
                 my $reason_type =
-                  _upgrade_get_legacy_reason_type( $class, $table );
-                my $noreason = _upgrade_get_no_reason( $class, $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 =
-                          _upgrade_get_reason( $class, $reason, $reason_type );
+                          $class->_upgrade_get_reason( $reason, $reason_type );
                         $record_to_upgrade->set( $fieldname . 'num',
                             $reason->reasonnum );
                     }
@@ -99,15 +116,14 @@ sub _upgrade_reasonnum {    # class method
     }
 }
 
+# internal methods for upgrade
+
 # _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 $table = shift;
-
-    my $reason_class =
-      ( $table =~ /void/ ) ? 'X' : 'F';    # see FS::reason_type (%class_name)
+    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) {
@@ -126,7 +142,7 @@ sub _upgrade_get_no_reason {
 
     my $class       = shift;
     my $reason_type = shift;
-    return _upgrade_get_reason( $class, '(none)', $reason_type );
+    return $class->_upgrade_get_reason( '(none)', $reason_type );
 }
 
 # _upgrade_get_reason is class method supposed to be used only within the
diff --git a/FS/FS/reason_type.pm b/FS/FS/reason_type.pm
index 4042972b4..1d049861d 100644
--- a/FS/FS/reason_type.pm
+++ b/FS/FS/reason_type.pm
@@ -3,15 +3,18 @@ package FS::reason_type;
 use strict;
 use vars qw( @ISA );
 use FS::Record qw( qsearch qsearchs );
+use Tie::IxHash;
 
 @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' => 'credit void',
+  'I' => 'invoice void',
+  'P' => 'payment void',
 );
 
 our %class_purpose = (  
@@ -19,7 +22,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 +65,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/browse/reason_type.html b/httemplate/browse/reason_type.html
index 6b444bad1..0cb6e7a39 100644
--- a/httemplate/browse/reason_type.html
+++ b/httemplate/browse/reason_type.html
@@ -1,49 +1,62 @@
-<% include( 'elements/browse.html',
-                 'title'       => ucfirst($classname) . " Reason Types",
-                 'menubar'     => [ ucfirst($classname) . " reasons" =>
-		                     $p.'browse/reason.html?class=' .  $class,
-                                  ],
-                 'html_init'   => $html_init,
-                 'name'        => $classname . " reason types",
-                 'query'       => { 'table'     => 'reason_type',
-                                    'hashref'   => {},
-                                    'extra_sql' => $where_clause .
-				                   'ORDER BY typenum',
-                                  },
-                 'count_query' => $count_query,
-                 'header'      => [ '#',
-                                    ucfirst($classname) . ' Reason Type',
-                                    ucfirst($classname) . ' Reasons',
-                                  ],
-                 'fields'      => [ 'typenum',
-                                    'type',
-                                    $reasons_sub,
-                                  ],
-                 'links'       => [ $link,
-                                    $link,
-                                    '',
-                                  ],
-             )
-%>
+<& elements/browse.html,
+  'title'       => ucwords($classname) . " Reasons",
+  'html_init'   => $html_init,
+  'name'        => $classname . " reason types",
+  'query'       => { 'table'     => 'reason_type',
+                    'hashref'   => {},
+                    'extra_sql' => $where_clause .
+                                   'ORDER BY typenum',
+                  },
+  'count_query' => $count_query,
+  'header'      => [ '#',
+                    ucwords($classname) . ' Reason Type',
+                    ucwords($classname) . ' Reasons',
+                  ],
+  'fields'      => [ 'typenum',
+                    'type',
+                    $reasons_sub,
+                  ],
+  'links'       => [ $link,
+                    $link,
+                    '',
+                  ],
+  'disable_total' => 1,
+&>
 <%init>
 
+sub ucwords {
+  join(' ', map ucfirst($_), split(/ /, shift));
+}
+
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Configuration');
 
 $cgi->param('class') =~ /^(\w)$/ or die "illegal class";
 my $class=$1;
 
-my $classname = $FS::reason_type::class_name{$class};
+my $classname = ucfirst($FS::reason_type::class_name{$class});
+
+my $html_init = 'Reasons: ' .
+  include('/elements/menubar.html',
+    map {
+      ucfirst($FS::reason_type::class_name{$_}),
+      $p.'browse/reason_type.html?class=' .  $_
+    } keys (%FS::reason_type::class_name)
+  );
 
-my $html_init = ucfirst($classname) .
-  " reason types allow groups of $classname reasons for reporting purposes." .
-  qq!<BR><BR><A HREF="${p}edit/reason_type.html?class=$class"><I>Add a ! .
-  $classname . " reason type</I></A><BR><BR>";
+$html_init .= '<BR><P>' .
+  $classname . ' reasons ' .
+  $FS::reason_type::class_purpose{$class} .
+  '. Reason types allow reasons to be grouped for reporting purposes.' .
+  qq!<BR><BR><A HREF="${p}edit/reason_type.html?class=$class"><I>! .
+  ($classname =~ /^[aeiou]/i ? 'Add an ' : 'Add a ') .
+  lc($classname) . ' reason type</I></A>'.
+  '</P>';
 
 my $reasons_sub = sub {
   my $reason_type = shift;
 
-  [ map {
+  [ ( map {
           [
             {
               'data'  => $_->reason,
@@ -53,7 +66,15 @@ my $reasons_sub = sub {
             },
           ];
         }
-    $reason_type->enabled_reasons,
+    $reason_type->enabled_reasons ),
+    [
+      {
+        'data'  => '(add)',
+        'align' => 'left',
+        'link'  => $p. "edit/reason.html?class=$class",
+        'data_style' => 'i',
+      }
+    ]
 
   ];
   
diff --git a/httemplate/elements/menu.html b/httemplate/elements/menu.html
index 81c121259..fa44e86e7 100644
--- a/httemplate/elements/menu.html
+++ b/httemplate/elements/menu.html
@@ -621,10 +621,8 @@ $config_export_svc{'Hardware types'} = [ $fsurl.'browse/hardware_class.html', 'S
   if $curuser->access_right('Configuration');
 
 tie my %config_pkg_reason, 'Tie::IxHash',
-  'Cancel reasons' => [ $fsurl.'browse/reason.html?class=C', 'Cancel reasons explain why a service was cancelled.' ],
-  'Cancel reason types' => [ $fsurl.'browse/reason_type.html?class=C', 'Cancel reason types define groups of reasons.' ],
-  'Suspend reasons' => [ $fsurl.'browse/reason.html?class=S', 'Suspend reasons explain why a service was suspended.' ],
-  'Suspend reason types' => [ $fsurl.'browse/reason_type.html?class=S', 'Suspend reason types define groups of reasons.' ],
+  'Cancel reasons' => [ $fsurl.'browse/reason_type.html?class=C', 'Cancel reasons explain why a service was cancelled.' ],
+  'Suspend reasons' => [ $fsurl.'browse/reason_type.html?class=S', 'Suspend reasons explain why a service was suspended.' ],
 ;
 
 tie my %config_pkg, 'Tie::IxHash', ();
@@ -715,16 +713,13 @@ if ( $curuser->access_right('Configuration') ) {
   }
 
   $config_billing{'separator4'} = ''; #its a separator!
-  $config_billing{'Credit reasons'}  = [ $fsurl.'browse/reason.html?class=R', 'Credit reasons explain why a credit was issued.' ];
-  $config_billing{'Credit reason types'}  = [ $fsurl.'browse/reason_type.html?class=R', 'Credit reason types define groups of reasons.' ];
+  $config_billing{'Credit reasons'}  = [ $fsurl.'browse/reason_type.html?class=R', 'Credit reasons explain why a credit was issued.' ];
   
-  $config_billing{'separator5'} = ''; #its a separator!
-  $config_billing{'Refund reasons'}  = [ $fsurl.'browse/reason.html?class=F', 'Refund reasons explain why a refund was issued.' ];
-  $config_billing{'Refund reason types'}  = [ $fsurl.'browse/reason_type.html?class=F', 'Refund reason types define groups of reasons.' ];
+  $config_billing{'Refund reasons'}  = [ $fsurl.'browse/reason_type.html?class=F', 'Refund reasons explain why a refund was issued.' ];
 
-  $config_billing{'separator6'} = ''; #its a separator!
-  $config_billing{'Void reasons'}  = [ $fsurl.'browse/reason.html?class=X', 'Void reasons explain why a void was issued.' ];
-  $config_billing{'Void reason types'}  = [ $fsurl.'browse/reason_type.html?class=X', 'Void reason types define groups of reasons.' ];
+  $config_billing{'Invoice void reasons'}  = [ $fsurl.'browse/reason_type.html?class=I', 'Invoice void reasons explain why an invoice was voided.' ];
+  $config_billing{'Payment void reasons'}  = [ $fsurl.'browse/reason_type.html?class=P', 'Payment void reasons explain why a payment was voided.' ];
+  $config_billing{'Credit void reasons'}  = [ $fsurl.'browse/reason_type.html?class=X', 'Credit void reasons explain why a credit was voided.' ];
 }
 
 #XXX also to be unified
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/process/void-cust_bill.html b/httemplate/misc/process/void-cust_bill.html
index c0f432b70..7773b0ba9 100755
--- a/httemplate/misc/process/void-cust_bill.html
+++ b/httemplate/misc/process/void-cust_bill.html
@@ -1,6 +1,6 @@
 %if ( $error ) {
 %  $cgi->param('error', $error);
-<% $cgi->redirect(popurl(2). "void-cust_bill.cgi?". $cgi->query_string ) %>
+<% $cgi->redirect(popurl(2). "void-cust_bill.html?". $cgi->query_string ) %>
 %} else {
 <& /elements/header-popup.html, 'Invoice voided' &>
 <SCRIPT TYPE="text/javascript">
diff --git a/httemplate/misc/void-cust_bill.cgi b/httemplate/misc/void-cust_bill.cgi
deleted file mode 100755
index 213cf9566..000000000
--- a/httemplate/misc/void-cust_bill.cgi
+++ /dev/null
@@ -1,46 +0,0 @@
-<& /elements/header-popup.html, mt('Void invoice') &>
-
-<% include('/elements/error.html') %>
-
-<% emt('Are you sure you want to void this invoice?') %>
-<BR><BR>
-
-<% emt("Invoice #[_1] ([_2])",$cust_bill->display_invnum, $money_char. $cust_bill->owed) %>
-<BR><BR>
-
-<FORM METHOD="POST" ACTION="process/void-cust_bill.html">
-<INPUT TYPE="hidden" NAME="invnum" VALUE="<% $invnum %>">
-
-<% ntable("#cccccc", 2) %>
-<& /elements/tr-select-reason.html,
-             'field'          => 'reasonnum',
-             'reason_class'   => 'X',
-             'cgi'            => $cgi
-&>
-
-</TABLE>
-
-<BR>
-<CENTER>
-<BUTTON TYPE="submit">Yes, void invoice</BUTTON>&nbsp;&nbsp;&nbsp;\
-<BUTTON TYPE="button" onClick="parent.cClick();">No, do not void invoice</BUTTON>
-</CENTER>
-
-</FORM>
-</BODY>
-</HTML>
-<%init>
-
-die "access denied"
-  unless $FS::CurrentUser::CurrentUser->access_right('Void invoices');
-
-my $conf = new FS::Conf;
-my $money_char = $conf->config('money_char') || '$';
-
-#untaint invnum
-$cgi->param('invnum') =~ /^(\d+)$/ || die "Illegal invnum";
-my $invnum = $1;
-
-my $cust_bill = qsearchs('cust_bill',{'invnum'=>$invnum});
-
-</%init>
diff --git a/httemplate/misc/void-cust_bill.html b/httemplate/misc/void-cust_bill.html
index 39b071229..e4e4705d7 100644..100755
--- a/httemplate/misc/void-cust_bill.html
+++ b/httemplate/misc/void-cust_bill.html
@@ -12,10 +12,11 @@
 <INPUT TYPE="hidden" NAME="invnum" VALUE="<% $invnum %>">
 
 <% ntable("#cccccc", 2) %>
-<TR>
-  <TD ALIGN="right">Reason</TD>
-  <TD><INPUT TYPE="text" NAME="reason" VALUE="<% scalar($cgi->param('reason')) |h %>"></TD>
-</TR>
+<& /elements/tr-select-reason.html,
+             'field'          => 'reasonnum',
+             'reason_class'   => 'I',
+             'cgi'            => $cgi
+&>
 
 </TABLE>
 
diff --git a/httemplate/misc/void-cust_credit.cgi b/httemplate/misc/void-cust_credit.html
index f352e6090..1e71f0030 100755
--- a/httemplate/misc/void-cust_credit.cgi
+++ b/httemplate/misc/void-cust_credit.html
@@ -12,7 +12,7 @@
 
 <P ALIGN="center"><B><% mt('Void this credit?') |h %></B>
 
-<FORM action="<% ${p} %>misc/void-cust_credit.cgi">
+<FORM action="<% ${p} %>misc/void-cust_credit.html">
 <INPUT TYPE="hidden" NAME="crednum" VALUE="<% $crednum %>">
 
 <TABLE BGCOLOR="#cccccc" BORDER="0" CELLSPACING="2" STYLE="margin-left:auto; margin-right:auto">
diff --git a/httemplate/misc/void-cust_pay.cgi b/httemplate/misc/void-cust_pay.html
index 784bb9a94..205d93aa3 100755
--- a/httemplate/misc/void-cust_pay.cgi
+++ b/httemplate/misc/void-cust_pay.html
@@ -12,13 +12,13 @@
 
 <P ALIGN="center"><B><% mt('Void this payment?') |h %></B>
 
-<FORM action="<% ${p} %>misc/void-cust_pay.cgi">
+<FORM action="<% ${p} %>misc/void-cust_pay.html">
 <INPUT TYPE="hidden" NAME="paynum" VALUE="<% $paynum %>">
 
 <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>
diff --git a/httemplate/view/cust_bill.cgi b/httemplate/view/cust_bill.cgi
index 6d139747a..d73edbd79 100755
--- a/httemplate/view/cust_bill.cgi
+++ b/httemplate/view/cust_bill.cgi
@@ -17,7 +17,7 @@ function areyousure(href, message) {
     <& /elements/popup_link.html,
       'label'       => emt('Void this invoice'),
       'actionlabel' => emt('Void this invoice'),
-      'action'      => $p.'misc/void-cust_bill.cgi?invnum='.$invnum,
+      'action'      => $p.'misc/void-cust_bill.html?invnum='.$invnum,
     &>
 %   }
 %   if ( $can_void and $can_delete ) {
diff --git a/httemplate/view/cust_main/payment_history/credit.html b/httemplate/view/cust_main/payment_history/credit.html
index 81be1cd7a..db2e5e582 100644
--- a/httemplate/view/cust_main/payment_history/credit.html
+++ b/httemplate/view/cust_main/payment_history/credit.html
@@ -130,7 +130,7 @@ my $void = '';
 $void = ' ('.
                include( '/elements/popup_link.html',
                     'label'    => emt('void'),
-                    'action'   => "${p}misc/void-cust_credit.cgi?".
+                    'action'   => "${p}misc/void-cust_credit.html?".
                                   $cust_credit->crednum,
                     'actionlabel' => emt('Void credit'),
                 ).
diff --git a/httemplate/view/cust_main/payment_history/invoice.html b/httemplate/view/cust_main/payment_history/invoice.html
index ca59c15fd..613936e00 100644
--- a/httemplate/view/cust_main/payment_history/invoice.html
+++ b/httemplate/view/cust_main/payment_history/invoice.html
@@ -27,7 +27,7 @@ if ( $cust_bill->closed !~ /^Y/i && $opt{'Void invoices'} ) {
   $void =
     ' ('. include('/elements/popup_link.html',
                     'label'     => emt('void'),
-                    'action'    => "${p}misc/void-cust_bill.cgi?;invnum=".
+                    'action'    => "${p}misc/void-cust_bill.html?;invnum=".
                                     $cust_bill->invnum,
                     'actionlabel' => emt('Void Invoice'),
                  ).
diff --git a/httemplate/view/cust_main/payment_history/payment.html b/httemplate/view/cust_main/payment_history/payment.html
index 77013140e..6c93f7b27 100644
--- a/httemplate/view/cust_main/payment_history/payment.html
+++ b/httemplate/view/cust_main/payment_history/payment.html
@@ -186,7 +186,7 @@ my $voidmsg = $cust_pay->payby =~ /^(CARD|CHEK|TOKN)$/
 $void = ' ('.
                include( '/elements/popup_link.html',
                     'label'    => emt('void'),
-                    'action'   => "${p}misc/void-cust_pay.cgi?".$cust_pay->paynum,
+                    'action'   => "${p}misc/void-cust_pay.html?".$cust_pay->paynum,
                     'actionlabel' => emt('Void payment'),
                 ).
           ')'