[freeside-commits] branch master updated. c38b6699d83ba0d7e3fd582373b9c8e78c9217d2

Mitch Jackson mitch at freeside.biz
Sun Mar 25 16:59:07 PDT 2018


The branch, master has been updated
       via  c38b6699d83ba0d7e3fd582373b9c8e78c9217d2 (commit)
      from  b21f69637bdfe8a83bc8f8f1be8a701a332d714f (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 c38b6699d83ba0d7e3fd582373b9c8e78c9217d2
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sun Mar 25 18:55:17 2018 -0500

    RT# 79636 Taxes per section when using invoice_sections

diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm
index 9b891879b..721b7bdf9 100644
--- a/FS/FS/Conf.pm
+++ b/FS/FS/Conf.pm
@@ -1599,6 +1599,13 @@ and customer address. Include units.',
   },
 
   {
+    'key'         => 'invoice_sections_with_taxes',
+    'section'     => 'invoicing',
+    'description' => 'Include taxes within each section of mutli-section invoices.',
+    'type'        => 'checkbox',
+  },
+
+  {
     'key'         => 'summary_subtotals_method',
     'section'     => 'invoicing',
     'description' => 'How to group line items when calculating summary subtotals.  By default, it will be the same method used for grouping invoice sections.',
@@ -5961,4 +5968,3 @@ and customer address. Include units.',
 );
 
 1;
-
diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index 88fd4e87f..a1ed4b0c0 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -1196,6 +1196,8 @@ sub print_generic {
 
     my %options = ();
     $options{'section'} = $section if $multisection;
+    $options{'section_with_taxes'} = 1
+      if $conf->exists('invoice_sections_with_taxes');
     $options{'format'} = $format;
     $options{'escape_function'} = $escape_function;
     $options{'no_usage'} = 1 unless $unsquelched;
@@ -1208,6 +1210,8 @@ sub print_generic {
     warn "$me   searching for line items\n"
       if $DEBUG > 1;
 
+    my %section_tax_lines;
+    my %seen_tax_lines;
     foreach my $line_item ( $self->_items_pkg(%options),
                             $self->_items_fee(%options) ) {
 
@@ -1232,9 +1236,56 @@ sub print_generic {
       }
       $line_item->{'ext_description'} ||= [];
 
+      if ( $options{section_with_taxes} && ref $line_item->{pkg_tax} ) {
+        for my $line_tax ( @{$ line_item->{pkg_tax} } ) {
+
+          # It is rarely possible for the same tax record to be presented here
+          # multiple times.  See cust_bill_pkg::_pkg_tax_list for more info
+          next if $seen_tax_lines{ $line_tax->{billpkgtaxlocationnum} };
+          $seen_tax_lines{ $line_tax->{billpkgtaxlocationnum} } = 1;
+
+          $section_tax_lines{ $line_tax->{taxname} } += $line_tax->{amount};
+        }
+      }
+
       push @detail_items, $line_item;
     }
 
+    # If conf flag invoice_sections_with_taxes:
+    # - Add @detail_items for taxes into each section
+    # - Update section subtotal to include taxes
+    if ( $options{section_with_taxes} && %section_tax_lines ) {
+      for my $taxname ( keys %section_tax_lines ) {
+
+        push @detail_items, {
+          section => $section,
+          amount  => sprintf($money_char."%.2f",$section_tax_lines{$taxname}),
+          description => &$escape_function($taxname),
+        };
+
+        # Append taxes to total.  If line format resembles "$5.00 to $12.00"
+        # append to the second value.
+
+        # $section->{subtotal} = '$5.00 to 12.00'; # for testing:
+        if ($section->{subtotal} =~ /to/) {
+          my @subtotal = split /\s/, $section->{subtotal};
+          $subtotal[2] =~ s/[^\d\.]//g;
+          $subtotal[2] = sprintf(
+            $money_char."%.2f",
+            ( $subtotal[2] + $section_tax_lines{$taxname} )
+          );
+          $section->{subtotal} = join ' ', @subtotal;
+        } else {
+          $section->{subtotal} =~ s/[^\d\.]//g;
+          $section->{subtotal} = sprintf(
+            $money_char . "%.2f",
+            ( $section->{subtotal} + $section_tax_lines{$taxname} )
+          );
+        }
+
+      }
+    }
+
     if ( $section->{'description'} ) {
       push @buf, ( ['','-----------'],
                    [ $section->{'description'}. ' sub-total',
@@ -1335,13 +1386,20 @@ sub print_generic {
         $tax_section->{'description'} = $self->mt($tax_description);
         $tax_section->{'summarized'} = '';
 
-        # append it if it's not already there
-        if ( !grep $tax_section, @sections ) {
+        if ( $conf->exists('invoice_sections_with_taxes')) {
+
+          # remove tax section if taxes are itemized within other sections
+          @sections = grep{ $_ ne $tax_section } @sections;
+
+        } elsif ( !grep $tax_section, @sections ) {
+
+          # append it if it's not already there
           push @sections, $tax_section;
           push @summary_subtotals, $tax_section;
+
         }
-      }
 
+      }
     } else {
       unshift @total_items, $total;
     }
@@ -3086,11 +3144,15 @@ sub _items_fee {
     my $desc = $part_fee->itemdesc_locale($self->cust_main->locale);
     # but not escape the base description line
 
+    my @pkg_tax = $cust_bill_pkg->_pkg_tax_list
+      if $options{section_with_taxes};
+
     push @items,
       { feepart     => $cust_bill_pkg->feepart,
         amount      => sprintf('%.2f', $cust_bill_pkg->setup + $cust_bill_pkg->recur),
         description => $desc,
-        ext_description => \@ext_desc
+        pkg_tax     => \@pkg_tax,
+        ext_description => \@ext_desc,
         # sdate/edate?
       };
   }
@@ -3188,6 +3250,8 @@ location (whichever is defined).
 multisection: a flag indicating that this is a multisection invoice,
 which does something complicated.
 
+section_with_taxes:  Look up and include applied taxes for each record
+
 Returns a list of hashrefs, each of which may contain:
 
 pkgnum, description, amount, unit_amount, quantity, pkgpart, _is_setup, and
@@ -3347,6 +3411,9 @@ sub _items_cust_bill_pkg {
         # not normally used, but pass this to the template anyway
         $classname = $part_pkg->classname;
 
+        my @pkg_tax = $cust_bill_pkg->_pkg_tax_list
+          if $opt{section_with_taxes};
+
         if (    (!$type || $type eq 'S')
              && (    $cust_bill_pkg->setup != 0
                   || $cust_bill_pkg->setup_show_zero
@@ -3420,6 +3487,7 @@ sub _items_cust_bill_pkg {
             push @{ $s->{ext_description} }, @d;
           } else {
             $s = {
+              billpkgnum      => $cust_bill_pkg->billpkgnum,
               _is_setup       => 1,
               description     => $description,
               pkgpart         => $pkgpart,
@@ -3431,6 +3499,7 @@ sub _items_cust_bill_pkg {
               ext_description => \@d,
               svc_label       => ($svc_label || ''),
               locationnum     => $cust_pkg->locationnum, # sure, why not?
+              pkg_tax         => \@pkg_tax,
             };
           };
 
@@ -3584,6 +3653,7 @@ sub _items_cust_bill_pkg {
               push @{ $r->{ext_description} }, @d;
             } else {
               $r = {
+                billpkgnum      => $cust_bill_pkg->billpkgnum,
                 description     => $description,
                 pkgpart         => $pkgpart,
                 pkgnum          => $cust_bill_pkg->pkgnum,
@@ -3595,6 +3665,7 @@ sub _items_cust_bill_pkg {
                 ext_description => \@d,
                 svc_label       => ($svc_label || ''),
                 locationnum     => $cust_pkg->locationnum,
+                pkg_tax         => \@pkg_tax,
               };
               $r->{'seconds'} = \@seconds if grep {defined $_} @seconds;
             }
@@ -3613,6 +3684,7 @@ sub _items_cust_bill_pkg {
             } elsif ( $amount ) {
               # create a new usage line
               $u = {
+                billpkgnum      => $cust_bill_pkg->billpkgnum,
                 description     => $description,
                 pkgpart         => $pkgpart,
                 pkgnum          => $cust_bill_pkg->pkgnum,
@@ -3622,6 +3694,7 @@ sub _items_cust_bill_pkg {
                 %item_dates,
                 ext_description => \@d,
                 locationnum     => $cust_pkg->locationnum,
+                pkg_tax         => \@pkg_tax,
               };
             } # else this has no usage, so don't create a usage section
           }
@@ -3755,4 +3828,5 @@ sub _items_discounts_avail {
 
 }
 
+
 1;
diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index 77dce2476..a36520b77 100644
--- a/FS/FS/cust_bill_pkg.pm
+++ b/FS/FS/cust_bill_pkg.pm
@@ -1815,6 +1815,70 @@ sub upgrade_tax_location {
   '';
 }
 
+sub _pkg_tax_list {
+  # Return an array of hashrefs for each cust_bill_pkg_tax_location
+  # applied to this bill for this cust_bill_pkg.pkgnum.
+  #
+  # ! Important Note:
+  #   In some situations, this list will contain more tax records than the
+  #   ones directly related to $self->billpkgnum.  The returned list contains
+  #   all records, for this bill, charged against this billpkgnum's pkgnum.
+  #
+  #   One must keep this in mind when using data returned by this method.
+  #
+  #   An unaddressed deficiency in the cust_bill_pkg_tax_location model makes
+  #   this necessary:  When a linked-hidden package generates a tax/fee as a row
+  #   in cust_bill_pkg_tax_location, there is not enough information to surmise
+  #   with specificity which billpkgnum row represents the direct parent of the
+  #   the linked-hidden package's tax row.  The closest we can get to this
+  #   backwards reassociation is to use the pkgnum.  Therefore, when multiple
+  #   billpkgnum's appear with the same pkgnum, this method is going to return
+  #   the tax records for ALL of those billpkgnum's, not just $self->billpkgnum.
+  #
+  #   This could be addressed with an update to the model, and to the billing
+  #   routine that generates rows into cust_bill_pkg_tax_location.  Perhaps a
+  #   column, link_billpkgnum or parent_billpkgnum, recording the link. I'm not
+  #   doing that now, because there would be no possible repair of data stored
+  #   historically prior to such a fix.  I need _pkg_tax_list() to not be
+  #   broken for already-generated bills.
+  #
+  #   Any code you write relying on _pkg_tax_list() MUST be aware of, and
+  #   account for, the possible return of duplicated tax records returned
+  #   when method is called on multiple cust_bill_pkg_tax_location rows.
+  #   Duplicates can be identified by billpkgtaxlocationnum column.
+
+  my $self = shift;
+  return unless $self->pkgnum;
+
+  map +{
+      billpkgtaxlocationnum => $_->billpkgtaxlocationnum,
+      billpkgnum            => $_->billpkgnum,
+      taxnum                => $_->taxnum,
+      amount                => $_->amount,
+      taxname               => $_->taxname,
+  },
+  qsearch({
+    table  => 'cust_bill_pkg_tax_location',
+    addl_from => '
+      LEFT JOIN cust_bill_pkg
+	      ON cust_bill_pkg.billpkgnum
+         = cust_bill_pkg_tax_location.taxable_billpkgnum
+    ',
+    select => join( ', ', (qw|
+      cust_bill_pkg.billpkgnum
+      cust_bill_pkg_tax_location.billpkgtaxlocationnum
+      cust_bill_pkg_tax_location.taxnum
+      cust_bill_pkg_tax_location.amount
+    |)),
+    extra_sql =>
+      ' WHERE '.
+      ' cust_bill_pkg.invnum = ' . dbh->quote( $self->invnum ) .
+      ' AND '.
+      ' cust_bill_pkg_tax_location.pkgnum = ' . dbh->quote( $self->pkgnum ),
+  });
+
+}
+
 sub _upgrade_data {
   # Create a queue job to run upgrade_tax_location from January 1, 2012 to 
   # the present date.
@@ -1873,4 +1937,3 @@ from the base documentation.
 =cut
 
 1;
-

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

Summary of changes:
 FS/FS/Conf.pm           |  8 ++++-
 FS/FS/Template_Mixin.pm | 82 ++++++++++++++++++++++++++++++++++++++++++++++---
 FS/FS/cust_bill_pkg.pm  | 65 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 149 insertions(+), 6 deletions(-)




More information about the freeside-commits mailing list