RT#31594: Unapplied payment issues
[freeside.git] / FS / FS / cust_main / Billing.pm
index d0fd12c..908f486 100644 (file)
@@ -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,7 @@ use FS::cust_bill_pkg_tax_rate_location;
 use FS::part_event;
 use FS::part_event_condition;
 use FS::pkg_category;
-use FS::cust_event_fee;
+use FS::FeeOrigin_Mixin;
 use FS::Log;
 
 # 1 is mostly method/subroutine entry and options
@@ -106,8 +107,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;
 
@@ -123,6 +125,7 @@ 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";
@@ -131,6 +134,7 @@ sub bill_and_collect {
     else                                                     { warn   $error; }
   }
 
+  $log->debug('suspending adjourned packages', %logopt);
   $error = $self->suspend_adjourned_pkgs( $actual_time );
   if ( $error ) {
     $error = "Error adjourning custnum ". $self->custnum. ": $error";
@@ -139,6 +143,7 @@ sub bill_and_collect {
     else                                                     { warn   $error; }
   }
 
+  $log->debug('unsuspending resumed packages', %logopt);
   $error = $self->unsuspend_resumed_pkgs( $actual_time );
   if ( $error ) {
     $error = "Error resuming custnum ".$self->custnum. ": $error";
@@ -148,6 +153,7 @@ sub bill_and_collect {
   }
 
   $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";
@@ -157,6 +163,7 @@ sub bill_and_collect {
   }
 
   $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";
@@ -165,10 +172,11 @@ sub bill_and_collect {
     else                                                     { warn   $error; }
   }
 
-  $job->update_statustext('70,running collection events') if $job;
   unless ( $conf->exists('cancelled_cust-noevents')
            && ! $self->num_ncancelled_pkgs
   ) {
+    $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";
@@ -177,8 +185,9 @@ sub bill_and_collect {
       else                                                   { warn   $error; }
     }
   }
+
   $job->update_statustext('100,finished') if $job;
-  $log->debug('finish', object => $self, agentnum => $self->agentnum);
+  $log->debug('finish', %logopt);
 
   '';
 
@@ -371,7 +380,10 @@ sub bill {
   return '' if $self->payby eq 'COMP';
 
   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;
 
@@ -400,11 +412,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;
 
@@ -420,6 +434,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;
 
@@ -450,6 +465,7 @@ sub bill {
 
     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... ?
@@ -481,13 +497,31 @@ sub bill {
       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 $pass = ($cust_pkg->no_auto || $part_pkg->no_auto) ? 'no_auto' : '';
+      my $pass = '';
+      if ( $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 = $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 };
+          $taxlisthash{$pass} = {};
+          $cust_bill_pkg{$pass} = [];
+        }
+      } elsif ( ($cust_pkg->no_auto || $part_pkg->no_auto) ) {
+        $pass = 'no_auto';
+      }
 
       my $next_bill = $cust_pkg->getfield('bill') || 0;
       my $error;
@@ -529,13 +563,7 @@ 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} });
 
@@ -547,17 +575,17 @@ sub bill {
     # process fees
     ###
 
-    my @pending_event_fees = FS::cust_event_fee->by_cust($self->custnum,
+    my @pending_fees = FS::FeeOrigin_Mixin->by_cust($self->custnum,
       hashref => { 'billpkgnum' => '' }
     );
-    warn "$me found pending fee events:\n".Dumper(\@pending_event_fees)."\n"
-      if @pending_event_fees and $DEBUG > 1;
+    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 $event_fee (@pending_event_fees) {
-      $generate_bill = 1 unless $event_fee->nextbill;
+    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 
@@ -566,38 +594,11 @@ sub bill {
 
     # calculate fees...
     my @fee_items;
-    foreach my $event_fee (@pending_event_fees) {
-      my $object = $event_fee->cust_event->cust_X;
-      my $part_fee = $event_fee->part_fee;
-      my $cust_bill;
-      if ( $object->isa('FS::cust_main')
-           or $object->isa('FS::cust_pkg')
-           or $object->isa('FS::cust_pay_batch') )
-      {
-        # Not 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 this is a package event, only apply the fee to line items 
-        # from that package.
-        if ($object->isa('FS::cust_pkg')) {
-          $cust_bill->set('cust_bill_pkg', 
-            [ grep  { $_->pkgnum == $object->pkgnum } @cust_bill_pkg ]
-          );
-        }
+    foreach my $fee_origin (@pending_fees) {
+      my $part_fee = $fee_origin->part_fee;
 
-      } elsif ( $object->isa('FS::cust_bill') ) {
-        # simple case: applying the fee to a previous invoice (late fee, 
-        # etc.)
-        $cust_bill = $object;
-      }
+      # 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.
@@ -608,10 +609,41 @@ sub bill {
       }
       # 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('cust_event_fee', $event_fee);
+      $fee_item->set('fee_origin', $fee_origin);
       push @fee_items, $fee_item;
 
     }
@@ -630,6 +662,7 @@ sub bill {
         $taxlisthash{$pass},
         $fee_item,
         location => $fee_location
+        # probably not right to pass cancel => 1 for fees
       );
       return $error if $error;
 
@@ -729,15 +762,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_bill = $cust_bill[-1] if @cust_bill;
-    my $previous_balance = 0;
-    if ( $previous_bill ) {
-      $previous_balance = $previous_bill->billing_balance 
-                        + $previous_bill->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 ( {
@@ -868,7 +904,9 @@ sub calculate_taxes {
   # $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
+  # then a cust_bill_pkg object the tax applies to, then the charge class
+  # on that object (setup, recur, a usage class number, or '')
+  # For internal taxes the charge class is always undef.
 
   local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
 
@@ -876,88 +914,155 @@ sub calculate_taxes {
        #.Dumper($self, $cust_bill_pkg, $taxlisthash, $invoice_time). "\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 $custnum = $self->custnum;
+  # The main tax accumulator.  One bin for each tax name (itemdesc).
+  # For each subdivision of tax under this name, push a cust_bill_pkg item 
+  # for the calculated tax into the arrayref.
+  # keys are tax names
+  # values are arrayrefs of tax lines
   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!)
+  # keys are taxlisthash keys
   # 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} };
+  # For tax on tax calculation, we need to remember which taxable items 
+  # (and charge classes) had which taxes applied to them.
+  #
+  # keys are cust_bill_pkg objects (taxable items)
+  # values are hashrefs
+  #   keys are charge classes
+  #   values are hashrefs
+  #     keys are taxnums (in tax_rate only; cust_main_county doesn't use this)
+  #     values are the taxlines generated for those taxes
+  tie my %item_has_tax, 'Tie::RefHash', 
+    map { $_ => {} } @$cust_bill_pkg;
+
+  foreach my $tax_id ( keys %$taxlisthash ) {
+    # $tax_id: the identifier of the tax we are calculating in this pass
+
+    my $taxables = $taxlisthash->{$tax_id};
+    my $tax_object = shift @$taxables;
+    my $taxnum = $tax_object->taxnum;
     # $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;
+    # the rest of @{ $taxlisthash->{$tax_id} } is cust_bill_pkg objects,
+    # optionally followed by their charge classes.
+    warn "found ". $tax_object->taxname. " as $tax_id\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
+    # first (arrayref) argument.
+    #
+    # Note that non-monthly exemptions have already been calculated and 
+    # attached to the items.  Monthly exemptions will be attached in this
+    # step.
+    my $exemptions = $tax_exemption{$tax_id} ||= [];
+    if ( $tax_object->isa('FS::tax_rate') ) { # EXTERNAL TAXES
+      # STILL have tax_rate-specific crap in here...
+      my @taxlines = $tax_object->taxline( $taxables,
+                              'custnum'      => $custnum,
+                              'invoice_time' => $invoice_time,
+                              'exemptions'   => $exemptions,
+                              );
+      next if !@taxlines;
+      if (!ref $taxlines[0]) {
+        # it's an error string
+        warn "error evaluating $tax_id on custnum $custnum\n";
+        return $taxlines[0];
+      }
+      foreach my $taxline (@taxlines) {
+        push @{ $taxname{ $taxline->itemdesc } }, $taxline;
+        my $link = $taxline->get('cust_bill_pkg_tax_rate_location')->[0];
+        my $taxable_item = $link->taxable_cust_bill_pkg;
+        $item_has_tax{$taxable_item}{$taxline->_class}{$taxnum} = $taxline;
+      }
+
+    } else { # INTERNAL TAXES
+      # we can do this in a single taxline, because it's not stupid
+
+      my $taxline =  $tax_object->taxline( $taxables,
+                        'custnum'      => $custnum,
+                        'invoice_time' => $invoice_time,
+                        'exemptions'   => $exemptions,
+                      );
+      next if !$taxline;
+      if (!ref $taxline) {
+        # it's an error string
+        warn "error evaluating $tax_id on custnum $custnum\n";
+        return $taxline;
+      }
+      # if the calculated tax is zero, don't even keep it
+      next if $taxline->setup < 0.001;
       push @{ $taxname{ $taxline->itemdesc } }, $taxline;
+    }
+  }
+  $DB::single = 1; # XXX
 
-    } 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
+  # all first-tier taxes are calculated.  now for tax on tax:
+
+  foreach my $taxable_item ( @$cust_bill_pkg ) {
+    # taxes that apply to this item
+    my $this_has_tax = $item_has_tax{$taxable_item};
+
+    my $location = $taxable_item->tax_location;
+
+    foreach my $charge_class (keys %$this_has_tax) {
+      # taxes that apply to this item and charge class
+      my $this_class_has_tax = $this_has_tax->{$charge_class};
+      foreach my $taxnum (keys %$this_class_has_tax) {
+
+        # for each tax item that was calculated in phase 1, get the 
+        # tax definition
+        my $tax_object = FS::tax_rate->by_key($taxnum);
+        # and find all taxes that apply to it in this location
+        my @tot = $tax_object->tax_on_tax( $location );
+        next if !@tot;
+        warn "found possible taxed taxnum $taxnum\n"
+          if $DEBUG > 2;
+        # Calculate ToT separately for each taxable item and class, and only 
+        # if _that class on the item_ is already taxed under the ToT.  This is
+        # counterintuitive.
+        # See RT#5243 and RT#36380.
+        foreach my $tot (@tot) {
+          my $totnum = $tot->taxnum;
+          warn "checking taxnum $totnum which we call ". $tot->taxname ."\n"
+            if $DEBUG > 2;
+          # note: if the _null class_ on this item is taxed under the ToT, 
+          # then this specific class is taxed also (because null class 
+          # includes all classes) and so ToT is applicable.
+          if (
+                exists $this_class_has_tax->{ $totnum }
+             or exists $this_has_tax->{''}{ $totnum }
+          ) {
+
+            warn "calculating tax on tax: taxnum $totnum on $taxnum\n"
+              if $DEBUG;
+            my @taxlines = $tot->taxline(
+                              $this_class_has_tax->{ $taxnum }, # the first-stage tax
+                              'custnum'       => $custnum,
+                              'invoice_time'  => $invoice_time,
+                             );
+            next if (!@taxlines); # it didn't apply after all
+            if (!ref($taxlines[0])) {
+              warn "error evaluating taxnum $totnum TOT on custnum $custnum\n";
+              return $taxlines[0];
+            }
+            foreach my $taxline (@taxlines) {
+              push @{ $taxname{ $taxline->itemdesc } }, $taxline;
+            }
+          } # if $has_tax
+        } # foreach my $tot (tax-on-tax rate definition)
+      } # foreach $taxnum (first-tier rate definition)
+    } # foreach $charge_class
+  } # foreach $taxable_item
 
   #consolidate and create tax line items
   warn "consolidating and generating...\n" if $DEBUG > 2;
+  my %final_tax_items; # taxname => item
   foreach my $taxname ( keys %taxname ) {
     my @cust_bill_pkg_tax_location;
     my @cust_bill_pkg_tax_rate_location;
@@ -975,22 +1080,23 @@ sub calculate_taxes {
     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);
+      next if $taxitem->get('setup') == 0;
+      # if ( ref($taxitem) eq 'FS::cust_bill_pkg' )  # always true
+      # 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;
+      my @links = @{
+        $taxitem->get('cust_bill_pkg_tax_location') ||
+        $taxitem->get('cust_bill_pkg_tax_rate_location') ||
+        []
+      };
+      foreach my $link ( @links ) {
+        $link->set('tax_cust_bill_pkg', $tax_cust_bill_pkg);
+        if ($link->isa('FS::cust_bill_pkg_tax_location')) {
           push @cust_bill_pkg_tax_location, $link;
+        } elsif ($link->isa('FS::cust_bill_pkg_tax_rate_location')) {
+          push @cust_bill_pkg_tax_rate_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;
@@ -1018,10 +1124,21 @@ sub calculate_taxes {
     }
     $tax_cust_bill_pkg->set('display', \@display);
 
-    push @tax_line_items, $tax_cust_bill_pkg;
+    $final_tax_items{$taxname} = $tax_cust_bill_pkg;
+  } # foreach $taxname
+  
+  # fix ToT backlinks for taxes that have been consolidated
+  # (has to be done in a separate pass)
+  foreach my $tax_item (values %final_tax_items) {
+    foreach my $taxable_link (@{ $tax_item->cust_bill_pkg_tax_rate_location }) {
+      my $taxed_item = $taxable_link->taxable_cust_bill_pkg;
+      next if $taxed_item->pkgnum > 0; # primary taxes
+      my $taxname = $taxed_item->itemdesc;
+      $taxable_link->set('taxable_cust_bill_pkg', $final_tax_items{ $taxname });
+    }
   }
 
-  \@tax_line_items;
+  [ values %final_tax_items ]
 }
 
 sub _make_lines {
@@ -1068,7 +1185,19 @@ 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}
+                    );
+  # 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
+  #
+  # The last condition used to check the "disable_setup_suspended" option but 
+  # that's obsolete. We now never set the setup date on a suspended package.
   if (     ! $options{recurring_only}
        and ! $options{cancel}
        and ( $options{'resetup'}
@@ -1076,12 +1205,8 @@ 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') )
+              )
            )
      )
   {
@@ -1117,10 +1242,17 @@ sub _make_lines {
   my @recur_discounts = ();
   my $sdate;
   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'
@@ -1173,6 +1305,14 @@ 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
 
@@ -1328,7 +1468,8 @@ sub _make_lines {
       # handle taxes
       ###
 
-      my $error = $self->_handle_taxes( $taxlisthash, $cust_bill_pkg );
+      my $error = $self->_handle_taxes( $taxlisthash, $cust_bill_pkg,
+        cancel => $options{cancel} );
       return $error if $error;
 
       $cust_bill_pkg->set_display(
@@ -1448,11 +1589,18 @@ 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 {
@@ -1482,86 +1630,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;
-    push @classes, 'setup' if $cust_bill_pkg->setup;
-    push @classes, 'recur' if $cust_bill_pkg->recur;
-
-    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_item, $class, $location);
-        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_item, '', $location);
-        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; # grrr
-    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;
-        # $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;
-
-      }
-
-      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( $location ) ) {
-          my $totname = ref( $tot ). ' '. $tot->taxnum;
+        # 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 "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;
-          # calculate the tax amount that the tax_on_tax will apply to
-          my $hashref_or_error = 
-            $tax_object->taxline( $localtaxlisthash{$tax} );
-          return $hashref_or_error
-            unless ref($hashref_or_error);
-          
-          # and append it to the list of taxable items
-          $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
 
-        }
       }
     }
 
@@ -1601,6 +1736,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;
     }
 
@@ -1759,7 +1895,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 = 
   '( '
@@ -2072,7 +2209,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'},
       );
@@ -2111,7 +2249,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'}
@@ -2224,6 +2363,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.
@@ -2366,6 +2506,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.
 
@@ -2395,13 +2536,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