[freeside-commits] branch FREESIDE_4_BRANCH updated. 504ec7fd59d124142d98dd0539aa48ee5e8963ae

Mitch Jackson mitch at freeside.biz
Sat May 19 15:50:04 PDT 2018


The branch, FREESIDE_4_BRANCH has been updated
       via  504ec7fd59d124142d98dd0539aa48ee5e8963ae (commit)
       via  63c66b013cbb8429b4f0f3796dd8ea5fe221a2c1 (commit)
       via  a1b5ab1539ec4fd31154975f5066adbea978826c (commit)
       via  cf744d8318d418ce2f1312436bac4d19f20fac9c (commit)
       via  0e73b93c4d08c645047e3b606d7a25e55c3f9235 (commit)
       via  6ea1cdac9e057d04c334e993416b90e569227f6f (commit)
       via  f2a59eb8033c12862461c87312d7e61c41e1f3c4 (commit)
       via  d310f2f3e2048c2d178576cb42c633de70e30c49 (commit)
       via  c059d891317e7bc09d5384e9c39bf43e01e346f0 (commit)
      from  9baab880cb616a2b9bd7812306563e2f13a4880d (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 504ec7fd59d124142d98dd0539aa48ee5e8963ae
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat May 19 20:31:02 2018 +0000

    RT# 79363 Hide empty tax section, invoice_sections_with_taxes

diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index ebdcd6d46..1bae5bc40 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -1396,6 +1396,14 @@ sub print_generic {
       $other_money_char. sprintf('%.2f', $self->charged - $taxtotal );
 
     if ( $multisection ) {
+
+      if ( $conf->config_bool('invoice_sections_with_taxes', $cust_main->agentnum) ) {
+        # If all tax items are displayed in location/category sections,
+        # remove the empty tax section
+        @sections = grep{ $_ ne $tax_section } @sections
+          unless grep{ $_->{section} eq $tax_section } @detail_items;
+      }
+
       if ( $taxtotal > 0 ) {
         # there are taxes, so prepare the section to be displayed.
         # $taxtotal already includes any line items that were already in the
@@ -1409,14 +1417,7 @@ sub print_generic {
         $tax_section->{'description'} = $self->mt($tax_description);
         $tax_section->{'summarized'} = '';
 
-        if ( $conf->config_bool('invoice_sections_with_taxes', $cust_main->agentnum) ) {
-
-          # If all tax items are displayed in location/category sections,
-          # remove the empty tax section
-          @sections = grep{ $_ ne $tax_section } @sections
-            unless grep{ $_->{section} eq $tax_section } @detail_items;
-
-        } elsif ( !grep $tax_section, @sections ) {
+        if ( !grep $tax_section, @sections ) {
 
           # append it if it's not already there
           push @sections, $tax_section;

commit 63c66b013cbb8429b4f0f3796dd8ea5fe221a2c1
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat May 19 20:05:15 2018 +0000

    RT# 78190 Fix taxes on fees for sectioned invoices
    
    Fix taxes charged on a billing-event fee, such as a late fee,
    displayed incorrectly on some sectioned invoices

diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index 0d8687846..ebdcd6d46 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -3184,6 +3184,7 @@ sub _items_fee {
 
     push @items,
       { feepart     => $cust_bill_pkg->feepart,
+        billpkgnum  => $cust_bill_pkg->billpkgnum,
         amount      => sprintf('%.2f', $cust_bill_pkg->setup + $cust_bill_pkg->recur),
         description => $desc,
         pkg_tax     => \@pkg_tax,
diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index a36520b77..983f62bed 100644
--- a/FS/FS/cust_bill_pkg.pm
+++ b/FS/FS/cust_bill_pkg.pm
@@ -1848,7 +1848,29 @@ sub _pkg_tax_list {
   #   Duplicates can be identified by billpkgtaxlocationnum column.
 
   my $self = shift;
-  return unless $self->pkgnum;
+
+  my $search_selector;
+  if ( $self->pkgnum ) {
+
+    # For taxes applied to normal billing items
+    $search_selector =
+      ' cust_bill_pkg_tax_location.pkgnum = '
+      . dbh->quote( $self->pkgnum );
+
+  } elsif ( $self->feepart ) {
+
+    # For taxes applied to fees, when the fee is not attached to a package
+    # i.e. late fees, billing events fees
+    $search_selector =
+      ' cust_bill_pkg_tax_location.taxable_billpkgnum = '
+      . dbh->quote( $self->billpkgnum );
+
+  } else {
+    warn "_pkg_tax_list() unhandled case breaking taxes into sections";
+    warn "_pkg_tax_list() $_: ".$self->$_
+      for qw(pkgnum billpkgnum feepart);
+    return;
+  }
 
   map +{
       billpkgtaxlocationnum => $_->billpkgtaxlocationnum,
@@ -1874,7 +1896,7 @@ sub _pkg_tax_list {
       ' WHERE '.
       ' cust_bill_pkg.invnum = ' . dbh->quote( $self->invnum ) .
       ' AND '.
-      ' cust_bill_pkg_tax_location.pkgnum = ' . dbh->quote( $self->pkgnum ),
+      $search_selector
   });
 
 }

commit a1b5ab1539ec4fd31154975f5066adbea978826c
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Fri May 18 22:40:08 2018 +0000

    RT# 78190 Fix billing event/late fee on sectioned invoices

diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index d3d6bbf8b..0d8687846 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -1346,27 +1346,36 @@ sub print_generic {
   #$tax_section->{'summarized'} = ''; #why? $summarypage && !$tax_weight ? 'Y' : '';
   #$tax_section->{'sort_weight'} = $tax_weight;
 
+  my $invoice_sections_with_taxes = $conf->config_bool(
+    'invoice_sections_with_taxes', $cust_main->agentnum
+  );
+
   foreach my $tax ( @items_tax ) {
 
-    $taxtotal += $tax->{'amount'};
 
     my $description = &$escape_function( $tax->{'description'} );
     my $amount      = sprintf( '%.2f', $tax->{'amount'} );
 
     if ( $multisection ) {
+      if ( !$invoice_sections_with_taxes ) {
 
-      push @detail_items, {
-        ext_description => [],
-        ref          => '',
-        quantity     => '',
-        description  => $description,
-        amount       => $money_char. $amount,
-        product_code => '',
-        section      => $tax_section,
-      };
+        $taxtotal += $tax->{'amount'};
 
+        push @detail_items, {
+          ext_description => [],
+          ref          => '',
+          quantity     => '',
+          description  => $description,
+          amount       => $money_char. $amount,
+          product_code => '',
+          section      => $tax_section,
+        };
+
+      }
     } else {
 
+      $taxtotal += $tax->{'amount'};
+
       push @total_items, {
         'total_item'   => $description,
         'total_amount' => $other_money_char. $amount,
@@ -1402,8 +1411,10 @@ sub print_generic {
 
         if ( $conf->config_bool('invoice_sections_with_taxes', $cust_main->agentnum) ) {
 
-          # remove tax section if taxes are itemized within other sections
-          @sections = grep{ $_ ne $tax_section } @sections;
+          # If all tax items are displayed in location/category sections,
+          # remove the empty tax section
+          @sections = grep{ $_ ne $tax_section } @sections
+            unless grep{ $_->{section} eq $tax_section } @detail_items;
 
         } elsif ( !grep $tax_section, @sections ) {
 
@@ -3122,17 +3133,31 @@ sub _items_fee {
       warn "fee definition not found for line item #".$cust_bill_pkg->billpkgnum."\n";
       next;
     }
-    if ( exists($options{section}) and exists($options{section}{category}) )
-    {
-      my $categoryname = $options{section}{category};
-      # then filter for items that have that section
-      if ( $part_fee->categoryname ne $categoryname ) {
-        warn "skipping fee '".$part_fee->itemdesc."'--not in section $categoryname\n" if $DEBUG;
-        next;
-      }
-    } # otherwise include them all in the main section
-    # XXX what to do when sectioning by location?
-    
+
+    # If _items_fee is called while building a sectioned invoice,
+    #   - invoice_sections_method: category
+    #     Skip fee records that do not match the section category.
+    #   - invoice_sections_method: location
+    #     Skip fee records always for location sections.
+    #     The fee records will be presented in the tax/fee section instead.
+    if (
+      exists( $options{section} )
+      and
+      (
+        (
+          exists( $options{section}{category} )
+          and
+          $part_fee->categoryname ne $options{section}{category}
+        )
+        or
+        exists( $options{section}{location})
+      )
+    ) {
+      warn "skipping fee '".$part_fee->itemdesc.
+           "'--not in section $options{section}{category}\n" if $DEBUG;
+      next;
+    }
+
     my @ext_desc;
     my %base_invnums; # invnum => invoice date
     foreach ($cust_bill_pkg->cust_bill_pkg_fee) {

commit cf744d8318d418ce2f1312436bac4d19f20fac9c
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sun Apr 15 16:41:32 2018 -0500

    RT# 78190,42357 Correct layout discrepancies for bill summary

diff --git a/conf/invoice_htmlsummary b/conf/invoice_htmlsummary
index 249db9b07..c07d7e229 100644
--- a/conf/invoice_htmlsummary
+++ b/conf/invoice_htmlsummary
@@ -35,15 +35,6 @@
           <td align="right"><b><%= $dollar.$current_less_finance %></b></td>
         </tr>
         <tr><th colspan=2><br></th></tr>
-        <tr>
-          <td><b><u><br>Summary of Payments and Credits<br></u></b></td>
-          <td></td>
-        </tr>
-        <tr>
-          <td><b>Payments and Credits</b></td>
-          <td align="right"><b>-<%= $dollar.$balance_adjustments %></b></td>
-        </tr>
-        <tr><th colspan=2><br></th></tr>
         <tr><td colspan=2><br></td></tr>
         <tr>
           <td><b><u>Invoice Summary</u></b></td>
@@ -61,16 +52,14 @@
           <td><b>New Charges</b></td>
           <td align="right"><b><%= $dollar.$current_less_finance %></b></td>
         </tr>
-        <%=
-          foreach my $section ( grep $_->{adjust_section}, @sections) {
-            $OUT .= '<tr><td><b>'. ($section->{'description'} ? $section->{'description'} : 'Charges' ). '</b></td>';
-            $OUT .= qq(<th align="right"><b>). $section->{'subtotal'}. "</b></th></tr>";
-          }
-        %>
-        <tr>
-          <td><b>Payments and Credits</b></td>
-          <th align="right"><b>-<%= $dollar.sprintf('%.2f', $balance_adjustments) %></b></th>
-        </tr>
+        <%= if ( $balance_adjustments > 0 ) {
+          $OUT .= "
+            <tr>
+              <td><b>Payments and Credits</b></td>
+              <th align='right'><b>-$dollar" . sprintf('%.2f', $balance_adjustments). "</b></th>
+            </tr>
+          ";
+        } %>
         <tr>
           <td><b>Total Amount Due</b></td>
           <td align="right"><b><%= $dollar.sprintf('%.2f', $balance) %></b></td>
diff --git a/conf/invoice_latexsummary b/conf/invoice_latexsummary
index 52868419b..3b13327dc 100644
--- a/conf/invoice_latexsummary
+++ b/conf/invoice_latexsummary
@@ -9,12 +9,9 @@
 \begin{tabular}{lr}
 \hline
 &\\
-\textbf{\underline{Summary of Previous Balance and Payments}} & \\
+\textbf{\underline{Summary of Previous Balance}} & \\
 &\\
-\textbf{Previous Balance}&\textbf{\dollar[@-- $true_previous_balance --@]}\\
-\textbf{Payments}&\textbf{\dollar[@-- $balance_adjustments --@]}\\
-\cline{2-2}
-\textbf{Balance Outstanding}&\textbf{\dollar[@-- sprintf('%.2f', $true_previous_balance) --@]}\\
+\textbf{Previous Balance}&\textbf{\dollar[@-- sprintf('%.2f', $true_previous_balance) --@]}\\
 &\\
 \hline
 &\\
@@ -36,15 +33,11 @@
 \textbf{Previous Past Due Charges}&\textbf{\dollar[@-- sprintf('%.2f', $true_previous_balance) --@]}\\
 \textbf{Finance charges on overdue amount}&\textbf{\dollar[@-- $finance_amount --@]}\\
 \textbf{New Charges}&\textbf{\dollar[@-- $current_less_finance --@]}\\
-
 [@--
-  #false laziness w/invoice_htmlsummary and above
-  foreach my $section ( grep $_->{adjust_section}, @sections ) {
-    $OUT .= '\textbf{'. ($section->{'description'} ? $section->{'description'} : 'Charges' ). '}';
-    $OUT .= '&\textbf{'. $section->{'subtotal'}. '}\\\\';
+  if ( $balance_adjustments > 0 ) {
+    $OUT .= '\textbf{Payments and Credits}&\textbf{-\dollar'.$balance_adjustments.'}\\\\'
   }
 --@]
-
 \cline{2-2}
 \textbf{Total Amount Due}&\textbf{\dollar[@-- sprintf('%.2f', $balance) --@]}\\
 &\\

commit 0e73b93c4d08c645047e3b606d7a25e55c3f9235
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat Apr 14 19:35:12 2018 -0500

    RT# 42357,78190 Fix Fees appearing twice within sectioned invoices

diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index 6905cf1f1..d3d6bbf8b 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -1219,6 +1219,17 @@ sub print_generic {
     foreach my $line_item ( $self->_items_pkg(%options),
                             $self->_items_fee(%options) ) {
 
+      # When bill is sectioned by location, fees may be displayed within the
+      # appropriate location section.  Suppress this fee from the taxes/fees
+      # end section, so it doesn't appear to be charged twice and make the
+      # subtotals seem incorrect
+      next
+        if $line_item->{locationnum}
+        && ref $options{section}
+        && !exists $options{section}->{locationnum}
+        && $self->has_sections
+        && $conf->config($tc.'sections_method') eq 'location';
+
       warn "$me     adding line item ".
            join(', ', map "$_=>".$line_item->{$_}, keys %$line_item). "\n"
         if $DEBUG > 1;

commit 6ea1cdac9e057d04c334e993416b90e569227f6f
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat Apr 14 12:56:14 2018 -0500

    POD Documentation

diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm
index 93606589d..ec6e3d341 100644
--- a/FS/FS/cust_bill.pm
+++ b/FS/FS/cust_bill.pm
@@ -2713,7 +2713,7 @@ sub _items_svc_phone_sections {
 
 }
 
-=sub _items_usage_class_summary OPTIONS
+=item _items_usage_class_summary OPTIONS
 
 Returns a list of detail items summarizing the usage charges on this 
 invoice.  Each one will have 'amount', 'description' (the usage charge name),
@@ -2762,7 +2762,7 @@ sub _items_usage_class_summary {
   return @l;
 }
 
-=sub _items_previous()
+=item _items_previous()
 
   Returns an array of hashrefs, each hashref representing a line-item on
   the current bill for previous unpaid invoices.
@@ -2896,7 +2896,7 @@ sub _items_previous {
 
 }
 
-=sub _items_previous_total
+=item _items_previous_total
 
   Return sum of amounts from all items returned by _items_previous
   Results will vary based on invoicing conf flags
@@ -2946,7 +2946,7 @@ sub __items_previous_map_invoice {
   }
 }
 
-=sub _items_credits()
+=item _items_credits()
 
   Return array of hashrefs containing credits to be shown as line-items
   when rendering this bill.
@@ -3085,7 +3085,7 @@ sub _items_credits {
   @return;
   }
 
-=sub _items_credits_total
+=item _items_credits_total
 
   Return the total of al items from _items_credits
   Will vary based on invoice display conf flag
@@ -3101,7 +3101,7 @@ sub _items_credits_total {
 
 
 
-=sub _items_credits_postbill()
+=item _items_credits_postbill()
 
   Returns an array of hashrefs for credits where
   - Credit issued after this invoice
@@ -3143,7 +3143,7 @@ sub _items_credits_postbill {
   }} @cust_credit_bill;
 }
 
-=sub _items_payments_postbill()
+=item _items_payments_postbill()
 
   Returns an array of hashrefs for payments where
   - Payment occured after this invoice
@@ -3179,7 +3179,7 @@ sub _items_payments_postbill {
   }} @cust_bill_pay;
 }
 
-=sub _items_payments()
+=item _items_payments()
 
   Return array of hashrefs containing payments to be shown as line-items
   when rendering this bill.
@@ -3297,7 +3297,7 @@ sub _items_payments {
   return @{ $self->get('_items_payments') };
   }
 
-=sub _items_payments_total
+=item _items_payments_total
 
   Return a total of all records returned by _items_payments
   Results vary based on invoicing conf flags
@@ -3354,7 +3354,7 @@ sub __items_payments_make_hashref {
   return @return;
   }
 
-=sub _items_total()
+=item _items_total()
 
   Generate the line-items to be shown on the bill in the "Totals" section
 

commit f2a59eb8033c12862461c87312d7e61c41e1f3c4
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat Apr 14 12:41:30 2018 -0500

    RT# 78190 Update totals for latex summary

diff --git a/conf/invoice_latexsummary b/conf/invoice_latexsummary
index bd4ea6975..52868419b 100644
--- a/conf/invoice_latexsummary
+++ b/conf/invoice_latexsummary
@@ -14,7 +14,7 @@
 \textbf{Previous Balance}&\textbf{\dollar[@-- $true_previous_balance --@]}\\
 \textbf{Payments}&\textbf{\dollar[@-- $balance_adjustments --@]}\\
 \cline{2-2}
-\textbf{Balance Outstanding}&\textbf{\dollar[@-- sprintf('%.2f', $true_previous_balance -$balance_adjustments) --@]}\\
+\textbf{Balance Outstanding}&\textbf{\dollar[@-- sprintf('%.2f', $true_previous_balance) --@]}\\
 &\\
 \hline
 &\\
@@ -33,7 +33,7 @@
 &\\
 \textbf{\underline{Invoice Summary}} & \\
 & \\
-\textbf{Previous Past Due Charges}&\textbf{\dollar[@-- sprintf('%.2f', $true_previous_balance - $balance_adjustments) --@]}\\
+\textbf{Previous Past Due Charges}&\textbf{\dollar[@-- sprintf('%.2f', $true_previous_balance) --@]}\\
 \textbf{Finance charges on overdue amount}&\textbf{\dollar[@-- $finance_amount --@]}\\
 \textbf{New Charges}&\textbf{\dollar[@-- $current_less_finance --@]}\\
 

commit d310f2f3e2048c2d178576cb42c633de70e30c49
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat Apr 14 12:25:26 2018 -0500

    RT# 78190 Remove debug statements

diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm
index d4383f91e..93606589d 100644
--- a/FS/FS/cust_bill.pm
+++ b/FS/FS/cust_bill.pm
@@ -3240,7 +3240,6 @@ sub _items_payments {
 
   if ( $self->conf->exists('previous_balance-payments_since') ) {
     if ($template eq 'statement') {
-print "\nCASE 3\n";
       # Case 3 (see above)
       # Return payments timestamped between the previous and following bills
 
@@ -3264,7 +3263,7 @@ print "\nCASE 3\n";
     } else {
       # Case 2 (see above)
       # Return payments timestamped between this and the previous bill
-print "\nCASE 2\n";
+
       my $date_start = 0;
       my $date_end = $self->_date;
 

commit c059d891317e7bc09d5384e9c39bf43e01e346f0
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Tue May 8 03:03:08 2018 +0000

    Fixed invoice inconsistencies with various conf flags RT#78190
    
    Applying different invoicing conf flags manifested different
    variations of the same problem.  Addressed by this fix:
    
     - Incorrect items listed for Previous Balance
     - Incorrect Items listed for applied payments and credits
     - Incorrect subtotals for various sections
     - Invoice amounts, subtotals, balances displayed did not reconcile.
       Because of which data was selected for display, columns could appear
       to have bad math.  No account balances were factually incorrect.
     - Items disappearing from invoices used a payment receipts or
       "statements" giving a false impression of overpayment or credits
     - Applied payments or credits appearing on the wrong statements
     - A single applied credit appearing on up to 3 invoices
     - When viewing older invoices, future payments for future bills
       shown on, and appearing to apply to, the older invoice
     - Inconsistencies of line items and numbers between website, email,
       pdf and txt version invoices.
     - Invoice summary page numbers not matching the invoice
     - Incorrect balances shown on on aging line
     - Update item order on invoice_htmlsummary mason template
    
    Conf flags involved in these issues:
    
     - disable_previous_balance
     - previous_balance-payments_since
     - previous_balance-summary_only
     - previous_balance-show_on_statements
     - previous_balance-section
     - previous_balance-exclude_from_total
     - invoice_include_aging
     - invoice_show_prior_due_date
     - invoice_usesummary
    
    New invoice template stash variables made available:
    
     - aged_balance_current
     - aged_balance_30d
     - aged_balance_60d
     - aged_balance_90d
    
    Solved by updating, or creating, FS::cust_bill helper methods that
    generate data to be displayed on invoices.  These helper methods
    are responsive to various conf flags.  Updated template pipeline to
    use these helpers instead of inconsistent sql queries.
    
    Resolves: #78190
    See Also: #75709, #76161, #74426

diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index 07252d05c..6905cf1f1 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -344,6 +344,8 @@ sub print_generic {
   $templatefile .= "_$template"
     if length($template) && $conf->exists($templatefile."_$template");
 
+  $self->set('_template',$template);
+
   # the base template
   my @invoice_template = map "$_\n", $conf->config($templatefile)
     or die "cannot load config data $templatefile";
@@ -692,18 +694,34 @@ sub print_generic {
   $invoice_data{'barcode_cid'} = $params{'barcode_cid'}
     if $params{'barcode_cid'};
 
-  my( $pr_total, @pr_cust_bill ) = $self->previous; #previous balance
-#  my( $cr_total, @cr_cust_credit ) = $self->cust_credit; #credits
-  #my $balance_due = $self->owed + $pr_total - $cr_total;
-  my $balance_due = $self->owed;
-  if ( $self->enable_previous ) {
-    $balance_due += $pr_total;
-  }
-  # otherwise the previous balance is not shown, so including it in the
-  # balance due is just confusing
 
-  # the sum of amount owed on all invoices
-  # (this is used in the summary & on the payment coupon)
+  # re: rt:78190
+  #   using owed_on_invoice() instead of owed() here for $balance_due
+  #   using _items_previous_total() instead of ->previous() for $pr_total
+  #
+  #   owed_on_invoice() is aware of configuration flags that affect how an
+  #     invoice is rendered.  May not return actual current balance. Will
+  #     return balance appropriate for the invoice being rendered, based
+  #     on which past due items, current charges, and future payments are
+  #     displayed.
+  #
+  #   Going forward, usage of owed(), or bypassing cust_bill helper methods
+  #     when generating invoice lines may lead to incorrect or misleading
+  #     math on invoices.
+  #
+  #   Helper methods that are aware of invoicing conf flags:
+  #   - owed_on_invoice          # use instead of owed()
+  #   - _items_previous()        # use instead of previous()
+  #   - _items_credits()         # use instead of cust_credit()
+  #   - _items_payments()
+  #   - _items_total()
+  #   - _items_previous_total()  # use instead of previous()
+  #   - _items_payments_total()
+  #   - _items_credits_total()   # use instead of cust_credit()
+
+  my $pr_total    = $self->_items_previous_total();
+
+  my $balance_due = $self->owed_on_invoice();
   $invoice_data{'balance'} = sprintf("%.2f", $balance_due);
 
   # flag telling this invoice to have a first-page summary
@@ -717,124 +735,97 @@ sub print_generic {
     # summary formats
     $invoice_data{'last_bill'} = {};
  
-    my $last_bill = $self->previous_bill;
-    if ( $last_bill ) {
+    #    my $last_bill = $self->previous_bill;
+    # if ( $last_bill ) {
 
-      # "balance_date_range" unfortunately is unsuitable for this, since it
-      # cares about application dates.  We want to know the sum of all 
-      # _top-level transactions_ dated before the last invoice.
-      #
-      # still do this for the "Previous Balance" line of the summary block
-      my @sql =
-        map "$_ WHERE _date <= ? AND custnum = ?", (
-          "SELECT      COALESCE( SUM(charged), 0 ) FROM cust_bill",
-          "SELECT -1 * COALESCE( SUM(amount),  0 ) FROM cust_credit",
-          "SELECT -1 * COALESCE( SUM(paid),    0 ) FROM cust_pay",
-          "SELECT      COALESCE( SUM(refund),  0 ) FROM cust_refund",
+    # Populate template stash for previous balance and payments
+    if ($pr_total) {
+      # Used on summary page as "Previous Balance"
+      $invoice_data{'true_previous_balance'} = sprintf("%.2f", $pr_total);
+
+      # Used on summary page as "Payments"
+      $invoice_data{'balance_adjustments'} = sprintf("%.2f",
+        $self->_items_payments_total() + $self->_items_credits_total()
         );
 
-      # the customer's current balance immediately after generating the last 
-      # bill
+      # Used in invoice template as "Previous Balance"
+      $invoice_data{'previous_balance'} = sprintf("%.2f", $pr_total);
 
-      my $last_bill_balance = $last_bill->charged;
-      foreach (@sql) {
-        my $delta = FS::Record->scalar_sql(
-          $_,
-          $last_bill->_date - 1,
-          $self->custnum,
-        );
-        $last_bill_balance += $delta;
-      }
+      # $invoice_data{last_bill}{_date}:
+      # Not used in default templates, but may be in use by someone
+      #
+      # ! May be a problem field if they are using it... this field
+      #   stores the date of the previous invoice... it is possible to
+      #   carry a balance, but have the immediately previous invoice paid off.
+      #   In this case, this field might be presenting bad data?  Not
+      #   altering the problematic behavior, because someone might be
+      #   expecting this bad behavior in their templates for some other
+      #   purpose, such as a "your last bill was dated %_date%"
+      my $last_bill = $self->previous_bill;
+      $invoice_data{'last_bill'}{'_date'}
+        = ref $last_bill
+        ? $last_bill->_date()
+        : undef;
+
+      # $invoice_data{previous_payments}
+      # Not used in default templates, but may be in use by someone
+      #
+      # Returns an array of hrefs representing payments, each with keys:
+      #  - _date:       epoch timestamp
+      #  - date:        text formatted date
+      #  - amount:      money formatted amount string
+      #  - payinfo:     string from payby_payinfo_pretty()
+      #  - paynum:      id for cust_pay
+      #  - description: Text description for bill line item
+      #
+      my @payments = $self->_items_payments();
+      $invoice_data{previous_payments} = \@payments;
 
-      $last_bill_balance = sprintf("%.2f", $last_bill_balance);
-
-      warn sprintf("LAST BILL: INVNUM %d, DATE %s, BALANCE %.2f\n\n",
-        $last_bill->invnum,
-        $self->time2str_local('%D', $last_bill->_date),
-        $last_bill_balance
-      ) if $DEBUG > 0;
-      # ("true_previous_balance" is a terrible name, but at least it's no
-      # longer stored in the database)
-      $invoice_data{'true_previous_balance'} = $last_bill_balance;
-
-      # Now, get all applications of credits/payments dated on or after the
-      # previous bill, to invoices before the current bill. (The
-      # credit/payment date restriction prevents these from intersecting
-      # the "Previous Balance" set.)
-      # These are "adjustments". The past due balance will be shown as
-      # Previous Balance - Adjustments.
-      my $adjustments = 0;
-      @sql = map {
-        "SELECT COALESCE(SUM(y.amount),0) FROM $_ JOIN cust_bill USING (invnum)
-         WHERE cust_bill._date < ?
-           AND x._date >= ?
-           AND cust_bill.custnum = ?"
-        } "cust_credit AS x JOIN cust_credit_bill y USING (crednum)",
-          "cust_pay    AS x JOIN cust_bill_pay    y USING (paynum)"
-      ;
-      foreach (@sql) {
-        my $delta = FS::Record->scalar_sql(
-          $_,
-          $self->_date,
-          $last_bill->_date,
-          $self->custnum,
+      # $invoice_data{previous_credits}
+      # Not used in default templates, but may be in use by someone
+      #
+      # Returns an array of hrefs representing credits, each with keys:
+      #  - _date:        epoch timestamp
+      #  - date:         text formatted date
+      #  - amount:       money formatted amount string
+      #  - crednum:      id for cust_credit
+      #  - description:  Text description for bill line item
+      #  - creditreason: reason() from cust_credit
+      #
+      my @credits = $self->_items_credits();
+      $invoice_data{previous_credits} = \@credits;
+
+      # Populate formatted date field
+      for my $pmt_href (@payments, @credits) {
+        $pmt_href->{date} = $self->time2str_local(
+          'long',
+          $pmt_href->{_date},
+          $format
         );
-        $adjustments += $delta;
       }
-      $invoice_data{'balance_adjustments'} = sprintf("%.2f", $adjustments);
-
-      warn sprintf("BALANCE ADJUSTMENTS: %.2f\n\n",
-                   $invoice_data{'balance_adjustments'}
-      ) if $DEBUG > 0;
-
-      # the sum of amount owed on all previous invoices
-      # ($pr_total is used elsewhere but not as $previous_balance)
-      $invoice_data{'previous_balance'} = sprintf("%.2f", $pr_total);
 
-      $invoice_data{'last_bill'}{'_date'} = $last_bill->_date; #unformatted
-      my (@payments, @credits);
-      # for formats that itemize previous payments
-      foreach my $cust_pay ( qsearch('cust_pay', {
-                              'custnum' => $self->custnum,
-                              '_date'   => { op => '>=',
-                                             value => $last_bill->_date }
-                             } ) )
-      {
-        next if $cust_pay->_date > $self->_date;
-        push @payments, {
-            '_date'       => $cust_pay->_date,
-            'date'        => $self->time2str_local('long', $cust_pay->_date, $format),
-            'payinfo'     => $cust_pay->payby_payinfo_pretty,
-            'amount'      => sprintf('%.2f', $cust_pay->paid),
-        };
-        # not concerned about applications
-      }
-      foreach my $cust_credit ( qsearch('cust_credit', {
-                              'custnum' => $self->custnum,
-                              '_date'   => { op => '>=',
-                                             value => $last_bill->_date }
-                             } ) )
-      {
-        next if $cust_credit->_date > $self->_date;
-        push @credits, {
-            '_date'       => $cust_credit->_date,
-            'date'        => $self->time2str_local('long', $cust_credit->_date, $format),
-            'creditreason'=> $cust_credit->reason,
-            'amount'      => sprintf('%.2f', $cust_credit->amount),
-        };
-      }
-      $invoice_data{'previous_payments'} = \@payments;
-      $invoice_data{'previous_credits'}  = \@credits;
     } else {
-      # there is no $last_bill
+      # There are no outstanding invoices    = YAPH
       $invoice_data{'true_previous_balance'} =
       $invoice_data{'balance_adjustments'}   =
       $invoice_data{'previous_balance'}      = '0.00';
-      $invoice_data{'previous_payments'} = [];
+      $invoice_data{'previous_payments'}     =
       $invoice_data{'previous_credits'} = [];
     }
- 
-    if ( $conf->config_bool('invoice_usesummary', $agentnum) ) {
+
+    # Condencing a lot of debug staements here
+    if ($DEBUG) {
+      warn "\$invoice_data{$_}: $invoice_data{$_}"
+        for qw(
+          true_previous_balance
+          balance_adjustments
+          previous_balance
+          previous_payments
+          previous_credits
+        );
+    }
+
+    if ( $conf->exists('invoice_usesummary', $agentnum) ) {
       $invoice_data{'summarypage'} = $summarypage = 1;
     }
 
@@ -970,11 +961,21 @@ sub print_generic {
                                             sprintf('%.2f', $pr_total),
                            'summarized'  => '', #why? $summarypage ? 'Y' : '',
                          };
-    $previous_section->{posttotal} = '0 / 30 / 60 / 90 days overdue '. 
-      join(' / ', map { $cust_main->balance_date_range(@$_) }
-                  $self->_prior_month30s
-          )
-      if $conf->exists('invoice_include_aging');
+
+    # Include balance aging line and template variables
+    my @aged_balances = $self->_items_aging_balances();
+    ( $invoice_data{aged_balance_current},
+      $invoice_data{aged_balance_30d},
+      $invoice_data{aged_balance_60d},
+      $invoice_data{aged_balance_90d}
+    ) = @aged_balances;
+
+    if ($conf->exists('invoice_include_aging')) {
+      $previous_section->{posttotal} = sprintf(
+        '0 / 30 / 60 / 90 days overdue %.2f / %.2f / %.2f / %.2f',
+        @aged_balances,
+      );
+    }
 
   } else {
     # otherwise put them in the main section
@@ -1131,7 +1132,7 @@ sub print_generic {
 
   }
 
-  if ( @pr_cust_bill && $self->enable_previous ) {
+  if ( $pr_total && $self->enable_previous ) {
     push @buf, ['','-----------'];
     push @buf, [ $self->mt('Total Previous Balance'),
                  $money_char. sprintf("%10.2f", $pr_total) ];
@@ -1552,7 +1553,7 @@ sub print_generic {
                                                #  ? $self->charged +
                                                #    $self->billing_balance
                                                #  :
-                                                   $self->owed + $pr_total
+                                                   $balance_due
                                       )
           );
         if ( $multisection && !$adjust_section->{sort_weight} ) {
@@ -1598,7 +1599,7 @@ sub print_generic {
       $total->{'total_item'} = &$embolden_function($self->balance_due_msg);
       $total->{'total_amount'} =
         &$embolden_function(
-          $other_money_char. sprintf('%.2f', $self->owed + $pr_total)
+          $other_money_char. sprintf('%.2f', $balance_due)
         );
       my $last_section = pop @sections;
       $last_section->{'posttotal'} = $total->{'total_item'}. ' '.
@@ -1785,22 +1786,6 @@ sub template_conf { warn "bare FS::Template_Mixin::template_conf";
   'invoice_';
 }
 
-# helper routine for generating date ranges
-sub _prior_month30s {
-  my $self = shift;
-  my @ranges = (
-   [ 1,       2592000 ], # 0-30 days ago
-   [ 2592000, 5184000 ], # 30-60 days ago
-   [ 5184000, 7776000 ], # 60-90 days ago
-   [ 7776000, 0       ], # 90+   days ago
-  );
-
-  map { [ $_->[0] ? $self->_date - $_->[0] - 1 : '',
-          $_->[1] ? $self->_date - $_->[1] - 1 : '',
-      ] }
-  @ranges;
-}
-
 =item print_ps HASHREF | [ TIME [ , TEMPLATE ] ]
 
 Returns an postscript invoice, as a scalar.
diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm
index 92a0f9a92..d4383f91e 100644
--- a/FS/FS/cust_bill.pm
+++ b/FS/FS/cust_bill.pm
@@ -11,6 +11,7 @@ use Fcntl qw(:flock); #for spool_csv
 use Cwd;
 use List::Util qw(min max sum);
 use Date::Format;
+use DateTime;
 use File::Temp 0.14;
 use HTML::Entities;
 use Storable qw( freeze thaw );
@@ -449,6 +450,27 @@ sub previous_bill {
   $self->get('previous_bill');
 }
 
+=item following_bill
+
+Returns the customer's invoice that follows this one
+
+=cut
+
+sub following_bill {
+  my $self = shift;
+  if (!$self->get('following_bill')) {
+    $self->set('following_bill', qsearchs({
+      table   => 'cust_bill',
+      hashref => {
+        custnum => $self->custnum,
+        invnum  => { op => '>', value => $self->invnum },
+      },
+      order_by => 'ORDER BY invnum ASC LIMIT 1',
+    }));
+  }
+  $self->get('following_bill');
+}
+
 =item previous
 
 Returns a list consisting of the total previous balance for this customer, 
@@ -860,6 +882,35 @@ sub owed {
   $balance;
 }
 
+=item owed_on_invoice
+
+Returns the amount to be displayed as the "Balance Due" on this
+invoice.  Amount returned depends on conf flags for invoicing
+
+See L<FS::cust_bill::owed> for the true amount currently owed
+
+=cut
+
+sub owed_on_invoice {
+  my $self = shift;
+
+  #return $self->owed()
+  #  unless $self->conf->exists('previous_balance-payments_since')
+
+  # Add charges from this invoice
+  my $owed = $self->charged();
+
+  # Add carried balances from previous invoices
+  #   If previous items aren't to be displayed on the invoice,
+  #   _items_previous() is aware of this and responds appropriately.
+  $owed += $_->{amount} for $self->_items_previous();
+
+  # Subtract payments and credits displayed on this invoice
+  $owed -= $_->{amount} for $self->_items_payments(), $self->_items_credits();
+
+  return $owed;
+}
+
 sub owed_pkgnum {
   my( $self, $pkgnum ) = @_;
 
@@ -1650,6 +1701,9 @@ sub print_csv {
 
   my $time = $opt{'time'} || time;
 
+  $self->set('_template', $opt{template})
+    if exists $opt{template};
+
   my $tracctnum = ''; #leaking out from billco-specific sections :/
   if ( $format eq 'billco' ) {
 
@@ -2708,207 +2762,784 @@ sub _items_usage_class_summary {
   return @l;
 }
 
+=sub _items_previous()
+
+  Returns an array of hashrefs, each hashref representing a line-item on
+  the current bill for previous unpaid invoices.
+
+  keys for each previous_item:
+  - amount (see notes)
+  - pkgnum
+  - description
+  - invnum
+  - _date
+
+  Payments and credits shown on this invoice may vary based on configuraiton.
+
+  when conf flag previous_balance-payments_since is set:
+    This method works backwards to rebuild the invoice as a snapshot in time.
+    The invoice displayed will have the balances owed, and payments made,
+    reflecting the state of the account at the time of invoice generation.
+
+=cut
+
 sub _items_previous {
+
   my $self = shift;
-  my $conf = $self->conf;
-  my $cust_main = $self->cust_main;
-  my( $pr_total, @pr_cust_bill ) = $self->previous; #previous balance
-  my @b = ();
-  foreach ( @pr_cust_bill ) {
-    my $date = $conf->exists('invoice_show_prior_due_date')
-               ? 'due '. $_->due_date2str('short')
-               : $self->time2str_local('short', $_->_date);
-    push @b, {
-      'description' => $self->mt('Previous Balance, Invoice #'). $_->invnum. " ($date)",
-      #'pkgpart'     => 'N/A',
-      'pkgnum'      => 'N/A',
-      'amount'      => sprintf("%.2f", $_->owed),
-    };
+
+  # simple memoize
+  if ($self->get('_items_previous')) {
+    return sort { $a->{_date} <=> $b->{_date} }
+         values %{ $self->get('_items_previous') };
+  }
+
+  # Gets the customer's current balance and outstanding invoices.
+  my ($prev_balance, @open_invoices) = $self->previous;
+
+  my %invoices = map {
+    $_->invnum => $self->__items_previous_map_invoice($_)
+  } @open_invoices;
+
+  # Which credits and payments displayed on the bill will vary based on
+  # conf flag previous_balance-payments_since.
+  my @credits  = $self->_items_credits();
+  my @payments = $self->_items_payments();
+
+
+  if ($self->conf->exists('previous_balance-payments_since')) {
+    # For each credit or payment, determine which invoices it was applied to.
+    # Manipulate data displayed so the invoice displayed appears as a
+    # snapshot in time... with previous balances and balance owed displayed
+    # as they were at the time of invoice creation.
+
+    my @credits_postbill = $self->_items_credits_postbill();
+    my @payments_postbill = $self->_items_payments_postbill();
+
+    my %pmnt_dupechk;
+    my %cred_dupechk;
+
+    # Each section below follows this pattern on a payment/credit
+    #
+    # - Dupe check, avoid adjusting for the same item twice
+    # - If invoice being adjusted for isn't in our list, add it
+    # - Adjust the invoice balance to refelct balnace without the
+    #   credit or payment applied
+    #
+
+    # Working with payments displayed on this bill
+    for my $pmt_hash (@payments) {
+      my $pmt_obj = qsearchs('cust_pay', {paynum => $pmt_hash->{paynum}});
+      for my $cust_bill_pay ($pmt_obj->cust_bill_pay) {
+        next if exists $pmnt_dupechk{$cust_bill_pay->billpaynum};
+        $pmnt_dupechk{$cust_bill_pay->billpaynum} = 1;
+
+        my $invnum = $cust_bill_pay->invnum;
+
+        $invoices{$invnum} = $self->__items_previous_get_invoice($invnum)
+          unless exists $invoices{$invnum};
+
+        $invoices{$invnum}->{amount} += $cust_bill_pay->amount;
+      }
+    }
+
+    # Working with credits displayed on this bill
+    for my $cred_hash (@credits) {
+      my $cred_obj = qsearchs('cust_credit', {crednum => $cred_hash->{crednum}});
+      for my $cust_credit_bill ($cred_obj->cust_credit_bill) {
+        next if exists $cred_dupechk{$cust_credit_bill->creditbillnum};
+        $cred_dupechk{$cust_credit_bill->creditbillnum} = 1;
+
+        my $invnum = $cust_credit_bill->invnum;
+
+        $invoices{$invnum} = $self->__items_previous_get_invoice($invnum)
+          unless exists $invoices{$invnum};
+
+        $invoices{$invnum}->{amount} += $cust_credit_bill->amount;
+      }
+    }
+
+    # Working with both credits and payments which are not displayed
+    # on this bill, but which have affected this bill's balances
+    for my $postbill (@payments_postbill, @credits_postbill) {
+
+      if ($postbill->{billpaynum}) {
+        next if exists $pmnt_dupechk{$postbill->{billpaynum}};
+        $pmnt_dupechk{$postbill->{billpaynum}} = 1;
+      } elsif ($postbill->{creditbillnum}) {
+        next if exists $cred_dupechk{$postbill->{creditbillnum}};
+        $cred_dupechk{$postbill->{creditbillnum}} = 1;
+      } else {
+        die "Missing creditbillnum or billpaynum";
+      }
+
+      my $invnum = $postbill->{invnum};
+
+      $invoices{$invnum} = $self->__items_previous_get_invoice($invnum)
+        unless exists $invoices{$invnum};
+
+      $invoices{$invnum}->{amount} += $postbill->{amount};
+    }
+
+    # Make sure current invoice doesn't appear in previous items
+    delete $invoices{$self->invnum}
+      if exists $invoices{$self->invnum};
+
   }
-  @b;
 
-  #{
-  #    'description'     => 'Previous Balance',
-  #    #'pkgpart'         => 'N/A',
-  #    'pkgnum'          => 'N/A',
-  #    'amount'          => sprintf("%10.2f", $pr_total ),
-  #    'ext_description' => [ map {
-  #                                 "Invoice ". $_->invnum.
-  #                                 " (". time2str("%x",$_->_date). ") ".
-  #                                 sprintf("%10.2f", $_->owed)
-  #                         } @pr_cust_bill ],
+  # Make sure amount is formatted as a dollar string
+  # (Formatting should happen on the template side, but is not?)
+  $invoices{$_}->{amount} = sprintf('%.2f',$invoices{$_}->{amount})
+    for keys %invoices;
+
+  $self->set('_items_previous', \%invoices);
+  return sort { $a->{_date} <=> $b->{_date} } values %invoices;
 
-  #};
 }
 
+=sub _items_previous_total
+
+  Return sum of amounts from all items returned by _items_previous
+  Results will vary based on invoicing conf flags
+
+=cut
+
+sub _items_previous_total {
+  my $self = shift;
+  my $tot = 0;
+  $tot += $_->{amount} for $self->_items_previous();
+  return $tot;
+}
+
+sub __items_previous_get_invoice {
+  # Helper function for _items_previous
+  #
+  # Read a record from cust_bill, return a hash of it's information
+  my ($self, $invnum) = @_;
+  die "Incorrect usage of __items_previous_get_invoice()" unless $invnum;
+
+  my $cust_bill = qsearchs('cust_bill', {invnum => $invnum});
+  return $self->__items_previous_map_invoice($cust_bill);
+}
+
+sub __items_previous_map_invoice {
+  # Helper function for _items_previous
+  #
+  # Transform a cust_bill object into a simple hash reference of the type
+  # required by _items_previous
+  my ($self, $cust_bill) = @_;
+  die "Incorrect usage of __items_previous_map_invoice" unless ref $cust_bill;
+
+  my $date = $self->conf->exists('invoice_show_prior_due_date')
+           ? 'due '.$cust_bill->due_date2str('short')
+           : $self->time2str_local('short', $cust_bill->_date);
+
+  return {
+    invnum => $cust_bill->invnum,
+    amount => $cust_bill->owed,
+    pkgnum => 'N/A',
+    _date  => $cust_bill->_date,
+    description => join(' ',
+      $self->mt('Previous Balance, Invoice #'),
+      $cust_bill->invnum,
+      "($date)"
+    ),
+  }
+}
+
+=sub _items_credits()
+
+  Return array of hashrefs containing credits to be shown as line-items
+  when rendering this bill.
+
+  keys for each credit item:
+  - crednum: id of payment
+  - amount: payment amount
+  - description: line item to be displayed on the bill
+
+  This method has three ways it selects which credits to display on
+  this bill:
+
+  1) Default Case: No Conf flag for 'previous_balance-payments_since'
+
+     Returns credits that have been applied to this bill only
+
+  2) Case:
+       Conf flag set for 'previous_balance-payments_since'
+
+     List all credits that have been recorded during the time period
+     between the timestamps of the last invoice and this invoice
+
+  3) Case:
+        Conf flag set for 'previous_balance-payments_since'
+        $opt{'template'} eq 'statement'
+
+    List all payments that have been recorded between the timestamps
+    of the previous invoice and the following invoice.
+
+     This is used to give the customer a receipt for a payment
+     in the form of their last bill with the payment amended.
+
+     I am concerned with this implementation, but leaving in place as is
+     If this option is selected, while viewing an older bill, the old bill
+     will show ALL future credits for future bills, but no charges for
+     future bills.  Somebody could be misled into believing they have a
+     large account credit when they don't.  Also, interrupts the chain of
+     invoices as an account history... the customer could have two invoices
+     in their fileing cabinet, for two different dates, both with a line item
+     for the same duplicate credit.  The accounting is technically accurate,
+     but somebody could easily become confused and think two credits were
+     made, when really those two line items on two different bills represent
+     only a single credit
+
+=cut
+
 sub _items_credits {
-  my( $self, %opt ) = @_;
-  my $trim_len = $opt{'trim_len'} || 40;
 
-  my @b;
-  #credits
-  my @objects;
+  my $self= shift;
+
+  # Simple memoize
+  return @{$self->get('_items_credits')} if $self->get('_items_credits');
+
+  my %opt = @_;
+  my $template = $opt{template} || $self->get('_template');
+  my $trim_len = $opt{template} || $self->get('trim_len') || 40;
+
+  my @return;
+  my @cust_credit_objs;
+
   if ( $self->conf->exists('previous_balance-payments_since') ) {
-    if ( $opt{'template'} eq 'statement' ) {
-      # then the current bill is a "statement" (i.e. an invoice sent as
-      # a payment receipt)
-      # and in that case we want to see payments on or after THIS invoice
-      @objects = qsearch('cust_credit', {
-          'custnum' => $self->custnum,
-          '_date'   => {op => '>=', value => $self->_date},
-      });
+    if ($template eq 'statement') {
+      # Case 3 (see above)
+      # Return credits timestamped between the previous and following bills
+
+      my $previous_bill  = $self->previous_bill;
+      my $following_bill = $self->following_bill;
+
+      my $date_start = ref $previous_bill  ? $previous_bill->_date  : 0;
+      my $date_end   = ref $following_bill ? $following_bill->_date : undef;
+
+      my %query = (
+        table => 'cust_credit',
+        hashref => {
+          custnum => $self->custnum,
+          _date => { op => '>=', value => $date_start },
+        },
+      );
+      $query{extra_sql} = " AND _date <= $date_end " if $date_end;
+
+      @cust_credit_objs = qsearch(\%query);
+
     } else {
-      my $date = 0;
-      $date = $self->previous_bill->_date if $self->previous_bill;
-      @objects = qsearch('cust_credit', {
-          'custnum' => $self->custnum,
-          '_date'   => {op => '>=', value => $date},
+      # Case 2 (see above)
+      # Return credits timestamps between this and the previous bills
+
+      my $date_start = 0;
+      my $date_end = $self->_date;
+
+      my $previous_bill = $self->previous_bill;
+      if (ref $previous_bill) {
+        $date_start = $previous_bill->_date;
+      }
+
+      @cust_credit_objs = qsearch({
+        table => 'cust_credit',
+        hashref => {
+          custnum => $self->custnum,
+          _date => {op => '>=', value => $date_start},
+        },
+        extra_sql => " AND _date <= $date_end ",
       });
     }
+
   } else {
-    @objects = $self->cust_credited;
+    # Case 1 (see above)
+    # Return only credits that have been applied to this bill
+
+    @cust_credit_objs = $self->cust_credited;
+
   }
 
-  foreach my $obj ( @objects ) {
+  # Translate objects into hashrefs
+  foreach my $obj ( @cust_credit_objs ) {
     my $cust_credit = $obj->isa('FS::cust_credit') ? $obj : $obj->cust_credit;
+    my %r_obj = (
+      amount       => sprintf('%.2f',$cust_credit->amount),
+      crednum      => $cust_credit->crednum,
+      _date        => $cust_credit->_date,
+      creditreason => $cust_credit->reason,
+    );
 
     my $reason = substr($cust_credit->reason, 0, $trim_len);
     $reason .= '...' if length($reason) < length($cust_credit->reason);
     $reason = " ($reason) " if $reason;
 
-    push @b, {
-      #'description' => 'Credit ref\#'. $_->crednum.
-      #                 " (". time2str("%x",$_->cust_credit->_date) .")".
-      #                 $reason,
-      'description' => $self->mt('Credit applied').' '.
-                       $self->time2str_local('short', $obj->_date). $reason,
-      'amount'      => sprintf("%.2f",$obj->amount),
-    };
+    $r_obj{description} = join(' ',
+      $self->mt('Credit applied'),
+      $self->time2str_local('short', $cust_credit->_date),
+      $reason,
+    );
+
+    push @return, \%r_obj;
+  }
+  $self->set('_items_credits',\@return);
+  @return;
   }
 
-  @b;
+=sub _items_credits_total
+
+  Return the total of al items from _items_credits
+  Will vary based on invoice display conf flag
+
+=cut
 
+sub _items_credits_total {
+  my $self = shift;
+  my $tot = 0;
+  $tot += $_->{amount} for $self->_items_credits();
+  return $tot;
 }
 
+
+
+=sub _items_credits_postbill()
+
+  Returns an array of hashrefs for credits where
+  - Credit issued after this invoice
+  - Credit applied to an invoice before this invoice
+
+  Returned hashrefs are of the format returned by _items_credits()
+
+=cut
+
+sub _items_credits_postbill {
+  my $self = shift;
+
+  my @cust_credit_bill = qsearch({
+    table   => 'cust_credit_bill',
+    select  => join(', ',qw(
+      cust_credit_bill.creditbillnum
+      cust_credit_bill._date
+      cust_credit_bill.invnum
+      cust_credit_bill.amount
+    )),
+    addl_from => ' LEFT JOIN cust_credit'.
+                 ' ON (cust_credit_bill.crednum = cust_credit.crednum) ',
+    extra_sql => ' WHERE cust_credit.custnum     = '.$self->custnum.
+                 ' AND   cust_credit_bill._date  > '.$self->_date.
+                 ' AND   cust_credit_bill.invnum < '.$self->invnum.' ',
+#! did not investigate why hashref doesn't work for this join query
+#    hashref => {
+#      'cust_credit.custnum'     => {op => '=', value => $self->custnum},
+#      'cust_credit_bill._date'  => {op => '>', value => $self->_date},
+#      'cust_credit_bill.invnum' => {op => '<', value => $self->invnum},
+#    },
+  });
+
+  return map {{
+    _date         => $_->_date,
+    invnum        => $_->invnum,
+    amount        => $_->amount,
+    creditbillnum => $_->creditbillnum,
+  }} @cust_credit_bill;
+}
+
+=sub _items_payments_postbill()
+
+  Returns an array of hashrefs for payments where
+  - Payment occured after this invoice
+  - Payment applied to an invoice before this invoice
+
+  Returned hashrefs are of the format returned by _items_payments()
+
+=cut
+
+sub _items_payments_postbill {
+  my $self = shift;
+
+  my @cust_bill_pay = qsearch({
+    table    => 'cust_bill_pay',
+    select => join(', ',qw(
+      cust_bill_pay.billpaynum
+      cust_bill_pay._date
+      cust_bill_pay.invnum
+      cust_bill_pay.amount
+    )),
+    addl_from => ' LEFT JOIN cust_bill'.
+                 ' ON (cust_bill_pay.invnum = cust_bill.invnum) ',
+    extra_sql => ' WHERE cust_bill.custnum     = '.$self->custnum.
+                 ' AND   cust_bill_pay._date   > '.$self->_date.
+                 ' AND   cust_bill_pay.invnum  < '.$self->invnum.' ',
+  });
+
+  return map {{
+    _date      => $_->_date,
+    invnum     => $_->invnum,
+    amount     => $_->amount,
+    billpaynum => $_->billpaynum,
+  }} @cust_bill_pay;
+}
+
+=sub _items_payments()
+
+  Return array of hashrefs containing payments to be shown as line-items
+  when rendering this bill.
+
+  keys for each payment item:
+  - paynum: id of payment
+  - amount: payment amount
+  - description: line item to be displayed on the bill
+
+  This method has three ways it selects which payments to display on
+  this bill:
+
+  1) Default Case: No Conf flag for 'previous_balance-payments_since'
+
+     Returns payments that have been applied to this bill only
+
+  2) Case:
+       Conf flag set for 'previous_balance-payments_since'
+
+     List all payments that have been recorded between the timestamps
+     of the previous invoice and this invoice
+
+  3) Case:
+        Conf flag set for 'previous_balance-payments_since'
+        $opt{'template'} eq 'statement'
+
+     List all payments that have been recorded between the timestamps
+     of the previous invoice and the following invoice.
+
+     I am concerned with this implementation, but leaving in place as is
+     If this option is selected, while viewing an older bill, the old bill
+     will show ALL future payments for future bills, but no charges for
+     future bills.  Somebody could be misled into believing they have a
+     large account credit when they don't.  Also, interrupts the chain of
+     invoices as an account history... the customer could have two invoices
+     in their fileing cabinet, for two different dates, both with a line item
+     for the same duplicate payment.  The accounting is technically accurate,
+     but somebody could easily become confused and think two payments were
+     made, when really those two line items on two different bills represent
+     only a single payment.
+
+=cut
+
 sub _items_payments {
+
   my $self = shift;
+
+  # Simple memoize
+  return @{$self->get('_items_payments')} if $self->get('_items_payments');
+
   my %opt = @_;
+  my $template = $opt{template} || $self->get('_template');
+
+  my @return;
+  my @cust_pay_objs;
+
+  my $c_invoice_payment_details = $self->conf->exists('invoice_payment_details');
 
-  my @b;
-  my $detailed = $self->conf->exists('invoice_payment_details');
-  my @objects;
   if ( $self->conf->exists('previous_balance-payments_since') ) {
-    # then show payments dated on/after the previous bill...
-    if ( $opt{'template'} eq 'statement' ) {
-      # then the current bill is a "statement" (i.e. an invoice sent as
-      # a payment receipt)
-      # and in that case we want to see payments on or after THIS invoice
-      @objects = qsearch('cust_pay', {
-          'custnum' => $self->custnum,
-          '_date'   => {op => '>=', value => $self->_date},
-      });
+    if ($template eq 'statement') {
+print "\nCASE 3\n";
+      # Case 3 (see above)
+      # Return payments timestamped between the previous and following bills
+
+      my $previous_bill  = $self->previous_bill;
+      my $following_bill = $self->following_bill;
+
+      my $date_start = ref $previous_bill  ? $previous_bill->_date  : 0;
+      my $date_end   = ref $following_bill ? $following_bill->_date : undef;
+
+      my %query = (
+        table => 'cust_pay',
+        hashref => {
+          custnum => $self->custnum,
+          _date => { op => '>=', value => $date_start },
+        },
+      );
+      $query{extra_sql} = " AND _date <= $date_end " if $date_end;
+
+      @cust_pay_objs = qsearch(\%query);
+
     } else {
-      # the normal case: payments on or after the previous invoice
-      my $date = 0;
-      $date = $self->previous_bill->_date if $self->previous_bill;
-      @objects = qsearch('cust_pay', {
-        'custnum' => $self->custnum,
-        '_date'   => {op => '>=', value => $date},
+      # Case 2 (see above)
+      # Return payments timestamped between this and the previous bill
+print "\nCASE 2\n";
+      my $date_start = 0;
+      my $date_end = $self->_date;
+
+      my $previous_bill = $self->previous_bill;
+      if (ref $previous_bill) {
+        $date_start = $previous_bill->_date;
+      }
+
+      @cust_pay_objs = qsearch({
+        table => 'cust_pay',
+        hashref => {
+          custnum => $self->custnum,
+          _date => {op => '>=', value => $date_start},
+        },
+        extra_sql => " AND _date <= $date_end ",
       });
-      # and before the current bill...
-      @objects = grep { $_->_date < $self->_date } @objects;
     }
+
   } else {
-    @objects = $self->cust_bill_pay;
+    # Case 1 (see above)
+    # Return payments applied only to this bill
+
+    @cust_pay_objs = $self->cust_bill_pay;
+
   }
 
-  foreach my $obj (@objects) {
-    my $cust_pay = $obj->isa('FS::cust_pay') ? $obj : $obj->cust_pay;
-    my $desc = $self->mt('Payment received').' '.
-               $self->time2str_local('short', $cust_pay->_date );
-    $desc .= $self->mt(' via ') .
-             $cust_pay->payby_payinfo_pretty( $self->cust_main->locale )
-      if $detailed;
-
-    push @b, {
-      'description' => $desc,
-      'amount'      => sprintf("%.2f", $obj->amount )
-    };
+  $self->set(
+    '_items_payments',
+    [ $self->__items_payments_make_hashref(@cust_pay_objs) ]
+  );
+  return @{ $self->get('_items_payments') };
   }
 
-  @b;
+=sub _items_payments_total
 
-}
+  Return a total of all records returned by _items_payments
+  Results vary based on invoicing conf flags
 
-sub _items_total {
+=cut
+
+sub _items_payments_total {
   my $self = shift;
-  my $conf = $self->conf;
+  my $tot = 0;
+  $tot += $_->{amount} for $self->_items_payments();
+  return $tot;
+}
+
+sub __items_payments_make_hashref {
+  # Transform a FS::cust_pay object into a simple hashref for invoice
+  my ($self, @cust_pay_objs) = @_;
+  my $c_invoice_payment_details = $self->conf->exists('invoice_payment_details');
+  my @return;
+
+  for my $obj (@cust_pay_objs) {
+
+    # In case we're passed FS::cust_bill_pay (or something else?)
+    # Below, we use $obj to render amount rather than $cust_apy.
+    #   If we were passed cust_bill_pay objs, then:
+    #   $obj->amount represents the amount applied to THIS invoice
+    #   $cust_pay->amount represents the total payment, which may have
+    #       been applied accross several invoices.
+    # If we were passed cust_bill_pay objects, then the conf flag
+    # previous_balance-payments_since is NOT set, so we should not
+    # present any payments not applied to this invoice.
+    my $cust_pay = $obj->isa('FS::cust_pay') ? $obj : $obj->cust_pay;
 
-  my @items;
-  my ($pr_total) = $self->previous;
-  my ($previous_charges_desc, $new_charges_desc, $new_charges_amount);
+    my %r_obj = (
+      _date   => $cust_pay->_date,
+      amount  => sprintf("%.2f", $obj->amount),
+      paynum  => $cust_pay->paynum,
+      payinfo => $cust_pay->payby_payinfo_pretty(),
+      description => join(' ',
+        $self->mt('Payment received'),
+        $self->time2str_local('short', $cust_pay->_date),
+      ),
+    );
 
-  if ( $conf->exists('previous_balance-exclude_from_total') ) {
-    # if enabled, specifically add a line for the previous balance total
-    $previous_charges_desc = $self->mt(
-      $conf->config('previous_balance-text') || 'Previous Balance'
+    if ($c_invoice_payment_details) {
+      $r_obj{description} = join(' ',
+        $r_obj{description},
+        $self->mt('via'),
+        $cust_pay->payby_payinfo_pretty($self->cust_main->locale),
     );
+    }
 
-    # then return separate lines for previous balance and total new charges
-    if ( $pr_total ) {
-      push @items,
-        { total_item    => $previous_charges_desc,
-          total_amount  => sprintf('%.2f',$pr_total)
-        };
+    push @return, \%r_obj;
     }
+  return @return;
   }
 
-  if (   $conf->exists('previous_balance-exclude_from_total')
-      or !$self->enable_previous ) {
-    # show new charges only
+=sub _items_total()
+
+  Generate the line-items to be shown on the bill in the "Totals" section
+
+  Returns a list of hashrefs, each with the keys:
+  - total_item: description field
+  - total_amount: dollar-formatted number amount
+
+  Information presented by this method varies based on Conf
+
+  Conf previous_balance-payments_due
+  - default, flag not set
+      Only transactions that were applied to this bill bill be
+      displayed and calculated intothe total.  If items exist in
+      the past-due section, those items will disappear from this
+      invoice if they have been paid off.
+
+  - previous_balance-payments_due flag is set
+      Transactions occuring after the timestsamp of this
+      invoice are not reflected on invoice line items
+
+      Only payments/credits applied between the previous invoice
+      and this one are displayed and calculated into the total
+
+  - previous_balance-payments_due && $opt{template} eq 'statement'
+      Same as above, except payments/credits occuring before the date
+      of the following invoice are also displayed and calculated into
+      the total
+
+  Conf previous_balance-exclude_from_total
+  - default, flag not set
+      The "Totals" section contains a single line item.
+      The dollar amount of this line items is a sum of old and new charges
+  - previous_balance-exclude_from_total flag is set
+      The "Totals" section contains two line items.
+      One for previous balance, one for new charges
+  !NOTE: Avent virtualization flag 'disable_previous_balance' can
+      override the global conf flag previous_balance-exclude_from_total
+
+  Conf invoice_show_prior_due_date
+  - default, flag not set
+    Total line item in the "Totals" section does not mention due date
+  - invoice_show_prior_due_date flag is set
+    Total line item in the "Totals" section includes either the due
+    date of the invoice, or the specified invoice terms
+    ? Not sure why this is called "Prior" due date, since we seem to be
+      displaying THIS due date...
+=cut
+
+sub _items_total {
+  my $self = shift;
+  my $conf = $self->conf;
+
+  my $c_multi_line_total = 0;
+  $c_multi_line_total    = 1
+    if $conf->exists('previous_balance-exclude_from_total')
+    && $self->enable_previous();
+
+  my @line_items;
+  my $invoice_charges  = $self->charged();
 
-    $new_charges_desc = $self->mt(
+  # _items_previous() is aware of conf flags
+  my $previous_balance = 0;
+  $previous_balance += $_->{amount} for $self->_items_previous();
+
+  my $total_charges;
+  my $total_descr;
+
+  if ( $previous_balance && $c_multi_line_total ) {
+    # previous balance, new charges on separate lines
+
+    push @line_items, {
+      total_amount => sprintf('%.2f',$previous_balance),
+      total_item   => $self->mt(
+        $conf->config('previous_balance-text') || 'Previous Balance'
+      ),
+    };
+
+    $total_charges = $invoice_charges;
+    $total_descr   = $self->mt(
       $conf->config('previous_balance-text-total_new_charges')
        || 'Total New Charges'
     );
 
-    $new_charges_amount = $self->charged;
-
-  } else {
-    # show new charges + previous invoice total
-
-    $new_charges_desc = $self->mt('Total Charges');
-    if ( $self->enable_previous ) {
-      $new_charges_amount = sprintf('%.2f', $self->charged + $pr_total);
     } else {
-      $new_charges_amount = sprintf('%.2f', $self->charged);
-    }
-
+    # previous balance and new charges combined into a single total line
+    $total_charges = $invoice_charges + $previous_balance;
+    $total_descr = $self->mt('Total Charges');
   }
 
   if ( $conf->exists('invoice_show_prior_due_date') ) {
     # then the due date should be shown with Total New Charges,
     # and should NOT be shown with the Balance Due message.
+
     if ( $self->due_date ) {
-      # localize the "Please pay by" message and the date itself
-      # (grammar issues with this, yeah)
-      $new_charges_desc .= ' - ' . $self->mt('Please pay by') . ' ' .
-                           $self->due_date2str('short');
+      $total_descr = join(' ',
+        $total_descr,
+        '-',
+        $self->mt('Please pay by'),
+        $self->due_date2str('short')
+      );
     } elsif ( $self->terms ) {
-      # phrases like "due on receipt" should be localized
-      $new_charges_desc .= ' - ' . $self->mt($self->terms);
+      $total_descr = join(' ',
+        $total_descr,
+        '-',
+        $self->mt($self->terms)
+      );
     }
   }
 
-  push @items,
-    { total_item    => $new_charges_desc,
-      total_amount  => $new_charges_amount,
+  push @line_items, {
+    total_amount => sprintf('%.2f', $total_charges),
+    total_item   => $total_descr,
     };
 
-  @items;
+  return @line_items;
 }
 
+=item _items_aging_balances
+
+  Returns an array of aged balance amounts from a given epoch timestamp.
+
+  The time of day is ignored for this calculation, so that slight differences
+  on the generation time of an invoice doesn't determine which column an
+  aged balance falls into.
+
+  Will not include any balances dated after the given timestamp in
+  the calculated totals
 
+  usage:
+  @aged_balances = $b->_items_aging_balances( $b->_date )
+
+  @aged_balances = (
+    under30d,
+    30d-60d,
+    60d-90d,
+    over90d
+  )
+
+=cut
+
+sub _items_aging_balances {
+  my ($self, $basetime) = @_;
+  die "Incorrect usage of _items_aging_balances()" unless ref $self;
+
+  $basetime = $self->_date unless $basetime;
+  my @aging_balances = (0, 0, 0, 0);
+  my @open_invoices = $self->_items_previous();
+  my $d30 = 2592000; # 60 * 60 * 24 * 30,
+  my $d60 = 5184000; # 60 * 60 * 24 * 60,
+  my $d90 = 7776000; # 60 * 60 * 24 * 90
+
+  # Move the clock back on our given day to 12:00:01 AM
+  my $dt_basetime = DateTime->from_epoch(epoch => $basetime);
+  my $dt_12am = DateTime->new(
+    year   => $dt_basetime->year,
+    month  => $dt_basetime->month,
+    day    => $dt_basetime->day,
+    hour   => 0,
+    minute => 0,
+    second => 1,
+  )->epoch();
+
+  # set our epoch breakpoints
+  $_ = $dt_12am - $_ for $d30, $d60, $d90;
+
+  # grep the aged balances
+  for my $oinv (@open_invoices) {
+    if ($oinv->{_date} <= $basetime && $oinv->{_date} > $d30) {
+      # If post invoice dated less than 30days ago
+      $aging_balances[0] += $oinv->{amount};
+    } elsif ($oinv->{_date} <= $d30 && $oinv->{_date} > $d60) {
+      # If past invoice dated between 30-60 days ago
+      $aging_balances[1] += $oinv->{amount};
+    } elsif ($oinv->{_date} <= $d60 && $oinv->{_date} > $d90) {
+      # If past invoice dated between 60-90 days ago
+      $aging_balances[2] += $oinv->{amount};
+    } else {
+      # If past invoice dated 90+ days ago
+      $aging_balances[3] += $oinv->{amount};
+    }
+  }
+
+  return map{ sprintf('%.2f',$_) } @aging_balances;
+}
 
 =item has_call_details
 
diff --git a/conf/invoice_htmlsummary b/conf/invoice_htmlsummary
index 47bdbfb7c..249db9b07 100644
--- a/conf/invoice_htmlsummary
+++ b/conf/invoice_htmlsummary
@@ -9,29 +9,20 @@
       <table class="invoice_summary">
         <tr><th colspan=2><br></th></tr>
         <tr>
-          <td><b><u><br>Summary of Previous Balance and Payments<br></u></b></td>
+          <td><b><u><br>Summary of Previous Balance<br></u></b></td>
           <td></td>
         </tr>
         <tr>
           <td><b>Previous Balance</b></td>
-          <td align="right"><b><%= $dollar.$true_previous_balance %></b></td>
-        </tr>
-        <tr>
-          <td><b>Payments</b></td>
-          <th align="right"><b><%= $dollar.$balance_adjustments %></b></th>
-        </tr>
-        <tr>
-          <td><b>Balance Outstanding</b></td>
-          <td align="right"><b><%= $dollar.sprintf('%.2f', $true_previous_balance - $balance_adjustments) %></b></td>
+          <td align=right><b><%= "${dollar}${true_previous_balance}" %></b></td>
         </tr>
         <tr><th colspan=2><br></th></tr>
         <tr><td colspan=2><br></td></tr>
         <tr>
           <td><b><u>Summary of New Charges</u></b></td>
-          <td></td>
         </tr>
         <tr><td colspan=2><br></td></tr>
-        <%= 
+        <%=
           my $last = $summary_subtotals[-1];
           foreach my $section (@summary_subtotals) {
             $OUT .= '<tr><td><b>'. ($section->{'description'} ? $section->{'description'} : 'Charges' ). '</b></td>';
@@ -44,15 +35,23 @@
           <td align="right"><b><%= $dollar.$current_less_finance %></b></td>
         </tr>
         <tr><th colspan=2><br></th></tr>
+        <tr>
+          <td><b><u><br>Summary of Payments and Credits<br></u></b></td>
+          <td></td>
+        </tr>
+        <tr>
+          <td><b>Payments and Credits</b></td>
+          <td align="right"><b>-<%= $dollar.$balance_adjustments %></b></td>
+        </tr>
+        <tr><th colspan=2><br></th></tr>
         <tr><td colspan=2><br></td></tr>
         <tr>
           <td><b><u>Invoice Summary</u></b></td>
-          <td></td>
         </tr>
         <tr><td colspan=2><br></td></tr>
         <tr>
           <td><b>Previous Past Due Charges</b></td>
-          <td align="right"><b><%= $dollar.sprintf('%.2f', $true_previous_balance - $balance_adjustments) %></b></td>
+          <td align="right"><b><%= $dollar.$true_previous_balance %></b></td>
         </tr>
         <tr>
           <td><b>Finance charges on overdue amount</b></td>
@@ -60,18 +59,18 @@
         </tr>
         <tr>
           <td><b>New Charges</b></td>
-          <th align="right"><b><%= $dollar.$current_less_finance %></b></th>
+          <td align="right"><b><%= $dollar.$current_less_finance %></b></td>
         </tr>
-
-        <%= 
-          
-          #false laziness w/invoice_latexsummary and above
+        <%=
           foreach my $section ( grep $_->{adjust_section}, @sections) {
             $OUT .= '<tr><td><b>'. ($section->{'description'} ? $section->{'description'} : 'Charges' ). '</b></td>';
             $OUT .= qq(<th align="right"><b>). $section->{'subtotal'}. "</b></th></tr>";
           }
         %>
-
+        <tr>
+          <td><b>Payments and Credits</b></td>
+          <th align="right"><b>-<%= $dollar.sprintf('%.2f', $balance_adjustments) %></b></th>
+        </tr>
         <tr>
           <td><b>Total Amount Due</b></td>
           <td align="right"><b><%= $dollar.sprintf('%.2f', $balance) %></b></td>

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

Summary of changes:
 FS/FS/Template_Mixin.pm   | 355 +++++++++---------
 FS/FS/cust_bill.pm        | 892 +++++++++++++++++++++++++++++++++++++++-------
 FS/FS/cust_bill_pkg.pm    |  26 +-
 conf/invoice_htmlsummary  |  38 +-
 conf/invoice_latexsummary |  17 +-
 5 files changed, 992 insertions(+), 336 deletions(-)




More information about the freeside-commits mailing list