[freeside-commits] branch FREESIDE_2_3_BRANCH updated. 8350bd8c54302669ded9e20285b53a1cca996473
Mark Wells
mark at 420.am
Fri Dec 14 17:59:59 PST 2012
The branch, FREESIDE_2_3_BRANCH has been updated
via 8350bd8c54302669ded9e20285b53a1cca996473 (commit)
from 817b5b87447dc24ca004ee8ca014c4a4d527c741 (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 8350bd8c54302669ded9e20285b53a1cca996473
Author: Mark Wells <mark at freeside.biz>
Date: Fri Dec 14 17:57:46 2012 -0800
fixes for line item credit application, #18676
diff --git a/FS/FS/cust_credit.pm b/FS/FS/cust_credit.pm
index dfe55fb..1f54777 100644
--- a/FS/FS/cust_credit.pm
+++ b/FS/FS/cust_credit.pm
@@ -644,7 +644,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
@@ -706,7 +705,8 @@ sub credit_lineitems {
}
#my $subtotal = 0;
- my $taxlisthash = {};
+ # keys in all of these are invoice numbers
+ my %taxlisthash = ();
my %cust_credit_bill = ();
my %cust_bill_pkg = ();
my %cust_credit_bill_pkg = ();
@@ -721,6 +721,8 @@ sub credit_lineitems {
'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);
$cust_bill_pkg->recur(0);
@@ -733,8 +735,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;
@@ -742,22 +745,44 @@ 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";
+ $cust_bill_pay->set('amount',
+ sprintf('%.2f',
+ $cust_bill_pay->get('amount') - $unapplied_payments{$billpaynum})
+ );
+ if ( $cust_bill_pay->amount >= 0.005 ) {
+ $error = $cust_bill_pay->replace;
+ } else {
+ $error = $cust_bill_pay->delete;
+ }
+ 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,
};
+ $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,
@@ -779,7 +804,11 @@ sub credit_lineitems {
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 );
+ $cust_main->calculate_taxes(
+ $cust_bill_pkg{$invnum},
+ $taxlisthash{$invnum},
+ $cust_bill_pkg{$invnum}->[0]->cust_bill->_date
+ );
unless ( ref( $listref_or_error ) ) {
$dbh->rollback if $oldAutoCommit;
@@ -793,38 +822,78 @@ sub credit_lineitems {
#my $taxtotal = 0;
foreach my $taxline ( @$listref_or_error ) {
+ my $amount = $taxline->setup;
+
#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,
});
+ if (!$tax_cust_bill_pkg) {
+ # Very debatable. We expected the credit to include tax and
+ # the tax is not on the invoice. Perhaps we should just bail
+ # out in this case.
+ #die "missing tax line item for invnum $invnum, description ".
+ # $taxline->desc."\n";
+ $cust_credit->set('amount',
+ sprintf('%.2f',
+ $cust_credit->get('amount') - $amount)
+ );
+ my $error = $cust_credit->replace;
+ die "error correcting credit for missing tax line: $error\n"
+ if $error;
+ next; #$taxline
+ }
- my $amount = $taxline->setup;
- my $desc = $taxline->desc;
-
- foreach my $location ( $tax_cust_bill_pkg->cust_bill_pkg_tax_Xlocation ) {
-
- $location->cust_bill_pkg_desc($taxline->desc); #ugh @ that kludge
-
- #$taxtotal += $location->amount;
- $amount -= $location->amount;
+ # Tricky business:
+ # The existing tax_Xlocation records may not have the same pkgnum as
+ # the line item we're crediting. If there's another line item on
+ # this invoice with the same taxnum (tax table line) as this tax,
+ # then they may have its pkgnum instead. Under 2.3 there is no
+ # way to exactly find the taxes associated with a taxable item.
+ # Even if the record DOES have the same pkgnum, it may include taxes
+ # from _other_ line items, and we only want to credit the amount
+ # that's due to the selected line item.
+ #
+ # Index the tax_Xlocation records by calculate_taxes "tax identifier".
+ my %xlocation_map;
+ foreach my $old_loc
+ ( $tax_cust_bill_pkg->cust_bill_pkg_tax_Xlocation )
+ {
+ my $taxid = $old_loc->taxtype . ' ' . $old_loc->taxnum;
+ warn "DUPLICATE TAX BREAKDOWN RECORD inv#$invnum $taxid\n"
+ if defined($xlocation_map{$taxid});
+
+ $xlocation_map{$taxid} = $old_loc;
+ }
- #push @taxlines,
- # #[ $location->desc, $taxline->setup, $taxlocnum, $taxratelocnum ];
- # [ $location->desc, $location->amount, $taxlocnum, $taxratelocnum ];
- $cust_credit_bill{$invnum} += $location->amount;
- push @{ $cust_credit_bill_pkg{$invnum} },
- new FS::cust_credit_bill_pkg {
- 'billpkgnum' => $tax_cust_bill_pkg->billpkgnum,
- 'amount' => $location->amount,
- 'setuprecur' => 'setup',
- 'billpkgtaxlocationnum' => $location->billpkgtaxlocationnum,
- 'billpkgtaxratelocationnum' => $location->billpkgtaxratelocationnum,
- };
+ #now loop over the calculated taxes
+ foreach my $new_loc
+ ( @{ $taxline->get('cust_bill_pkg_tax_location') },
+ @{ $taxline->get('cust_bill_pkg_tax_rate_location') } )
+ {
+ my $taxid = $new_loc->taxtype . ' ' . $new_loc->taxnum;
+ # $taxid MUST match
+ my $old_loc = $xlocation_map{$taxid};
+ if ( $old_loc ) {
+ # then apply the amount of $new_loc to it
+ $amount -= $new_loc->amount;
+
+ $cust_credit_bill{$invnum} += $new_loc->amount;
+ push @{ $cust_credit_bill_pkg{$invnum} },
+ new FS::cust_credit_bill_pkg {
+ 'billpkgnum' => $tax_cust_bill_pkg->billpkgnum,
+ 'amount' => $new_loc->amount,
+ 'setuprecur' => 'setup',
+ 'billpkgtaxlocationnum' => $old_loc->billpkgtaxlocationnum,
+ 'billpkgtaxratelocationnum' => $old_loc->billpkgtaxratelocationnum,
+ };
+ } else {
+ # do nothing, and apply the leftover amount nonspecifically
+ }
+ } #foreach my $new_loc
- }
if ($amount > 0) {
#$taxtotal += $amount;
#push @taxlines,
@@ -838,17 +907,17 @@ sub credit_lineitems {
'setuprecur' => 'setup',
};
- }
- }
+ } # if $amount > 0
+ } #foreach $taxline
- }
+ } # if @{ $cust_bill_pkg{$invnum} }
#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 ca0f8e5..0fb91ef 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -798,9 +798,9 @@ sub calculate_taxes {
#move the cust_tax_exempt_pkg records to the cust_bill_pkgs we will commit
my %packagemap = map { $_->pkgnum => $_ } @$cust_bill_pkg;
foreach my $tax ( keys %$taxlisthash ) {
- foreach ( @{ $taxlisthash->{$tax} }[1 ... scalar(@{ $taxlisthash->{$tax} })] ) {
- next unless ref($_) eq 'FS::cust_bill_pkg';
-
+ foreach ( @{ $taxlisthash->{$tax} }[1 .. scalar(@{ $taxlisthash->{$tax}}) - 1] ) {
+ #next unless ref($_) eq 'FS::cust_bill_pkg'; #no longer needed
+
my @cust_tax_exempt_pkg = splice( @{ $_->_cust_tax_exempt_pkg } );
next unless @cust_tax_exempt_pkg; #just avoiding the prob when irrelevant?
diff --git a/httemplate/edit/credit-cust_bill_pkg.html b/httemplate/edit/credit-cust_bill_pkg.html
index e317936..1c6d5e5 100644
--- a/httemplate/edit/credit-cust_bill_pkg.html
+++ b/httemplate/edit/credit-cust_bill_pkg.html
@@ -69,6 +69,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,
&>
@@ -94,6 +95,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 + ')');
@@ -166,14 +169,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/cust_credit.pm | 139 +++++++++++++++++++++-------
FS/FS/cust_main/Billing.pm | 6 +-
httemplate/edit/credit-cust_bill_pkg.html | 9 ++-
3 files changed, 114 insertions(+), 40 deletions(-)
More information about the freeside-commits
mailing list