From 014e2c547f63bcd283eac4d0210a7976ca5fcb90 Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Fri, 14 Nov 2014 15:13:12 -0800 Subject: [PATCH] show active discounts on invoices more cleanly, #31273 --- FS/FS/Template_Mixin.pm | 106 ++++++++++++++++++++++++++++++--------- FS/FS/part_pkg/discount_Mixin.pm | 51 ++++++++++--------- conf/invoice_html | 35 +++++++------ conf/invoice_latex | 19 ++++--- 4 files changed, 142 insertions(+), 69 deletions(-) diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm index f70ac3ce2..29c50b2ee 100644 --- a/FS/FS/Template_Mixin.pm +++ b/FS/FS/Template_Mixin.pm @@ -344,13 +344,13 @@ sub print_generic { my @invoice_template = map "$_\n", $conf->config($templatefile) or die "cannot load config data $templatefile"; - my $old_latex = ''; if ( $format eq 'latex' && grep { /^%%Detail/ } @invoice_template ) { #change this to a die when the old code is removed - warn "old-style invoice template $templatefile; ". + # it's been almost ten years, changing it to a die. + die "old-style invoice template $templatefile; ". "patch with conf/invoice_latex.diff or use new conf/invoice_latex*\n"; - $old_latex = 'true'; - @invoice_template = _translate_old_latex_format(@invoice_template); + #$old_latex = 'true'; + #@invoice_template = _translate_old_latex_format(@invoice_template); } warn "$me print_generic creating T:T object\n" @@ -880,6 +880,7 @@ sub print_generic { ); my $money_char = $money_chars{$format}; + # extremely dubious my %other_money_chars = ( 'latex' => '\dollar ',#XXX should be a config too 'html' => $conf->config('money_char') || '$', 'template' => '', @@ -1091,8 +1092,7 @@ sub print_generic { ext_description => [ map { &$escape_function($_) } @{ $line_item->{'ext_description'} || [] } ], - amount => ( $old_latex ? '' : $money_char). - $line_item->{'amount'}, + amount => $money_char . $line_item->{'amount'}, product_code => $line_item->{'pkgpart'} || 'N/A', }; @@ -1178,8 +1178,9 @@ sub print_generic { $line_item->{'product_code'} = $line_item->{'pkgpart'} || 'N/A'; # mt()? $line_item->{'section'} = $section; $line_item->{'description'} = &$escape_function($line_item->{'description'}); - if (!$old_latex) { # dubious; templates should provide this - $line_item->{'amount'} = $money_char.$line_item->{'amount'}; + $line_item->{'amount'} = $money_char.$line_item->{'amount'}; + + if ( length($line_item->{'unit_amount'}) ) { $line_item->{'unit_amount'} = $money_char.$line_item->{'unit_amount'}; } $line_item->{'ext_description'} ||= []; @@ -1226,13 +1227,12 @@ sub print_generic { if ( $multisection ) { - my $money = $old_latex ? '' : $money_char; push @detail_items, { ext_description => [], ref => '', quantity => '', description => $description, - amount => $money. $amount, + amount => $money_char. $amount, product_code => '', section => $tax_section, }; @@ -1360,13 +1360,12 @@ sub print_generic { $total->{'total_amount'} = $minus.$other_money_char.$credit->{'amount'}; $adjusttotal += $credit->{'amount'}; if ( $multisection ) { - my $money = $old_latex ? '' : $money_char; push @detail_items, { ext_description => [], ref => '', quantity => '', description => &$escape_function($credit->{'description'}), - amount => $money. $credit->{'amount'}, + amount => $money_char . $credit->{'amount'}, product_code => '', section => $adjust_section, }; @@ -1395,13 +1394,12 @@ sub print_generic { $total->{'total_amount'} = $minus.$other_money_char.$payment->{'amount'}; $adjusttotal += $payment->{'amount'}; if ( $multisection ) { - my $money = $old_latex ? '' : $money_char; push @detail_items, { ext_description => [], ref => '', quantity => '', description => &$escape_function($payment->{'description'}), - amount => $money. $payment->{'amount'}, + amount => $money_char . $payment->{'amount'}, product_code => '', section => $adjust_section, }; @@ -2635,16 +2633,21 @@ sub _items_cust_bill_pkg { my $cust_main = $self->cust_main;#for per-agent cust_bill-line_item-ate_style # and location labels - my @b = (); - my ($s, $r, $u) = ( undef, undef, undef ); + my @b = (); # accumulator for the line item hashes that we'll return + my ($s, $r, $u, $d) = ( undef, undef, undef ); + # the 'current' line item hashes for setup, recur, usage, discount foreach my $cust_bill_pkg ( @$cust_bill_pkgs ) { - - foreach ( $s, $r, ($opt{skip_usage} ? () : $u ) ) { + # if the current line item is waiting to go out, and the one we're about + # to start is not bundled, then push out the current one and start a new + # one. + foreach ( $s, $r, ($opt{skip_usage} ? () : $u ) , $d ) { if ( $_ && !$cust_bill_pkg->hidden ) { - $_->{amount} = sprintf( "%.2f", $_->{amount} ), + $_->{amount} = sprintf( "%.2f", $_->{amount} ); $_->{amount} =~ s/^\-0\.00$/0.00/; - $_->{unit_amount} = sprintf( "%.2f", $_->{unit_amount} ), + if (exists($_->{unit_amount})) { + $_->{unit_amount} = sprintf( "%.2f", $_->{unit_amount} ); + } push @b, { %$_ } if $_->{amount} != 0 || $discount_show_always @@ -2715,6 +2718,7 @@ sub _items_cust_bill_pkg { # quotation_pkgs are never fees, so don't worry about the case where # part_pkg is undefined + # and I guess they're never bundled either? if ( $cust_bill_pkg->setup != 0 ) { my $description = $desc; $description .= ' Setup' @@ -2739,7 +2743,8 @@ sub _items_cust_bill_pkg { }; } - } elsif ( $cust_bill_pkg->pkgnum > 0 ) { # and it's not a quotation_pkg + } elsif ( $cust_bill_pkg->pkgnum > 0 ) { + # a "normal" package line item (not a quotation, not a fee, not a tax) warn "$me _items_cust_bill_pkg cust_bill_pkg is non-tax\n" if $DEBUG > 1; @@ -3018,6 +3023,58 @@ sub _items_cust_bill_pkg { } # recurring or usage with recurring charge + # decide whether to show active discounts here + if ( + # case 1: we are showing a single line for the package + ( !$type ) + # case 2: we are showing a setup line for a package that has + # no base recurring fee + or ( $type eq 'S' and $cust_bill_pkg->unitrecur == 0 ) + # case 3: we are showing a recur line for a package that has + # a base recurring fee + or ( $type eq 'R' and $cust_bill_pkg->unitrecur > 0 ) + ) { + + my @discounts = $cust_bill_pkg->cust_bill_pkg_discount; + if( @discounts ) { + warn "$me _items_cust_bill_pkg including discounts for ". + $cust_bill_pkg->billpkgnum."\n" + if $DEBUG; + my $discount_amount = sum( map {$_->amount} @discounts ); + my $orig_amount = $cust_bill_pkg->setup + $cust_bill_pkg->recur + + $discount_amount; + # if multiple discounts apply to the same package, how to display + # them? ext_description lines, apparently + if ( $d and $cust_bill_pkg->hidden ) { + $d->{amount} += $discount_amount; + $d->{orig_amount} += $orig_amount; + } else { + my @ext; + # make a placeholder for the original price, if necessary + # (if unit prices are enabled, it won't be necessary) + push @ext, '' if !$conf->exists('invoice-unitprice'); + $d = { + _is_discount => 1, + description => $self->mt('Discount included'), + amount => $discount_amount, + orig_amount => $orig_amount, + ext_description => \@ext, + }; + foreach my $cust_bill_pkg_discount (@discounts) { + my $def = $cust_bill_pkg_discount->cust_pkg_discount->discount; + push @ext, &{$escape_function}( $def->description ); + } + } + + # update the placeholder to show the original price in the + # first ext_description line + if ( !$conf->exists('invoice-unitprice') ) { + $d->{ext_description}->[0] = + sprintf('Original price: %.2f', $d->{orig_amount}); + } + } # if there are any discounts + } # if this is an appropriate place to show discounts + } else { # taxes and fees warn "$me _items_cust_bill_pkg cust_bill_pkg is tax\n" @@ -3039,13 +3096,14 @@ sub _items_cust_bill_pkg { } - foreach ( $s, $r, ($opt{skip_usage} ? () : $u ) ) { + foreach ( $s, $r, ($opt{skip_usage} ? () : $u, $d ) ) { if ( $_ ) { $_->{amount} = sprintf( "%.2f", $_->{amount} ), if exists($_->{amount}); $_->{amount} =~ s/^\-0\.00$/0.00/; - $_->{unit_amount} = sprintf('%.2f', $_->{unit_amount}) - if exists($_->{unit_amount}); + if (exists($_->{unit_amount})) { + $_->{unit_amount} = sprintf( "%.2f", $_->{unit_amount} ); + } push @b, { %$_ } if $_->{amount} != 0 diff --git a/FS/FS/part_pkg/discount_Mixin.pm b/FS/FS/part_pkg/discount_Mixin.pm index be0200c77..50b331236 100644 --- a/FS/FS/part_pkg/discount_Mixin.pm +++ b/FS/FS/part_pkg/discount_Mixin.pm @@ -32,7 +32,7 @@ sub calc_recur { Takes all the arguments of calc_recur. Calculates and returns the amount by which to reduce the recurring fee; also increments months used on the -discount and generates an invoice detail describing it. +discount. =cut @@ -164,29 +164,32 @@ sub calc_discount { push @{ $param->{'discounts'} }, $cust_bill_pkg_discount; #add details on discount to invoice - my $money_char = $conf->config('money_char') || '$'; - $months = sprintf('%.2f', $months) if $months =~ /\./; - - my $d = 'Includes '; - my $format; - - if ( $months eq '1' ) { - $d .= "discount of $money_char$amount"; - $d .= " each" if $cust_pkg->quantity > 1; - $format = 'Undiscounted amount: %s%.2f'; - } else { - $d .= 'setup ' if defined $param->{'setup_charge'}; - $d .= 'discount of '. $discount->description_short; - $d .= " for $months months" - unless defined $param->{'setup_charge'}; - $d .= ": $money_char$amount" if $discount->percent; - $format = 'Undiscounted monthly amount: %s%.2f'; - } - - push @$details, $d; - push @$details, sprintf( $format, $money_char, $br ); - - $tot_discount += $amount; + # no longer! this is now done during rendering based on the existence + # of the cust_bill_pkg_discount record + # + #my $money_char = $conf->config('money_char') || '$'; + #$months = sprintf('%.2f', $months) if $months =~ /\./; + + #my $d = 'Includes '; + #my $format; + + #if ( $months eq '1' ) { + # $d .= "discount of $money_char$amount"; + # $d .= " each" if $cust_pkg->quantity > 1; + # $format = 'Undiscounted amount: %s%.2f'; + #} else { + # $d .= 'setup ' if defined $param->{'setup_charge'}; + # $d .= 'discount of '. $discount->description_short; + # $d .= " for $months months" + # unless defined $param->{'setup_charge'}; + # $d .= ": $money_char$amount" if $discount->percent; + # $format = 'Undiscounted monthly amount: %s%.2f'; + #} + + #push @$details, $d; + #push @$details, sprintf( $format, $money_char, $br ); + + #$tot_discount += $amount; } sprintf('%.2f', $tot_discount); diff --git a/conf/invoice_html b/conf/invoice_html index 509bf950d..bd99899e0 100644 --- a/conf/invoice_html +++ b/conf/invoice_html @@ -161,23 +161,28 @@ ) } @detail_items ) { - $OUT .= - '{description_generator}}($line); } else { - $OUT .= ( ($line->{'ref'} && $line->{'ref'} ne $lastref) ? '' : '_more' ). - '">'. - ''. - ( $line->{'ref'} ne $lastref ? $line->{'ref'} : '' ). ''. - ''. $line->{'description'}. ''. - ( $unitprices - ? ''. $line->{'unit_amount'}. ''. - ''. $line->{'quantity'}. '' - : '' - ). - - ''. $line->{'amount'}. ''; + my $class = 'invoice_desc_more'; + if ( $line->{'ref'} and $line->{'ref'} ne $lastref ) { + # then it's a new package (not a continuation) + $class = 'invoice_desc'; + } + $OUT .= ' + '; + if ( $line->{'ref'} ne $lastref ) { + $OUT .= $line->{'ref'}; + } + $OUT .= ' + '. $line->{'description'}. ''; + if ( $unitprices ) { + $OUT .= + ''. $line->{'unit_amount'}. ''. + ''. $line->{'quantity'}. ''; + } + $OUT .= ''. $line->{'amount'}. ''; } $OUT .= ''; $lastref = $line->{'ref'}; diff --git a/conf/invoice_latex b/conf/invoice_latex index 6a5b53dd5..99d12d5c7 100644 --- a/conf/invoice_latex +++ b/conf/invoice_latex @@ -187,11 +187,11 @@ \newcommand{\FSdesc}[5]{ \multicolumn{1}{c}{\rule{0pt}{2.5ex}\textbf{#1}} & \multicolumn{[@-- $unitprices ? '4' : '6' --@]}{l}{\textbf{#2}} & -[@-- $unitprices ? ' \multicolumn{1}{r}{\textbf{\dollar #3}} &'."\n". +[@-- $unitprices ? ' \multicolumn{1}{r}{\textbf{#3}} &'."\n". ' \multicolumn{1}{r}{\textbf{#4}} &'."\n" : '' --@] - \multicolumn{1}{r}{\textbf{\dollar #5}}\\ + \multicolumn{1}{r}{\textbf{#5}}\\ } % ...extended description... \newcommand{\FSextdesc}[1]{ @@ -333,10 +333,17 @@ } else { $OUT .= '\FSdesc'. '{' . ( $line->{'ref'} ne $lastref ? $line->{'ref'} : '' ) . '}'. - '{' . $line->{'description'} . '}' . - '{' . ( $unitprices ? $line->{'unit_amount'} : '' ) . '}'. - '{' . ( $unitprices ? $line->{'quantity'} : '' ) . '}' . - '{' . $line->{'amount'} . "}${rowbreak}\n"; + '{' . $line->{'description'} . '}' ; + if ( $unitprices and length($line->{'unit_amount'}) ) { + # then show the unit amount and quantity + $OUT .= + '{\\dollar' . $line->{'unit_amount'} . '}'. + '{' . $line->{'quantity'} . '}'; + } else { + # leave those columns blank + $OUT .= '{}{}'; + } + $OUT .= '{\\dollar' . $line->{'amount'} . "}${rowbreak}\n"; } $lastref = $line->{'ref'}; -- 2.11.0