From 46dc4e18008835d3f8f76afb7156fb44e7f75e32 Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Tue, 25 Feb 2014 23:12:52 -0800 Subject: [PATCH] redesign the "nextbill" flag a little, #25899 --- FS/FS/Schema.pm | 2 +- FS/FS/cust_event_fee.pm | 4 ++++ FS/FS/cust_main/Billing.pm | 18 ++++++++++-------- FS/FS/part_event/Action/Mixin/fee.pm | 28 ++++++++++++++++++++++++---- FS/FS/part_event/Action/cust_bill_fee.pm | 16 ++++++++++++++++ FS/FS/part_event/Action/cust_fee.pm | 2 ++ FS/FS/part_fee.pm | 11 ++++------- 7 files changed, 61 insertions(+), 20 deletions(-) diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm index 69681d05a..795b97fb6 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 78794fdfe..d924485e7 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). +=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 1703aec80..a7e7d1976 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 8eb86fa1d..a49782de0 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 fc185e439..5d962b131 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 a6f1078e8..9373091ab 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 d1e6477a7..b0e5473eb 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; } -- 2.11.0