From 90596b8e3f530a9a9f6ab36c2844b719a34e55ae Mon Sep 17 00:00:00 2001 From: mark Date: Thu, 13 Oct 2011 07:56:12 +0000 Subject: [PATCH] fix payment application to term discounts, #14524 --- FS/FS/cust_bill.pm | 151 +++++++++++++++++++--------------------- FS/FS/cust_pay.pm | 44 ++++++++---- FS/FS/part_pkg/prorate_Mixin.pm | 6 +- 3 files changed, 108 insertions(+), 93 deletions(-) diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm index bd8c1df19..4b450537c 100644 --- a/FS/FS/cust_bill.pm +++ b/FS/FS/cust_bill.pm @@ -5181,105 +5181,100 @@ a setup fee if the discount is allowed to apply to setup fees. sub _items_discounts_avail { my $self = shift; - my %total; - my $pkgnums = 0; - my $pkgnums_times_discounts = 0; - # tricky, because packages may not all be eligible for the same discounts - foreach my $cust_bill_pkg ( $self->cust_bill_pkg ) { - my $cust_pkg = $cust_bill_pkg->cust_pkg or next; - my $part_pkg = $cust_pkg->part_pkg or next; - # for simplicity, skip all this if the customer already has a term discount - return () if $cust_pkg->cust_pkg_discount_active; - - $pkgnums++; - next if $part_pkg->freq ne '1'; - - foreach my $discount ( - map { $_->discount } $part_pkg->part_pkg_discount - ) { - - $total{$discount->discountnum} ||= - { - discount => $discount, - pkgnums => [], - base_current => 0, - base_permonth => 0, - setup_include => 0, - setup_exclude => 0, - }; - my $hash = $total{$discount->discountnum}; - $hash->{discount} = $discount; - $hash->{thismonth} += $cust_bill_pkg->recur || 0; - $hash->{setup} += $cust_bill_pkg->setup || 0; - $hash->{base_permonth} += $part_pkg->base_recur_permonth; - - # and make a list of pkgnums - push @{ $hash->{pkgnums} }, $cust_pkg->pkgnum; - $pkgnums_times_discounts++; + my %terms; + my $list_pkgnums = 0; # if any packages are not eligible for all discounts + + my ($previous_balance) = $self->previous; + + foreach (qsearch('discount',{ 'months' => { op => '>', value => 1} })) { + $terms{$_->months} = { + pkgnums => [], + base => $previous_balance || 0, # pre-discount sum of charges + discounted => $previous_balance || 0, # post-discount sum + list_pkgnums => 0, # whether any packages are not discounted } } + foreach my $months (keys %terms) { + my $hash = $terms{$months}; - # Test for the simple case where all packages on the invoice - # are eligible for the same set of discounts. If not, we need - # to list eligibility in the ext_description. - my $list_pkgnums = ( $pkgnums_times_discounts != $pkgnums * keys(%total) ); - - foreach my $hash (values %total) { - my $discount = $hash->{discount}; - my ($amount, $term_total, $percent, $permonth); - my $months = $discount->months; - $hash->{months} = $months; - - if ( $discount->percent ) { - - # per discount_Mixin, percent discounts are calculated on the base - # recurring fee, not the prorated fee. - $percent = $discount->percent; - $amount = sprintf('%.2f', 0.01 * $percent * $hash->{base_permonth}); - # percent discounts apply to setup fee - if ( $discount->setup ) { - $hash->{setup} *= (1 - 0.01*$percent); - } + # tricky, because packages may not all be eligible for the same discounts + foreach my $cust_bill_pkg ( $self->cust_bill_pkg ) { + my $cust_pkg = $cust_bill_pkg->cust_pkg or next; + my $part_pkg = $cust_pkg->part_pkg or next; - } - elsif ( $discount->amount > 0 ) { + next if $part_pkg->freq ne '1'; + my $setup = $cust_bill_pkg->setup || 0; + my $recur = $cust_bill_pkg->recur || 0; + my $permonth = $part_pkg->base_recur_permonth || 0; - # amount discounts are amount * number of packages - $amount = $discount->amount * scalar(@{ $hash->{pkgnums} }); - $percent = sprintf('%.0f', 100 * $amount / $hash->{base_permonth}); + my ($discount) = grep { $_->months == $months } + map { $_->discount } $part_pkg->part_pkg_discount; - # flat discounts are applied to setup and recur together - if ( $discount->setup ) { - $hash->{thismonth} += $hash->{setup}; - $hash->{setup} = 0; + $hash->{base} += $setup + $recur + ($months - 1) * $permonth; + if ($discount) { + + my $discountable; + if ( $discount->setup ) { + $discountable += $setup; + } + else { + $hash->{discounted} += $setup; + } + + if ( $discount->percent ) { + $discountable += $months * $permonth; + $discountable -= ($discountable * $discount->percent / 100); + $discountable -= ($permonth - $recur); # correct for prorate + $hash->{discounted} += $discountable; + } + else { + $discountable += $recur; + $discountable -= $discount->amount * $recur/$permonth; + + $discountable += ($months - 1) * max($permonth - $discount->amount,0); + } + + $hash->{discounted} += $discountable; + push @{ $hash->{pkgnums} }, $cust_pkg->pkgnum; + } + else { #no discount + $hash->{discounted} += $setup + $recur + ($months - 1) * $permonth; + $hash->{list_pkgnums} = 1; } + } #foreach $cust_bill_pkg - } + # don't show this line if no packages have discounts at this term + # or if there are no new charges to apply the discount to + delete $terms{$months} if $hash->{base} == $hash->{discounted} + or $hash->{base} == 0; - $permonth = max( $hash->{base_permonth} - $amount, 0); - $term_total = max( $hash->{thismonth} - $amount , 0 ) # this month - + $permonth * ($months - 1) # rest of the term - + $hash->{setup}; # setup fee + } + + $list_pkgnums = grep { $_->{list_pkgnums} > 0 } values %terms; + + foreach my $months (keys %terms) { + my $hash = $terms{$months}; + my $term_total = sprintf('%.2f', $hash->{discounted}); + # possibly shouldn't include previous balance in these? + my $percent = sprintf('%.0f', 100 * (1 - $term_total / $hash->{base}) ); + my $permonth = sprintf('%.2f', $term_total / $months); $hash->{description} = $self->mt('Save [_1]% by paying for [_2] months', - $percent, $months, + $percent, $months ); $hash->{amount} = $self->mt('[_1] ([_2] per month)', - sprintf('%.2f',$term_total), #no money_char to accommodate template quirk - $money_char.sprintf('%.2f',$permonth) ); + $term_total, $money_char.$permonth + ); my @detail; if ( $list_pkgnums ) { - push @detail, $self->mt('for item'). ' '. + push @detail, $self->mt('discount on item'). ' '. join(', ', map { "#$_" } @{ $hash->{pkgnums} }); } - if ( !$discount->setup and $hash->{setup} ) { - push @detail, $self->mt('excluding setup fees'); - } $hash->{ext_description} = join ', ', @detail; } - sort { -( $a->{months} <=> $b->{months} ) } values(%total); + map { $terms{$_} } sort {$b <=> $a} keys %terms; } =item call_details [ OPTION => VALUE ... ] diff --git a/FS/FS/cust_pay.pm b/FS/FS/cust_pay.pm index 3888cd6d0..60c437a92 100644 --- a/FS/FS/cust_pay.pm +++ b/FS/FS/cust_pay.pm @@ -189,29 +189,43 @@ sub insert { if ( my $credit_type = $conf->config('prepayment_discounts-credit_type') ) { if ( my $months = $self->discount_term ) { - #hmmm... error handling - my ($credit, $savings, $total) = - $cust_main->discount_term_values($months); + # XXX this should be moved out somewhere, but discount_term_values + # doesn't fit right + my ($cust_bill) = ($cust_main->cust_bill)[-1]; # most recent invoice + return "can't accept prepayment for an unbilled customer" if !$cust_bill; + + my %billing_pkgs = map { $_->pkgnum => $_ } $cust_main->billing_pkgs; + my $credit = 0; # sum of recurring charges from that invoice + my $last_bill_date = 0; # the real bill date + foreach my $item ( $cust_bill->cust_bill_pkg ) { + next if !exists($billing_pkgs{$item->pkgnum}); # skip inactive packages + $credit += $item->recur; + $last_bill_date = $item->cust_pkg->last_bill + if defined($item->cust_pkg) + and $item->cust_pkg->last_bill > $last_bill_date + } + my $cust_credit = new FS::cust_credit { 'custnum' => $self->custnum, - 'amount' => $credit, + 'amount' => sprintf('%.2f', $credit), 'reason' => 'customer chose to prepay for discount', }; $error = $cust_credit->insert('reason_type' => $credit_type); if ( $error ) { $dbh->rollback if $oldAutoCommit; - return "error inserting cust_pay: $error"; + return "error inserting prepayment credit: $error"; } - my @pkgs = $cust_main->_discount_pkgs_and_bill; - my $cust_bill = shift(@pkgs); - @pkgs = &FS::cust_main::Billing_Discount::_discountable_pkgs_at_term($months, @pkgs); - $_->bill($_->last_bill) foreach @pkgs; - $error = $cust_main->bill( - 'recurring_only' => 1, - 'time' => $cust_bill->invoice_date, + # don't apply it yet + + # bill for the entire term + $_->bill($_->last_bill) foreach (values %billing_pkgs); + $error = $cust_main->bill( + # no recurring_only, we want unbilled packages with start dates to + # get billed 'no_usage_reset' => 1, - 'pkg_list' => \@pkgs, - 'freq_override' => $months, + 'time' => $last_bill_date, # not $cust_bill->_date + 'pkg_list' => [ values %billing_pkgs ], + 'freq_override' => $months, ); if ( $error ) { $dbh->rollback if $oldAutoCommit; @@ -227,6 +241,8 @@ sub insert { $dbh->rollback if $oldAutoCommit; return "balance after prepay discount attempt: $new_balance"; } + # user friendly: override the "apply only to this invoice" mode + $self->invnum(''); } diff --git a/FS/FS/part_pkg/prorate_Mixin.pm b/FS/FS/part_pkg/prorate_Mixin.pm index 7d68be0bd..380f0b029 100644 --- a/FS/FS/part_pkg/prorate_Mixin.pm +++ b/FS/FS/part_pkg/prorate_Mixin.pm @@ -98,7 +98,11 @@ sub calc_prorate { my $months = ( ( $self->freq - 1 ) + ($mend-$mnow) / ($mend-$mstart) ); # add a full period if currently billing for a partial period - if ( ( $self->option('add_full_period',1) + # or periods up to freq_override if billing for an override interval + if ( ($param->{'freq_override'} || 0) > 1 ) { + $months += $param->{'freq_override'} - 1; + } + elsif ( ( $self->option('add_full_period',1) or $self->option('prorate_defer_bill',1) ) # necessary and $months < $self->freq ) { $months += $self->freq; -- 2.11.0