diff options
| -rw-r--r-- | FS/FS/Schema.pm | 16 | ||||
| -rw-r--r-- | FS/FS/Upgrade.pm | 3 | ||||
| -rw-r--r-- | FS/FS/cust_bill_pkg.pm | 11 | ||||
| -rw-r--r-- | FS/FS/cust_bill_pkg_tax_location.pm | 281 | ||||
| -rw-r--r-- | FS/FS/cust_credit.pm | 198 | ||||
| -rw-r--r-- | FS/FS/cust_main/Billing.pm | 18 | ||||
| -rw-r--r-- | FS/FS/log_context.pm | 2 | ||||
| -rw-r--r-- | httemplate/edit/credit-cust_bill_pkg.html | 9 |
8 files changed, 449 insertions, 89 deletions
diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm index 172ac8296..b6fd3b67b 100644 --- a/FS/FS/Schema.pm +++ b/FS/FS/Schema.pm @@ -805,13 +805,19 @@ sub tables_hashref { 'billpkgnum', 'int', '', '', '', '', 'taxnum', 'int', '', '', '', '', 'taxtype', 'varchar', '', $char_d, '', '', - 'pkgnum', 'int', '', '', '', '', - 'locationnum', 'int', '', '', '', '', #redundant? + 'pkgnum', 'int', '', '', '', '', #redundant + 'locationnum', 'int', '', '', '', '', #redundant 'amount', @money_type, '', '', + 'taxable_billpkgnum', 'int', 'NULL', '', '', '', ], 'primary_key' => 'billpkgtaxlocationnum', 'unique' => [], - 'index' => [ [ 'billpkgnum' ], [ 'taxnum' ], [ 'pkgnum' ], [ 'locationnum' ] ], + 'index' => [ [ 'billpkgnum' ], + [ 'taxnum' ], + [ 'pkgnum' ], + [ 'locationnum' ], + [ 'taxable_billpkgnum' ], + ], }, 'cust_bill_pkg_tax_rate_location' => { @@ -823,10 +829,12 @@ sub tables_hashref { 'locationtaxid', 'varchar', 'NULL', $char_d, '', '', 'taxratelocationnum', 'int', '', '', '', '', 'amount', @money_type, '', '', + 'taxable_billpkgnum', 'int', 'NULL', '', '', '', ], 'primary_key' => 'billpkgtaxratelocationnum', 'unique' => [], - 'index' => [ [ 'billpkgnum' ], [ 'taxnum' ], [ 'taxratelocationnum' ] ], + 'index' => [ [ 'billpkgnum' ], [ 'taxnum' ], [ 'taxratelocationnum' ], + [ 'taxable_billpkgnum' ], ], }, 'cust_bill_pkg_void' => { diff --git a/FS/FS/Upgrade.pm b/FS/FS/Upgrade.pm index 45cba82a4..fea53a235 100644 --- a/FS/FS/Upgrade.pm +++ b/FS/FS/Upgrade.pm @@ -305,6 +305,9 @@ sub upgrade_data { #kick off tax location history upgrade 'cust_bill_pkg' => [], + + #fix taxable line item links + 'cust_bill_pkg_tax_location' => [], ; \%hash; diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm index a83af1326..5cff140bb 100644 --- a/FS/FS/cust_bill_pkg.pm +++ b/FS/FS/cust_bill_pkg.pm @@ -1310,9 +1310,10 @@ sub upgrade_tax_location { ); $cents_remaining -= $part; push @tax_links, { - taxnum => $taxdef->taxnum, - pkgnum => $nontax->pkgnum, - cents => $part, + taxnum => $taxdef->taxnum, + pkgnum => $nontax->pkgnum, + billpkgnum => $nontax->billpkgnum, + cents => $part, }; } #foreach $nontax } #foreach $taxclass @@ -1355,6 +1356,7 @@ sub upgrade_tax_location { taxnum => $_->{taxnum}, pkgnum => $_->{pkgnum}, amount => sprintf('%.2f', $_->{cents} / 100), + taxable_billpkgnum => $_->{billpkgnum}, }); my $error = $link->insert; if ( $error ) { @@ -1443,6 +1445,9 @@ sub _upgrade_data { # Then mark the upgrade as done, so that we don't queue the job twice # and somehow run two of them concurrently. FS::upgrade_journal->set_done($upgrade); + # This upgrade now does the job of assigning taxable_billpkgnums to + # cust_bill_pkg_tax_location, so set that task done also. + FS::upgrade_journal->set_done('tax_location_taxable_billpkgnum'); } =back diff --git a/FS/FS/cust_bill_pkg_tax_location.pm b/FS/FS/cust_bill_pkg_tax_location.pm index 44dd6e3c4..723d6e0a3 100644 --- a/FS/FS/cust_bill_pkg_tax_location.pm +++ b/FS/FS/cust_bill_pkg_tax_location.pm @@ -9,6 +9,9 @@ use FS::cust_location; use FS::cust_bill_pay_pkg; use FS::cust_credit_bill_pkg; use FS::cust_main_county; +use FS::Log; + +use List::Util qw(sum min); =head1 NAME @@ -65,6 +68,11 @@ locationnum amount +=item taxable_billpkgnum + +The billpkgnum of the L<FS::cust_bill_pkg> that this tax was charged on. +It may specifically be on any portion of that line item (setup, recurring, +or a usage class). =back @@ -119,6 +127,7 @@ sub check { || $self->ut_foreign_key('pkgnum', 'cust_pkg', 'pkgnum' ) || $self->ut_foreign_key('locationnum', 'cust_location', 'locationnum' ) || $self->ut_money('amount') + || $self->ut_foreign_key('taxable_billpkgnum', 'cust_bill_pkg', 'billpkgnum') ; return $error if $error; @@ -127,7 +136,7 @@ sub check { =item cust_bill_pkg -Returns the associated cust_bill_pkg object +Returns the associated cust_bill_pkg object (i.e. the tax charge). =cut @@ -136,6 +145,10 @@ sub cust_bill_pkg { qsearchs( 'cust_bill_pkg', { 'billpkgnum' => $self->billpkgnum } ); } +=item taxable_cust_bill_pkg + +Returns the cust_bill_pkg object for the I<taxable> charge. + =item cust_location Returns the associated cust_location object @@ -208,12 +221,274 @@ sub cust_main_county { } } +sub _upgrade_data { + eval { + use FS::queue; + use Date::Parse 'str2time'; + }; + my $class = shift; + my $upgrade = 'tax_location_taxable_billpkgnum'; + return if FS::upgrade_journal->is_done($upgrade); + my $job = FS::queue->new({ job => + 'FS::cust_bill_pkg_tax_location::upgrade_taxable_billpkgnum' + }); + $job->insert($class, 's' => str2time('2012-01-01')); + FS::upgrade_journal->set_done($upgrade); +} + +sub upgrade_taxable_billpkgnum { + # Associate these records to the correct taxable line items. + # The cust_bill_pkg upgrade now does this also for pre-3.0 records that + # aren't broken out by pkgnum, so we only need to deal with the case of + # multiple line items for the same pkgnum. + # Despite appearances, this has almost no relation to the upgrade in + # FS::cust_bill_pkg. + + my ($class, %opt) = @_; + my $dbh = FS::UID::dbh(); + my $oldAutoCommit = $FS::UID::AutoCommit; + local $FS::UID::AutoCommit = 0; + my $log = FS::Log->new('upgrade_taxable_billpkgnum'); + + my $date_where = ''; + if ( $opt{s} ) { + $date_where .= " AND cust_bill._date >= $opt{s}"; + } + if ( $opt{e} ) { + $date_where .= " AND cust_bill._date < $opt{e}"; + } + + my @need_to_upgrade = qsearch({ + select => 'cust_bill_pkg_tax_location.*', + table => 'cust_bill_pkg_tax_location', + hashref => { taxable_billpkgnum => '' }, + addl_from => 'JOIN cust_bill_pkg USING (billpkgnum)'. + 'JOIN cust_bill USING (invnum)', + extra_sql => $date_where, + }); + $log->info('Starting upgrade of '.scalar(@need_to_upgrade). + ' cust_bill_pkg_tax_location records.'); + + # keys are billpkgnums + my %cust_bill_pkg; + my %tax_location; + foreach (@need_to_upgrade) { + my $tax_billpkgnum = $_->billpkgnum; + $cust_bill_pkg{ $tax_billpkgnum } ||= FS::cust_bill_pkg->by_key($tax_billpkgnum); + $tax_location{ $tax_billpkgnum } ||= []; + push @{ $tax_location{ $tax_billpkgnum } }, $_; + } + + TAX_ITEM: foreach my $tax_item (values %cust_bill_pkg) { + my $tax_locations = $tax_location{ $tax_item->billpkgnum }; + my $invnum = $tax_item->invnum; + my $cust_bill = FS::cust_bill->by_key($tax_item->invnum); + my %tax_on_pkg; # keys are tax identifiers + TAX_LOCATION: foreach my $tax_location (@$tax_locations) { + # recapitulate the "cust_main_county $taxnum $pkgnum" tax identifier, + # in a way + my $taxid = join(' ', + $tax_location->taxtype, + $tax_location->taxnum, + $tax_location->pkgnum, + $tax_location->locationnum + ); + $tax_on_pkg{$taxid} ||= []; + push @{ $tax_on_pkg{$taxid} }, $tax_location; + } + PKGNUM: foreach my $taxid (keys %tax_on_pkg) { + my ($taxtype, $taxnum, $pkgnum, $locationnum) = split(' ', $taxid); + $log->info("tax#$taxnum, pkg#$pkgnum", object => $cust_bill); + my @pkg_items = $cust_bill->cust_bill_pkg_pkgnum($pkgnum); + if (!@pkg_items) { + # then how is there tax on it? should never happen + $log->error("no line items with pkg#$pkgnum", object => $cust_bill); + next PKGNUM; + } + my $pkg_amount = 0; + foreach my $pkg_item (@pkg_items) { + # find the taxable amount of each one + my $amount = $pkg_item->setup + $pkg_item->recur; + # subtract any exemptions that apply to this taxdef + foreach (qsearch('cust_tax_exempt_pkg', { + taxnum => $taxnum, + billpkgnum => $pkg_item->billpkgnum + }) ) + { + $amount -= $_->amount; + } + $pkg_item->set('amount' => $pkg_item->setup + $pkg_item->recur); + $pkg_amount += $amount; + } #$pkg_item + next PKGNUM if $pkg_amount == 0; # probably because it's fully exempted + # now sort them descending by taxable amount + @pkg_items = sort { $b->amount <=> $a->amount } + @pkg_items; + # and do the same with the tax links + # (there should be one per taxed item) + my @tax_links = sort { $b->amount <=> $a->amount } + @{ $tax_on_pkg{$taxid} }; + + if (scalar(@tax_links) == scalar(@pkg_items)) { + # the relatively simple case: they match 1:1 + for my $i (0 .. scalar(@tax_links) - 1) { + $tax_links[$i]->set('taxable_billpkgnum', + $pkg_items[$i]->billpkgnum); + my $error = $tax_links[$i]->replace; + if ( $error ) { + $log->error("failed to set taxable_billpkgnum in tax on pkg#$pkgnum", + object => $cust_bill); + next PKGNUM; + } + } #for $i + } else { + # the more complicated case + $log->warn("mismatched charges and tax links in pkg#$pkgnum", + object => $cust_bill); + my $tax_amount = sum(map {$_->amount} @tax_links); + # remove all tax link records and recreate them to be 1:1 with + # taxable items + my (%billpaynum, %creditbillnum); + my $link_type; + foreach my $tax_link (@tax_links) { + $link_type ||= ref($tax_link); + my $error = $tax_link->delete; + if ( $error ) { + $log->error("error unlinking tax#$taxnum pkg#$pkgnum", + object => $cust_bill); + next PKGNUM; + } + my $pkey = $tax_link->primary_key; + # also remove all applications that reference this tax link + # (they will be applications to the tax item) + my %hash = ($pkey => $tax_link->get($pkey)); + foreach (qsearch('cust_bill_pay_pkg', \%hash)) { + $billpaynum{$_->billpaynum} += $_->amount; + my $error = $_->delete; + die "error unapplying payment: $error" if ( $error ); + } + foreach (qsearch('cust_credit_bill_pkg', \%hash)) { + $creditbillnum{$_->creditbillnum} += $_->amount; + my $error = $_->delete; + die "error unapplying credit: $error" if ( $error ); + } + } + @tax_links = (); + my $cents_remaining = int(100 * $tax_amount); + foreach my $pkg_item (@pkg_items) { + my $cents = int(100 * $pkg_item->amount * $tax_amount / $pkg_amount); + my $tax_link = $link_type->new({ + taxable_billpkgnum => $pkg_item->billpkgnum, + billpkgnum => $tax_item->billpkgnum, + taxnum => $taxnum, + taxtype => $taxtype, + pkgnum => $pkgnum, + locationnum => $locationnum, + cents => $cents, + }); + push @tax_links, $tax_link; + $cents_remaining -= $cents; + } + my $nlinks = scalar @tax_links; + my $i = 0; + while ($cents_remaining) { + $tax_links[$i % $nlinks]->set('cents' => + $tax_links[$i % $nlinks]->cents + 1 + ); + $cents_remaining--; + $i++; + } + foreach my $tax_link (@tax_links) { + $tax_link->set('amount' => sprintf('%.2f', $tax_link->cents / 100)); + my $error = $tax_link->insert; + if ( $error ) { + $log->error("error relinking tax#$taxnum pkg#$pkgnum", + object => $cust_bill); + next PKGNUM; + } + } + + $i = 0; + my $error; + my $left = 0; # the amount "left" on the last tax link after + # applying payments, but before credits, so that + # it can receive both a payment and a credit if + # necessary + # reapply payments/credits...this sucks + foreach my $billpaynum (keys %billpaynum) { + my $pay_amount = $billpaynum{$billpaynum}; + while ($i < $nlinks and $pay_amount > 0) { + my $this_amount = min($pay_amount, $tax_links[$i]->amount); + $left = $tax_links[$i]->amount - $this_amount; + my $app = FS::cust_bill_pay_pkg->new({ + billpaynum => $billpaynum, + billpkgnum => $tax_links[$i]->billpkgnum, + billpkgtaxlocationnum => $tax_links[$i]->billpkgtaxlocationnum, + amount => $this_amount, + setuprecur => 'setup', + # sdate/edate are null + }); + my $error ||= $app->insert; + $pay_amount -= $this_amount; + $i++ if $left == 0; + } + } + foreach my $creditbillnum (keys %creditbillnum) { + my $credit_amount = $creditbillnum{$creditbillnum}; + while ($i < $nlinks and $credit_amount > 0) { + my $this_amount = min($left, $credit_amount, $tax_links[$i]->amount); + $left = $credit_amount * 2; # just so it can't be selected twice + $i++ if $this_amount == $left + or $this_amount == $tax_links[$i]->amount; + my $app = FS::cust_credit_bill_pkg->new({ + creditbillnum => $creditbillnum, + billpkgnum => $tax_links[$i]->billpkgnum, + billpkgtaxlocationnum => $tax_links[$i]->billpkgtaxlocationnum, + amount => $this_amount, + setuprecur => 'setup', + # sdate/edate are null + }); + my $error ||= $app->insert; + $credit_amount -= $this_amount; + } + } + if ( $error ) { + # we've just unapplied a bunch of stuff, so if it won't reapply + # we really need to revert the whole transaction + die "error reapplying payments/credits: $error; upgrade halted"; + } + } # scalar(@tax_links) ?= scalar(@pkg_items) + } #taxnum/pkgnum + } #TAX_ITEM + + $log->info('finish'); + + $dbh->commit if $oldAutoCommit; + return; +} + +=cut + =back =head1 BUGS -The presense of FS::cust_main_county::delete makes the cust_main_county method -unreliable +The presence of FS::cust_main_county::delete makes the cust_main_county method +unreliable. + +Pre-3.0 versions of Freeside would only create one cust_bill_pkg_tax_location +per tax definition (taxtype/taxnum) per invoice. The pkgnum and locationnum +fields were arbitrarily set to those of the first line item subject to the +tax. This created problems if the tax contribution of each line item ever +needed to be determined (for example, when applying credits). For several +months in 2012, this was changed to create one record per tax definition +per I<package> per invoice, which was still not specific enough to identify +a line item. + +The current behavior is to create one record per tax definition per taxable +line item, and to store the billpkgnum of the taxed line item in the record. +The upgrade will try to convert existing records to the new format, but this +is not perfectly reliable. =head1 SEE ALSO diff --git a/FS/FS/cust_credit.pm b/FS/FS/cust_credit.pm index fe9572f6b..7741bbee2 100644 --- a/FS/FS/cust_credit.pm +++ b/FS/FS/cust_credit.pm @@ -646,7 +646,6 @@ Example: use FS::cust_bill_pkg; sub credit_lineitems { my( $class, %arg ) = @_; - my $curuser = $FS::CurrentUser::CurrentUser; #some false laziness w/misc/xmlhttp-cust_bill_pkg-calculate_taxes.html @@ -713,10 +712,11 @@ sub credit_lineitems { } #my $subtotal = 0; - my $taxlisthash = {}; + # keys in all of these are invoice numbers my %cust_credit_bill = (); my %cust_bill_pkg = (); my %cust_credit_bill_pkg = (); + my %taxlisthash = (); foreach my $billpkgnum ( @{$arg{billpkgnums}} ) { my $setuprecur = shift @{$arg{setuprecurs}}; my $amount = shift @{$arg{amounts}}; @@ -727,6 +727,8 @@ sub credit_lineitems { 'addl_from' => 'LEFT JOIN cust_bill USING (invnum)', 'extra_sql' => 'AND custnum = '. $cust_main->custnum, }) or die "unknown billpkgnum $billpkgnum"; + + my $invnum = $cust_bill_pkg->invnum; if ( $setuprecur eq 'setup' ) { $cust_bill_pkg->setup($amount); @@ -740,8 +742,9 @@ sub credit_lineitems { $cust_bill_pkg->unitsetup(0); } - push @{$cust_bill_pkg{$cust_bill_pkg->invnum}}, $cust_bill_pkg; + push @{$cust_bill_pkg{$invnum}}, $cust_bill_pkg; + my %unapplied_payments; # billpaynum => amount #unapply any payments applied to this line item (other credits too?) foreach my $cust_bill_pay_pkg ( $cust_bill_pkg->cust_bill_pay_pkg($setuprecur) ) { $error = $cust_bill_pay_pkg->delete; @@ -749,25 +752,51 @@ sub credit_lineitems { $dbh->rollback if $oldAutoCommit; return "Error unapplying payment: $error"; } + $unapplied_payments{$cust_bill_pay_pkg->billpaynum} + += $cust_bill_pay_pkg->amount; + } + # also unapply that amount from the invoice so it doesn't screw up + # application of the credit + foreach my $billpaynum (keys %unapplied_payments) { + my $cust_bill_pay = FS::cust_bill_pay->by_key($billpaynum) + or die "broken payment application $billpaynum"; + $error = $cust_bill_pay->delete; # can't replace + + my $new_cust_bill_pay = FS::cust_bill_pay->new({ + $cust_bill_pay->hash, + billpaynum => '', + amount => sprintf('%.2f', + $cust_bill_pay->amount - $unapplied_payments{$billpaynum}), + }); + + if ( $new_cust_bill_pay->amount > 0 ) { + $error ||= $new_cust_bill_pay->insert; + } + if ( $error ) { + $dbh->rollback if $oldAutoCommit; + return "Error unapplying payment: $error"; + } } #$subtotal += $amount; - $cust_credit_bill{$cust_bill_pkg->invnum} += $amount; - push @{ $cust_credit_bill_pkg{$cust_bill_pkg->invnum} }, + $cust_credit_bill{$invnum} += $amount; + push @{ $cust_credit_bill_pkg{$invnum} }, new FS::cust_credit_bill_pkg { 'billpkgnum' => $cust_bill_pkg->billpkgnum, - 'amount' => $amount, + 'amount' => sprintf('%.2f',$amount), 'setuprecur' => $setuprecur, 'sdate' => $cust_bill_pkg->sdate, 'edate' => $cust_bill_pkg->edate, }; + # recalculate taxes with new amounts + $taxlisthash{$invnum} ||= {}; my $part_pkg = $cust_bill_pkg->part_pkg; $cust_main->_handle_taxes( $part_pkg, - $taxlisthash, + $taxlisthash{$invnum}, $cust_bill_pkg, $cust_bill_pkg->cust_pkg, - $cust_bill_pkg->cust_bill->_date, + $cust_bill_pkg->cust_bill->_date, #invoice time $cust_bill_pkg->cust_pkg->pkgpart, ); } @@ -781,83 +810,108 @@ sub credit_lineitems { foreach my $invnum ( sort { $a <=> $b } keys %cust_credit_bill ) { - #taxes + my $arrayref_or_error = + $cust_main->calculate_taxes( + $cust_bill_pkg{$invnum}, # list of taxable items that we're crediting + $taxlisthash{$invnum}, # list of tax-item bindings + $cust_bill_pkg{$invnum}->[0]->cust_bill->_date, # invoice time + ); - if ( @{ $cust_bill_pkg{$invnum} } ) { - - my $listref_or_error = - $cust_main->calculate_taxes( $cust_bill_pkg{$invnum}, $taxlisthash, $cust_bill_pkg{$invnum}->[0]->cust_bill->_date ); + unless ( ref( $arrayref_or_error ) ) { + $dbh->rollback if $oldAutoCommit; + return "Error calculating taxes: $arrayref_or_error"; + } + + my %tax_links; # {tax billpkgnum}{nontax billpkgnum} - unless ( ref( $listref_or_error ) ) { - $dbh->rollback if $oldAutoCommit; - return "Error calculating taxes: $listref_or_error"; + #taxes + foreach my $cust_bill_pkg ( @{ $cust_bill_pkg{$invnum} } ) { + my $billpkgnum = $cust_bill_pkg->billpkgnum; + my %hash = ( 'taxable_billpkgnum' => $billpkgnum ); + # gather up existing tax links (we need their billpkgtaxlocationnums) + my @tax_links = qsearch('cust_bill_pkg_tax_location', \%hash), + qsearch('cust_bill_pkg_tax_rate_location', \%hash); + + foreach ( @tax_links ) { + $tax_links{$_->billpkgnum} ||= {}; + $tax_links{$_->billpkgnum}{$_->taxable_billpkgnum} = $_; } + } - # so, loop through the taxlines, apply just that amount to the tax line - # item (save for later insert) & add to $ - - #my @taxlines = (); - #my $taxtotal = 0; - foreach my $taxline ( @$listref_or_error ) { - - #find equivalent tax line items on the existing invoice - # (XXX need a more specific/deterministic way to find these than itemdesc..) - my $tax_cust_bill_pkg = qsearchs('cust_bill_pkg', { - 'invnum' => $invnum, - 'pkgnum' => 0, #$taxline->invnum - 'itemdesc' => $taxline->desc, - }); - - my $amount = $taxline->setup; - my $desc = $taxline->desc; - - foreach my $location ( $tax_cust_bill_pkg->cust_bill_pkg_tax_Xlocation ) { - - #support partial credits: use $amount if smaller - # (so just distribute to the first location? perhaps should - # do so evenly...) - my $loc_amount = min( $amount, $location->amount); - - $amount -= $loc_amount; - - #push @taxlines, - # #[ $location->desc, $taxline->setup, $taxlocnum, $taxratelocnum ]; - # [ $location->desc, $location->amount, $taxlocnum, $taxratelocnum ]; - $cust_credit_bill{$invnum} += $loc_amount; - push @{ $cust_credit_bill_pkg{$invnum} }, - new FS::cust_credit_bill_pkg { - 'billpkgnum' => $tax_cust_bill_pkg->billpkgnum, - 'amount' => $loc_amount, - 'setuprecur' => 'setup', - 'billpkgtaxlocationnum' => $location->billpkgtaxlocationnum, - 'billpkgtaxratelocationnum' => $location->billpkgtaxratelocationnum, - }; - - } - if ($amount > 0) { - #$taxtotal += $amount; - #push @taxlines, - # [ $taxline->itemdesc. ' (default)', sprintf('%.2f', $amount), '', '' ]; - - $cust_credit_bill{$invnum} += $amount; - push @{ $cust_credit_bill_pkg{$invnum} }, - new FS::cust_credit_bill_pkg { - 'billpkgnum' => $tax_cust_bill_pkg->billpkgnum, - 'amount' => $amount, - 'setuprecur' => 'setup', - }; - + foreach my $taxline ( @$arrayref_or_error ) { + + my $amount = $taxline->setup; + + # find equivalent tax line item on the existing invoice + my $tax_item = qsearchs('cust_bill_pkg', { + 'invnum' => $invnum, + 'pkgnum' => 0, + 'itemdesc' => $taxline->desc, + }); + if (!$tax_item) { + # or should we just exit if this happens? + $cust_credit->set('amount', + sprintf('%.2f', $cust_credit->get('amount') - $amount) + ); + my $error = $cust_credit->replace; + if ( $error ) { + $dbh->rollback if $oldAutoCommit; + return "error correcting credit for missing tax line: $error"; } } - } + # but in the new era, we no longer have the problem of uniquely + # identifying the tax_Xlocation record. The billpkgnums of the + # tax and the taxed item are known. + foreach my $new_loc + ( @{ $taxline->get('cust_bill_pkg_tax_location') }, + @{ $taxline->get('cust_bill_pkg_tax_rate_location') } ) + { + # the existing tax_Xlocation object + my $old_loc = + $tax_links{$tax_item->billpkgnum}{$new_loc->taxable_billpkgnum}; + + next if !$old_loc; # apply the leftover amount nonspecifically + + #support partial credits: use $amount if smaller + # (so just distribute to the first location? perhaps should + # do so evenly...) + my $loc_amount = min( $amount, $new_loc->amount); + + $amount -= $loc_amount; + + $cust_credit_bill{$invnum} += $loc_amount; + push @{ $cust_credit_bill_pkg{$invnum} }, + new FS::cust_credit_bill_pkg { + 'billpkgnum' => $tax_item->billpkgnum, + 'amount' => $loc_amount, + 'setuprecur' => 'setup', + 'billpkgtaxlocationnum' => $old_loc->billpkgtaxlocationnum, + 'billpkgtaxratelocationnum' => $old_loc->billpkgtaxratelocationnum, + }; + + } #foreach my $new_loc + + # we still have to deal with the possibility that the tax links don't + # cover the whole amount of tax because of an incomplete upgrade... + if ($amount > 0) { + $cust_credit_bill{$invnum} += $amount; + push @{ $cust_credit_bill_pkg{$invnum} }, + new FS::cust_credit_bill_pkg { + 'billpkgnum' => $tax_item->billpkgnum, + 'amount' => $amount, + 'setuprecur' => 'setup', + }; + + } # if $amount > 0 + } #foreach $taxline #insert cust_credit_bill my $cust_credit_bill = new FS::cust_credit_bill { 'crednum' => $cust_credit->crednum, 'invnum' => $invnum, - 'amount' => $cust_credit_bill{$invnum}, + 'amount' => sprintf('%.2f', $cust_credit_bill{$invnum}), }; $error = $cust_credit_bill->insert; if ( $error ) { diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm index 0ec6b5429..6d3ff9146 100644 --- a/FS/FS/cust_main/Billing.pm +++ b/FS/FS/cust_main/Billing.pm @@ -760,11 +760,13 @@ sub calculate_taxes { my %tax_exemption; foreach my $tax ( keys %$taxlisthash ) { - # $tax is a tax identifier + # $tax is a tax identifier (intersection of a tax definition record + # and a cust_bill_pkg record) my $tax_object = shift @{ $taxlisthash->{$tax} }; # $tax_object is a cust_main_county or tax_rate - # (with pkgnum and locationnum set) - # the rest of @{ $taxlisthash->{$tax} } is cust_bill_pkg objects + # (with billpkgnum, pkgnum, locationnum set) + # the rest of @{ $taxlisthash->{$tax} } is cust_bill_pkg component objects + # (setup, recurring, usage classes) warn "found ". $tax_object->taxname. " as $tax\n" if $DEBUG > 2; warn " ". join('/', @{ $taxlisthash->{$tax} } ). "\n" if $DEBUG > 2; # taxline calculates the tax on all cust_bill_pkgs in the @@ -808,6 +810,7 @@ sub calculate_taxes { 'pkgnum' => $tax_object->get('pkgnum'), 'locationnum' => $tax_object->get('locationnum'), 'amount' => sprintf('%.2f', $amount ), + 'taxable_billpkgnum' => $tax_object->get('billpkgnum'), }; } elsif ( ref($tax_object) eq 'FS::tax_rate' ) { @@ -820,6 +823,7 @@ sub calculate_taxes { 'amount' => sprintf('%.2f', $amount ), 'locationtaxid' => $tax_object->location, 'taxratelocationnum' => $taxratelocationnum, + 'taxable_billpkgnum' => $tax_object->get('billpkgnum'), }; } @@ -1284,6 +1288,10 @@ sub _handle_taxes { foreach (@taxes) { # These could become cust_bill_pkg_tax_location records, # or cust_tax_exempt_pkg. We'll decide later. + # + # The most important thing here: record which charge is being taxed. + $_->set('billpkgnum', $cust_bill_pkg->billpkgnum); + # also these, for historical reasons $_->set('pkgnum', $cust_pkg->pkgnum); $_->set('locationnum', $cust_pkg->tax_locationnum); } @@ -1325,8 +1333,8 @@ sub _handle_taxes { # this is the tax identifier, not the taxname my $taxname = ref( $tax ). ' '. $tax->taxnum; - $taxname .= ' pkgnum'. $cust_pkg->pkgnum; - # We need to create a separate $taxlisthash entry for each pkgnum + $taxname .= ' billpkgnum'. $cust_bill_pkg->billpkgnum; + # We need to create a separate $taxlisthash entry for each billpkgnum # on the invoice, so that cust_bill_pkg_tax_location records will # be linked correctly. diff --git a/FS/FS/log_context.pm b/FS/FS/log_context.pm index 372bdaa39..a25490588 100644 --- a/FS/FS/log_context.pm +++ b/FS/FS/log_context.pm @@ -12,6 +12,8 @@ my @contexts = ( qw( spool_upload daily queue + upgrade + upgrade_taxable_billpkgnum ) ); =head1 NAME diff --git a/httemplate/edit/credit-cust_bill_pkg.html b/httemplate/edit/credit-cust_bill_pkg.html index e0ca04b5e..3d1cf2438 100644 --- a/httemplate/edit/credit-cust_bill_pkg.html +++ b/httemplate/edit/credit-cust_bill_pkg.html @@ -80,6 +80,7 @@ 'field' => 'reasonnum', 'reason_class' => 'R', #XXX reconcile both this and show_taxes wanteding to enable this + 'id' => 'select_reason', 'control_button' => "document.getElementById('credit_button')", 'cgi' => $cgi, &> @@ -114,6 +115,8 @@ %> <SCRIPT TYPE="text/javascript"> +document.getElementById('select_reason').disabled = true; + // start it disabled because no line items are selected yet function show_taxes(arg) { var argsHash = eval('(' + arg + ')'); @@ -186,14 +189,16 @@ function show_taxes(arg) { //XXX reconcile both this and the reason selector wanteding to enable this if ( total > 0 ) { - document.getElementById('credit_button').disabled = false; + //document.getElementById('credit_button').disabled = false; + document.getElementById('select_reason').disabled = false; } } function calc_total(what) { - document.getElementById('credit_button').disabled = true; + //document.getElementById('credit_button').disabled = true; + document.getElementById('select_reason').disabled = true; var subtotal = 0; // bah, a pain, just using an attribute var re = /^billpkgnum(\d+)$/; |