From f9930edeaceb217a6503fa49078fcff2b588caf8 Mon Sep 17 00:00:00 2001 From: Ivan Kohler Date: Sat, 9 Aug 2014 15:19:27 -0700 Subject: [PATCH] optimizations for large package lists, RT#28526 --- FS/FS/cust_pkg.pm | 36 ++++++++++++++++---- FS/FS/location_Mixin.pm | 17 ++++++++-- FS/FS/part_pkg.pm | 10 +++++- httemplate/view/cust_main/packages.html | 44 ++++++++++++++----------- httemplate/view/cust_main/packages/package.html | 6 ++-- httemplate/view/cust_main/packages/section.html | 5 +-- 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) for this invoice. +Returns the customer billing events (see L) 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) for this invoice. +Returns the number of customer billing events (see L) 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) 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). =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), otherwise returns the customer (see L). 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); -% 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, - &> - -% } + +% $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 &> + +% } + @@ -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 % ) { ( <%pkg_event_link($cust_pkg)%> ) % } 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') -- 2.11.0