X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=blobdiff_plain;f=FS%2FFS%2Fcust_main%2FBilling.pm;h=08b10c1ffe2601c1a8803120de59acd98b86769e;hp=939a625c758867e466bf6a8ee93df23842c4ad7b;hb=2cdb0b3f9e3b778fd914d847fc7851948a9930e4;hpb=54a357b171aa44f9399b4c146acd2afd3b686075

diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index 939a625c7..08b10c1ff 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -8,6 +8,7 @@ use List::Util qw( min );
 use FS::UID qw( dbh );
 use FS::Record qw( qsearch qsearchs dbdef );
 use FS::Misc::DateTime qw( day_end );
+use Tie::RefHash;
 use FS::cust_bill;
 use FS::cust_bill_pkg;
 use FS::cust_bill_pkg_display;
@@ -21,7 +22,9 @@ use FS::cust_bill_pkg_tax_rate_location;
 use FS::part_event;
 use FS::part_event_condition;
 use FS::pkg_category;
+use FS::FeeOrigin_Mixin;
 use FS::Log;
+use FS::TaxEngine;
 
 # 1 is mostly method/subroutine entry and options
 # 2 traces progress of some operations
@@ -54,7 +57,7 @@ Cancels and suspends any packages due, generates bills, applies payments and
 credits, and applies collection events to run cards, send bills and notices,
 etc.
 
-By default, warns on errors and continues with the next operation (but see the
+Any errors prevent subsequent operations from continuing and die (but see the
 "fatal" flag below).
 
 Options are passed as name-value pairs.  Currently available options are:
@@ -105,8 +108,9 @@ options of those methods are also available.
 sub bill_and_collect {
   my( $self, %options ) = @_;
 
-  my $log = FS::Log->new('bill_and_collect');
-  $log->debug('start', object => $self, agentnum => $self->agentnum);
+  my $log = FS::Log->new('FS::cust_main::Billing::bill_and_collect');
+  my %logopt = (object => $self);
+  $log->debug('start', %logopt);
 
   my $error;
 
@@ -122,62 +126,76 @@ sub bill_and_collect {
                     );
 
   $job->update_statustext('0,cleaning expired packages') if $job;
+  $log->debug('canceling expired packages', %logopt);
   $error = $self->cancel_expired_pkgs( $actual_time );
   if ( $error ) {
     $error = "Error expiring custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
-    elsif ( $options{fatal}                                ) { die    $error; }
-    else                                                     { warn   $error; }
+    else                                                     { die    $error; }
   }
 
+  $log->debug('suspending adjourned packages', %logopt);
   $error = $self->suspend_adjourned_pkgs( $actual_time );
   if ( $error ) {
     $error = "Error adjourning custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
-    elsif ( $options{fatal}                                ) { die    $error; }
-    else                                                     { warn   $error; }
+    else                                                     { die    $error; }
   }
 
+  $log->debug('unsuspending resumed packages', %logopt);
   $error = $self->unsuspend_resumed_pkgs( $actual_time );
   if ( $error ) {
     $error = "Error resuming custnum ".$self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
-    elsif ( $options{fatal}                                ) { die    $error; }
-    else                                                     { warn   $error; }
+    else                                                     { die    $error; }
   }
 
   $job->update_statustext('20,billing packages') if $job;
+  $log->debug('billing packages', %logopt);
   $error = $self->bill( %options );
   if ( $error ) {
     $error = "Error billing custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
-    elsif ( $options{fatal}                                ) { die    $error; }
-    else                                                     { warn   $error; }
+    else                                                     { die    $error; }
   }
 
   $job->update_statustext('50,applying payments and credits') if $job;
+  $log->debug('applying payments and credits', %logopt);
   $error = $self->apply_payments_and_credits;
   if ( $error ) {
     $error = "Error applying custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
-    elsif ( $options{fatal}                                ) { die    $error; }
-    else                                                     { warn   $error; }
+    else                                                     { die    $error; }
   }
 
-  $job->update_statustext('70,running collection events') if $job;
-  unless ( $conf->exists('cancelled_cust-noevents')
-           && ! $self->num_ncancelled_pkgs
-  ) {
+  # In a batch tax environment, do not run collection if any pending 
+  # invoices were created.  Collection will run after the next tax batch.
+  my $tax = FS::TaxEngine->new;
+  if ( $tax->info->{batch} and 
+       qsearch('cust_bill', { custnum => $self->custnum, pending => 'Y' })
+     )
+  {
+    warn "skipped collection for custnum ".$self->custnum.
+         " due to pending invoices\n" if $DEBUG;
+  } elsif ( $conf->exists('cancelled_cust-noevents')
+             && ! $self->num_ncancelled_pkgs )
+  {
+    warn "skipped collection for custnum ".$self->custnum.
+         " because they have no active packages\n" if $DEBUG;
+  } else {
+    # run collection normally
+    $job->update_statustext('70,running collection events') if $job;
+    $log->debug('running collection events', %logopt);
     $error = $self->collect( %options );
     if ( $error ) {
       $error = "Error collecting custnum ". $self->custnum. ": $error";
       if    ($options{fatal} && $options{fatal} eq 'return') { return $error; }
-      elsif ($options{fatal}                               ) { die    $error; }
-      else                                                   { warn   $error; }
+      else                                                   { die    $error; }
     }
   }
+
   $job->update_statustext('100,finished') if $job;
-  $log->debug('finish', object => $self, agentnum => $self->agentnum);
+  $log->debug('finish', %logopt);
 
   '';
 
@@ -192,15 +210,42 @@ sub cancel_expired_pkgs {
 
   my @errors = ();
 
-  foreach my $cust_pkg ( @cancel_pkgs ) {
+  my @really_cancel_pkgs = ();
+  my @cancel_reasons = ();
+
+  CUST_PKG: foreach my $cust_pkg ( @cancel_pkgs ) {
     my $cpr = $cust_pkg->last_cust_pkg_reason('expire');
-    my $error = $cust_pkg->cancel($cpr ? ( 'reason'        => $cpr->reasonnum,
-                                           'reason_otaker' => $cpr->otaker,
-                                           'time'          => $time,
-                                         )
-                                       : ()
-                                 );
-    push @errors, 'pkgnum '.$cust_pkg->pkgnum.": $error" if $error;
+
+    if ( $cust_pkg->change_to_pkgnum ) {
+
+      my $new_pkg = FS::cust_pkg->by_key($cust_pkg->change_to_pkgnum);
+      if ( !$new_pkg ) {
+        push @errors, 'can\'t change pkgnum '.$cust_pkg->pkgnum.' to pkgnum '.
+                      $cust_pkg->change_to_pkgnum.'; not expiring';
+        next CUST_PKG;
+      }
+      my $error = $cust_pkg->change( 'cust_pkg'        => $new_pkg,
+                                     'unprotect_svcs'  => 1,
+                                   );
+      push @errors, $error if $error && ref($error) ne 'FS::cust_pkg';
+
+    } else { # just cancel it
+
+      push @really_cancel_pkgs, $cust_pkg;
+      push @cancel_reasons, $cpr;
+
+    }
+  }
+
+  if (@really_cancel_pkgs) {
+
+    my %cancel_opt = ( 'cust_pkg' => \@really_cancel_pkgs,
+                       'cust_pkg_reason' => \@cancel_reasons,
+                       'time' => $time,
+                     );
+
+    push @errors, $self->cancel_pkgs(%cancel_opt);
+
   }
 
   join(' / ', @errors);
@@ -312,6 +357,10 @@ An array ref of specific packages (objects) to attempt billing, instead trying a
 
 A hashref of pkgparts to exclude from this billing run (can also be specified as a comma-separated scalar).
 
+=item no_prepaid
+
+Do not bill prepaid packages.  Used by freeside-daily.
+
 =item invoice_time
 
 Used in conjunction with the I<time> option, this option specifies the date of for the generated invoices.  Other calculations, such as whether or not to generate the invoice in the first place, are not affected.
@@ -335,6 +384,12 @@ Do not save the generated bill in the database.  Useful with return_bill
 
 A list reference on which the generated bill(s) will be returned.
 
+=item estimate
+
+Boolean value; indicates that this is an estimate rather than a "tax invoice".
+This will be passed through to the tax engine, as online tax services 
+sometimes need to know it for reporting purposes. Otherwise it has no effect.
+
 =item invoice_terms
 
 Optional terms to be printed on this invoice.  Otherwise, customer-specific
@@ -347,16 +402,24 @@ terms or the default terms are used.
 sub bill {
   my( $self, %options ) = @_;
 
-  return '' if $self->payby eq 'COMP';
+  return '' if $self->complimentary eq 'Y';
 
   local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
+  my $log = FS::Log->new('FS::cust_main::Billing::bill');
+  my %logopt = (object => $self);
 
+  $log->debug('start', %logopt);
   warn "$me bill customer ". $self->custnum. "\n"
     if $DEBUG;
 
   my $time = $options{'time'} || time;
   my $invoice_time = $options{'invoice_time'} || $time;
 
+  my $cmp_time = ( $conf->exists('next-bill-ignore-time')
+                     ? day_end( $time )
+                     : $time
+                 );
+
   $options{'not_pkgpart'} ||= {};
   $options{'not_pkgpart'} = { map { $_ => 1 }
                                   split(/\s*,\s*/, $options{'not_pkgpart'})
@@ -374,11 +437,13 @@ sub bill {
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
 
+  $log->debug('acquiring lock', %logopt);
   warn "$me acquiring lock on customer ". $self->custnum. "\n"
     if $DEBUG;
 
   $self->select_for_update; #mutex
 
+  $log->debug('running pre-bill events', %logopt);
   warn "$me running pre-bill events for customer ". $self->custnum. "\n"
     if $DEBUG;
 
@@ -394,6 +459,7 @@ sub bill {
     return $error;
   }
 
+  $log->debug('done running pre-bill events', %logopt);
   warn "$me done running pre-bill events for customer ". $self->custnum. "\n"
     if $DEBUG;
 
@@ -410,51 +476,110 @@ sub bill {
   my %total_setup   = map { my $z = 0; $_ => \$z; } @passes;
   my %total_recur   = map { my $z = 0; $_ => \$z; } @passes;
 
-  my %taxlisthash = map { $_ => {} } @passes;
-
   my @precommit_hooks = ();
 
   $options{'pkg_list'} ||= [ $self->ncancelled_pkgs ];  #param checks?
+  
+  my %tax_engines;
+  my $tax_is_batch = '';
+  foreach (@passes) {
+    $tax_engines{$_} = FS::TaxEngine->new(cust_main    => $self,
+                                          invoice_time => $invoice_time,
+                                          cancel       => $options{cancel},
+                                          estimate     => $options{estimate},
+                                         );
+    $tax_is_batch ||= $tax_engines{$_}->info->{batch};
+  }
 
   foreach my $cust_pkg ( @{ $options{'pkg_list'} } ) {
 
     next if $options{'not_pkgpart'}->{$cust_pkg->pkgpart};
 
+    my $part_pkg = $cust_pkg->part_pkg;
+
+    next if $options{'no_prepaid'} && $part_pkg->is_prepaid;
+
+    $log->debug('bill package '. $cust_pkg->pkgnum, %logopt);
     warn "  bill package ". $cust_pkg->pkgnum. "\n" if $DEBUG;
 
     #? to avoid use of uninitialized value errors... ?
     $cust_pkg->setfield('bill', '')
       unless defined($cust_pkg->bill);
  
-    #my $part_pkg = $cust_pkg->part_pkg;
-
     my $real_pkgpart = $cust_pkg->pkgpart;
     my %hash = $cust_pkg->hash;
 
     # we could implement this bit as FS::part_pkg::has_hidden, but we already
     # suffer from performance issues
     $options{has_hidden} = 0;
-    my @part_pkg = $cust_pkg->part_pkg->self_and_bill_linked;
+    my @part_pkg = $part_pkg->self_and_bill_linked;
     $options{has_hidden} = 1 if ($part_pkg[1] && $part_pkg[1]->hidden);
  
+    # if this package was changed from another package,
+    # and it hasn't been billed since then,
+    # and package balances are enabled,
+    if ( $cust_pkg->change_pkgnum
+        and $cust_pkg->change_date >= ($cust_pkg->last_bill || 0)
+        and $cust_pkg->change_date <  $invoice_time
+      and $conf->exists('pkg-balances') )
+    {
+      # _transfer_balance will also create the appropriate credit
+      my @transfer_items = $self->_transfer_balance($cust_pkg);
+      # $part_pkg[0] is the "real" part_pkg
+      my $pass = ($cust_pkg->no_auto || $part_pkg[0]->no_auto) ? 
+                  'no_auto' : '';
+      push @{ $cust_bill_pkg{$pass} }, @transfer_items;
+      # treating this as recur, just because most charges are recur...
+      ${$total_recur{$pass}} += $_->recur foreach @transfer_items;
+
+      # currently not considering separate_bill here, as it's for 
+      # one-time charges only
+    }
+
     foreach my $part_pkg ( @part_pkg ) {
 
-      $cust_pkg->set($_, $hash{$_}) foreach qw ( setup last_bill bill );
+      my $this_cust_pkg = $cust_pkg;
+      # for add-on packages, copy the object to avoid leaking changes back to
+      # the caller if pkg_list is in use; see RT#73607
+      if ( $part_pkg->get('pkgpart') != $real_pkgpart ) {
+        $this_cust_pkg = FS::cust_pkg->new({ %hash });
+      }
 
-      my $pass = ($cust_pkg->no_auto || $part_pkg->no_auto) ? 'no_auto' : '';
+      my $pass = '';
+      if ( $this_cust_pkg->separate_bill ) {
+        # if no_auto is also set, that's fine. we just need to not have
+        # invoices that are both auto and no_auto, and since the package
+        # gets an invoice all to itself, it will only be one or the other.
+        $pass = $this_cust_pkg->pkgnum;
+        if (!exists $cust_bill_pkg{$pass}) { # it may not exist yet
+          push @passes, $pass;
+          $total_setup{$pass} = do { my $z = 0; \$z };
+          $total_recur{$pass} = do { my $z = 0; \$z };
+          # it also needs its own tax context
+          $tax_engines{$pass} = FS::TaxEngine->new(
+                                  cust_main    => $self,
+                                  invoice_time => $invoice_time,
+                                  cancel       => $options{cancel},
+                                  estimate     => $options{estimate},
+                                );
+          $cust_bill_pkg{$pass} = [];
+        }
+      } elsif ( ($this_cust_pkg->no_auto || $part_pkg->no_auto) ) {
+        $pass = 'no_auto';
+      }
 
-      my $next_bill = $cust_pkg->getfield('bill') || 0;
+      my $next_bill = $this_cust_pkg->getfield('bill') || 0;
       my $error;
       # let this run once if this is the last bill upon cancellation
-      while ( $next_bill <= $time or $options{cancel} ) {
+      while ( $next_bill <= $cmp_time or $options{cancel} ) {
         $error =
           $self->_make_lines( 'part_pkg'            => $part_pkg,
-                              'cust_pkg'            => $cust_pkg,
+                              'cust_pkg'            => $this_cust_pkg,
                               'precommit_hooks'     => \@precommit_hooks,
                               'line_items'          => $cust_bill_pkg{$pass},
                               'setup'               => $total_setup{$pass},
                               'recur'               => $total_recur{$pass},
-                              'tax_matrix'          => $taxlisthash{$pass},
+                              'tax_engine'          => $tax_engines{$pass},
                               'time'                => $time,
                               'real_pkgpart'        => $real_pkgpart,
                               'options'             => \%options,
@@ -464,12 +589,12 @@ sub bill {
         last if $error;
 
         # or if we're not incrementing the bill date.
-        last if ($cust_pkg->getfield('bill') || 0) == $next_bill;
+        last if ($this_cust_pkg->getfield('bill') || 0) == $next_bill;
 
         # or if we're letting it run only once
         last if $options{cancel};
 
-        $next_bill = $cust_pkg->getfield('bill') || 0;
+        $next_bill = $this_cust_pkg->getfield('bill') || 0;
 
         #stop if -o was passed to freeside-daily
         last if $options{'one_recur'};
@@ -483,22 +608,108 @@ sub bill {
 
   } #foreach my $cust_pkg
 
-  #if the customer isn't on an automatic payby, everything can go on a single
-  #invoice anyway?
-  #if ( $cust_main->payby !~ /^(CARD|CHEK)$/ ) {
-    #merge everything into one list
-  #}
-
-  foreach my $pass (@passes) { # keys %cust_bill_pkg ) {
+  foreach my $pass (@passes) { # keys %cust_bill_pkg )
 
     my @cust_bill_pkg = _omit_zero_value_bundles(@{ $cust_bill_pkg{$pass} });
 
-    next unless @cust_bill_pkg; #don't create an invoice w/o line items
-
     warn "$me billing pass $pass\n"
            #.Dumper(\@cust_bill_pkg)."\n"
       if $DEBUG > 2;
 
+    ###
+    # process fees
+    ###
+
+    my @pending_fees = FS::FeeOrigin_Mixin->by_cust($self->custnum,
+      hashref => { 'billpkgnum' => '' }
+    );
+    warn "$me found pending fees:\n".Dumper(\@pending_fees)."\n"
+      if @pending_fees and $DEBUG > 1;
+
+    # determine whether to generate an invoice
+    my $generate_bill = scalar(@cust_bill_pkg) > 0;
+
+    foreach my $fee (@pending_fees) {
+      $generate_bill = 1 unless $fee->nextbill;
+    }
+    
+    # don't create an invoice with no line items, or where the only line 
+    # items are fees that are supposed to be held until the next invoice
+    next if !$generate_bill;
+
+    # calculate fees...
+    my @fee_items;
+    foreach my $fee_origin (@pending_fees) {
+      my $part_fee = $fee_origin->part_fee;
+
+      # check whether the fee is applicable before doing anything expensive:
+      #
+      # if the fee def belongs to a different agent, don't charge the fee.
+      # event conditions should prevent this, but just in case they don't,
+      # skip the fee.
+      if ( $part_fee->agentnum and $part_fee->agentnum != $self->agentnum ) {
+        warn "tried to charge fee#".$part_fee->feepart .
+             " on customer#".$self->custnum." from a different agent.\n";
+        next;
+      }
+      # also skip if it's disabled
+      next if $part_fee->disabled eq 'Y';
+
+      # Decide which invoice to base the fee on.
+      my $cust_bill = $fee_origin->cust_bill;
+      if (!$cust_bill) {
+        # Then link it to the current invoice. This isn't the real cust_bill
+        # object that will be inserted--in particular there are no taxes yet.
+        # If you want to charge a fee on the total invoice amount including
+        # taxes, you have to put the fee on the next invoice.
+        $cust_bill = FS::cust_bill->new({
+            'custnum'       => $self->custnum,
+            'cust_bill_pkg' => \@cust_bill_pkg,
+            'charged'       => ${ $total_setup{$pass} } +
+                               ${ $total_recur{$pass} },
+        });
+
+        # If the origin is for a specific package, then only apply the fee to
+        # line items from that package.
+        if ( my $cust_pkg = $fee_origin->cust_pkg ) {
+          my @charge_fee_on_item;
+          my $charge_fee_on_amount = 0;
+          foreach (@cust_bill_pkg) {
+            if ($_->pkgnum == $cust_pkg->pkgnum) {
+              push @charge_fee_on_item, $_;
+              $charge_fee_on_amount += $_->setup + $_->recur;
+            }
+          }
+          $cust_bill->set('cust_bill_pkg', \@charge_fee_on_item);
+          $cust_bill->set('charged', $charge_fee_on_amount);
+        }
+
+      } # $cust_bill is now set
+      # calculate the fee
+      my $fee_item = $part_fee->lineitem($cust_bill) or next;
+      # link this so that we can clear the marker on inserting the line item
+      $fee_item->set('fee_origin', $fee_origin);
+      push @fee_items, $fee_item;
+
+    }
+    
+    # add fees to the invoice
+    foreach my $fee_item (@fee_items) {
+
+      push @cust_bill_pkg, $fee_item;
+      ${ $total_setup{$pass} } += $fee_item->setup;
+      ${ $total_recur{$pass} } += $fee_item->recur;
+
+      my $part_fee = $fee_item->part_fee;
+      my $fee_location = $self->ship_location; # I think?
+      
+      my $error = $tax_engines{''}->add_sale($fee_item);
+
+      return $error if $error;
+
+    }
+
+    # XXX implementation of fees is supposed to make this go away...
     if ( scalar( grep { $_->recur && $_->recur > 0 } @cust_bill_pkg) ||
            !$conf->exists('postal_invoice-recurring_only')
        )
@@ -530,7 +741,7 @@ sub bill {
                                 'line_items'          => \@cust_bill_pkg,
                                 'setup'               => $total_setup{$pass},
                                 'recur'               => $total_recur{$pass},
-                                'tax_matrix'          => $taxlisthash{$pass},
+                                'tax_engine'          => $tax_engines{$pass},
                                 'time'                => $time,
                                 'real_pkgpart'        => $real_pkgpart,
                                 'options'             => \%postal_options,
@@ -548,21 +759,8 @@ sub bill {
 
     }
 
-    my $listref_or_error =
-      $self->calculate_taxes( \@cust_bill_pkg, $taxlisthash{$pass}, $invoice_time);
-
-    unless ( ref( $listref_or_error ) ) {
-      $dbh->rollback if $oldAutoCommit && !$options{no_commit};
-      return $listref_or_error;
-    }
-
-    foreach my $taxline ( @$listref_or_error ) {
-      ${ $total_setup{$pass} } =
-        sprintf('%.2f', ${ $total_setup{$pass} } + $taxline->setup );
-      push @cust_bill_pkg, $taxline;
-    }
-
     #add tax adjustments
+    #XXX does this work with batch tax engines?
     warn "adding tax adjustments...\n" if $DEBUG > 2;
     foreach my $cust_tax_adjustment (
       qsearch('cust_tax_adjustment', { 'custnum'    => $self->custnum,
@@ -592,17 +790,18 @@ sub bill {
 
     my $charged = sprintf('%.2f', ${ $total_setup{$pass} } + ${ $total_recur{$pass} } );
 
-    my @cust_bill = $self->cust_bill;
     my $balance = $self->balance;
-    my $previous_balance = scalar(@cust_bill)
-                             ? ( $cust_bill[$#cust_bill]->billing_balance || 0 )
-                             : 0;
 
-    $previous_balance += $cust_bill[$#cust_bill]->charged
-      if scalar(@cust_bill);
-    #my $balance_adjustments =
-    #  sprintf('%.2f', $balance - $prior_prior_balance - $prior_charged);
+    my $previous_bill = qsearchs({ 'table'     => 'cust_bill',
+                                   'hashref'   => { custnum=>$self->custnum },
+                                   'extra_sql' => 'ORDER BY _date DESC LIMIT 1',
+                                });
+    my $previous_balance =
+      $previous_bill
+        ? ( $previous_bill->billing_balance + $previous_bill->charged )
+        : 0;
 
+    $log->debug('creating the new invoice', %logopt);
     warn "creating the new invoice\n" if $DEBUG;
     #create the new invoice
     my $cust_bill = new FS::cust_bill ( {
@@ -613,12 +812,64 @@ sub bill {
       'previous_balance'    => $previous_balance,
       'invoice_terms'       => $options{'invoice_terms'},
       'cust_bill_pkg'       => \@cust_bill_pkg,
+      'pending'             => 'Y', # clear this after doing taxes
     } );
-    $error = $cust_bill->insert unless $options{no_commit};
-    if ( $error ) {
-      $dbh->rollback if $oldAutoCommit && !$options{no_commit};
-      return "can't create invoice for customer #". $self->custnum. ": $error";
+
+    if (!$options{no_commit}) {
+      # probably we ought to insert it as pending, and then rollback
+      # without ever un-pending it
+      $error = $cust_bill->insert;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit && !$options{no_commit};
+        return "can't create invoice for customer #". $self->custnum. ": $error";
+      }
+
     }
+
+    # calculate and append taxes
+    if ( ! $tax_is_batch) {
+      local $@;
+      my $arrayref = eval { $tax_engines{$pass}->calculate_taxes($cust_bill) };
+
+      if ( $@ ) {
+        $dbh->rollback if $oldAutoCommit && !$options{no_commit};
+        return $@;
+      }
+
+      # or should this be in TaxEngine?
+      my $total_tax = 0;
+      foreach my $taxline ( @$arrayref ) {
+        $total_tax += $taxline->setup;
+        $taxline->set('invnum' => $cust_bill->invnum); # just to be sure
+        push @cust_bill_pkg, $taxline; # for return_bill
+
+        if (!$options{no_commit}) {
+          my $error = $taxline->insert;
+          if ( $error ) {
+            $dbh->rollback if $oldAutoCommit;
+            return $error;
+          }
+        }
+
+      }
+
+      # add tax to the invoice amount and finalize it
+      ${ $total_setup{$pass} } = sprintf('%.2f', ${ $total_setup{$pass} } + $total_tax);
+      $charged = sprintf('%.2f', $charged + $total_tax);
+      $cust_bill->set('charged', $charged);
+      $cust_bill->set('pending', '');
+
+      if (!$options{no_commit}) {
+        my $error = $cust_bill->replace;
+        if ( $error ) {
+          $dbh->rollback if $oldAutoCommit;
+          return $error;
+        }
+      }
+
+    } # if !$tax_is_batch
+      # if it IS batch, then we'll do all this in process_tax_batch
+
     push @{$options{return_bill}}, $cust_bill if $options{return_bill};
 
   } #foreach my $pass ( keys %cust_bill_pkg )
@@ -639,254 +890,66 @@ sub bill {
 }
 
 #discard bundled packages of 0 value
+# XXX we should reconsider whether we even need this
 sub _omit_zero_value_bundles {
   my @in = @_;
 
-  my @cust_bill_pkg = ();
-  my @cust_bill_pkg_bundle = ();
-  my $sum = 0;
-  my $discount_show_always = 0;
-
+  my @out = ();
+  my @bundle = ();
+  my $discount_show_always = $conf->exists('discount-show-always');
+  my $show_this = 0;
+
+  # Sort @in the same way we do during invoice rendering, so we can identify
+  # bundles.  See FS::Template_Mixin::_items_nontax.
+  @in = sort { $a->pkgnum <=> $b->pkgnum        or
+               $a->sdate  <=> $b->sdate         or
+               ($a->pkgpart_override ? 0 : -1)  or
+               ($b->pkgpart_override ? 0 : 1)   or
+               $b->hidden cmp $a->hidden        or
+               $a->pkgpart_override <=> $b->pkgpart_override
+             } @in;
+
+  # this is a pack-and-deliver pattern. every time there's a cust_bill_pkg
+  # _without_ pkgpart_override, that's the start of the new bundle. if there's
+  # an existing bundle, and it contains a nonzero amount (or a zero amount 
+  # that's displayable anyway), push all line items in the bundle.
   foreach my $cust_bill_pkg ( @in ) {
 
-    $discount_show_always = ($cust_bill_pkg->get('discounts')
-				&& scalar(@{$cust_bill_pkg->get('discounts')})
-				&& $conf->exists('discount-show-always'));
-
-    warn "  pkgnum ". $cust_bill_pkg->pkgnum. " sum $sum, ".
-         "setup_show_zero ". $cust_bill_pkg->setup_show_zero.
-         "recur_show_zero ". $cust_bill_pkg->recur_show_zero. "\n"
-      if $DEBUG > 0;
-
-    if (scalar(@cust_bill_pkg_bundle) && !$cust_bill_pkg->pkgpart_override) {
-      push @cust_bill_pkg, @cust_bill_pkg_bundle 
-        if $sum > 0
-        || ($sum == 0 && (    $discount_show_always
-                           || grep {$_->recur_show_zero || $_->setup_show_zero}
-                                   @cust_bill_pkg_bundle
-                         )
-           );
-      @cust_bill_pkg_bundle = ();
-      $sum = 0;
+    if (scalar(@bundle) and !$cust_bill_pkg->pkgpart_override) {
+      # ship out this bundle and reset it
+      if ( $show_this ) {
+        push @out, @bundle;
+      }
+      @bundle = ();
+      $show_this = 0;
     }
 
-    $sum += $cust_bill_pkg->setup + $cust_bill_pkg->recur;
-    push @cust_bill_pkg_bundle, $cust_bill_pkg;
+    # add this item to the current bundle
+    push @bundle, $cust_bill_pkg;
 
+    # determine if it makes the bundle displayable
+    if (   $cust_bill_pkg->setup > 0
+        or $cust_bill_pkg->recur > 0
+        or $cust_bill_pkg->setup_show_zero
+        or $cust_bill_pkg->recur_show_zero
+        or ($discount_show_always 
+          and scalar(@{ $cust_bill_pkg->get('discounts')}) 
+          )
+    ) {
+      $show_this++;
+    }
   }
 
-  push @cust_bill_pkg, @cust_bill_pkg_bundle
-    if $sum > 0
-    || ($sum == 0 && (    $discount_show_always
-                       || grep {$_->recur_show_zero || $_->setup_show_zero}
-                               @cust_bill_pkg_bundle
-                     )
-       );
+  # last bundle
+  if ( $show_this) {
+    push @out, @bundle;
+  }
 
   warn "  _omit_zero_value_bundles: ". scalar(@in).
-       '->'. scalar(@cust_bill_pkg). "\n" #. Dumper(@cust_bill_pkg). "\n"
-    if $DEBUG > 2;
-
-  (@cust_bill_pkg);
-
-}
-
-=item calculate_taxes LINEITEMREF TAXHASHREF INVOICE_TIME
-
-Generates tax line items (see L<FS::cust_bill_pkg>) for this customer.
-Usually used internally by bill method B<bill>.
-
-If there is an error, returns the error, otherwise returns reference to a
-list of line items suitable for insertion.
-
-=over 4
-
-=item LINEITEMREF
-
-An array ref of the line items being billed.
-
-=item TAXHASHREF
-
-A strange beast.  The keys to this hash are internal identifiers consisting
-of the name of the tax object type, a space, and its unique identifier ( e.g.
- 'cust_main_county 23' ).  The values of the hash are listrefs.  The first
-item in the list is the tax object.  The remaining items are either line
-items or floating point values (currency amounts).
-
-The taxes are calculated on this entity.  Calculated exemption records are
-transferred to the LINEITEMREF items on the assumption that they are related.
-
-Read the source.
-
-=item INVOICE_TIME
-
-This specifies the date appearing on the associated invoice.  Some
-jurisdictions (i.e. Texas) have tax exemptions which are date sensitive.
-
-=back
-
-=cut
-
-sub calculate_taxes {
-  my ($self, $cust_bill_pkg, $taxlisthash, $invoice_time) = @_;
-
-  # $taxlisthash is a hashref
-  # keys are identifiers, values are arrayrefs
-  # each arrayref starts with a tax object (cust_main_county or tax_rate)
-  # then any cust_bill_pkg objects the tax applies to
-
-  local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
-
-  warn "$me calculate_taxes\n"
-       #.Dumper($self, $cust_bill_pkg, $taxlisthash, $invoice_time). "\n"
+       '->'. scalar(@out). "\n" #. Dumper(@out). "\n"
     if $DEBUG > 2;
 
-  my @tax_line_items = ();
-
-  # keys are tax names (as printed on invoices / itemdesc )
-  # values are arrayrefs of taxlisthash keys (internal identifiers)
-  my %taxname = ();
-
-  # keys are taxlisthash keys (internal identifiers)
-  # values are (cumulative) amounts
-  my %tax_amount = ();
-
-  # keys are taxlisthash keys (internal identifiers)
-  # values are arrayrefs of cust_bill_pkg_tax_location hashrefs
-  my %tax_location = ();
-
-  # keys are taxlisthash keys (internal identifiers)
-  # values are arrayrefs of cust_bill_pkg_tax_rate_location hashrefs
-  my %tax_rate_location = ();
-
-  # keys are taxlisthash keys (internal identifiers!)
-  # values are arrayrefs of cust_tax_exempt_pkg objects
-  my %tax_exemption;
-
-  foreach my $tax ( keys %$taxlisthash ) {
-    # $tax is a tax identifier (intersection of a tax definition record
-    # and a cust_bill_pkg record)
-    my $tax_object = shift @{ $taxlisthash->{$tax} };
-    # $tax_object is a cust_main_county or tax_rate 
-    # (with billpkgnum, pkgnum, locationnum set)
-    # the rest of @{ $taxlisthash->{$tax} } is cust_bill_pkg component objects
-    # (setup, recurring, usage classes)
-    warn "found ". $tax_object->taxname. " as $tax\n" if $DEBUG > 2;
-    warn " ". join('/', @{ $taxlisthash->{$tax} } ). "\n" if $DEBUG > 2;
-    # taxline calculates the tax on all cust_bill_pkgs in the 
-    # first (arrayref) argument, and returns a hashref of 'name' 
-    # (the line item description) and 'amount'.
-    # It also calculates exemptions and attaches them to the cust_bill_pkgs
-    # in the argument.
-    my $taxables = $taxlisthash->{$tax};
-    my $exemptions = $tax_exemption{$tax} ||= [];
-    my $taxline = $tax_object->taxline(
-                            $taxables,
-                            'custnum'      => $self->custnum,
-                            'invoice_time' => $invoice_time,
-                            'exemptions'   => $exemptions,
-                          );
-    return $taxline unless ref($taxline);
-
-    unshift @{ $taxlisthash->{$tax} }, $tax_object;
-
-    if ( $tax_object->isa('FS::cust_main_county') ) {
-      # then $taxline is a real line item
-      push @{ $taxname{ $taxline->itemdesc } }, $taxline;
-
-    } else {
-      # leave this as is for now
-
-      my $name   = $taxline->{'name'};
-      my $amount = $taxline->{'amount'};
-
-      #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 }  },
-        {
-          'taxnum'             => $tax_object->taxnum, 
-          'taxtype'            => ref($tax_object),
-          'amount'             => sprintf('%.2f', $amount ),
-          'locationtaxid'      => $tax_object->location,
-          'taxratelocationnum' => $taxratelocationnum,
-        };
-    } #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 = ();
-    warn "adding $taxname\n" if $DEBUG > 1;
-    foreach my $taxitem ( @{ $taxname{$taxname} } ) {
-      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'     => '',
-                                                 },
-                               );
-
-    my @display = ();
-    if ( $pkg_category and
-         $conf->config('invoice_latexsummary') ||
-         $conf->config('invoice_htmlsummary')
-       )
-    {
-
-      my %hash = (  'section' => $pkg_category->categoryname );
-      push @display, new FS::cust_bill_pkg_display { type => 'S', %hash };
-
-    }
-    $tax_cust_bill_pkg->set('display', \@display);
-
-    push @tax_line_items, $tax_cust_bill_pkg;
-  }
-
-  \@tax_line_items;
+  @out;
 }
 
 sub _make_lines {
@@ -896,14 +959,16 @@ sub _make_lines {
 
   my $part_pkg = $params{part_pkg} or die "no part_pkg specified";
   my $cust_pkg = $params{cust_pkg} or die "no cust_pkg specified";
+  my $cust_location = $cust_pkg->tax_location;
   my $precommit_hooks = $params{precommit_hooks} or die "no precommit_hooks specified";
   my $cust_bill_pkgs = $params{line_items} or die "no line buffer specified";
   my $total_setup = $params{setup} or die "no setup accumulator specified";
   my $total_recur = $params{recur} or die "no recur accumulator specified";
-  my $taxlisthash = $params{tax_matrix} or die "no tax accumulator specified";
   my $time = $params{'time'} or die "no time specified";
   my (%options) = %{$params{options}};
 
+  my $tax_engine = $params{tax_engine};
+
   if ( $part_pkg->freq ne '1' and ($options{'freq_override'} || 0) > 0 ) {
     # this should never happen
     die 'freq_override billing attempted on non-monthly package '.
@@ -932,7 +997,22 @@ sub _make_lines {
   my $setup = 0;
   my $unitsetup = 0;
   my @setup_discounts = ();
-  my %setup_param = ( 'discounts' => \@setup_discounts );
+  my %setup_param = ( 'discounts'     => \@setup_discounts,
+                      'real_pkgpart'  => $params{real_pkgpart}
+                    );
+  my $setup_billed_currency = '';
+  my $setup_billed_amount = 0;
+  # Conditions for setting setup date and charging the setup fee:
+  # - this is not a recurring-only billing run
+  # - and the package is not currently being canceled
+  # - and, unless we're specifically told otherwise via 'resetup':
+  #   - it doesn't already HAVE a setup date
+  #   - or a start date in the future
+  #   - and it's not suspended
+  # - and it doesn't have an expire date in the past
+  #
+  # The "disable_setup_suspended" option is now obsolete; we never set the
+  # setup date on a suspended package.
   if (     ! $options{recurring_only}
        and ! $options{cancel}
        and ( $options{'resetup'}
@@ -940,13 +1020,11 @@ sub _make_lines {
                   && ( ! $cust_pkg->start_date
                        || $cust_pkg->start_date <= $cmp_time
                      )
-                  && ( ! $conf->exists('disable_setup_suspended_pkgs')
-                       || ( $conf->exists('disable_setup_suspended_pkgs') &&
-                            ! $cust_pkg->getfield('susp')
-                          )
-                     )
+                  && ( ! $cust_pkg->getfield('susp') )
                 )
            )
+       and ( ! $cust_pkg->expire
+             || $cust_pkg->expire > $cmp_time )
      )
   {
     
@@ -959,13 +1037,30 @@ sub _make_lines {
         return "$@ running calc_setup for $cust_pkg\n"
           if $@;
 
-        $unitsetup = $cust_pkg->part_pkg->unit_setup || $setup; #XXX uuh
+        # Only increment unitsetup here if there IS a setup fee.
+        # prorate_defer_bill may cause calc_setup on a setup-stage package
+        # to return zero, and the setup fee to be charged later. (This happens
+        # when it's first billed on the prorate cutoff day. RT#31276.)
+        if ( $setup ) {
+          $unitsetup = $cust_pkg->base_setup()
+                         || $setup; #XXX uuh
+        }
+
+        if ( $setup_param{'billed_currency'} ) {
+          $setup_billed_currency = delete $setup_param{'billed_currency'};
+          $setup_billed_amount   = delete $setup_param{'billed_amount'};
+        }
     }
 
-    $cust_pkg->setfield('setup', $time)
-      unless $cust_pkg->setup;
-          #do need it, but it won't get written to the db
-          #|| $cust_pkg->pkgpart != $real_pkgpart;
+    if ( $cust_pkg->get('setup') ) {
+      # don't change it
+    } elsif ( $cust_pkg->get('start_date') ) {
+      # this allows start_date to be used to set the first bill date
+      $cust_pkg->set('setup', $cust_pkg->get('start_date'));
+    } else {
+      # if unspecified, start it right now
+      $cust_pkg->set('setup', $time);
+    }
 
     $cust_pkg->setfield('start_date', '')
       if $cust_pkg->start_date;
@@ -979,18 +1074,50 @@ sub _make_lines {
   my $recur = 0;
   my $unitrecur = 0;
   my @recur_discounts = ();
+  my $recur_billed_currency = '';
+  my $recur_billed_amount = 0;
   my $sdate;
+
+  my $override_quantity;
+
+  # Conditions for billing the recurring fee:
+  # - the package doesn't have a future start date
+  # - and it's not suspended
+  #   - unless suspend_bill is enabled on the package or package def
+  #     - but still not, if the package is on hold
+  #   - or it's suspended for a delayed cancellation
+  # - and its next bill date is in the past
+  #   - or it doesn't have a next bill date yet
+  #   - or it's a one-time charge
+  #   - or it's a CDR plan with the "bill_every_call" option
+  #   - or it's being canceled
+  # - and it doesn't have an expire date in the past (this can happen with
+  #   advance billing)
+  #   - again, unless it's being canceled
   if (     ! $cust_pkg->start_date
-       and ( ! $cust_pkg->susp || $cust_pkg->option('suspend_bill',1)
-                               || ( $part_pkg->option('suspend_bill', 1) )
-                                     && ! $cust_pkg->option('no_suspend_bill',1)
-                                  )
+       and 
+           ( ! $cust_pkg->susp
+               || ( $cust_pkg->susp != $cust_pkg->order_date
+                      && (    $cust_pkg->option('suspend_bill',1)
+                           || ( $part_pkg->option('suspend_bill', 1)
+                                 && ! $cust_pkg->option('no_suspend_bill',1)
+                              )
+                         )
+                  )
+               || $cust_pkg->is_status_delay_cancel
+           )
        and
             ( $part_pkg->freq ne '0' && ( $cust_pkg->bill || 0 ) <= $cmp_time )
          || ( $part_pkg->plan eq 'voip_cdr'
                && $part_pkg->option('bill_every_call')
             )
          || $options{cancel}
+
+       and
+          ( ! $cust_pkg->expire
+            || $cust_pkg->expire > $cmp_time
+            || $options{cancel}
+          )
   ) {
 
     # XXX should this be a package event?  probably.  events are called
@@ -1037,9 +1164,27 @@ sub _make_lines {
     return "$@ running $method for $cust_pkg\n"
       if ( $@ );
 
+    if ($recur eq 'NOTHING') {
+      # then calc_cancel (or calc_recur but that's not used) has declined to
+      # generate a recurring lineitem at all. treat this as zero, but also 
+      # try not to generate a lineitem.
+      $recur = 0;
+      $lineitems--;
+    }
+
     #base_cancel???
     $unitrecur = $cust_pkg->base_recur( \$sdate ) || $recur; #XXX uuh, better
 
+    if ( $param{'billed_currency'} ) {
+      $recur_billed_currency = delete $param{'billed_currency'};
+      $recur_billed_amount   = delete $param{'billed_amount'};
+    }
+
+    if ( $param{'override_quantity'} ) {
+      $override_quantity = $param{'override_quantity'};
+      $unitrecur = $recur / $override_quantity;
+    }
+
     if ( $increment_next_bill ) {
 
       my $next_bill;
@@ -1050,21 +1195,42 @@ sub _make_lines {
         # its frequency
         my $main_pkg_freq = $main_pkg->part_pkg->freq;
         my $supp_pkg_freq = $part_pkg->freq;
-        my $ratio = $supp_pkg_freq / $main_pkg_freq;
-        if ( $ratio != int($ratio) ) {
+        if ( $supp_pkg_freq == 0 or $main_pkg_freq == 0 ) {
           # the UI should prevent setting up packages like this, but just
           # in case
-          return "supplemental package period is not an integer multiple of main  package period";
+          return "unable to calculate supplemental package period ratio";
         }
-        $next_bill = $sdate;
-        for (1..$ratio) {
-          $next_bill = $part_pkg->add_freq( $next_bill, $main_pkg_freq );
+        my $ratio = $supp_pkg_freq / $main_pkg_freq;
+        if ( $ratio == int($ratio) ) {
+          # simple case: main package is X months, supp package is X*A months,
+          # advance supp package to where the main package will be in A cycles.
+          $next_bill = $sdate;
+          for (1..$ratio) {
+            $next_bill = $part_pkg->add_freq( $next_bill, $main_pkg_freq );
+          }
+        } else {
+          # harder case: main package is X months, supp package is Y months.
+          # advance supp package by Y months. then if they're within half a 
+          # month of each other, resync them. this may result in the period
+          # not being exactly Y months.
+          $next_bill = $part_pkg->add_freq( $sdate, $supp_pkg_freq );
+          my $main_next_bill = $main_pkg->bill;
+          if ( $main_pkg->bill <= $time ) {
+            # then the main package has not yet been billed on this cycle;
+            # predict what its bill date will be.
+            $main_next_bill =
+              $part_pkg->add_freq( $main_next_bill, $main_pkg_freq );
+          }
+          if ( abs($main_next_bill - $next_bill) < 86400*15 ) {
+            $next_bill = $main_next_bill;
+          }
         }
 
       } else {
-        # the normal case
+      # the normal case, not a supplemental package
       $next_bill = $part_pkg->add_freq($sdate, $options{freq_override} || 0);
-      return "unparsable frequency: ". $part_pkg->freq
+      return "unparsable frequency: ".
+        ($options{freq_override} || $part_pkg->freq)
         if $next_bill == -1;
       }  
   
@@ -1083,7 +1249,7 @@ sub _make_lines {
       # Add an additional setup fee at the billing stage.
       # Used for prorate_defer_bill.
       $setup += $param{'setup_fee'};
-      $unitsetup += $param{'setup_fee'};
+      $unitsetup = $cust_pkg->base_setup();
       $lineitems++;
     }
 
@@ -1093,7 +1259,7 @@ sub _make_lines {
         }
     }
 
-  }
+  } # end of recurring fee
 
   warn "\$setup is undefined" unless defined($setup);
   warn "\$recur is undefined" unless defined($recur);
@@ -1155,16 +1321,20 @@ sub _make_lines {
       push @details, @cust_pkg_detail;
 
       my $cust_bill_pkg = new FS::cust_bill_pkg {
-        'pkgnum'    => $cust_pkg->pkgnum,
-        'setup'     => $setup,
-        'unitsetup' => $unitsetup,
-        'recur'     => $recur,
-        'unitrecur' => $unitrecur,
-        'quantity'  => $cust_pkg->quantity,
-        'details'   => \@details,
-        'discounts' => [ @setup_discounts, @recur_discounts ],
-        'hidden'    => $part_pkg->hidden,
-        'freq'      => $part_pkg->freq,
+        'pkgnum'                => $cust_pkg->pkgnum,
+        'setup'                 => $setup,
+        'unitsetup'             => sprintf('%.2f', $unitsetup),
+        'setup_billed_currency' => $setup_billed_currency,
+        'setup_billed_amount'   => $setup_billed_amount,
+        'recur'                 => $recur,
+        'unitrecur'             => sprintf('%.2f', $unitrecur),
+        'recur_billed_currency' => $recur_billed_currency,
+        'recur_billed_amount'   => $recur_billed_amount,
+        'quantity'              => $override_quantity || $cust_pkg->quantity,
+        'details'               => \@details,
+        'discounts'             => [ @setup_discounts, @recur_discounts ],
+        'hidden'                => $part_pkg->hidden,
+        'freq'                  => $part_pkg->freq,
       };
 
       if ( $part_pkg->option('prorate_defer_bill',1) 
@@ -1176,7 +1346,7 @@ sub _make_lines {
         $cust_bill_pkg->sdate( $hash{last_bill} );
         $cust_bill_pkg->edate( $sdate - 86399   ); #60s*60m*24h-1
         $cust_bill_pkg->edate( $time ) if $options{cancel};
-      } else { #if ( $part_pkg->recur_temporality eq 'upcoming' ) {
+      } else { #if ( $part_pkg->recur_temporality eq 'upcoming' )
         $cust_bill_pkg->sdate( $sdate );
         $cust_bill_pkg->edate( $cust_pkg->bill );
         #$cust_bill_pkg->edate( $time ) if $options{cancel};
@@ -1191,19 +1361,9 @@ sub _make_lines {
       ###
       # handle taxes
       ###
-
-      #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
-      );
+      
+      my $error = $tax_engine->add_sale($cust_bill_pkg);
       return $error if $error;
-      #}
 
       $cust_bill_pkg->set_display(
         part_pkg     => $part_pkg,
@@ -1220,42 +1380,142 @@ 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.
+=item _transfer_balance TO_PKG [ FROM_PKGNUM ]
+
+Takes one argument, a cust_pkg object that is being billed.  This will 
+be called only if the package was created by a package change, and has
+not been billed since the package change, and package balance tracking
+is enabled.  The second argument can be an alternate package number to 
+transfer the balance from; this should not be used externally.
+
+Transfers the balance from the previous package (now canceled) to
+this package, by crediting one package and creating an invoice item for 
+the other.  Inserts the credit and returns the invoice item (so that it 
+can be added to an invoice that's being built).
+
+If the previous package was never billed, and was also created by a package
+change, then this will also transfer the balance from I<its> previous 
+package, and so on, until reaching a package that either has been billed
+or was not created by a package change.
+
+=cut
+
+my $balance_transfer_reason;
+
+sub _transfer_balance {
+  my $self = shift;
+  my $cust_pkg = shift;
+  my $from_pkgnum = shift || $cust_pkg->change_pkgnum;
+  my $from_pkg = FS::cust_pkg->by_key($from_pkgnum);
+
+  my @transfers;
+
+  # if $from_pkg is not the first package in the chain, and it was never 
+  # billed, walk back
+  if ( $from_pkg->change_pkgnum and scalar($from_pkg->cust_bill_pkg) == 0 ) {
+    @transfers = $self->_transfer_balance($cust_pkg, $from_pkg->change_pkgnum);
+  }
+
+  my $prev_balance = $self->balance_pkgnum($from_pkgnum);
+  if ( $prev_balance != 0 ) {
+    $balance_transfer_reason ||= FS::reason->new_or_existing(
+      'reason' => 'Package balance transfer',
+      'type'   => 'Internal adjustment',
+      'class'  => 'R'
+    );
+
+    my $credit = FS::cust_credit->new({
+        'custnum'   => $self->custnum,
+        'amount'    => abs($prev_balance),
+        'reasonnum' => $balance_transfer_reason->reasonnum,
+        '_date'     => $cust_pkg->change_date,
+    });
+
+    my $cust_bill_pkg = FS::cust_bill_pkg->new({
+        'setup'     => 0,
+        'recur'     => abs($prev_balance),
+        #'sdate'     => $from_pkg->last_bill, # not sure about this
+        #'edate'     => $cust_pkg->change_date,
+        'itemdesc'  => $self->mt('Previous Balance, [_1]',
+                                 $from_pkg->part_pkg->pkg),
+    });
+
+    if ( $prev_balance > 0 ) {
+      # credit the old package, charge the new one
+      $credit->set('pkgnum', $from_pkgnum);
+      $cust_bill_pkg->set('pkgnum', $cust_pkg->pkgnum);
+    } else {
+      # the reverse
+      $credit->set('pkgnum', $cust_pkg->pkgnum);
+      $cust_bill_pkg->set('pkgnum', $from_pkgnum);
+    }
+    my $error = $credit->insert;
+    die "error transferring package balance from #".$from_pkgnum.
+        " to #".$cust_pkg->pkgnum.": $error\n" if $error;
+
+    push @transfers, $cust_bill_pkg;
+  } # $prev_balance != 0
+
+  return @transfers;
+}
+
+#### vestigial code ####
+
+=item handle_taxes TAXLISTHASH CUST_BILL_PKG [ OPTIONS ]
+
+This is _handle_taxes.  It's called once for each cust_bill_pkg generated
+from _make_lines.
+
+TAXLISTHASH is a hashref 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.
+
+OPTIONS may include:
+- part_item: a part_pkg or part_fee object to be used as the package/fee 
+  definition.
+- location: a cust_location to be used as the billing location.
+- cancel: true if this package is being billed on cancellation.  This 
+  allows tax to be calculated on usage charges only.
+
+If not supplied, part_item will be inferred from the pkgnum or feepart of the
+cust_bill_pkg, and location from the pkgnum (or, for fees, the invnum and 
+the customer's default service location).
+
+This method will also calculate exemptions for any taxes that apply to the
+line item (using the C<set_exemptions> method of L<FS::cust_bill_pkg>) and
+attach them.  This is the only place C<set_exemptions> is called in normal
+invoice processing.
+
+=cut
 
 sub _handle_taxes {
   my $self = shift;
-  my $part_pkg = shift;
   my $taxlisthash = shift;
   my $cust_bill_pkg = shift;
-  my $cust_pkg = shift;
-  my $invoice_time = shift;
-  my $real_pkgpart = shift;
-  my $options = shift;
+  my %options = @_;
+
+  # at this point I realize that we have enough information to infer all this
+  # stuff, instead of passing around giant honking argument lists
+  my $location = $options{location} || $cust_bill_pkg->tax_location;
+  my $part_item = $options{part_item} || $cust_bill_pkg->part_X;
 
   local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
 
   return if ( $self->payby eq 'COMP' ); #dubious
 
-  if ( $conf->exists('enable_taxproducts')
-       && ( scalar($part_pkg->part_pkg_taxoverride)
-            || $part_pkg->has_taxproduct
+  if ( $conf->config('enable_taxproducts')
+       && ( scalar($part_item->part_pkg_taxoverride)
+            || $part_item->has_taxproduct
           )
      )
     {
@@ -1265,94 +1525,73 @@ sub _handle_taxes {
     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;
+    my $usage = $cust_bill_pkg->usage || 0;
+    push @classes, $cust_bill_pkg->usage_classes if $usage;
+    push @classes, 'setup' if $cust_bill_pkg->setup and !$options{cancel};
+    push @classes, 'recur' if ($cust_bill_pkg->recur - $usage)
+        and !$options{cancel};
+    # that's better--probably don't even need $options{cancel} now
+    # but leave it for now, just to be safe
+    #
+    # About $options{cancel}: This protects against charging per-line or
+    # per-customer or other flat-rate surcharges on a package that's being
+    # billed on cancellation (which is an out-of-cycle bill and should only
+    # have usage charges).  See RT#29443.
+
+    # customer exemption is now handled in the 'taxline' method
+    #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 ) {
+    #$exempt = ($exempt eq 'Y') ? 'Y' : '';
+    #
+    #if ( !$exempt ) {
+
+    unless (exists $taxes{''}) {
+      # unsure what purpose this serves, but last time I deleted something
+      # from here just because I didn't see the point, it actually did
+      # something important.
+      my $err_or_ref = $self->_gather_taxes($part_item, '', $location);
+      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;
-      }
+    # NO DISINTEGRATIONS.
+    # my %tax_cust_bill_pkg = $cust_bill_pkg->disintegrate;
+    #
+    # do not call taxline() with any argument except the entire set of
+    # cust_bill_pkgs on an invoice that are eligible for the tax.
 
-      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;
-      }
+    # only calculate exemptions once for each tax rate, even if it's used
+    # for multiple classes
+    my %tax_seen = ();
+ 
+    foreach my $class (@classes) {
+      my $err_or_ref = $self->_gather_taxes($part_item, $class, $location);
+      return $err_or_ref unless ref($err_or_ref);
+      my @taxes = @$err_or_ref;
 
-    }
+      next if !@taxes;
 
-    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.
+        my $tax_id = ref( $tax ). ' '. $tax->taxnum;
+        # $taxlisthash: keys are tax identifiers ('FS::tax_rate 123456').
         # 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;
-
-      }
+        # or tax_rate), then the cust_bill_pkg object that the 
+        # tax applies to, then the tax class (setup, recur, usage classnum).
+        $taxlisthash->{ $tax_id } ||= [ $tax ];
+        push @{ $taxlisthash->{ $tax_id  } }, $cust_bill_pkg, $class;
+
+        # determine any exemptions that apply
+        if (!$tax_seen{$tax_id}) {
+          $cust_bill_pkg->set_exemptions( $tax, custnum => $self->custnum );
+          $tax_seen{$tax_id} = 1;
+        }
 
-      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};
+        # tax on tax will be done later, when we actually create the tax
+        # line items
 
-        }
       }
     }
 
@@ -1364,10 +1603,9 @@ sub _handle_taxes {
     # because we need to record that fact.
 
     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;
+    $taxhash{'taxclass'} = $part_item->taxclass;
 
     warn "taxhash:\n". Dumper(\%taxhash) if $DEBUG > 2;
 
@@ -1393,6 +1631,7 @@ sub _handle_taxes {
     foreach (@taxes) {
       my $tax_id = 'cust_main_county '.$_->taxnum;
       $taxlisthash->{$tax_id} ||= [ $_ ];
+      $cust_bill_pkg->set_exemptions($_, custnum => $self->custnum);
       push @{ $taxlisthash->{$tax_id} }, $cust_bill_pkg;
     }
 
@@ -1400,52 +1639,33 @@ sub _handle_taxes {
   '';
 }
 
+=item _gather_taxes PART_ITEM CLASS CUST_LOCATION
+
+Internal method used with vendor-provided tax tables.  PART_ITEM is a part_pkg
+or part_fee (which will define the tax eligibility of the product), CLASS is
+'setup', 'recur', null, or a C<usage_class> number, and CUST_LOCATION is the 
+location where the service was provided (or billed, depending on 
+configuration).  Returns an arrayref of L<FS::tax_rate> objects that 
+can apply to this line item.
+
+=cut
+
 sub _gather_taxes {
   my $self = shift;
-  my $part_pkg = shift;
+  my $part_item = shift;
   my $class = shift;
-  my $cust_pkg = shift;
+  my $location = shift;
 
   local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
 
-  my $geocode;
-  if ( $cust_pkg->locationnum && $conf->exists('tax-pkg_address') ) {
-    $geocode = $cust_pkg->cust_location->geocode('cch');
-  } else {
-    $geocode = $self->geocode('cch');
-  }
-
-  my @taxes = ();
+  my $geocode = $location->geocode('cch');
 
-  my @taxclassnums = map { $_->taxclassnum }
-                     $part_pkg->part_pkg_taxoverride($class);
-
-  unless (@taxclassnums) {
-    @taxclassnums = map { $_->taxclassnum }
-                    grep { $_->taxable eq 'Y' }
-                    $part_pkg->part_pkg_taxrate('cch', $geocode, $class);
-  }
-  warn "Found taxclassnum values of ". join(',', @taxclassnums)
-    if $DEBUG;
-
-  my $extra_sql =
-    "AND (".
-    join(' OR ', map { "taxclassnum = $_" } @taxclassnums ). ")";
-
-  @taxes = qsearch({ 'table' => 'tax_rate',
-                     'hashref' => { 'geocode' => $geocode, },
-                     'extra_sql' => $extra_sql,
-                  })
-    if scalar(@taxclassnums);
-
-  warn "Found taxes ".
-       join(',', map{ ref($_). " ". $_->get($_->primary_key) } @taxes). "\n" 
-   if $DEBUG;
-
-  [ @taxes ];
+  [ $part_item->tax_rates('cch', $geocode, $class) ]
 
 }
 
+#### end vestigial code ####
+
 =item collect [ HASHREF | OPTION => VALUE ... ]
 
 (Attempt to) collect money for this customer's outstanding invoices (see
@@ -1572,7 +1792,8 @@ sub retry_realtime {
 
   #a little false laziness w/due_cust_event (not too bad, really)
 
-  my $join = FS::part_event_condition->join_conditions_sql;
+  # I guess this is always as of now?
+  my $join = FS::part_event_condition->join_conditions_sql('', 'time' => time);
   my $order = FS::part_event_condition->order_conditions_sql;
   my $mine = 
   '( '
@@ -1885,7 +2106,8 @@ sub due_cust_event {
 
       #some false laziness w/Cron::bill bill_where
 
-      my $join  = FS::part_event_condition->join_conditions_sql( $eventtable);
+      my $join  = FS::part_event_condition->join_conditions_sql( $eventtable,
+        'time' => $opt{'time'});
       my $where = FS::part_event_condition->where_conditions_sql($eventtable,
         'time'=>$opt{'time'},
       );
@@ -1924,7 +2146,8 @@ sub due_cust_event {
       my $pkey = $object->primary_key;
       $cross_where = "$eventtable.$pkey = ". $object->$pkey();
 
-      my $join = FS::part_event_condition->join_conditions_sql( $eventtable );
+      my $join = FS::part_event_condition->join_conditions_sql( $eventtable,
+        'time' => $opt{'time'});
       my $extra_sql =
         FS::part_event_condition->where_conditions_sql( $eventtable,
                                                         'time'=>$opt{'time'}
@@ -2037,6 +2260,7 @@ sub due_cust_event {
 =item apply_payments_and_credits [ OPTION => VALUE ... ]
 
 Applies unapplied payments and credits.
+Payments with the no_auto_apply flag set will not be applied.
 
 In most cases, this new method should be used in place of sequential
 apply_payments and apply_credits methods.
@@ -2179,6 +2403,7 @@ sub apply_credits {
 
 Applies (see L<FS::cust_bill_pay>) unapplied payments (see L<FS::cust_pay>)
 to outstanding invoice balances in chronological order.
+Payments with the no_auto_apply flag set will not be applied.
 
  #and returns the value of any remaining unapplied payments.
 
@@ -2208,13 +2433,9 @@ sub apply_payments {
 
   #return 0 unless
 
-  my @payments = sort { $b->_date <=> $a->_date }
-                 grep { $_->unapplied > 0 }
-                 $self->cust_pay;
+  my @payments = grep { !$_->no_auto_apply } $self->unapplied_cust_pay;
 
-  my @invoices = sort { $a->_date <=> $b->_date}
-                 grep { $_->owed > 0 }
-                 $self->cust_bill;
+  my @invoices = $self->open_cust_bill;
 
   if ( $conf->exists('pkg-balances') ) {
     # limit @payments to those w/ a pkgnum grepped from $self
@@ -2288,8 +2509,6 @@ sub apply_payments {
     bill
       (do_cust_event pre-bill)
       _make_lines
-        _handle_taxes
-          (vendor-only) _gather_taxes
       _omit_zero_value_bundles
       calculate_taxes