RT# 78547 bill_and_collect() small optimization

[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 6932647..71d5c9b 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;
+use feature 'state';
 use vars qw( $conf $DEBUG $me );
 use Carp;
 use Data::Dumper;
@@ -57,7 +58,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:
@@ -131,8 +132,7 @@ sub bill_and_collect {
   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);
@@ -140,8 +140,7 @@ sub bill_and_collect {
   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);
@@ -149,8 +148,7 @@ sub bill_and_collect {
   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;
@@ -159,8 +157,7 @@ sub bill_and_collect {
   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;
@@ -169,17 +166,13 @@ sub bill_and_collect {
   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.
-  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')
@@ -195,8 +188,7 @@ sub bill_and_collect {
     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; }
     }
   }
 
@@ -216,12 +208,11 @@ sub cancel_expired_pkgs {
 
   my @errors = ();
 
-  my @really_cancel_pkgs;
-  my @cancel_reasons;
+  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;
 
     if ( $cust_pkg->change_to_pkgnum ) {
 
@@ -231,9 +222,10 @@ sub cancel_expired_pkgs {
                       $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
 
@@ -1058,6 +1050,9 @@ sub _make_lines {
         }
     }
 
+    $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') ) {