projects / freeside.git / blobdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | inline | side by side
spacing, RT#83503
[freeside.git] / FS / FS / cust_main / Billing.pm
diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index 9e2082f..47cbbf1 100644 (file)
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -1,6 +1,7 @@
 package FS::cust_main::Billing;
 
 use strict;
 package FS::cust_main::Billing;
 
 use strict;
+use feature 'state';
 use vars qw( $conf $DEBUG $me );
 use Carp;
 use Data::Dumper;
 use vars qw( $conf $DEBUG $me );
 use Carp;
 use Data::Dumper;
@@ -8,6 +9,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 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;
 use FS::cust_bill;
 use FS::cust_bill_pkg;
 use FS::cust_bill_pkg_display;
@@ -21,9 +23,10 @@ use FS::cust_bill_pkg_tax_rate_location;
 use FS::part_event;
 use FS::part_event_condition;
 use FS::pkg_category;
 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;
 use FS::TaxEngine;
 use FS::Log;
 use FS::TaxEngine;
+use FS::Misc::Savepoint;
 
 # 1 is mostly method/subroutine entry and options
 # 2 traces progress of some operations
 
 # 1 is mostly method/subroutine entry and options
 # 2 traces progress of some operations
@@ -56,7 +59,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.
 
 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:
 "fatal" flag below).
 
 Options are passed as name-value pairs.  Currently available options are:
@@ -130,8 +133,7 @@ sub bill_and_collect {
   if ( $error ) {
     $error = "Error expiring custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
   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);
   }
 
   $log->debug('suspending adjourned packages', %logopt);
@@ -139,8 +141,7 @@ sub bill_and_collect {
   if ( $error ) {
     $error = "Error adjourning custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
   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);
   }
 
   $log->debug('unsuspending resumed packages', %logopt);
@@ -148,8 +149,42 @@ sub bill_and_collect {
   if ( $error ) {
     $error = "Error resuming custnum ".$self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
   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; }
+  }
+
+  my $tax_district_method = $conf->config('tax_district_method');
+  if ( $tax_district_method && $tax_district_method eq 'wa_sales' ) {
+    # When using Washington State Sales Tax Districts,
+    # Bail out of billing customer if sales tax district for location is missing
+
+    $log->debug('checking cust_location tax districts', %logopt);
+
+    if (
+      my @cust_locations_missing_district =
+        $self->cust_locations_missing_district
+    ) {
+      $error = sprintf
+        'cust_location missing tax district: '.
+        join( ', ' => (
+          map(
+            {
+              sprintf
+                'locationnum(%s) %s %s %s %s',
+                 $_->locationnum,
+                 $_->address1,
+                 $_->city,
+                 $_->state,
+                 $_->zip
+            }
+            @cust_locations_missing_district
+          )
+        ));
+    }
+  }
+  if ( $error ) {
+    $error = "Error calculating taxes ".$self->custnum. ": $error";
+    if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
+    else                                                     { die    $error; }
   }
 
   $job->update_statustext('20,billing packages') if $job;
   }
 
   $job->update_statustext('20,billing packages') if $job;
@@ -158,8 +193,7 @@ sub bill_and_collect {
   if ( $error ) {
     $error = "Error billing custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
   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;
   }
 
   $job->update_statustext('50,applying payments and credits') if $job;
@@ -168,17 +202,13 @@ sub bill_and_collect {
   if ( $error ) {
     $error = "Error applying custnum ". $self->custnum. ": $error";
     if    ( $options{fatal} && $options{fatal} eq 'return' ) { return $error; }
   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; }
   }
 
   # In a batch tax environment, do not run collection if any pending 
   # invoices were created.  Collection will run after the next tax batch.
   }
 
   # 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' })
-     )
-  {
+  state $is_batch_tax = FS::TaxEngine->new->info->{batch} ? 1 : 0;
+  if ( $is_batch_tax && $self->pending_invoice_count ) {
     warn "skipped collection for custnum ".$self->custnum.
          " due to pending invoices\n" if $DEBUG;
   } elsif ( $conf->exists('cancelled_cust-noevents')
     warn "skipped collection for custnum ".$self->custnum.
          " due to pending invoices\n" if $DEBUG;
   } elsif ( $conf->exists('cancelled_cust-noevents')
@@ -194,8 +224,7 @@ sub bill_and_collect {
     if ( $error ) {
       $error = "Error collecting custnum ". $self->custnum. ": $error";
       if    ($options{fatal} && $options{fatal} eq 'return') { return $error; }
     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; }
     }
   }
 
     }
   }
 
@@ -215,9 +244,11 @@ sub cancel_expired_pkgs {
 
   my @errors = ();
 
 
   my @errors = ();
 
+  my @really_cancel_pkgs = ();
+  my @cancel_reasons = ();
+
   CUST_PKG: foreach my $cust_pkg ( @cancel_pkgs ) {
     my $cpr = $cust_pkg->last_cust_pkg_reason('expire');
   CUST_PKG: foreach my $cust_pkg ( @cancel_pkgs ) {
     my $cpr = $cust_pkg->last_cust_pkg_reason('expire');
-    my $error;
 
     if ( $cust_pkg->change_to_pkgnum ) {
 
 
     if ( $cust_pkg->change_to_pkgnum ) {
 
@@ -227,19 +258,28 @@ sub cancel_expired_pkgs {
                       $cust_pkg->change_to_pkgnum.'; not expiring';
         next CUST_PKG;
       }
                       $cust_pkg->change_to_pkgnum.'; not expiring';
         next CUST_PKG;
       }
-      $error = $cust_pkg->change( 'cust_pkg'        => $new_pkg,
-                                  'unprotect_svcs'  => 1 );
-      $error = '' if ref $error eq 'FS::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
 
     } else { # just cancel it
-       $error = $cust_pkg->cancel($cpr ? ( 'reason'        => $cpr->reasonnum,
-                                           'reason_otaker' => $cpr->otaker,
-                                           'time'          => $time,
-                                         )
-                                       : ()
-                                 );
+
+      push @really_cancel_pkgs, $cust_pkg;
+      push @cancel_reasons, $cpr;
+
     }
     }
-    push @errors, 'pkgnum '.$cust_pkg->pkgnum.": $error" if $error;
+  }
+
+  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);
   }
 
   join(' / ', @errors);
@@ -378,6 +418,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.
 
 
 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
 =item invoice_terms
 
 Optional terms to be printed on this invoice.  Otherwise, customer-specific
@@ -390,7 +436,7 @@ terms or the default terms are used.
 sub bill {
   my( $self, %options ) = @_;
 
 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');
 
   local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
   my $log = FS::Log->new('FS::cust_main::Billing::bill');
@@ -473,7 +519,8 @@ sub bill {
   foreach (@passes) {
     $tax_engines{$_} = FS::TaxEngine->new(cust_main    => $self,
                                           invoice_time => $invoice_time,
   foreach (@passes) {
     $tax_engines{$_} = FS::TaxEngine->new(cust_main    => $self,
                                           invoice_time => $invoice_time,
-                                          cancel       => $options{cancel}
+                                          cancel       => $options{cancel},
+                                          estimate     => $options{estimate},
                                          );
     $tax_is_batch ||= $tax_engines{$_}->info->{batch};
   }
                                          );
     $tax_is_batch ||= $tax_engines{$_}->info->{batch};
   }
@@ -518,21 +565,50 @@ 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;
       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 ) {
 
     }
 
     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 <= $cmp_time or $options{cancel} ) {
         $error =
           $self->_make_lines( 'part_pkg'            => $part_pkg,
       my $error;
       # let this run once if this is the last bill upon cancellation
       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},
                               'precommit_hooks'     => \@precommit_hooks,
                               'line_items'          => $cust_bill_pkg{$pass},
                               'setup'               => $total_setup{$pass},
@@ -547,12 +623,12 @@ sub bill {
         last if $error;
 
         # or if we're not incrementing the bill date.
         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};
 
 
         # 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'};
 
         #stop if -o was passed to freeside-daily
         last if $options{'one_recur'};
@@ -566,13 +642,7 @@ sub bill {
 
   } #foreach my $cust_pkg
 
 
   } #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} });
 
 
     my @cust_bill_pkg = _omit_zero_value_bundles(@{ $cust_bill_pkg{$pass} });
 
@@ -584,17 +654,17 @@ sub bill {
     # process fees
     ###
 
     # 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' => '' }
     );
       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;
 
 
     # 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 
     }
     
     # don't create an invoice with no line items, or where the only line 
@@ -603,38 +673,11 @@ sub bill {
 
     # calculate fees...
     my @fee_items;
 
     # 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} },
-        });
+    foreach my $fee_origin (@pending_fees) {
+      my $part_fee = $fee_origin->part_fee;
 
 
-        # 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 ]
-          );
-        }
-
-      } 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.
       # 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.
@@ -645,10 +688,41 @@ sub bill {
       }
       # also skip if it's disabled
       next if $part_fee->disabled eq 'Y';
       }
       # 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
       # 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;
 
     }
       push @fee_items, $fee_item;
 
     }
@@ -788,16 +862,17 @@ sub bill {
 
     # calculate and append taxes
     if ( ! $tax_is_batch) {
 
     # calculate and append taxes
     if ( ! $tax_is_batch) {
-      my $arrayref_or_error = $tax_engines{$pass}->calculate_taxes($cust_bill);
+      local $@;
+      my $arrayref = eval { $tax_engines{$pass}->calculate_taxes($cust_bill) };
 
 
-      unless ( ref( $arrayref_or_error ) ) {
+      if ( $@ ) {
         $dbh->rollback if $oldAutoCommit && !$options{no_commit};
         $dbh->rollback if $oldAutoCommit && !$options{no_commit};
-        return $arrayref_or_error;
+        return $@;
       }
 
       # or should this be in TaxEngine?
       my $total_tax = 0;
       }
 
       # or should this be in TaxEngine?
       my $total_tax = 0;
-      foreach my $taxline ( @$arrayref_or_error ) {
+      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
         $total_tax += $taxline->setup;
         $taxline->set('invnum' => $cust_bill->invnum); # just to be sure
         push @cust_bill_pkg, $taxline; # for return_bill
@@ -849,56 +924,66 @@ sub bill {
 }
 
 #discard bundled packages of 0 value
 }
 
 #discard bundled packages of 0 value
+# XXX we should reconsider whether we even need this
 sub _omit_zero_value_bundles {
   my @in = @_;
 
 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 ) {
 
   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).
 
   warn "  _omit_zero_value_bundles: ". scalar(@in).
-       '->'. scalar(@cust_bill_pkg). "\n" #. Dumper(@cust_bill_pkg). "\n"
+       '->'. scalar(@out). "\n" #. Dumper(@out). "\n"
     if $DEBUG > 2;
 
     if $DEBUG > 2;
 
-  (@cust_bill_pkg);
-
+  @out;
 }
 
 sub _make_lines {
 }
 
 sub _make_lines {
@@ -958,9 +1043,10 @@ sub _make_lines {
   #   - it doesn't already HAVE a setup date
   #   - or a start date in the future
   #   - and it's not suspended
   #   - 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 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.
+  # 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'}
   if (     ! $options{recurring_only}
        and ! $options{cancel}
        and ( $options{'resetup'}
@@ -971,6 +1057,8 @@ sub _make_lines {
                   && ( ! $cust_pkg->getfield('susp') )
                 )
            )
                   && ( ! $cust_pkg->getfield('susp') )
                 )
            )
+       and ( ! $cust_pkg->expire
+             || $cust_pkg->expire > $cmp_time )
      )
   {
     
      )
   {
     
@@ -983,8 +1071,14 @@ sub _make_lines {
         return "$@ running calc_setup for $cust_pkg\n"
           if $@;
 
         return "$@ running calc_setup for $cust_pkg\n"
           if $@;
 
-        $unitsetup = $cust_pkg->base_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'};
 
         if ( $setup_param{'billed_currency'} ) {
           $setup_billed_currency = delete $setup_param{'billed_currency'};
@@ -992,10 +1086,20 @@ sub _make_lines {
         }
     }
 
         }
     }
 
-    $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;
+    $lineitems++
+      if $cust_pkg->waive_setup
+      && $part_pkg->can('prorate_setup')
+      && $part_pkg->prorate_setup($cust_pkg, $time);
+
+    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;
 
     $cust_pkg->setfield('start_date', '')
       if $cust_pkg->start_date;
@@ -1012,6 +1116,23 @@ sub _make_lines {
   my $recur_billed_currency = '';
   my $recur_billed_amount = 0;
   my $sdate;
   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
   if (     ! $cust_pkg->start_date
        and 
            ( ! $cust_pkg->susp
@@ -1022,6 +1143,7 @@ sub _make_lines {
                               )
                          )
                   )
                               )
                          )
                   )
+               || $cust_pkg->is_status_delay_cancel
            )
        and
             ( $part_pkg->freq ne '0' && ( $cust_pkg->bill || 0 ) <= $cmp_time )
            )
        and
             ( $part_pkg->freq ne '0' && ( $cust_pkg->bill || 0 ) <= $cmp_time )
@@ -1029,6 +1151,12 @@ sub _make_lines {
                && $part_pkg->option('bill_every_call')
             )
          || $options{cancel}
                && $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
   ) {
 
     # XXX should this be a package event?  probably.  events are called
@@ -1075,6 +1203,14 @@ sub _make_lines {
     return "$@ running $method for $cust_pkg\n"
       if ( $@ );
 
     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
 
     #base_cancel???
     $unitrecur = $cust_pkg->base_recur( \$sdate ) || $recur; #XXX uuh, better
 
@@ -1083,6 +1219,11 @@ sub _make_lines {
       $recur_billed_amount   = delete $param{'billed_amount'};
     }
 
       $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;
     if ( $increment_next_bill ) {
 
       my $next_bill;
@@ -1093,21 +1234,42 @@ sub _make_lines {
         # its frequency
         my $main_pkg_freq = $main_pkg->part_pkg->freq;
         my $supp_pkg_freq = $part_pkg->freq;
         # 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
           # 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 {
         }
 
       } else {
-        # the normal case
+      # the normal case, not a supplemental package
       $next_bill = $part_pkg->add_freq($sdate, $options{freq_override} || 0);
       $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;
       }  
   
         if $next_bill == -1;
       }  
   
@@ -1126,7 +1288,7 @@ sub _make_lines {
       # Add an additional setup fee at the billing stage.
       # Used for prorate_defer_bill.
       $setup += $param{'setup_fee'};
       # 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++;
     }
 
       $lineitems++;
     }
 
@@ -1136,7 +1298,7 @@ sub _make_lines {
         }
     }
 
         }
     }
 
-  }
+  } # end of recurring fee
 
   warn "\$setup is undefined" unless defined($setup);
   warn "\$recur is undefined" unless defined($recur);
 
   warn "\$setup is undefined" unless defined($setup);
   warn "\$recur is undefined" unless defined($recur);
@@ -1200,14 +1362,14 @@ sub _make_lines {
       my $cust_bill_pkg = new FS::cust_bill_pkg {
         'pkgnum'                => $cust_pkg->pkgnum,
         'setup'                 => $setup,
       my $cust_bill_pkg = new FS::cust_bill_pkg {
         'pkgnum'                => $cust_pkg->pkgnum,
         'setup'                 => $setup,
-        'unitsetup'             => $unitsetup,
+        'unitsetup'             => sprintf('%.2f', $unitsetup),
         'setup_billed_currency' => $setup_billed_currency,
         'setup_billed_amount'   => $setup_billed_amount,
         'recur'                 => $recur,
         'setup_billed_currency' => $setup_billed_currency,
         'setup_billed_amount'   => $setup_billed_amount,
         'recur'                 => $recur,
-        'unitrecur'             => $unitrecur,
+        'unitrecur'             => sprintf('%.2f', $unitrecur),
         'recur_billed_currency' => $recur_billed_currency,
         'recur_billed_amount'   => $recur_billed_amount,
         'recur_billed_currency' => $recur_billed_currency,
         'recur_billed_amount'   => $recur_billed_amount,
-        'quantity'              => $cust_pkg->quantity,
+        'quantity'              => $override_quantity || $cust_pkg->quantity,
         'details'               => \@details,
         'discounts'             => [ @setup_discounts, @recur_discounts ],
         'hidden'                => $part_pkg->hidden,
         'details'               => \@details,
         'discounts'             => [ @setup_discounts, @recur_discounts ],
         'hidden'                => $part_pkg->hidden,
@@ -1368,6 +1530,11 @@ 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).
 
 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 {
 =cut
 
 sub _handle_taxes {
@@ -1385,7 +1552,7 @@ sub _handle_taxes {
 
   return if ( $self->payby eq 'COMP' ); #dubious
 
 
   return if ( $self->payby eq 'COMP' ); #dubious
 
-  if ( $conf->exists('enable_taxproducts')
+  if ( $conf->config('enable_taxproducts')
        && ( scalar($part_item->part_pkg_taxoverride)
             || $part_item->has_taxproduct
           )
        && ( scalar($part_item->part_pkg_taxoverride)
             || $part_item->has_taxproduct
           )
@@ -1397,85 +1564,73 @@ sub _handle_taxes {
     my %taxes = ();
 
     my @classes;
     my %taxes = ();
 
     my @classes;
-    push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->usage;
+    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, 'setup' if $cust_bill_pkg->setup and !$options{cancel};
-    push @classes, 'recur' if $cust_bill_pkg->recur and !$options{cancel};
-
-    my $exempt = $conf->exists('cust_class-tax_exempt')
-                   ? ( $self->cust_class ? $self->cust_class->tax : '' )
-                   : $self->tax;
+    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
     # 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 ) {
 
       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
         # 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( $location ) ) {
-          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;
-          # 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
 
 
-        }
       }
     }
 
       }
     }
 
@@ -1515,6 +1670,7 @@ sub _handle_taxes {
     foreach (@taxes) {
       my $tax_id = 'cust_main_county '.$_->taxnum;
       $taxlisthash->{$tax_id} ||= [ $_ ];
     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;
     }
 
       push @{ $taxlisthash->{$tax_id} }, $cust_bill_pkg;
     }
 
@@ -1635,7 +1791,10 @@ sub collect {
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   #never want to roll back an event just because it returned an error
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   #never want to roll back an event just because it returned an error
-  local $FS::UID::AutoCommit = 1; #$oldAutoCommit;
+  # unless $FS::UID::ForceObeyAutoCommit is set
+  local $FS::UID::AutoCommit = 1
+    unless !$oldAutoCommit
+        && $FS::UID::ForceObeyAutoCommit;
 
   $self->do_cust_event(
     'debug'      => ( $options{'debug'} || 0 ),
 
   $self->do_cust_event(
     'debug'      => ( $options{'debug'} || 0 ),
@@ -1843,9 +2002,13 @@ sub do_cust_event {
   }
 
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   }
 
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+
   #never want to roll back an event just because it or a different one
   # returned an error
   #never want to roll back an event just because it or a different one
   # returned an error
-  local $FS::UID::AutoCommit = 1; #$oldAutoCommit;
+  # unless $FS::UID::ForceObeyAutoCommit is set
+  local $FS::UID::AutoCommit = 1
+    unless !$oldAutoCommit
+        && $FS::UID::ForceObeyAutoCommit;
 
   foreach my $cust_event ( @$due_cust_event ) {
 
 
   foreach my $cust_event ( @$due_cust_event ) {
 
@@ -2143,6 +2306,7 @@ sub due_cust_event {
 =item apply_payments_and_credits [ OPTION => VALUE ... ]
 
 Applies unapplied payments and credits.
 =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.
 
 In most cases, this new method should be used in place of sequential
 apply_payments and apply_credits methods.
@@ -2169,16 +2333,21 @@ sub apply_payments_and_credits {
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
 
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
 
+  my $savepoint_label = 'Billing__apply_payments_and_credits';
+  savepoint_create( $savepoint_label );
+
   $self->select_for_update; #mutex
 
   foreach my $cust_bill ( $self->open_cust_bill ) {
     my $error = $cust_bill->apply_payments_and_credits(%options);
     if ( $error ) {
   $self->select_for_update; #mutex
 
   foreach my $cust_bill ( $self->open_cust_bill ) {
     my $error = $cust_bill->apply_payments_and_credits(%options);
     if ( $error ) {
+      savepoint_rollback_and_release( $savepoint_label );
       $dbh->rollback if $oldAutoCommit;
       return "Error applying: $error";
     }
   }
 
       $dbh->rollback if $oldAutoCommit;
       return "Error applying: $error";
     }
   }
 
+  savepoint_release( $savepoint_label );
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   ''; #no error
 
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   ''; #no error
 
@@ -2285,6 +2454,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.
 
 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.
 
 
  #and returns the value of any remaining unapplied payments.
 
@@ -2314,7 +2484,7 @@ sub apply_payments {
 
   #return 0 unless
 
 
   #return 0 unless
 
-  my @payments = $self->unapplied_cust_pay;
+  my @payments = grep { !$_->no_auto_apply } $self->unapplied_cust_pay;
 
   my @invoices = $self->open_cust_bill;
 
 
   my @invoices = $self->open_cust_bill;
 
Freeside billing, trouble ticketing, network monitoring and provisioning