optimize customer view: avoid looking up config values inside loops, RT#4728

diff --git a/httemplate/view/cust_main/packages.html b/httemplate/view/cust_main/packages.html
index afd9941..5c0ee48 100755 (executable)
--- a/httemplate/view/cust_main/packages.html
+++ b/httemplate/view/cust_main/packages.html
@@ -145,10 +145,24 @@ Current packages
 %     $bgcolor = $bgcolor1;
 %   }
 %
+%   my $countrydefault = scalar($conf->config('countrydefault')) || 'US';
 %   my %iopt = (
-%     'bgcolor'  => $bgcolor,
-%     'cust_pkg' => $cust_pkg,
-%     'part_pkg' => $cust_pkg->part_pkg,
+%     'bgcolor'                  => $bgcolor,
+%     'cust_pkg'                 => $cust_pkg,
+%     'part_pkg'                 => $cust_pkg->part_pkg,
+%
+%     #for services.html and status.html
+%     'cust_pkg-display_times'   => $conf->exists('cust_pkg-display_times'),
+%
+%     #for location.html
+%     'countrydefault'           => $countrydefault,
+%     'statedefault'             => ( scalar($conf->config('statedefault'))
+%                                    || ($countrydefault eq 'US' ? 'CA' : '') ),
+%
+%     #for services.html
+%     'svc_external-skip_manual' => $conf->exists('svc_external-skip_manual'),
+%     'legacy_link'              => $conf->exists('legacy_link'),
+%
 %   );
 
     <!--pkgnum: <% $cust_pkg->pkgnum %>-->

diff --git a/httemplate/view/cust_main/packages/location.html b/httemplate/view/cust_main/packages/location.html
index b51616d..59efce1 100644 (file)
--- a/httemplate/view/cust_main/packages/location.html
+++ b/httemplate/view/cust_main/packages/location.html
@@ -33,14 +33,11 @@
 
 my %opt = @_;
 
-my $conf = new FS::Conf;
-
 my $bgcolor        = $opt{'bgcolor'};
 my $cust_pkg       = $opt{'cust_pkg'};
 my $part_pkg       = $opt{'part_pkg'};
-my $conf           = new FS::Conf;
-my $countrydefault = $conf->config('countrydefault') || 'US';
-my $statedefault   = $conf->config('statedefault')
+my $countrydefault = $opt{'countrydefault'} || 'US';
+my $statedefault   = $opt{'statedefault'}
                      || ($countrydefault eq 'US' ? 'CA' : '');
 
 my $loc = $cust_pkg->cust_location_or_main;

diff --git a/httemplate/view/cust_main/packages/services.html b/httemplate/view/cust_main/packages/services.html
index f46afb9..1e47373 100644 (file)
--- a/httemplate/view/cust_main/packages/services.html
+++ b/httemplate/view/cust_main/packages/services.html
@@ -20,7 +20,7 @@
       <TR>
         <TD ALIGN="right" COLSPAN="3" VALIGN="top" STYLE="padding-bottom:1px;padding-top:0px"><FONT SIZE="-2" COLOR="#FFD000">
 
-            <% $cust_svc->overlimit ? "Overlimit: ". time2str('%b %o %Y' . ($conf->exists('cust_pkg-display_times') ? ' %l:%M %P' : ''), $cust_svc->overlimit) : '' %>
+            <% $cust_svc->overlimit ? "Overlimit: ". time2str('%b %o %Y' . ($opt{'cust_pkg-display_times'} ? ' %l:%M %P' : ''), $cust_svc->overlimit) : '' %>
           </FONT></TD>
       </TR>
 
@@ -55,7 +55,7 @@
 
       <TR>
         <TD COLSPAN=3 ALIGN="center" STYLE="padding-bottom:4px;padding-top:0px">
-          <B><% svc_provision_link($cust_pkg, $part_svc, $conf, $curuser) %></B>
+          <B><% svc_provision_link($cust_pkg, $part_svc, \%opt, $curuser) %></B>
         </TD>
       </TR>
 
@@ -74,17 +74,16 @@ my $bgcolor  = $opt{'bgcolor'};
 my $cust_pkg = $opt{'cust_pkg'};
 my $part_pkg = $opt{'part_pkg'};
 my $curuser  = $FS::CurrentUser::CurrentUser;
-my $conf     = new FS::Conf;
 
 sub svc_provision_link {
-  my ($cust_pkg, $part_svc, $conf, $curuser) = @_;
+  my ($cust_pkg, $part_svc, $opt, $curuser) = @_;
   ( my $svc_nbsp = $part_svc->svc ) =~ s/\s+/&nbsp;/g;
   my $num_avail = $part_svc->num_avail;
   my $pkgnum_svcpart = "pkgnum=". $cust_pkg->pkgnum. ';'.
                        "svcpart=". $part_svc->svcpart;
   my $url;
   if ( $part_svc->svcdb eq 'svc_external' #could be generalized
-       && $conf->exists('svc_external-skip_manual')
+       && $opt->{'svc_external-skip_manual'}
   ) {
     $url = "${p}edit/process/". $part_svc->svcdb. ".cgi?$pkgnum_svcpart";
   } else {
@@ -99,7 +98,7 @@ sub svc_provision_link {
 
   my $link = qq!<A CLASS="provision" HREF="$url">!.
              "Provision&nbsp;$svc_nbsp&nbsp;($num_avail)</A>";
-  if ( $conf->exists('legacy_link')
+  if ( $opt->{'legacy_link'}
        && $curuser->access_right('View/link unlinked services')
      )
   {

diff --git a/httemplate/view/cust_main/packages/status.html b/httemplate/view/cust_main/packages/status.html
index 4e7462b..e0a188e 100644 (file)
--- a/httemplate/view/cust_main/packages/status.html
+++ b/httemplate/view/cust_main/packages/status.html
@@ -6,7 +6,7 @@
 % if ( $cust_pkg->get('cancel') ) { #status: cancelled
 %   my $cpr = $cust_pkg->last_cust_pkg_reason('cancel');
 
-    <% pkg_status_row($cust_pkg, 'Cancelled', 'cancel', 'color'=>'FF0000', conf=>$conf ) %>
+    <% pkg_status_row($cust_pkg, 'Cancelled', 'cancel', 'color'=>'FF0000', %opt ) %>
 
     <% pkg_status_row_colspan(
          ( $cpr ? $cpr->reasontext. ' by '. $cpr->otaker : '' ), '',
@@ -20,10 +20,10 @@
 
 %   } else { 
 
-       <% pkg_status_row( $cust_pkg, 'Setup', 'setup', conf=>$conf ) %>
-       <% pkg_status_row_changed( $cust_pkg, conf=>$conf ) %>
-       <% pkg_status_row_if( $cust_pkg, $last_bill_or_renewed, 'last_bill', conf=>$conf, curuser=>$curuser ) %>
-       <% pkg_status_row_if( $cust_pkg, 'Suspended', 'susp', conf=>$conf, curuser=>$curuser ) %>
+       <% pkg_status_row( $cust_pkg, 'Setup', 'setup', %opt ) %>
+       <% pkg_status_row_changed( $cust_pkg, %opt ) %>
+       <% pkg_status_row_if( $cust_pkg, $last_bill_or_renewed, 'last_bill', %opt, curuser=>$curuser ) %>
+       <% pkg_status_row_if( $cust_pkg, 'Suspended', 'susp', %opt, curuser=>$curuser ) %>
 
 %   } 
 %
@@ -32,7 +32,7 @@
 %   if ( $cust_pkg->get('susp') ) { #status: suspended
 %     my $cpr = $cust_pkg->last_cust_pkg_reason('susp');
 
-    <% pkg_status_row( $cust_pkg, 'Suspended', 'susp', 'color'=>'FF9900', conf=>$conf ) %>
+    <% pkg_status_row( $cust_pkg, 'Suspended', 'susp', 'color'=>'FF9900', %opt ) %>
 
     <% pkg_status_row_colspan(
          ( $cpr ? $cpr->reasontext. ' by '. $cpr->otaker : '' ), '',
@@ -43,13 +43,13 @@
 %   unless ( $cust_pkg->get('setup') ) { 
       <% pkg_status_row_colspan('Never billed') %>
 %   } else { 
-      <% pkg_status_row($cust_pkg, 'Setup', 'setup', conf=>$conf ) %>
+      <% pkg_status_row($cust_pkg, 'Setup', 'setup', %opt ) %>
 %   } 
 
-    <% pkg_status_row_changed( $cust_pkg, conf=>$conf ) %>
-    <% pkg_status_row_if( $cust_pkg, $last_bill_or_renewed, 'last_bill', conf=>$conf, curuser=>$curuser ) %>
-%   # pkg_status_row($cust_pkg, 'Next bill', 'bill', conf=>$conf)
-    <% pkg_status_row_if( $cust_pkg, 'Expires', 'expire', conf=>$conf, curuser=>$curuser ) %>
+    <% pkg_status_row_changed( $cust_pkg, %opt ) %>
+    <% pkg_status_row_if( $cust_pkg, $last_bill_or_renewed, 'last_bill', %opt, curuser=>$curuser ) %>
+%   # pkg_status_row($cust_pkg, 'Next bill', 'bill', %opt)
+    <% pkg_status_row_if( $cust_pkg, 'Expires', 'expire', %opt, curuser=>$curuser ) %>
 
     <TR>
       <TD COLSPAN=<%$colspan%>>
@@ -94,7 +94,7 @@
 
           <% pkg_status_row_colspan('One-time&nbsp;charge') %>
 
-          <% pkg_status_row($cust_pkg, 'Billed', 'setup', conf=>$conf) %>
+          <% pkg_status_row($cust_pkg, 'Billed', 'setup', %opt) %>
 
 %       } else { 
 %
@@ -116,7 +116,7 @@
             %>
 %         } 
 
-          <% pkg_status_row($cust_pkg, 'Setup', 'setup', conf=>$conf) %>
+          <% pkg_status_row($cust_pkg, 'Setup', 'setup', %opt) %>
 
 %       } 
 %
@@ -124,12 +124,12 @@
 %   my $autosuspend = pkg_autosuspend_time( $cust_pkg );
 %   $cust_pkg->set('autosuspend', $autosuspend) if $autosuspend;
 
-      <% pkg_status_row_changed( $cust_pkg, conf=>$conf ) %>
-      <% pkg_status_row_if( $cust_pkg, $last_bill_or_renewed, 'last_bill', conf=>$conf, curuser=>$curuser ) %>
-      <% pkg_status_row_if( $cust_pkg, $next_bill_or_prepaid_until, 'bill', conf=>$conf, curuser=>$curuser ) %>
-      <% pkg_status_row_if($cust_pkg, 'Will automatically suspend by', 'autosuspend', conf=>$conf) %>
-      <% pkg_status_row_if( $cust_pkg, 'Will suspend on', 'adjourn', conf=>$conf, curuser=>$curuser ) %>
-      <% pkg_status_row_if( $cust_pkg, 'Expires', 'expire', conf=>$conf, curuser=>$curuser ) %>
+      <% pkg_status_row_changed( $cust_pkg, %opt ) %>
+      <% pkg_status_row_if( $cust_pkg, $last_bill_or_renewed, 'last_bill', %opt, curuser=>$curuser ) %>
+      <% pkg_status_row_if( $cust_pkg, $next_bill_or_prepaid_until, 'bill', %opt, curuser=>$curuser ) %>
+      <% pkg_status_row_if($cust_pkg, 'Will automatically suspend by', 'autosuspend', %opt) %>
+      <% pkg_status_row_if( $cust_pkg, 'Will suspend on', 'adjourn', %opt, curuser=>$curuser ) %>
+      <% pkg_status_row_if( $cust_pkg, 'Expires', 'expire', %opt, curuser=>$curuser ) %>
 
 %     if ( $part_pkg->freq ) { 
 
@@ -171,9 +171,8 @@ my $bgcolor  = $opt{'bgcolor'};
 my $cust_pkg = $opt{'cust_pkg'};
 my $part_pkg = $opt{'part_pkg'};
 my $curuser  = $FS::CurrentUser::CurrentUser;
-my $conf     = new FS::Conf;
-my $colspan  = $conf->exists('cust_pkg-display_times') ? 8 : 4;
-my $width    = $conf->exists('cust_pkg-display_times') ? '38%' : '56%';
+my $colspan  = $opt{'cust_pkg-display_times'} ? 8 : 4;
+my $width    = $opt{'cust_pkg-display_times'} ? '38%' : '56%';
 
 #false laziness w/edit/REAL_cust_pkg.cgi
 my( $billed_or_prepaid, $last_bill_or_renewed, $next_bill_or_prepaid_until );
@@ -213,7 +212,7 @@ sub pkg_status_row {
   $html   .= qq($title&nbsp;);
   $html   .= qq(</B></FONT>) if length($color);
   $html   .= qq(</TD>);
-  $html   .= pkg_datestr($cust_pkg, $field, $opt{conf}).'</TR>';
+  $html   .= pkg_datestr($cust_pkg, $field, %opt).'</TR>';
 
   $html;
 }
@@ -237,7 +236,7 @@ sub pkg_status_row_if {
 sub pkg_status_row_changed {
   my( $cust_pkg, %opt ) = @_;
   return '' unless $cust_pkg->change_date;
-  my $html = pkg_status_row( $cust_pkg, 'Package&nbsp;changed', 'change_date', conf=>$opt{'conf'} );
+  my $html = pkg_status_row( $cust_pkg, 'Package&nbsp;changed', 'change_date', %opt );
   my $old = $cust_pkg->old_cust_pkg;
   if ( $old ) {
     my $part_pkg = $old->part_pkg;
@@ -269,7 +268,7 @@ sub pkg_status_row_colspan {
 }
 
 sub pkg_datestr {
-  my($cust_pkg, $field, $conf) = @_ or return '';
+  my($cust_pkg, $field, %opt) = @_ or return '';
   return '&nbsp;' unless $cust_pkg->get($field);
   my $format = '<TD align="left"><B>%b</B></TD>'.
                '<TD align="right"><B>&nbsp;%o,</B></TD>'.
@@ -279,7 +278,7 @@ sub pkg_datestr {
              '<TD ALIGN="center"><B>:</B></TD>'.
              '<TD ALIGN="left"><B>%M</B></TD>'.
              '<TD ALIGN="left"><B>&nbsp;%P</B></TD>'
-    if $conf->exists('cust_pkg-display_times');
+    if $opt{'cust_pkg-display_times'};
   my $strip = time2str($format, $cust_pkg->get($field) );
   $strip =~ s/ (\d)/$1/g;
   $strip;

diff --git a/httemplate/view/cust_main/payment_history.html b/httemplate/view/cust_main/payment_history.html
index c3a9019..9628d74 100644 (file)
--- a/httemplate/view/cust_main/payment_history.html
+++ b/httemplate/view/cust_main/payment_history.html
@@ -157,7 +157,7 @@
 <TR>
   <TH CLASS="grid" BGCOLOR="#cccccc">Date</TH>
   <TH CLASS="grid" BGCOLOR="#cccccc">Description</TH>
-  <TH CLASS="grid" BGCOLOR="#cccccc"><FONT SIZE=-1>Charge</FONT></TH>
+  <TH CLASS="grid" BGCOLOR="#cccccc"><FONT SIZE=-1>Invoice</FONT></TH>
   <TH CLASS="grid" BGCOLOR="#cccccc"><FONT SIZE=-1>Payment</FONT></TH>
   <TH CLASS="grid" BGCOLOR="#cccccc"><FONT SIZE=-1>In-house<BR>Credit</FONT></TH>
   <TH CLASS="grid" BGCOLOR="#cccccc"><FONT SIZE=-1>Refund</FONT></TH>
@@ -344,11 +344,19 @@ my %status = (
 #get payment history
 my @history = ();
 
+my %opt =
+  ( map { $_ => scalar($conf->config($_)) }
+        qw( card_refund-days )
+  ),
+  ( map { $_ => $conf->exists($_) } 
+        qw( deletepayments deleterefunds )
+  );
+
 #invoices
 foreach my $cust_bill ($cust_main->cust_bill) {
   push @history, {
     'date'   => $cust_bill->_date,
-    'desc'   => include('payment_history/invoice.html', $cust_bill),
+    'desc'   => include('payment_history/invoice.html', $cust_bill, %opt ),
     'charge' => $cust_bill->charged,
   };
 }
@@ -357,7 +365,7 @@ foreach my $cust_bill ($cust_main->cust_bill) {
 foreach my $cust_pay ($cust_main->cust_pay) {
   push @history, {
     'date'    => $cust_pay->_date,
-    'desc'    => include('payment_history/payment.html', $cust_pay),
+    'desc'    => include('payment_history/payment.html', $cust_pay, %opt ),
     'payment' => $cust_pay->paid,
     #'target'  => $target, #XXX
   };

diff --git a/httemplate/view/cust_main/payment_history/payment.html b/httemplate/view/cust_main/payment_history/payment.html
index 346c0d0..2e24b17 100644 (file)
--- a/httemplate/view/cust_main/payment_history/payment.html
+++ b/httemplate/view/cust_main/payment_history/payment.html
@@ -4,7 +4,6 @@
 
 my( $cust_pay, %opt ) = @_;
 
-my $conf = new FS::Conf;
 my $curuser = $FS::CurrentUser::CurrentUser;
 
 my $payby = $cust_pay->payby;
@@ -139,7 +138,7 @@ my $view =
    ')';
 
 my $refund = '';
-my $refund_days = $conf->config('card_refund-days') || 120;
+my $refund_days = $opt{'card_refund-days'} || 120;
 if (    $cust_pay->closed !~ /^Y/i
      && $cust_pay->payby =~ /^(CARD|CHEK)$/
      && time-$cust_pay->_date < $refund_days*86400
@@ -179,7 +178,7 @@ if (    $cust_pay->closed !~ /^Y/i
 
 my $delete = '';
 if ( $cust_pay->closed !~ /^Y/i
-     && $conf->exists('deletepayments')
+     && $opt{'deletepayments'}
      && $curuser->access_right('Delete payment')
    )
 {

diff --git a/httemplate/view/cust_main/payment_history/refund.html b/httemplate/view/cust_main/payment_history/refund.html
index 7978821..4a48fea 100644 (file)
--- a/httemplate/view/cust_main/payment_history/refund.html
+++ b/httemplate/view/cust_main/payment_history/refund.html
@@ -5,7 +5,6 @@ by <% $cust_refund->otaker %><% $view %><% $delete %>
 
 my( $cust_refund, %opt ) = @_;
 
-my $conf = new FS::Conf;
 my $curuser = $FS::CurrentUser::CurrentUser;
 
 my $payby = $cust_refund->payby;
@@ -36,7 +35,7 @@ my $view =
 
 my $delete = '';
 if ( $cust_refund->closed !~ /^Y/i
-     && $conf->exists('deleterefunds')
+     && $opt{'deleterefunds'}
      && $curuser->access_right('Delete refund')
    )
 {