avoid billing packages that will have expired before the billing date, #30049 and...

[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 5c10c63..29535f2 100644 (file)
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -880,6 +880,7 @@ sub bill {
 }
 
 #discard bundled packages of 0 value
+# XXX we should reconsider whether we even need this
 sub _omit_zero_value_bundles {
   my @in = @_;
 
@@ -888,11 +889,20 @@ sub _omit_zero_value_bundles {
   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 ) {
 
     if (scalar(@bundle) and !$cust_bill_pkg->pkgpart_override) {
@@ -989,9 +999,10 @@ sub _make_lines {
   #   - 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'}
@@ -1002,6 +1013,8 @@ sub _make_lines {
                   && ( ! $cust_pkg->getfield('susp') )
                 )
            )
+       and ( ! $cust_pkg->expire
+             || $cust_pkg->expire > $cmp_time )
      )
   {
     
@@ -1014,8 +1027,14 @@ sub _make_lines {
         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'};
@@ -1043,6 +1062,20 @@ sub _make_lines {
   my $recur_billed_currency = '';
   my $recur_billed_amount = 0;
   my $sdate;
+  # 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
@@ -1061,6 +1094,12 @@ sub _make_lines {
                && $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
@@ -1167,7 +1206,8 @@ sub _make_lines {
       } else {
       # the normal case, not a supplemental package
       $next_bill = $part_pkg->add_freq($sdate, $options{freq_override} || 0);
-      return "unparsable frequency: ". $part_pkg->freq
+      return "unparsable frequency: ".
+        ($options{freq_override} || $part_pkg->freq)
         if $next_bill == -1;
       }  
   
@@ -1186,7 +1226,7 @@ sub _make_lines {
       # Add an additional setup fee at the billing stage.
       # Used for prorate_defer_bill.
       $setup += $param{'setup_fee'};
-      $unitsetup += $param{'setup_fee'};
+      $unitsetup = $cust_pkg->base_setup();
       $lineitems++;
     }