redesign the "nextbill" flag a little, #25899

7 files changed, 61 insertions, 20 deletions

diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 69681d0..795b97f 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -943,6 +943,7 @@ sub tables_hashref {
         'eventnum',       'int', '', '', '', '',
         'billpkgnum',     'int', 'NULL', '', '', '',
         'feepart',        'int', '', '', '', '',
+        'nextbill',      'char', 'NULL',  1, '', '',
       ],
       'primary_key'  => 'eventfeenum', # I'd rather just use eventnum
       'unique' => [ [ 'billpkgnum' ], [ 'eventnum' ] ], # one-to-one link
@@ -3148,7 +3149,6 @@ sub tables_hashref {
         'limit_credit',  'char',    'NULL',  1, '', '',
         'setuprecur',    'char',     '',     5, '', '',
         'taxable',       'char',    'NULL',  1, '', '',
-        'nextbill',      'char',    'NULL',  1, '', '',
       ],
       'primary_key'  => 'feepart',
       'unique'       => [],
diff --git a/FS/FS/cust_event_fee.pm b/FS/FS/cust_event_fee.pm
index 78794fd..d924485 100644
--- a/FS/FS/cust_event_fee.pm
+++ b/FS/FS/cust_event_fee.pm
@@ -45,6 +45,9 @@ time billing runs for the customer.
 
 =item feepart - key of the fee definition (L<FS::part_fee>).
 
+=item nextbill - 'Y' if the fee should be charged on the customer's next
+bill, rather than causing a bill to be produced immediately.
+
 =back
 
 =head1 METHODS
@@ -93,6 +96,7 @@ sub check {
     || $self->ut_foreign_key('eventnum', 'cust_event', 'eventnum')
     || $self->ut_foreign_keyn('billpkgnum', 'cust_bill_pkg', 'billpkgnum')
     || $self->ut_foreign_key('feepart', 'part_fee', 'feepart')
+    || $self->ut_flag('nextbill')
   ;
   return $error if $error;
 
diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index 1703aec..a7e7d19 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -545,17 +545,24 @@ sub bill {
       hashref => { 'billpkgnum' => '' }
     );
     warn "$me found pending fee events:\n".Dumper(\@pending_event_fees)."\n"
-      if @pending_event_fees;
+      if @pending_event_fees and $DEBUG > 1;
 
-    # whether to generate an invoice
+    # 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;
+    }
+    
+    # don't create an invoice with no line items, or where the only line 
+    # items are fees that are supposed to be held until the next invoice
+    next if !$generate_bill;
+
     # calculate fees...
     my @fee_items;
     foreach my $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') ) {
         # Not the real cust_bill object that will be inserted--in particular
@@ -589,13 +596,8 @@ sub bill {
       $fee_item->set('cust_event_fee', $event_fee);
       push @fee_items, $fee_item;
 
-      $generate_bill = 1 unless $part_fee->nextbill;
     }
     
-    # don't create an invoice with no line items, or where the only line 
-    # items are fees that are supposed to be held until the next invoice
-    next if !$generate_bill;
-
     # add fees to the invoice
     foreach my $fee_item (@fee_items) {
 
diff --git a/FS/FS/part_event/Action/Mixin/fee.pm b/FS/FS/part_event/Action/Mixin/fee.pm
index 8eb86fa..a49782d 100644
--- a/FS/FS/part_event/Action/Mixin/fee.pm
+++ b/FS/FS/part_event/Action/Mixin/fee.pm
@@ -2,6 +2,7 @@ package FS::part_event::Action::Mixin::fee;
 
 use strict;
 use base qw( FS::part_event::Action );
+use FS::Record qw( qsearch );
 
 sub event_stage { 'pre-bill'; }
 
@@ -15,16 +16,34 @@ sub option_fields {
                     value_col => 'feepart',
                     disable_empty => 1,
                   },
-  );
+  ),
+
 }
 
 sub default_weight { 10; }
 
+sub hold_until_bill { 1 }
+
 sub do_action {
   my( $self, $cust_object, $cust_event ) = @_;
 
-  die "no fee definition selected for event '".$self->event."'\n"
-    unless $self->option('feepart');
+  my $feepart = $self->option('feepart')
+    or die "no fee definition selected for event '".$self->event."'\n";
+  my $tablenum = $cust_object->get($cust_object->primary_key);
+
+  # see if there's already a pending fee for this customer/invoice
+  my @existing = qsearch({
+      table     => 'cust_event_fee',
+      addl_from => 'JOIN cust_event USING (eventnum)',
+      hashref   => { feepart    => $feepart,
+                     billpkgnum => '' },
+      extra_sql => " AND tablenum = $tablenum",
+  });
+  if (scalar @existing > 0) {
+    warn $self->event." event, object $tablenum: already scheduled\n"
+      if $FS::part_fee::DEBUG;
+    return;
+  }
 
   # mark the event so that the fee will be charged
   # the logic for calculating the fee amount is in FS::part_fee
@@ -32,8 +51,9 @@ sub do_action {
   # FS::cust_bill_pkg
   my $cust_event_fee = FS::cust_event_fee->new({
       'eventnum'    => $cust_event->eventnum,
-      'feepart'     => $self->option('feepart'),
+      'feepart'     => $feepart,
       'billpkgnum'  => '',
+      'nextbill'    => $self->hold_until_bill ? 'Y' : '',
   });
 
   my $error = $cust_event_fee->insert;
diff --git a/FS/FS/part_event/Action/cust_bill_fee.pm b/FS/FS/part_event/Action/cust_bill_fee.pm
index fc185e4..5d962b1 100644
--- a/FS/FS/part_event/Action/cust_bill_fee.pm
+++ b/FS/FS/part_event/Action/cust_bill_fee.pm
@@ -9,4 +9,20 @@ sub eventtable_hashref {
     { 'cust_bill' => 1 };
 }
 
+sub option_fields {
+  (
+    __PACKAGE__->SUPER::option_fields,
+    'nextbill'  => { label    => 'Hold fee until the customer\'s next bill',
+                     type     => 'checkbox',
+                     value    => 'Y'
+                   },
+  )
+}
+
+# it makes sense for this to be optional for previous-invoice fees
+sub hold_until_bill {
+  my $self = shift;
+  $self->option('nextbill');
+}
+
 1;
diff --git a/FS/FS/part_event/Action/cust_fee.pm b/FS/FS/part_event/Action/cust_fee.pm
index a6f1078..9373091 100644
--- a/FS/FS/part_event/Action/cust_fee.pm
+++ b/FS/FS/part_event/Action/cust_fee.pm
@@ -9,6 +9,8 @@ sub eventtable_hashref {
     { 'cust_main' => 1 };
 }
 
+sub hold_until_bill { 1 }
+
 # Otherwise identical to cust_bill_fee.  We only have a separate event 
 # because it behaves differently as an invoice event than as a customer
 # event, and needs a different description.
diff --git a/FS/FS/part_fee.pm b/FS/FS/part_fee.pm
index d1e6477..b0e5473 100644
--- a/FS/FS/part_fee.pm
+++ b/FS/FS/part_fee.pm
@@ -5,7 +5,7 @@ use base qw( FS::o2m_Common FS::Record );
 use vars qw( $DEBUG );
 use FS::Record qw( qsearch qsearchs );
 
-$DEBUG = 1;
+$DEBUG = 0;
 
 =head1 NAME
 
@@ -54,9 +54,6 @@ the invoice
 Currently, taxable fees will be treated like they exist at the customer's
 default service location.
 
-=item nextbill - 'Y' if this fee should be delayed until the customer is 
-billed for a package.
-
 =item taxclass - the tax class the fee belongs to, as a string, for the 
 internal tax system
 
@@ -138,7 +135,6 @@ sub check {
     || $self->ut_flag('disabled')
     || $self->ut_foreign_keyn('classnum', 'pkg_class', 'classnum')
     || $self->ut_flag('taxable')
-    || $self->ut_flag('nextbill')
     || $self->ut_textn('taxclass')
     || $self->ut_numbern('taxproductnum')
     || $self->ut_floatn('pay_weight')
@@ -294,7 +290,7 @@ sub lineitem {
       $maximum = -1 * $balance;
     }
   }
-  if ( $maximum ne '' ) {
+  if ( $maximum ne '' and $amount > $maximum ) {
     warn "Applying maximum fee\n" if $DEBUG;
     $amount = $maximum;
   }
@@ -312,7 +308,7 @@ sub lineitem {
   });
 
   if ( $maximum and $self->taxable ) {
-    warn "Estimating taxes on fee.\n";
+    warn "Estimating taxes on fee.\n" if $DEBUG;
     # then we need to estimate tax to respect the maximum
     # XXX currently doesn't work with external (tax_rate) taxes
     # or batch taxes, obviously
@@ -331,6 +327,7 @@ sub lineitem {
     if ($total_rate > 0) {
       my $max_cents = $maximum * 100;
       my $charge_cents = sprintf('%0.f', $max_cents * 100/(100 + $total_rate));
+      # the actual maximum that we can charge...
       $maximum = sprintf('%.2f', $charge_cents / 100.00);
       $amount = $maximum if $amount > $maximum;
     }