optimizations for large package lists, RT#28526

6 files changed, 84 insertions, 34 deletions

diff --git a/FS/FS/cust_pkg.pm b/FS/FS/cust_pkg.pm
index 915f2297b..93763dc8e 100644
--- a/FS/FS/cust_pkg.pm
+++ b/FS/FS/cust_pkg.pm
@@ -2718,7 +2718,7 @@ sub set_cust_pkg_detail {
 
 =item cust_event
 
-Returns the new-style customer billing events (see L<FS::cust_event>) for this invoice.
+Returns the customer billing events (see L<FS::cust_event>) for this invoice.
 
 =cut
 
@@ -2735,19 +2735,41 @@ sub cust_event {
 
 =item num_cust_event
 
-Returns the number of new-style customer billing events (see L<FS::cust_event>) for this invoice.
+Returns the number of customer billing events (see L<FS::cust_event>) for this package.
 
 =cut
 
 #false laziness w/cust_bill.pm
 sub num_cust_event {
   my $self = shift;
-  my $sql =
-    "SELECT COUNT(*) FROM cust_event JOIN part_event USING ( eventpart ) ".
-    "  WHERE tablenum = ? AND eventtable = 'cust_pkg'";
+  my $sql = "SELECT COUNT(*) ". $self->_from_cust_event_where;
+  $self->_prep_ex($sql, $self->pkgnum)->fetchrow_arrayref->[0];
+}
+
+=item exists_cust_event
+
+Returns true if there are customer billing events (see L<FS::cust_event>) for this package.  More efficient than using num_cust_event.
+
+=cut
+
+sub exists_cust_event {
+  my $self = shift;
+  my $sql = "SELECT 1 ". $self->_from_cust_event_where. " LIMIT 1";
+  my $row = $self->_prep_ex($sql, $self->pkgnum)->fetchrow_arrayref;
+  $row ? $row->[0] : '';
+}
+
+sub _from_cust_event_where {
+  #my $self = shift;
+  " FROM cust_event JOIN part_event USING ( eventpart ) ".
+  "  WHERE tablenum = ? AND eventtable = 'cust_pkg' ";
+}
+
+sub _prep_ex {
+  my( $self, $sql, @args ) = @_;
   my $sth = dbh->prepare($sql) or die  dbh->errstr. " preparing $sql"; 
-  $sth->execute($self->pkgnum) or die $sth->errstr. " executing $sql";
-  $sth->fetchrow_arrayref->[0];
+  $sth->execute(@args)         or die $sth->errstr. " executing $sql";
+  $sth;
 }
 
 =item part_pkg_currency_option OPTIONNAME
diff --git a/FS/FS/location_Mixin.pm b/FS/FS/location_Mixin.pm
index d32b4a37c..b010374a9 100644
--- a/FS/FS/location_Mixin.pm
+++ b/FS/FS/location_Mixin.pm
@@ -11,9 +11,20 @@ Returns the location object, if any (see L<FS::cust_location>).
 =cut
 
 sub cust_location {
-  my $self = shift;
+  my( $self, %opt ) = @_;
+
   return '' unless $self->locationnum;
-  qsearchs( 'cust_location', { 'locationnum' => $self->locationnum } );
+
+  return $opt{_cache}->{$self->locationnum}
+    if $opt{_cache} && $opt{_cache}->{$self->locationnum};
+
+  my $cust_location = 
+    qsearchs( 'cust_location', { 'locationnum' => $self->locationnum } );
+
+  $opt{_cache}->{$self->locationnum} = $cust_location
+     if $opt{_cache};
+  
+  $cust_location;
 }
 
 =item cust_location_or_main
@@ -25,7 +36,7 @@ L<FS::cust_location>), otherwise returns the customer (see L<FS::cust_main>).
 
 sub cust_location_or_main {
   my $self = shift;
-  $self->cust_location || $self->cust_main;
+  $self->cust_location(@_) || $self->cust_main;
 }
 
 =item location_label [ OPTION => VALUE ... ]
diff --git a/FS/FS/part_pkg.pm b/FS/FS/part_pkg.pm
index 06f304a22..f408552e3 100644
--- a/FS/FS/part_pkg.pm
+++ b/FS/FS/part_pkg.pm
@@ -31,7 +31,7 @@ use FS::part_pkg_discount;
 use FS::part_pkg_vendor;
 use FS::part_pkg_currency;
 
-$DEBUG = 0;
+$DEBUG = 1;
 $setup_hack = 0;
 $skip_pkg_svc_hack = 0;
 
@@ -1288,6 +1288,12 @@ will be suppressed.
 
 sub option {
   my( $self, $opt, $ornull ) = @_;
+
+  #cache: was pulled up in the original part_pkg query
+  if ( $opt =~ /^(setup|recur)_fee$/ && defined($self->hashref->{"_$opt"}) ) {
+    return $self->hashref->{"_$opt"};
+  }
+
   cluck "$self -> option: searching for $opt"
     if $DEBUG;
   my $part_pkg_option =
@@ -1296,12 +1302,14 @@ sub option {
       optionname => $opt,
   } );
   return $part_pkg_option->optionvalue if $part_pkg_option;
+
   my %plandata = map { /^(\w+)=(.*)$/; ( $1 => $2 ); }
                      split("\n", $self->get('plandata') );
   return $plandata{$opt} if exists $plandata{$opt};
   cluck "WARNING: (pkgpart ". $self->pkgpart. ") Package def option $opt ".
         "not found in options or plandata!\n"
     unless $ornull;
+
   '';
 }
 
diff --git a/httemplate/view/cust_main/packages.html b/httemplate/view/cust_main/packages.html
index a05142f98..32a725df3 100755
--- a/httemplate/view/cust_main/packages.html
+++ b/httemplate/view/cust_main/packages.html
@@ -175,23 +175,18 @@ if ( el ) el.scrollIntoView(true);
 
   <TR>
     <TD COLSPAN=2>
-% if ( $conf->exists('cust_pkg-group_by_location') ) {
-<& locations.html,
-    'cust_main'     => $cust_main,
-    'packages'      => \@packages,
-    %opt,
- &>
-% }
-% else {
-% # in this format, put all packages in one section
-<& /elements/table-grid.html &>
-<& packages/section.html,
-    'cust_main'     => $cust_main,
-    'packages'      => \@packages,
-    %opt,
- &>
-</TABLE>
-% }
+
+%     $opt{cust_main} = $cust_main;
+%     $opt{packages}  = \@packages;
+%     $opt{cust_location_cache} = {};
+%     if ( $conf->exists('cust_pkg-group_by_location') ) {
+        <& locations.html, %opt &>
+%     } else { # in this format, put all packages in one section
+        <& /elements/table-grid.html &>
+        <& packages/section.html, %opt &>
+        </TABLE>
+%     }
+
     </TD>
   </TR>
 
@@ -221,7 +216,10 @@ my $cust_pkg_fields =
   join(', ', map { "cust_pkg.$_ AS $_"          } fields('cust_pkg') );
 
 my $part_pkg_fields =
-  join(', ', map { "part_pkg.$_ AS part_pkg_$_" } fields('part_pkg') );
+  join(', ', ( map { "part_pkg.$_ AS part_pkg_$_" } fields('part_pkg') ),
+             'setup_option.optionvalue AS part_pkg__setup_fee',
+             'recur_option.optionvalue AS part_pkg__recur_fee',
+      );
 
 my $group_by =
   join(', ', map "cust_pkg.$_", fields('cust_pkg') ). ', '.
@@ -233,7 +231,15 @@ my $num_svcs = '( SELECT COUNT(*) FROM cust_svc '.
 # don't exclude cancelled packages at this stage
 my @packages = $cust_main->all_pkgs( {
   'select'    => "$cust_pkg_fields, $part_pkg_fields, $num_svcs",
-  'addl_from' => 'LEFT JOIN part_pkg USING ( pkgpart )',
+  'addl_from' => qq{
+    LEFT JOIN part_pkg USING ( pkgpart )
+    LEFT JOIN part_pkg_option AS setup_option
+      ON (     cust_pkg.pkgpart = setup_option.pkgpart
+           AND setup_option.optionname = 'setup_fee' )
+    LEFT JOIN part_pkg_option AS recur_option
+      ON (     cust_pkg.pkgpart = recur_option.pkgpart
+           AND recur_option.optionname = 'recur_fee' )
+  },
 } );
 
 my %change_to_from; # target pkgnum => current cust_pkg, for future changes
diff --git a/httemplate/view/cust_main/packages/package.html b/httemplate/view/cust_main/packages/package.html
index 9f6169c1e..cf5c98a1c 100644
--- a/httemplate/view/cust_main/packages/package.html
+++ b/httemplate/view/cust_main/packages/package.html
@@ -76,7 +76,9 @@
 %             if ( $curuser->access_right('Discount customer package')
 %                  && $part_pkg->can_discount
 %                  && ! scalar( @{ $cust_pkg->{_cust_pkg_discount_active} } )
-%                  && ! scalar($cust_pkg->part_pkg->part_pkg_discount)
+%                  && (    ! $opt{'term_discounts'}
+%                       || ! scalar($cust_pkg->part_pkg->part_pkg_discount)
+%                     )
 %                )
 %             {
 %               $br=1;
@@ -94,7 +96,7 @@
 %           if (    (    $curuser->access_right('Billing event reports')
 %                     || $curuser->access_right('View customer billing events')
 %                   )
-%                && $cust_pkg->num_cust_event
+%                && $cust_pkg->exists_cust_event
 %              ) {
               (&nbsp;<%pkg_event_link($cust_pkg)%>&nbsp;)
 %           }
diff --git a/httemplate/view/cust_main/packages/section.html b/httemplate/view/cust_main/packages/section.html
index 95b486f05..4980feeac 100755
--- a/httemplate/view/cust_main/packages/section.html
+++ b/httemplate/view/cust_main/packages/section.html
@@ -11,8 +11,8 @@
 %   foreach my $cust_pkg (@$packages) {
     <& .packagerow, $cust_pkg,
         ( map { $_ => $opt{$_} } qw(
-            cust_main bgcolor
-            no_links before_pkg_callback before_svc_callback after_svc_callback
+            cust_main bgcolor no_links cust_location_cache
+            before_pkg_callback before_svc_callback after_svc_callback
         )),
         %conf_opt
     &>
@@ -99,6 +99,7 @@ my %conf_opt = (
   #for package.html
   'invoice-unitprice'         => $conf->exists('invoice-unitprice'),
   'show_pkgnum'               => $curuser->option('show_pkgnum'),
+  'part_pkg-term_discounts'   => $conf->exists('part_pkg-term_discounts'),
 
   #for services.html and status.html
   'cust_pkg-display_times'    => ($conf->exists('cust_pkg-display_times')