From dbc913a3ae4d582fbfacf290ee75debe5f0b5413 Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Tue, 7 Jul 2015 11:31:42 -0700 Subject: [PATCH] optionally include discounts in sales report, #37124 --- FS/FS/Report/Table.pm | 58 +++++++++++------------------- httemplate/graph/cust_bill_pkg.cgi | 57 +++++++++++++++++------------ httemplate/graph/report_cust_bill_pkg.html | 6 ++++ httemplate/search/cust_bill_pkg.cgi | 56 +++++++++++++++++++---------- 4 files changed, 98 insertions(+), 79 deletions(-) diff --git a/FS/FS/Report/Table.pm b/FS/FS/Report/Table.pm index eef983d68..cba968b43 100644 --- a/FS/FS/Report/Table.pm +++ b/FS/FS/Report/Table.pm @@ -439,8 +439,8 @@ sub cust_pkg_recur_cost { =item cust_bill_pkg: the total package charges on invoice line items. -'charges': limit the type of charges included (setup, recur, usage). -Should be a string containing one or more of 'S', 'R', or 'U'; if +'charges': limit the type of charges included (setup, recur, usage, discount). +Should be a string containing one or more of 'S', 'R', 'U', or 'D'; if unspecified, defaults to all three. 'classnum': limit to this package class. @@ -470,6 +470,7 @@ sub cust_bill_pkg { $sum += $self->cust_bill_pkg_setup(@_) if $charges{S}; $sum += $self->cust_bill_pkg_recur(@_) if $charges{R}; $sum += $self->cust_bill_pkg_detail(@_) if $charges{U}; + $sum += $self->cust_bill_pkg_discount(@_) if $charges{D}; if ($opt{'average_per_cust_pkg'}) { my $count = $self->cust_bill_pkg_count_pkgnum(@_); @@ -656,47 +657,28 @@ sub cust_bill_pkg_detail { } sub cust_bill_pkg_discount { - my( $self, $speriod, $eperiod, $agentnum, %opt ) = @_; - - #need to do this the new multi-classnum way if it gets re-enabled - #my $where = ''; - #my $comparison = ''; - #if ( $opt{'classnum'} =~ /^(\d+)$/ ) { - # if ( $1 == 0 ) { - # $comparison = "IS NULL"; - # } else { - # $comparison = "= $1"; - # } - # - # if ( $opt{'use_override'} ) { - # $where = "( - # part_pkg.classnum $comparison AND pkgpart_override IS NULL OR - # override.classnum $comparison AND pkgpart_override IS NOT NULL - # )"; - # } else { - # $where = "part_pkg.classnum $comparison"; - # } - #} + my $self = shift; + my ($speriod, $eperiod, $agentnum, %opt) = @_; + # apply all the same constraints here as for setup/recur $agentnum ||= $opt{'agentnum'}; - my $total_sql = - " SELECT COALESCE( SUM( cust_bill_pkg_discount.amount ), 0 ) "; + my @where = ( + '(pkgnum != 0 OR feepart IS NOT NULL)', + $self->with_classnum($opt{'classnum'}, $opt{'use_override'}), + $self->with_report_option(%opt), + $self->in_time_period_and_agent($speriod, $eperiod, $agentnum), + $self->with_refnum(%opt), + $self->with_cust_classnum(%opt) + ); - $total_sql .= - " FROM cust_bill_pkg_discount - LEFT JOIN cust_bill_pkg USING ( billpkgnum ) - LEFT JOIN cust_bill USING ( invnum ) - LEFT JOIN cust_main USING ( custnum ) - WHERE ". $self->in_time_period_and_agent($speriod, $eperiod, $agentnum); - # LEFT JOIN cust_pkg_discount USING ( pkgdiscountnum ) - # LEFT JOIN discount USING ( discountnum ) - # LEFT JOIN cust_pkg USING ( pkgnum ) - # LEFT JOIN part_pkg USING ( pkgpart ) - # LEFT JOIN part_pkg AS override ON pkgpart_override = override.pkgpart - - return $self->scalar_sql($total_sql); + my $total_sql = "SELECT COALESCE(SUM(cust_bill_pkg_discount.amount), 0) + FROM cust_bill_pkg_discount + JOIN cust_bill_pkg USING (billpkgnum) + $cust_bill_pkg_join + WHERE " . join(' AND ', grep $_, @where); + $self->scalar_sql($total_sql); } ##### package churn report ##### diff --git a/httemplate/graph/cust_bill_pkg.cgi b/httemplate/graph/cust_bill_pkg.cgi index ea7fee932..b5486f4af 100644 --- a/httemplate/graph/cust_bill_pkg.cgi +++ b/httemplate/graph/cust_bill_pkg.cgi @@ -27,6 +27,8 @@ my $bottom_link = "$link;"; my $use_usage = $cgi->param('use_usage') || 0; my $use_setup = $cgi->param('use_setup') || 0; +my $use_discount = $cgi->param('use_discount') || 2; + my $use_override = $cgi->param('use_override') ? 1 : 0; my $average_per_cust_pkg = $cgi->param('average_per_cust_pkg') ? 1 : 0; my $distribute = $cgi->param('distribute') ? 1 : 0; @@ -41,11 +43,13 @@ if ( $average_per_cust_pkg ) { } my %charge_labels = ( + 'SRU'=> 'setup + recurring', 'SR' => 'setup + recurring', 'RU' => 'recurring', 'S' => 'setup', 'R' => 'recurring', 'U' => 'usage', + 'D' => 'discount', ); #XXX or virtual @@ -186,6 +190,10 @@ elsif ( $use_usage == 2 ) { $components[-1] =~ s/U//; } +if ( $use_discount == 1 ) { + push @components, 'D'; +} # else leave discounts off entirely; never combine them with setup/recur + # Categorization of line items goes # Agent -> Referral -> Package class -> Component (setup/recur/usage) # If per-agent totals are enabled, they go under the Agent level. @@ -240,21 +248,26 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a 'charges' => $component, ); - # XXX this is very silly. we should cache it server-side and - # just put a cache identifier in the link - my $rowlink = "$link;". - ($all_agent ? '' : "agentnum=$row_agentnum;"). + my $row_link = "$link;". + "charges=$component;". + "distribute=$distribute;"; + + if ( $component eq 'D' ) { + # discounts ignore 'charges' and 'distribute' + $row_link = "${p}search/cust_bill_pkg_discount.html?"; + } + + $row_link .= ($all_agent ? '' : "agentnum=$row_agentnum;"). ($all_part_referral ? '' : "refnum=$row_refnum;"). (join('',map {"cust_classnum=$_;"} @cust_classnums)). - "distribute=$distribute;". - "use_override=$use_override;charges=$component;"; - $rowlink .= "$class_param=$_;" foreach @classnums; + "use_override=$use_override;"; + $row_link .= "$class_param=$_;" foreach @classnums; if ( $all_report_options ) { push @row_params, 'all_report_options', 1; - $rowlink .= 'all_report_options=1'; + $row_link .= 'all_report_options=1'; } push @params, \@row_params; - push @links, $rowlink; + push @links, $row_link; @colorbuf = @agent_colors unless @colorbuf; push @colors, shift @colorbuf; @@ -293,13 +306,22 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a ($all_part_referral ? () : ('refnum' => $row_refnum)), 'charges' => $component, ); + my $row_link = "$link;". - ($all_agent ? '' : "agentnum=$row_agentnum;"). + "charges=$component;". + "distribute=$distribute;"; + + if ( $component eq 'D' ) { + # discounts ignore 'charges' and 'distribute' + $row_link ="${p}search/cust_bill_pkg_discount.html?"; + } + + $row_link .= ($all_agent ? '' : "agentnum=$row_agentnum;"). ($all_part_referral ? '' : "refnum=$row_refnum;"). (join('',map {"cust_classnum=$_;"} @cust_classnums)). "$class_param=$row_classnum;". - "distribute=$distribute;". - "use_override=$use_override;charges=$component;"; + "use_override=$use_override;"; + if ( $class_param eq 'report_optionnum' ) { push @row_params, 'all_report_options' => 1, @@ -366,17 +388,6 @@ foreach my $agent ( $all_agent || $sel_agent || $FS::CurrentUser::CurrentUser->a } -# may be useful at some point... -#if ( $average_per_cust_pkg ) { -# @items = map { ('cust_bill_pkg', 'cust_bill_pkg_count_pkgnum') } @items; -# @labels = map { $_, "Packages" } @labels; -# @params = map { $_, $_ } @params; -# @links = map { $_, $_ } @links; -# @colors = map { $_, $_ } @colors; -# @no_graph = map { $_, 1 } @no_graph; -#} -# - #use Data::Dumper; if ( $cgi->param('debug') == 1 ) { $FS::Report::Table::DEBUG = 1; diff --git a/httemplate/graph/report_cust_bill_pkg.html b/httemplate/graph/report_cust_bill_pkg.html index 76d3a6ca3..e99671454 100644 --- a/httemplate/graph/report_cust_bill_pkg.html +++ b/httemplate/graph/report_cust_bill_pkg.html @@ -190,6 +190,12 @@ window.onload = class_mode_changed; 'onchange'=> 'enable_agent_totals', &> % } +<& /elements/tr-select.html, + 'label' => 'Discounts', + 'field' => 'use_discount', + 'options' => [ 2, 1 ], # 3.x only: make 2 the default + 'labels' => { 1 => 'Separate', 2 => 'Do not show' }, +&> Colors diff --git a/httemplate/search/cust_bill_pkg.cgi b/httemplate/search/cust_bill_pkg.cgi index cbe37bcc4..8892af123 100644 --- a/httemplate/search/cust_bill_pkg.cgi +++ b/httemplate/search/cust_bill_pkg.cgi @@ -186,8 +186,8 @@ my $conf = new FS::Conf; my $money_char = $conf->config('money_char') || '$'; my @select = ( 'cust_bill_pkg.*', 'cust_bill._date' ); -my @total = ( 'COUNT(*)', 'SUM(cust_bill_pkg.setup + cust_bill_pkg.recur)'); -my @total_desc = ( $money_char.'%.2f total' ); # sprintf strings +my @total = ( 'COUNT(*)' ); +my @total_desc = (); my @peritem = ( 'setup', 'recur' ); my @peritem_desc = ( 'Setup charges', 'Recurring charges' ); @@ -489,14 +489,32 @@ if ( $cgi->param('nottax') ) { # setup/recur/usage separation my %charges = map { $_ => 1 } split('', $cgi->param('charges') || 'SRU'); - if ( $charges{R} and $charges{U} ) { + if ( $charges{S} and $charges{R} and $charges{U} ) { + # in this case, show discounts - # default, don't change @peritem or @total - if ( !$charges{S} ) { - push @where, 'cust_bill_pkg.recur > 0'; - $total[1] = "SUM(cust_bill_pkg.recur)"; - $total_desc[0] = "$money_char%.2f recurring"; - } + $join_pkg .= ' JOIN ( + SELECT billpkgnum, COALESCE(SUM(amount), 0) AS discounted + FROM cust_bill_pkg_discount RIGHT JOIN cust_bill_pkg USING (billpkgnum) + GROUP BY billpkgnum + ) AS _discount ON (cust_bill_pkg.billpkgnum = _discount.billpkgnum) + '; + push @select, '_discount.discounted'; + + push @peritem, 'discounted'; + push @peritem_desc, 'Discount'; + push @total, 'SUM(cust_bill_pkg.setup + cust_bill_pkg.recur + discounted)', + 'SUM(discounted)', + 'SUM(cust_bill_pkg.setup + cust_bill_pkg.recur)'; + push @total_desc, "$money_char%.2f gross sales", + "− $money_char%.2f discounted", + "= $money_char%.2f invoiced"; + + } elsif ( $charges{R} and $charges{U} ) { + + # hide rows with no recurring fee, and show the sum of recurring fees only + push @where, 'cust_bill_pkg.recur > 0'; + push @total, "SUM(cust_bill_pkg.recur)"; + push @total_desc, "$money_char%.2f recurring"; } elsif ( $charges{R} and !$charges{U} ) { @@ -505,8 +523,8 @@ if ( $cgi->param('nottax') ) { push @select, "($recur_no_usage) AS recur_no_usage"; $peritem[1] = 'recur_no_usage'; $peritem_desc[1] = 'Recurring charges (excluding usage)'; - $total[1] = "SUM($recur_no_usage)"; - $total_desc[0] = "$money_char%.2f recurring"; + push @total, "SUM($recur_no_usage)"; + push @total_desc, "$money_char%.2f recurring"; if ( !$charges{S} ) { push @where, "($recur_no_usage) > 0"; } @@ -518,8 +536,8 @@ if ( $cgi->param('nottax') ) { # there's already a method named 'usage' $peritem[1] = '_usage'; $peritem_desc[1] = 'Usage charge'; - $total[1] = "SUM($usage)"; - $total_desc[0] = "$money_char%.2f usage charges"; + push @total, "SUM($usage)"; + push @total_desc, "$money_char%.2f usage charges"; if ( !$charges{S} ) { push @where, "($usage) > 0"; } @@ -527,8 +545,8 @@ if ( $cgi->param('nottax') ) { } elsif ( $charges{S} ) { push @where, "cust_bill_pkg.setup > 0"; - $total[1] = "SUM(cust_bill_pkg.setup)"; - $total_desc[0] = "$money_char%.2f setup"; + push @total, "SUM(cust_bill_pkg.setup)"; + push @total_desc, "$money_char%.2f setup"; } # else huh? you have to have SOME charges @@ -551,10 +569,11 @@ if ( $cgi->param('nottax') ) { } } - $total[1] = 'SUM( + push @total, 'SUM( COALESCE(cust_bill_pkg_tax_rate_location.amount, cust_bill_pkg.setup + cust_bill_pkg.recur) )'; + push @total_desc, "$money_char%.2f total"; } else { # the internal-tax case @@ -564,8 +583,9 @@ if ( $cgi->param('nottax') ) { '; # don't double-count the components of consolidated taxes - $total[0] = 'COUNT(DISTINCT cust_bill_pkg.billpkgnum)'; - $total[1] = 'SUM(cust_bill_pkg_tax_location.amount)'; + @total = ( 'COUNT(DISTINCT cust_bill_pkg.billpkgnum)', + 'SUM(cust_bill_pkg_tax_location.amount)' ); + @total_desc = "$money_char%.2f total"; # taxclass if ( $cgi->param('taxclassNULL') ) { -- 2.11.0