fix linking of cust_bill_pkg_tax_location records during billing, #20629
authorMark Wells <mark@freeside.biz>
Mon, 31 Dec 2012 01:03:58 +0000 (17:03 -0800)
committerMark Wells <mark@freeside.biz>
Mon, 31 Dec 2012 01:03:58 +0000 (17:03 -0800)
FS/FS/cust_bill_pkg.pm
FS/FS/cust_main/Billing.pm
FS/FS/cust_main_county.pm

index 5cff140..716c098 100644 (file)
@@ -201,16 +201,50 @@ sub insert {
 
   my $tax_location = $self->get('cust_bill_pkg_tax_location');
   if ( $tax_location ) {
-    foreach my $cust_bill_pkg_tax_location ( @$tax_location ) {
-      $cust_bill_pkg_tax_location->billpkgnum($self->billpkgnum);
-      $error = $cust_bill_pkg_tax_location->insert;
-      if ( $error ) {
-        $dbh->rollback if $oldAutoCommit;
-        return "error inserting cust_bill_pkg_tax_location: $error";
+    foreach my $link ( @$tax_location ) {
+      next if $link->billpkgtaxlocationnum; # don't try to double-insert
+      # This cust_bill_pkg can be linked on either side (i.e. it can be the
+      # tax or the taxed item).  If the other side is already inserted, 
+      # then set billpkgnum to ours, and insert the link.  Otherwise,
+      # set billpkgnum to ours and pass the link off to the cust_bill_pkg
+      # on the other side, to be inserted later.
+
+      my $tax_cust_bill_pkg = $link->get('tax_cust_bill_pkg');
+      if ( $tax_cust_bill_pkg && $tax_cust_bill_pkg->billpkgnum ) {
+        $link->set('billpkgnum', $tax_cust_bill_pkg->billpkgnum);
+        # break circular links when doing this
+        $link->set('tax_cust_bill_pkg', '');
       }
-    }
+      my $taxable_cust_bill_pkg = $link->get('taxable_cust_bill_pkg');
+      if ( $taxable_cust_bill_pkg && $taxable_cust_bill_pkg->billpkgnum ) {
+        $link->set('taxable_billpkgnum', $taxable_cust_bill_pkg->billpkgnum);
+        # XXX if we ever do tax-on-tax for these, this will have to change
+        # since pkgnum will be zero
+        $link->set('pkgnum', $taxable_cust_bill_pkg->pkgnum);
+        $link->set('locationnum', 
+          $taxable_cust_bill_pkg->cust_pkg->tax_locationnum);
+        $link->set('taxable_cust_bill_pkg', '');
+      }
+
+      if ( $link->billpkgnum and $link->taxable_billpkgnum ) {
+        $error = $link->insert;
+        if ( $error ) {
+          $dbh->rollback if $oldAutoCommit;
+          return "error inserting cust_bill_pkg_tax_location: $error";
+        }
+      } else { # handoff
+        my $other;
+        $other = $link->billpkgnum ? $link->get('taxable_cust_bill_pkg')
+                                   : $link->get('tax_cust_bill_pkg');
+        my $link_array = $other->get('cust_bill_pkg_tax_location') || [];
+        push @$link_array, $link;
+        $other->set('cust_bill_pkg_tax_location' => $link_array);
+      }
+    } #foreach my $link
   }
 
+  # someday you will be as awesome as cust_bill_pkg_tax_location...
+  # but not today
   my $tax_rate_location = $self->get('cust_bill_pkg_tax_rate_location');
   if ( $tax_rate_location ) {
     foreach my $cust_bill_pkg_tax_rate_location ( @$tax_rate_location ) {
index 6d3ff91..cd46c73 100644 (file)
@@ -687,8 +687,6 @@ sub _omit_zero_value_bundles {
 
 =item calculate_taxes LINEITEMREF TAXHASHREF INVOICE_TIME
 
-This is a weird one.  Perhaps it should not even be exposed.
-
 Generates tax line items (see L<FS::cust_bill_pkg>) for this customer.
 Usually used internally by bill method B<bill>.
 
@@ -755,7 +753,7 @@ sub calculate_taxes {
   # values are arrayrefs of cust_bill_pkg_tax_rate_location hashrefs
   my %tax_rate_location = ();
 
-  # keys are taxnums (not internal identifiers!)
+  # keys are taxlisthash keys (internal identifiers!)
   # values are arrayrefs of cust_tax_exempt_pkg objects
   my %tax_exemption;
 
@@ -775,45 +773,35 @@ sub calculate_taxes {
     # It also calculates exemptions and attaches them to the cust_bill_pkgs
     # in the argument.
     my $taxables = $taxlisthash->{$tax};
-    my $exemptions = $tax_exemption{$tax_object->taxnum} ||= [];
-    my $hashref_or_error =
-      $tax_object->taxline( $taxables,
+    my $exemptions = $tax_exemption{$tax} ||= [];
+    my $taxline = $tax_object->taxline(
+                            $taxables,
                             'custnum'      => $self->custnum,
                             'invoice_time' => $invoice_time,
                             'exemptions'   => $exemptions,
                           );
-    return $hashref_or_error unless ref($hashref_or_error);
-
-    # then collect any new exemptions generated for this tax
-    push @$exemptions, @{ $_->cust_tax_exempt_pkg }
-      foreach @$taxables;
+    return $taxline unless ref($taxline);
 
     unshift @{ $taxlisthash->{$tax} }, $tax_object;
 
-    my $name   = $hashref_or_error->{'name'};
-    my $amount = $hashref_or_error->{'amount'};
+    if ( $tax_object->isa('FS::cust_main_county') ) {
+      # then $taxline is a real line item
+      push @{ $taxname{ $taxline->itemdesc } }, $taxline;
 
-    #warn "adding $amount as $name\n";
-    $taxname{ $name } ||= [];
-    push @{ $taxname{ $name } }, $tax;
+    } else {
+      # leave this as is for now
 
-    $tax_amount{ $tax } += $amount;
+      my $name   = $taxline->{'name'};
+      my $amount = $taxline->{'amount'};
 
-    # link records between cust_main_county/tax_rate and cust_location
-    $tax_location{ $tax } ||= [];
-    $tax_rate_location{ $tax } ||= [];
-    if ( ref($tax_object) eq 'FS::cust_main_county' ) {
-      push @{ $tax_location{ $tax }  },
-        {
-          'taxnum'      => $tax_object->taxnum, 
-          'taxtype'     => ref($tax_object),
-          'pkgnum'      => $tax_object->get('pkgnum'),
-          'locationnum' => $tax_object->get('locationnum'),
-          'amount'      => sprintf('%.2f', $amount ),
-          'taxable_billpkgnum' => $tax_object->get('billpkgnum'),
-        };
-    }
-    elsif ( ref($tax_object) eq 'FS::tax_rate' ) {
+      #warn "adding $amount as $name\n";
+      $taxname{ $name } ||= [];
+      push @{ $taxname{ $name } }, $tax;
+
+      $tax_amount{ $tax } += $amount;
+
+      # link records between cust_main_county/tax_rate and cust_location
+      $tax_rate_location{ $tax } ||= [];
       my $taxratelocationnum =
         $tax_object->tax_rate_location->taxratelocationnum;
       push @{ $tax_rate_location{ $tax }  },
@@ -823,56 +811,53 @@ sub calculate_taxes {
           'amount'             => sprintf('%.2f', $amount ),
           'locationtaxid'      => $tax_object->location,
           'taxratelocationnum' => $taxratelocationnum,
-          'taxable_billpkgnum' => $tax_object->get('billpkgnum'),
         };
-    }
-
-  }
-
-  #move the cust_tax_exempt_pkg records to the cust_bill_pkgs we will commit
-  my %packagemap = map { $_->pkgnum => $_ } @$cust_bill_pkg;
-  foreach my $tax ( keys %$taxlisthash ) {
-    my $taxables = $taxlisthash->{$tax};
-    my $tax_object = shift @$taxables; # the rest are line items
-    foreach my $cust_bill_pkg ( @$taxables ) {
-      next unless ref($cust_bill_pkg) eq 'FS::cust_bill_pkg'; #IS needed for CCH tax-on-tax
-
-      my @cust_tax_exempt_pkg = splice @{ $cust_bill_pkg->cust_tax_exempt_pkg };
-
-      next unless @cust_tax_exempt_pkg;
-      # get the non-disintegrated version
-      my $real_cust_bill_pkg = $packagemap{$cust_bill_pkg->pkgnum}
-        or die "can't distribute tax exemptions: no line item for ".
-          Dumper($_). " in packagemap ". 
-          join(',', sort {$a<=>$b} keys %packagemap). "\n";
-
-      push @{ $real_cust_bill_pkg->cust_tax_exempt_pkg },
-           @cust_tax_exempt_pkg;
-    }
-  }
+    } #if ref($tax_object)...
+  } #foreach keys %$taxlisthash
 
   #consolidate and create tax line items
   warn "consolidating and generating...\n" if $DEBUG > 2;
   foreach my $taxname ( keys %taxname ) {
+    my @cust_bill_pkg_tax_location;
+    my @cust_bill_pkg_tax_rate_location;
+    my $tax_cust_bill_pkg = FS::cust_bill_pkg->new({
+        'pkgnum'    => 0,
+        'recur'     => 0,
+        'sdate'     => '',
+        'edate'     => '',
+        'itemdesc'  => $taxname,
+        'cust_bill_pkg_tax_location'      => \@cust_bill_pkg_tax_location,
+        'cust_bill_pkg_tax_rate_location' => \@cust_bill_pkg_tax_rate_location,
+    });
+
     my $tax_total = 0;
     my %seen = ();
-    my @cust_bill_pkg_tax_location = ();
-    my @cust_bill_pkg_tax_rate_location = ();
     warn "adding $taxname\n" if $DEBUG > 1;
     foreach my $taxitem ( @{ $taxname{$taxname} } ) {
-      next if $seen{$taxitem}++;
-      warn "adding $tax_amount{$taxitem}\n" if $DEBUG > 1;
-      $tax_total += $tax_amount{$taxitem};
-      push @cust_bill_pkg_tax_location,
-        map { new FS::cust_bill_pkg_tax_location $_ }
-            @{ $tax_location{ $taxitem } };
-      push @cust_bill_pkg_tax_rate_location,
-        map { new FS::cust_bill_pkg_tax_rate_location $_ }
-            @{ $tax_rate_location{ $taxitem } };
+      if ( ref($taxitem) eq 'FS::cust_bill_pkg' ) {
+        # then we need to transfer the amount and the links from the
+        # line item to the new one we're creating.
+        $tax_total += $taxitem->setup;
+        foreach my $link ( @{ $taxitem->get('cust_bill_pkg_tax_location') } ) {
+          $link->set('tax_cust_bill_pkg', $tax_cust_bill_pkg);
+          push @cust_bill_pkg_tax_location, $link;
+        }
+      } else {
+        # the tax_rate way
+        next if $seen{$taxitem}++;
+        warn "adding $tax_amount{$taxitem}\n" if $DEBUG > 1;
+        $tax_total += $tax_amount{$taxitem};
+        push @cust_bill_pkg_tax_rate_location,
+          map { new FS::cust_bill_pkg_tax_rate_location $_ }
+              @{ $tax_rate_location{ $taxitem } };
+      }
     }
     next unless $tax_total;
 
+    # we should really neverround this up...I guess it's okay if taxline 
+    # already returns amounts with 2 decimal places
     $tax_total = sprintf('%.2f', $tax_total );
+    $tax_cust_bill_pkg->set('setup', $tax_total);
   
     my $pkg_category = qsearchs( 'pkg_category', { 'categoryname' => $taxname,
                                                    'disabled'     => '',
@@ -890,19 +875,9 @@ sub calculate_taxes {
       push @display, new FS::cust_bill_pkg_display { type => 'S', %hash };
 
     }
+    $tax_cust_bill_pkg->set('display', \@display);
 
-    push @tax_line_items, new FS::cust_bill_pkg {
-      'pkgnum'   => 0,
-      'setup'    => $tax_total,
-      'recur'    => 0,
-      'sdate'    => '',
-      'edate'    => '',
-      'itemdesc' => $taxname,
-      'display'  => \@display,
-      'cust_bill_pkg_tax_location' => \@cust_bill_pkg_tax_location,
-      'cust_bill_pkg_tax_rate_location' => \@cust_bill_pkg_tax_rate_location,
-    };
-
+    push @tax_line_items, $tax_cust_bill_pkg;
   }
 
   \@tax_line_items;
@@ -1184,11 +1159,23 @@ sub _make_lines {
       # handle taxes
       ###
 
-      unless ( $discount_show_always ) {
-         my $error = 
-           $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg, $cust_pkg, $options{invoice_time}, $real_pkgpart, \%options);
-         return $error if $error;
-      }
+      #unless ( $discount_show_always ) { # oh, for god's sake
+      my $error = $self->_handle_taxes(
+        $part_pkg,
+        $taxlisthash,
+        $cust_bill_pkg,
+        $cust_pkg,
+        $options{invoice_time},
+        $real_pkgpart,
+        \%options # I have serious objections to this
+      );
+      return $error if $error;
+      #}
+
+      $cust_bill_pkg->set_display(
+        part_pkg     => $part_pkg,
+        real_pkgpart => $real_pkgpart,
+      );
 
       push @$cust_bill_pkgs, $cust_bill_pkg;
 
@@ -1200,6 +1187,25 @@ sub _make_lines {
 
 }
 
+# This is _handle_taxes.  It's called once for each cust_bill_pkg generated
+# from _make_lines, along with the part_pkg, cust_pkg, invoice time, the 
+# non-overridden pkgpart, a flag indicating whether the package is being
+# canceled, and a partridge in a pear tree.
+#
+# The most important argument is 'taxlisthash'.  This is shared across the 
+# entire invoice.  It looks like this:
+# {
+#   'cust_main_county 1001' => [ [FS::cust_main_county], ... ],
+#   'cust_main_county 1002' => [ [FS::cust_main_county], ... ],
+# }
+#
+# 'cust_main_county' can also be 'tax_rate'.  The first object in the array
+# is always the cust_main_county or tax_rate identified by the key.
+#
+# That "..." is a list of FS::cust_bill_pkg objects that will be fed to 
+# the 'taxline' method to calculate the amount of the tax.  This doesn't
+# happen until calculate_taxes, though.
+
 sub _handle_taxes {
   my $self = shift;
   my $part_pkg = shift;
@@ -1212,175 +1218,152 @@ sub _handle_taxes {
 
   local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
 
-  my %cust_bill_pkg = ();
-  my %taxes = ();
-    
-  my @classes;
-  #push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->type eq 'U';
-  push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->usage;
-  push @classes, 'setup' if ($cust_bill_pkg->setup && !$options->{cancel});
-  push @classes, 'recur' if ($cust_bill_pkg->recur && !$options->{cancel});
-
-  my $exempt = $conf->exists('cust_class-tax_exempt')
-                 ? ( $self->cust_class ? $self->cust_class->tax : '' )
-                 : $self->tax;
-  # standardize this just to be sure
-  $exempt = ($exempt eq 'Y') ? 'Y' : '';
-
-  #if ( $exempt !~ /Y/i && $self->payby ne 'COMP' ) {
-  if ( $self->payby ne 'COMP' ) {
-
-    if ( $conf->exists('enable_taxproducts')
-         && ( scalar($part_pkg->part_pkg_taxoverride)
-              || $part_pkg->has_taxproduct
-            )
-       )
-    {
+  return if ( $self->payby eq 'COMP' ); #dubious
 
-      if ( !$exempt ) {
+  if ( $conf->exists('enable_taxproducts')
+       && ( scalar($part_pkg->part_pkg_taxoverride)
+            || $part_pkg->has_taxproduct
+          )
+     )
+    {
 
-        foreach my $class (@classes) {
-          my $err_or_ref = $self->_gather_taxes( $part_pkg, $class, $cust_pkg );
-          return $err_or_ref unless ref($err_or_ref);
-          $taxes{$class} = $err_or_ref;
-        }
+    # EXTERNAL TAX RATES (via tax_rate)
+    my %cust_bill_pkg = ();
+    my %taxes = ();
+
+    my @classes;
+    #push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->type eq 'U';
+    push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->usage;
+    # debatable
+    push @classes, 'setup' if ($cust_bill_pkg->setup && !$options->{cancel});
+    push @classes, 'recur' if ($cust_bill_pkg->recur && !$options->{cancel});
+
+    my $exempt = $conf->exists('cust_class-tax_exempt')
+                   ? ( $self->cust_class ? $self->cust_class->tax : '' )
+                   : $self->tax;
+    # standardize this just to be sure
+    $exempt = ($exempt eq 'Y') ? 'Y' : '';
+  
+    if ( !$exempt ) {
 
-        unless (exists $taxes{''}) {
-          my $err_or_ref = $self->_gather_taxes( $part_pkg, '', $cust_pkg );
-          return $err_or_ref unless ref($err_or_ref);
-          $taxes{''} = $err_or_ref;
-        }
+      foreach my $class (@classes) {
+        my $err_or_ref = $self->_gather_taxes( $part_pkg, $class, $cust_pkg );
+        return $err_or_ref unless ref($err_or_ref);
+        $taxes{$class} = $err_or_ref;
+      }
 
+      unless (exists $taxes{''}) {
+        my $err_or_ref = $self->_gather_taxes( $part_pkg, '', $cust_pkg );
+        return $err_or_ref unless ref($err_or_ref);
+        $taxes{''} = $err_or_ref;
       }
 
-    } else { # cust_main_county tax system
+    }
 
-      # We fetch taxes even if the customer is completely exempt,
-      # because we need to record that fact.
+    my %tax_cust_bill_pkg = $cust_bill_pkg->disintegrate;
+    foreach my $key (keys %tax_cust_bill_pkg) {
+      # $key is "setup", "recur", or a usage class name. ('' is a usage class.)
+      # $tax_cust_bill_pkg{$key} is a cust_bill_pkg for that component of 
+      # the line item.
+      # $taxes{$key} is an arrayref of cust_main_county or tax_rate objects that
+      # apply to $key-class charges.
+      my @taxes = @{ $taxes{$key} || [] };
+      my $tax_cust_bill_pkg = $tax_cust_bill_pkg{$key};
+
+      my %localtaxlisthash = ();
+      foreach my $tax ( @taxes ) {
+
+        # this is the tax identifier, not the taxname
+        my $taxname = ref( $tax ). ' '. $tax->taxnum;
+        $taxname .= ' billpkgnum'. $cust_bill_pkg->billpkgnum;
+        # We need to create a separate $taxlisthash entry for each billpkgnum
+        # on the invoice, so that cust_bill_pkg_tax_location records will
+        # be linked correctly.
+
+        # $taxlisthash: keys are "setup", "recur", and usage classes.
+        # Values are arrayrefs, first the tax object (cust_main_county
+        # or tax_rate) and then any cust_bill_pkg objects that the 
+        # tax applies to.
+        $taxlisthash->{ $taxname } ||= [ $tax ];
+        push @{ $taxlisthash->{ $taxname  } }, $tax_cust_bill_pkg;
+
+        $localtaxlisthash{ $taxname } ||= [ $tax ];
+        push @{ $localtaxlisthash{ $taxname  } }, $tax_cust_bill_pkg;
 
-      my @loc_keys = qw( district city county state country );
-      my $location = $cust_pkg->tax_location;
-      my %taxhash = map { $_ => $location->$_ } @loc_keys;
+      }
 
-      $taxhash{'taxclass'} = $part_pkg->taxclass;
+      warn "finding taxed taxes...\n" if $DEBUG > 2;
+      foreach my $tax ( keys %localtaxlisthash ) {
+        my $tax_object = shift @{ $localtaxlisthash{$tax} };
+        warn "found possible taxed tax ". $tax_object->taxname. " we call $tax\n"
+          if $DEBUG > 2;
+        next unless $tax_object->can('tax_on_tax');
+
+        foreach my $tot ( $tax_object->tax_on_tax( $self ) ) {
+          my $totname = ref( $tot ). ' '. $tot->taxnum;
+
+          warn "checking $totname which we call ". $tot->taxname. " as applicable\n"
+            if $DEBUG > 2;
+          next unless exists( $localtaxlisthash{ $totname } ); # only increase
+                                                               # existing taxes
+          warn "adding $totname to taxed taxes\n" if $DEBUG > 2;
+          # we're calling taxline() right here?  wtf?
+          my $hashref_or_error = 
+            $tax_object->taxline( $localtaxlisthash{$tax},
+                                  'custnum'      => $self->custnum,
+                                  'invoice_time' => $invoice_time,
+                                );
+          return $hashref_or_error
+            unless ref($hashref_or_error);
+          
+          $taxlisthash->{ $totname } ||= [ $tot ];
+          push @{ $taxlisthash->{ $totname  } }, $hashref_or_error->{amount};
 
-      warn "taxhash:\n". Dumper(\%taxhash) if $DEBUG > 2;
+        }
+      }
+    }
 
-      my @taxes = (); # entries are cust_main_county objects
-      my %taxhash_elim = %taxhash;
-      my @elim = qw( district city county state );
-      do { 
+  } else {
 
-        #first try a match with taxclass
-        @taxes = qsearch( 'cust_main_county', \%taxhash_elim );
+    # INTERNAL TAX RATES (cust_main_county)
 
-        if ( !scalar(@taxes) && $taxhash_elim{'taxclass'} ) {
-          #then try a match without taxclass
-          my %no_taxclass = %taxhash_elim;
-          $no_taxclass{ 'taxclass' } = '';
-          @taxes = qsearch( 'cust_main_county', \%no_taxclass );
-        }
+    # We fetch taxes even if the customer is completely exempt,
+    # because we need to record that fact.
 
-        $taxhash_elim{ shift(@elim) } = '';
+    my @loc_keys = qw( district city county state country );
+    my $location = $cust_pkg->tax_location;
+    my %taxhash = map { $_ => $location->$_ } @loc_keys;
 
-      } while ( !scalar(@taxes) && scalar(@elim) );
+    $taxhash{'taxclass'} = $part_pkg->taxclass;
 
-      foreach (@taxes) {
-        # These could become cust_bill_pkg_tax_location records,
-        # or cust_tax_exempt_pkg.  We'll decide later.
-        #
-        # The most important thing here: record which charge is being taxed.
-        $_->set('billpkgnum',   $cust_bill_pkg->billpkgnum);
-        # also these, for historical reasons
-        $_->set('pkgnum',       $cust_pkg->pkgnum);
-        $_->set('locationnum',  $cust_pkg->tax_locationnum);
-      }
+    warn "taxhash:\n". Dumper(\%taxhash) if $DEBUG > 2;
 
-      $taxes{''} = [ @taxes ];
-      $taxes{'setup'} = [ @taxes ];
-      $taxes{'recur'} = [ @taxes ];
-      $taxes{$_} = [ @taxes ] foreach (@classes);
-
-      # # maybe eliminate this entirely, along with all the 0% records
-      # unless ( @taxes ) {
-      #   return
-      #     "fatal: can't find tax rate for state/county/country/taxclass ".
-      #     join('/', map $taxhash{$_}, qw(state county country taxclass) );
-      # }
-
-    } #if $conf->exists('enable_taxproducts') ...
-
-  } # if $self->payby eq 'COMP'
-
-  #what's this doing in the middle of _handle_taxes?  probably should split
-  #this into three parts above in _make_lines
-  $cust_bill_pkg->set_display(   part_pkg     => $part_pkg,
-                                 real_pkgpart => $real_pkgpart,
-                             );
-
-  my %tax_cust_bill_pkg = $cust_bill_pkg->disintegrate;
-  foreach my $key (keys %tax_cust_bill_pkg) {
-    # $key is "setup", "recur", or a usage class name. ('' is a usage class.)
-    # $tax_cust_bill_pkg{$key} is a cust_bill_pkg for that component of 
-    # the line item.
-    # $taxes{$key} is an arrayref of cust_main_county or tax_rate objects that
-    # apply to $key-class charges.
-    my @taxes = @{ $taxes{$key} || [] };
-    my $tax_cust_bill_pkg = $tax_cust_bill_pkg{$key};
-
-    my %localtaxlisthash = ();
-    foreach my $tax ( @taxes ) {
-
-      # this is the tax identifier, not the taxname
-      my $taxname = ref( $tax ). ' '. $tax->taxnum;
-      $taxname .= ' billpkgnum'. $cust_bill_pkg->billpkgnum;
-      # We need to create a separate $taxlisthash entry for each billpkgnum
-      # on the invoice, so that cust_bill_pkg_tax_location records will
-      # be linked correctly.
-
-      # $taxlisthash: keys are "setup", "recur", and usage classes.
-      # Values are arrayrefs, first the tax object (cust_main_county
-      # or tax_rate) and then any cust_bill_pkg objects that the 
-      # tax applies to.
-      $taxlisthash->{ $taxname } ||= [ $tax ];
-      push @{ $taxlisthash->{ $taxname  } }, $tax_cust_bill_pkg;
-
-      $localtaxlisthash{ $taxname } ||= [ $tax ];
-      push @{ $localtaxlisthash{ $taxname  } }, $tax_cust_bill_pkg;
+    my @taxes = (); # entries are cust_main_county objects
+    my %taxhash_elim = %taxhash;
+    my @elim = qw( district city county state );
+    do { 
 
-    }
+      #first try a match with taxclass
+      @taxes = qsearch( 'cust_main_county', \%taxhash_elim );
 
-    warn "finding taxed taxes...\n" if $DEBUG > 2;
-    foreach my $tax ( keys %localtaxlisthash ) {
-      my $tax_object = shift @{ $localtaxlisthash{$tax} };
-      warn "found possible taxed tax ". $tax_object->taxname. " we call $tax\n"
-        if $DEBUG > 2;
-      next unless $tax_object->can('tax_on_tax');
+      if ( !scalar(@taxes) && $taxhash_elim{'taxclass'} ) {
+        #then try a match without taxclass
+        my %no_taxclass = %taxhash_elim;
+        $no_taxclass{ 'taxclass' } = '';
+        @taxes = qsearch( 'cust_main_county', \%no_taxclass );
+      }
 
-      foreach my $tot ( $tax_object->tax_on_tax( $self ) ) {
-        my $totname = ref( $tot ). ' '. $tot->taxnum;
+      $taxhash_elim{ shift(@elim) } = '';
 
-        warn "checking $totname which we call ". $tot->taxname. " as applicable\n"
-          if $DEBUG > 2;
-        next unless exists( $localtaxlisthash{ $totname } ); # only increase
-                                                             # existing taxes
-        warn "adding $totname to taxed taxes\n" if $DEBUG > 2;
-        my $hashref_or_error = 
-          $tax_object->taxline( $localtaxlisthash{$tax},
-                                'custnum'      => $self->custnum,
-                                'invoice_time' => $invoice_time,
-                              );
-        return $hashref_or_error
-          unless ref($hashref_or_error);
-        
-        $taxlisthash->{ $totname } ||= [ $tot ];
-        push @{ $taxlisthash->{ $totname  } }, $hashref_or_error->{amount};
+    } while ( !scalar(@taxes) && scalar(@elim) );
 
-      }
+    foreach (@taxes) {
+      my $tax_id = 'cust_main_county '.$_->taxnum;
+      $taxlisthash->{$tax_id} ||= [ $_ ];
+      push @{ $taxlisthash->{$tax_id} }, $cust_bill_pkg;
     }
 
   }
-
   '';
 }
 
index 87c1ca7..db6be75 100644 (file)
@@ -258,10 +258,15 @@ sub _list_sql {
 
 =item taxline TAXABLES_ARRAYREF, [ OPTION => VALUE ... ]
 
-Returns an hashref of a name and an amount of tax calculated for the 
-line items (L<FS::cust_bill_pkg> objects) in TAXABLES_ARRAYREF.  The line 
-items must come from the same invoice.  Returns a scalar error message 
-on error.
+Takes an arrayref of L<FS::cust_bill_pkg> objects representing taxable
+line items, and returns a new L<FS::cust_bill_pkg> object representing
+the tax on them under this tax rate.
+
+This will have a pseudo-field, "cust_bill_pkg_tax_location", containing 
+an arrayref of L<FS::cust_bill_pkg_tax_location> objects.  Each of these 
+will in turn have a "taxable_cust_bill_pkg" pseudo-field linking it to one
+of the taxable items.  All of these links must be resolved as the objects
+are inserted.
 
 In addition to calculating the tax for the line items, this will calculate
 any appropriate tax exemptions and attach them to the line items.
@@ -275,8 +280,7 @@ tax exemption limit if there is one.
 
 =cut
 
-# XXX this should just return a cust_bill_pkg object for the tax,
-# but that requires changing stuff in tax_rate.pm also.
+# XXX change tax_rate.pm to work like this
 
 sub taxline {
   my( $self, $taxables, %opt ) = @_;
@@ -294,7 +298,8 @@ sub taxline {
   my $dbh = dbh;
 
   my $name = $self->taxname || 'Tax';
-  my $amount = 0;
+  my $taxable_cents = 0;
+  my $tax_cents = 0;
 
   my $cust_bill = $taxables->[0]->cust_bill;
   my $custnum   = $cust_bill ? $cust_bill->custnum : $opt{'custnum'};
@@ -325,6 +330,15 @@ sub taxline {
   push @existing_exemptions, @{ $_->cust_tax_exempt_pkg }
     for @$taxables;
 
+  my $tax_item = FS::cust_bill_pkg->new({
+      'pkgnum'    => 0,
+      'recur'     => 0,
+      'sdate'     => '',
+      'edate'     => '',
+      'itemdesc'  => $name,
+  });
+  my @tax_location;
+
   foreach my $cust_bill_pkg (@$taxables) {
 
     my $cust_pkg  = $cust_bill_pkg->cust_pkg;
@@ -472,37 +486,47 @@ sub taxline {
 
     $_->taxnum($self->taxnum) foreach @new_exemptions;
 
-    #if ( $cust_bill_pkg->billpkgnum ) {
-
-      #no, need to do this to e.g. calculate tax credit amounts
-      #die "tried to calculate tax exemptions on a previously billed line item\n";
-
-      # this is unnecessary
-#      foreach my $cust_tax_exempt_pkg (@new_exemptions) {
-#        my $error = $cust_tax_exempt_pkg->insert;
-#        if ( $error ) {
-#          $dbh->rollback if $oldAutoCommit;
-#          return "can't insert cust_tax_exempt_pkg: $error";
-#        }
-#      }
-    #}
-
     # attach them to the line item
     push @{ $cust_bill_pkg->cust_tax_exempt_pkg }, @new_exemptions;
     push @existing_exemptions, @new_exemptions;
 
-    # If we were smart, we'd also generate a cust_bill_pkg_tax_location 
-    # record at this point, but that would require redesigning more stuff.
     $taxable_charged = sprintf( "%.2f", $taxable_charged);
-
-    $amount += $taxable_charged * $self->tax / 100;
+    next if $taxable_charged == 0;
+
+    my $this_tax_cents = int($taxable_charged * $self->tax);
+    my $location = FS::cust_bill_pkg_tax_location->new({
+        'taxnum'      => $self->taxnum,
+        'taxtype'     => ref($self),
+        'cents'       => $this_tax_cents,
+        'taxable_cust_bill_pkg' => $cust_bill_pkg,
+        'tax_cust_bill_pkg'     => $tax_item,
+    });
+    push @tax_location, $location;
+
+    $taxable_cents += $taxable_charged;
+    $tax_cents += $this_tax_cents;
   } #foreach $cust_bill_pkg
-
-  return {
-    'name'   => $name,
-    'amount' => $amount,
-  };
-
+  
+  # now round and distribute
+  my $extra_cents = sprintf('%.2f', $taxable_cents * $self->tax / 100) * 100
+                    - $tax_cents;
+  if ( $extra_cents < 0 ) {
+    die "nonsense extra_cents value $extra_cents"; # because seriously, wtf
+  }
+  $tax_cents += $extra_cents;
+  my $i = 0;
+  foreach (@tax_location) { # can never require more than a single pass, yes?
+    my $cents = $_->get('cents');
+    if ( $extra_cents > 0 ) {
+      $cents++;
+      $extra_cents--;
+    }
+    $_->set('amount', sprintf('%.2f', $cents/100));
+  }
+  $tax_item->set('setup' => sprintf('%.2f', $tax_cents / 100));
+  $tax_item->set('cust_bill_pkg_tax_location', \@tax_location);
+  
+  return $tax_item;
 }
 
 =back