[freeside-commits] branch master updated. 014e2c547f63bcd283eac4d0210a7976ca5fcb90

Mark Wells mark at 420.am
Fri Nov 14 15:13:17 PST 2014


The branch, master has been updated
       via  014e2c547f63bcd283eac4d0210a7976ca5fcb90 (commit)
      from  a07ca8e4ca980b88a4d6a8f85b4f72c459599e0a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 014e2c547f63bcd283eac4d0210a7976ca5fcb90
Author: Mark Wells <mark at freeside.biz>
Date:   Fri Nov 14 15:13:12 2014 -0800

    show active discounts on invoices more cleanly, #31273

diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index f70ac3c..29c50b2 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 be0200c..50b3312 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 509bf95..bd99899 100644
--- a/conf/invoice_html
+++ b/conf/invoice_html
@@ -161,23 +161,28 @@
                  ) }
             @detail_items )
           {
-            $OUT .=
-              '<tr class="invoice_desc';
             if ( $section->{description_generator} ) {
-              $OUT .= &{$section->{description_generator}}($line);
+              $OUT .= '<tr class="invoice_desc' .
+                      &{$section->{description_generator}}($line);
             } else {
-              $OUT .=  ( ($line->{'ref'} && $line->{'ref'} ne $lastref) ? '' : '_more' ).
-                       '">'.
-                       '<td align="center">'. 
-                       ( $line->{'ref'} ne $lastref ? $line->{'ref'} : '' ). '</td>'.
-                       '<td align="left">'. $line->{'description'}. '</td>'.
-                       ( $unitprices 
-                           ? '<td align="right">'. $line->{'unit_amount'}. '</td>'.
-                             '<td align="right">'. $line->{'quantity'}. '</td>'
-                           : ''
-                       ).
-
-                       '<td align="right">'. $line->{'amount'}. '</td>';
+              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 .= '<tr class="'.$class.'">
+                       <td align="center">';
+              if ( $line->{'ref'} ne $lastref ) {
+                $OUT .= $line->{'ref'};
+              }
+              $OUT .= '</td>
+                       <td align="left">'. $line->{'description'}. '</td>';
+              if ( $unitprices ) {
+                $OUT .= 
+                  '<td align="right">'. $line->{'unit_amount'}. '</td>'.
+                  '<td align="right">'. $line->{'quantity'}. '</td>';
+              }
+              $OUT .= '<td align="right">'. $line->{'amount'}. '</td>';
             }
             $OUT .= '</tr>';
             $lastref = $line->{'ref'};
diff --git a/conf/invoice_latex b/conf/invoice_latex
index 6a5b53d..99d12d5 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'};
 

-----------------------------------------------------------------------

Summary of changes:
 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(-)




More information about the freeside-commits mailing list