[freeside-commits] branch master updated. 95cef2cea4c98d8fde7f58bacce3cf1da955c1a0
Mark Wells
mark at 420.am
Fri Dec 21 16:01:24 PST 2012
The branch, master has been updated
via 95cef2cea4c98d8fde7f58bacce3cf1da955c1a0 (commit)
from d837405e32b0c698e2c13d1080a2135a7e717f1b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 95cef2cea4c98d8fde7f58bacce3cf1da955c1a0
Author: Mark Wells <mark at freeside.biz>
Date: Fri Dec 21 15:49:42 2012 -0800
better internal tax links and various credit_lineitems fixes, #20629
diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 172ac82..b6fd3b6 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 45cba82..fea53a2 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 a83af13..5cff140 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 44dd6e3..723d6e0 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 fe9572f..7741bbe 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 0ec6b54..6d3ff91 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 372bdaa..a254905 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 e0ca04b..3d1cf24 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+)$/;
-----------------------------------------------------------------------
Summary of changes:
FS/FS/Schema.pm | 16 ++-
FS/FS/Upgrade.pm | 3 +
FS/FS/cust_bill_pkg.pm | 11 +-
FS/FS/cust_bill_pkg_tax_location.pm | 281 ++++++++++++++++++++++++++++-
FS/FS/cust_credit.pm | 198 +++++++++++++--------
FS/FS/cust_main/Billing.pm | 18 ++-
FS/FS/log_context.pm | 2 +
httemplate/edit/credit-cust_bill_pkg.html | 9 +-
8 files changed, 449 insertions(+), 89 deletions(-)
More information about the freeside-commits
mailing list