[freeside-commits] branch master updated. a14db43cc4ec18badd0aff4fbc3e6738f4f63f6c
Mark Wells
mark at 420.am
Sun Dec 30 20:54:47 PST 2012
The branch, master has been updated
via a14db43cc4ec18badd0aff4fbc3e6738f4f63f6c (commit)
via bce3e7d93335196a137f6dbfc75335668842ecff (commit)
via 49e08fcef54f4c4a3463d91f1cfcb69f121b2c5a (commit)
from 6e4fd158b8a797fc4702c7aa35f24c4bbd0f550f (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 a14db43cc4ec18badd0aff4fbc3e6738f4f63f6c
Author: Mark Wells <mark at freeside.biz>
Date: Sun Dec 30 17:03:58 2012 -0800
fix linking of cust_bill_pkg_tax_location records during billing, #20629
diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index 5cff140..716c098 100644
--- a/FS/FS/cust_bill_pkg.pm
+++ b/FS/FS/cust_bill_pkg.pm
@@ -201,16 +201,50 @@ sub insert {
my $tax_location = $self->get('cust_bill_pkg_tax_location');
if ( $tax_location ) {
- foreach my $cust_bill_pkg_tax_location ( @$tax_location ) {
- $cust_bill_pkg_tax_location->billpkgnum($self->billpkgnum);
- $error = $cust_bill_pkg_tax_location->insert;
- if ( $error ) {
- $dbh->rollback if $oldAutoCommit;
- return "error inserting cust_bill_pkg_tax_location: $error";
+ foreach my $link ( @$tax_location ) {
+ next if $link->billpkgtaxlocationnum; # don't try to double-insert
+ # This cust_bill_pkg can be linked on either side (i.e. it can be the
+ # tax or the taxed item). If the other side is already inserted,
+ # then set billpkgnum to ours, and insert the link. Otherwise,
+ # set billpkgnum to ours and pass the link off to the cust_bill_pkg
+ # on the other side, to be inserted later.
+
+ my $tax_cust_bill_pkg = $link->get('tax_cust_bill_pkg');
+ if ( $tax_cust_bill_pkg && $tax_cust_bill_pkg->billpkgnum ) {
+ $link->set('billpkgnum', $tax_cust_bill_pkg->billpkgnum);
+ # break circular links when doing this
+ $link->set('tax_cust_bill_pkg', '');
}
- }
+ my $taxable_cust_bill_pkg = $link->get('taxable_cust_bill_pkg');
+ if ( $taxable_cust_bill_pkg && $taxable_cust_bill_pkg->billpkgnum ) {
+ $link->set('taxable_billpkgnum', $taxable_cust_bill_pkg->billpkgnum);
+ # XXX if we ever do tax-on-tax for these, this will have to change
+ # since pkgnum will be zero
+ $link->set('pkgnum', $taxable_cust_bill_pkg->pkgnum);
+ $link->set('locationnum',
+ $taxable_cust_bill_pkg->cust_pkg->tax_locationnum);
+ $link->set('taxable_cust_bill_pkg', '');
+ }
+
+ if ( $link->billpkgnum and $link->taxable_billpkgnum ) {
+ $error = $link->insert;
+ if ( $error ) {
+ $dbh->rollback if $oldAutoCommit;
+ return "error inserting cust_bill_pkg_tax_location: $error";
+ }
+ } else { # handoff
+ my $other;
+ $other = $link->billpkgnum ? $link->get('taxable_cust_bill_pkg')
+ : $link->get('tax_cust_bill_pkg');
+ my $link_array = $other->get('cust_bill_pkg_tax_location') || [];
+ push @$link_array, $link;
+ $other->set('cust_bill_pkg_tax_location' => $link_array);
+ }
+ } #foreach my $link
}
+ # someday you will be as awesome as cust_bill_pkg_tax_location...
+ # but not today
my $tax_rate_location = $self->get('cust_bill_pkg_tax_rate_location');
if ( $tax_rate_location ) {
foreach my $cust_bill_pkg_tax_rate_location ( @$tax_rate_location ) {
diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index 6d3ff91..cd46c73 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -687,8 +687,6 @@ sub _omit_zero_value_bundles {
=item calculate_taxes LINEITEMREF TAXHASHREF INVOICE_TIME
-This is a weird one. Perhaps it should not even be exposed.
-
Generates tax line items (see L<FS::cust_bill_pkg>) for this customer.
Usually used internally by bill method B<bill>.
@@ -755,7 +753,7 @@ sub calculate_taxes {
# values are arrayrefs of cust_bill_pkg_tax_rate_location hashrefs
my %tax_rate_location = ();
- # keys are taxnums (not internal identifiers!)
+ # keys are taxlisthash keys (internal identifiers!)
# values are arrayrefs of cust_tax_exempt_pkg objects
my %tax_exemption;
@@ -775,45 +773,35 @@ sub calculate_taxes {
# It also calculates exemptions and attaches them to the cust_bill_pkgs
# in the argument.
my $taxables = $taxlisthash->{$tax};
- my $exemptions = $tax_exemption{$tax_object->taxnum} ||= [];
- my $hashref_or_error =
- $tax_object->taxline( $taxables,
+ my $exemptions = $tax_exemption{$tax} ||= [];
+ my $taxline = $tax_object->taxline(
+ $taxables,
'custnum' => $self->custnum,
'invoice_time' => $invoice_time,
'exemptions' => $exemptions,
);
- return $hashref_or_error unless ref($hashref_or_error);
-
- # then collect any new exemptions generated for this tax
- push @$exemptions, @{ $_->cust_tax_exempt_pkg }
- foreach @$taxables;
+ return $taxline unless ref($taxline);
unshift @{ $taxlisthash->{$tax} }, $tax_object;
- my $name = $hashref_or_error->{'name'};
- my $amount = $hashref_or_error->{'amount'};
+ if ( $tax_object->isa('FS::cust_main_county') ) {
+ # then $taxline is a real line item
+ push @{ $taxname{ $taxline->itemdesc } }, $taxline;
- #warn "adding $amount as $name\n";
- $taxname{ $name } ||= [];
- push @{ $taxname{ $name } }, $tax;
+ } else {
+ # leave this as is for now
- $tax_amount{ $tax } += $amount;
+ my $name = $taxline->{'name'};
+ my $amount = $taxline->{'amount'};
- # link records between cust_main_county/tax_rate and cust_location
- $tax_location{ $tax } ||= [];
- $tax_rate_location{ $tax } ||= [];
- if ( ref($tax_object) eq 'FS::cust_main_county' ) {
- push @{ $tax_location{ $tax } },
- {
- 'taxnum' => $tax_object->taxnum,
- 'taxtype' => ref($tax_object),
- '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' ) {
+ #warn "adding $amount as $name\n";
+ $taxname{ $name } ||= [];
+ push @{ $taxname{ $name } }, $tax;
+
+ $tax_amount{ $tax } += $amount;
+
+ # link records between cust_main_county/tax_rate and cust_location
+ $tax_rate_location{ $tax } ||= [];
my $taxratelocationnum =
$tax_object->tax_rate_location->taxratelocationnum;
push @{ $tax_rate_location{ $tax } },
@@ -823,56 +811,53 @@ sub calculate_taxes {
'amount' => sprintf('%.2f', $amount ),
'locationtaxid' => $tax_object->location,
'taxratelocationnum' => $taxratelocationnum,
- 'taxable_billpkgnum' => $tax_object->get('billpkgnum'),
};
- }
-
- }
-
- #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 ) {
- my $taxables = $taxlisthash->{$tax};
- my $tax_object = shift @$taxables; # the rest are line items
- foreach my $cust_bill_pkg ( @$taxables ) {
- next unless ref($cust_bill_pkg) eq 'FS::cust_bill_pkg'; #IS needed for CCH tax-on-tax
-
- my @cust_tax_exempt_pkg = splice @{ $cust_bill_pkg->cust_tax_exempt_pkg };
-
- next unless @cust_tax_exempt_pkg;
- # get the non-disintegrated version
- my $real_cust_bill_pkg = $packagemap{$cust_bill_pkg->pkgnum}
- or die "can't distribute tax exemptions: no line item for ".
- Dumper($_). " in packagemap ".
- join(',', sort {$a<=>$b} keys %packagemap). "\n";
-
- push @{ $real_cust_bill_pkg->cust_tax_exempt_pkg },
- @cust_tax_exempt_pkg;
- }
- }
+ } #if ref($tax_object)...
+ } #foreach keys %$taxlisthash
#consolidate and create tax line items
warn "consolidating and generating...\n" if $DEBUG > 2;
foreach my $taxname ( keys %taxname ) {
+ my @cust_bill_pkg_tax_location;
+ my @cust_bill_pkg_tax_rate_location;
+ my $tax_cust_bill_pkg = FS::cust_bill_pkg->new({
+ 'pkgnum' => 0,
+ 'recur' => 0,
+ 'sdate' => '',
+ 'edate' => '',
+ 'itemdesc' => $taxname,
+ 'cust_bill_pkg_tax_location' => \@cust_bill_pkg_tax_location,
+ 'cust_bill_pkg_tax_rate_location' => \@cust_bill_pkg_tax_rate_location,
+ });
+
my $tax_total = 0;
my %seen = ();
- my @cust_bill_pkg_tax_location = ();
- my @cust_bill_pkg_tax_rate_location = ();
warn "adding $taxname\n" if $DEBUG > 1;
foreach my $taxitem ( @{ $taxname{$taxname} } ) {
- next if $seen{$taxitem}++;
- warn "adding $tax_amount{$taxitem}\n" if $DEBUG > 1;
- $tax_total += $tax_amount{$taxitem};
- push @cust_bill_pkg_tax_location,
- map { new FS::cust_bill_pkg_tax_location $_ }
- @{ $tax_location{ $taxitem } };
- push @cust_bill_pkg_tax_rate_location,
- map { new FS::cust_bill_pkg_tax_rate_location $_ }
- @{ $tax_rate_location{ $taxitem } };
+ if ( ref($taxitem) eq 'FS::cust_bill_pkg' ) {
+ # then we need to transfer the amount and the links from the
+ # line item to the new one we're creating.
+ $tax_total += $taxitem->setup;
+ foreach my $link ( @{ $taxitem->get('cust_bill_pkg_tax_location') } ) {
+ $link->set('tax_cust_bill_pkg', $tax_cust_bill_pkg);
+ push @cust_bill_pkg_tax_location, $link;
+ }
+ } else {
+ # the tax_rate way
+ next if $seen{$taxitem}++;
+ warn "adding $tax_amount{$taxitem}\n" if $DEBUG > 1;
+ $tax_total += $tax_amount{$taxitem};
+ push @cust_bill_pkg_tax_rate_location,
+ map { new FS::cust_bill_pkg_tax_rate_location $_ }
+ @{ $tax_rate_location{ $taxitem } };
+ }
}
next unless $tax_total;
+ # we should really neverround this up...I guess it's okay if taxline
+ # already returns amounts with 2 decimal places
$tax_total = sprintf('%.2f', $tax_total );
+ $tax_cust_bill_pkg->set('setup', $tax_total);
my $pkg_category = qsearchs( 'pkg_category', { 'categoryname' => $taxname,
'disabled' => '',
@@ -890,19 +875,9 @@ sub calculate_taxes {
push @display, new FS::cust_bill_pkg_display { type => 'S', %hash };
}
+ $tax_cust_bill_pkg->set('display', \@display);
- push @tax_line_items, new FS::cust_bill_pkg {
- 'pkgnum' => 0,
- 'setup' => $tax_total,
- 'recur' => 0,
- 'sdate' => '',
- 'edate' => '',
- 'itemdesc' => $taxname,
- 'display' => \@display,
- 'cust_bill_pkg_tax_location' => \@cust_bill_pkg_tax_location,
- 'cust_bill_pkg_tax_rate_location' => \@cust_bill_pkg_tax_rate_location,
- };
-
+ push @tax_line_items, $tax_cust_bill_pkg;
}
\@tax_line_items;
@@ -1184,11 +1159,23 @@ sub _make_lines {
# handle taxes
###
- unless ( $discount_show_always ) {
- my $error =
- $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg, $cust_pkg, $options{invoice_time}, $real_pkgpart, \%options);
- return $error if $error;
- }
+ #unless ( $discount_show_always ) { # oh, for god's sake
+ my $error = $self->_handle_taxes(
+ $part_pkg,
+ $taxlisthash,
+ $cust_bill_pkg,
+ $cust_pkg,
+ $options{invoice_time},
+ $real_pkgpart,
+ \%options # I have serious objections to this
+ );
+ return $error if $error;
+ #}
+
+ $cust_bill_pkg->set_display(
+ part_pkg => $part_pkg,
+ real_pkgpart => $real_pkgpart,
+ );
push @$cust_bill_pkgs, $cust_bill_pkg;
@@ -1200,6 +1187,25 @@ sub _make_lines {
}
+# This is _handle_taxes. It's called once for each cust_bill_pkg generated
+# from _make_lines, along with the part_pkg, cust_pkg, invoice time, the
+# non-overridden pkgpart, a flag indicating whether the package is being
+# canceled, and a partridge in a pear tree.
+#
+# The most important argument is 'taxlisthash'. This is shared across the
+# entire invoice. It looks like this:
+# {
+# 'cust_main_county 1001' => [ [FS::cust_main_county], ... ],
+# 'cust_main_county 1002' => [ [FS::cust_main_county], ... ],
+# }
+#
+# 'cust_main_county' can also be 'tax_rate'. The first object in the array
+# is always the cust_main_county or tax_rate identified by the key.
+#
+# That "..." is a list of FS::cust_bill_pkg objects that will be fed to
+# the 'taxline' method to calculate the amount of the tax. This doesn't
+# happen until calculate_taxes, though.
+
sub _handle_taxes {
my $self = shift;
my $part_pkg = shift;
@@ -1212,175 +1218,152 @@ sub _handle_taxes {
local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
- my %cust_bill_pkg = ();
- my %taxes = ();
-
- my @classes;
- #push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->type eq 'U';
- push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->usage;
- push @classes, 'setup' if ($cust_bill_pkg->setup && !$options->{cancel});
- push @classes, 'recur' if ($cust_bill_pkg->recur && !$options->{cancel});
-
- my $exempt = $conf->exists('cust_class-tax_exempt')
- ? ( $self->cust_class ? $self->cust_class->tax : '' )
- : $self->tax;
- # standardize this just to be sure
- $exempt = ($exempt eq 'Y') ? 'Y' : '';
-
- #if ( $exempt !~ /Y/i && $self->payby ne 'COMP' ) {
- if ( $self->payby ne 'COMP' ) {
-
- if ( $conf->exists('enable_taxproducts')
- && ( scalar($part_pkg->part_pkg_taxoverride)
- || $part_pkg->has_taxproduct
- )
- )
- {
+ return if ( $self->payby eq 'COMP' ); #dubious
- if ( !$exempt ) {
+ if ( $conf->exists('enable_taxproducts')
+ && ( scalar($part_pkg->part_pkg_taxoverride)
+ || $part_pkg->has_taxproduct
+ )
+ )
+ {
- foreach my $class (@classes) {
- my $err_or_ref = $self->_gather_taxes( $part_pkg, $class, $cust_pkg );
- return $err_or_ref unless ref($err_or_ref);
- $taxes{$class} = $err_or_ref;
- }
+ # EXTERNAL TAX RATES (via tax_rate)
+ my %cust_bill_pkg = ();
+ my %taxes = ();
+
+ my @classes;
+ #push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->type eq 'U';
+ push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->usage;
+ # debatable
+ push @classes, 'setup' if ($cust_bill_pkg->setup && !$options->{cancel});
+ push @classes, 'recur' if ($cust_bill_pkg->recur && !$options->{cancel});
+
+ my $exempt = $conf->exists('cust_class-tax_exempt')
+ ? ( $self->cust_class ? $self->cust_class->tax : '' )
+ : $self->tax;
+ # standardize this just to be sure
+ $exempt = ($exempt eq 'Y') ? 'Y' : '';
+
+ if ( !$exempt ) {
- unless (exists $taxes{''}) {
- my $err_or_ref = $self->_gather_taxes( $part_pkg, '', $cust_pkg );
- return $err_or_ref unless ref($err_or_ref);
- $taxes{''} = $err_or_ref;
- }
+ foreach my $class (@classes) {
+ my $err_or_ref = $self->_gather_taxes( $part_pkg, $class, $cust_pkg );
+ return $err_or_ref unless ref($err_or_ref);
+ $taxes{$class} = $err_or_ref;
+ }
+ unless (exists $taxes{''}) {
+ my $err_or_ref = $self->_gather_taxes( $part_pkg, '', $cust_pkg );
+ return $err_or_ref unless ref($err_or_ref);
+ $taxes{''} = $err_or_ref;
}
- } else { # cust_main_county tax system
+ }
- # We fetch taxes even if the customer is completely exempt,
- # because we need to record that fact.
+ my %tax_cust_bill_pkg = $cust_bill_pkg->disintegrate;
+ foreach my $key (keys %tax_cust_bill_pkg) {
+ # $key is "setup", "recur", or a usage class name. ('' is a usage class.)
+ # $tax_cust_bill_pkg{$key} is a cust_bill_pkg for that component of
+ # the line item.
+ # $taxes{$key} is an arrayref of cust_main_county or tax_rate objects that
+ # apply to $key-class charges.
+ my @taxes = @{ $taxes{$key} || [] };
+ my $tax_cust_bill_pkg = $tax_cust_bill_pkg{$key};
+
+ my %localtaxlisthash = ();
+ foreach my $tax ( @taxes ) {
+
+ # this is the tax identifier, not the taxname
+ my $taxname = ref( $tax ). ' '. $tax->taxnum;
+ $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.
+
+ # $taxlisthash: keys are "setup", "recur", and usage classes.
+ # Values are arrayrefs, first the tax object (cust_main_county
+ # or tax_rate) and then any cust_bill_pkg objects that the
+ # tax applies to.
+ $taxlisthash->{ $taxname } ||= [ $tax ];
+ push @{ $taxlisthash->{ $taxname } }, $tax_cust_bill_pkg;
+
+ $localtaxlisthash{ $taxname } ||= [ $tax ];
+ push @{ $localtaxlisthash{ $taxname } }, $tax_cust_bill_pkg;
- my @loc_keys = qw( district city county state country );
- my $location = $cust_pkg->tax_location;
- my %taxhash = map { $_ => $location->$_ } @loc_keys;
+ }
- $taxhash{'taxclass'} = $part_pkg->taxclass;
+ warn "finding taxed taxes...\n" if $DEBUG > 2;
+ foreach my $tax ( keys %localtaxlisthash ) {
+ my $tax_object = shift @{ $localtaxlisthash{$tax} };
+ warn "found possible taxed tax ". $tax_object->taxname. " we call $tax\n"
+ if $DEBUG > 2;
+ next unless $tax_object->can('tax_on_tax');
+
+ foreach my $tot ( $tax_object->tax_on_tax( $self ) ) {
+ my $totname = ref( $tot ). ' '. $tot->taxnum;
+
+ warn "checking $totname which we call ". $tot->taxname. " as applicable\n"
+ if $DEBUG > 2;
+ next unless exists( $localtaxlisthash{ $totname } ); # only increase
+ # existing taxes
+ warn "adding $totname to taxed taxes\n" if $DEBUG > 2;
+ # we're calling taxline() right here? wtf?
+ my $hashref_or_error =
+ $tax_object->taxline( $localtaxlisthash{$tax},
+ 'custnum' => $self->custnum,
+ 'invoice_time' => $invoice_time,
+ );
+ return $hashref_or_error
+ unless ref($hashref_or_error);
+
+ $taxlisthash->{ $totname } ||= [ $tot ];
+ push @{ $taxlisthash->{ $totname } }, $hashref_or_error->{amount};
- warn "taxhash:\n". Dumper(\%taxhash) if $DEBUG > 2;
+ }
+ }
+ }
- my @taxes = (); # entries are cust_main_county objects
- my %taxhash_elim = %taxhash;
- my @elim = qw( district city county state );
- do {
+ } else {
- #first try a match with taxclass
- @taxes = qsearch( 'cust_main_county', \%taxhash_elim );
+ # INTERNAL TAX RATES (cust_main_county)
- if ( !scalar(@taxes) && $taxhash_elim{'taxclass'} ) {
- #then try a match without taxclass
- my %no_taxclass = %taxhash_elim;
- $no_taxclass{ 'taxclass' } = '';
- @taxes = qsearch( 'cust_main_county', \%no_taxclass );
- }
+ # We fetch taxes even if the customer is completely exempt,
+ # because we need to record that fact.
- $taxhash_elim{ shift(@elim) } = '';
+ my @loc_keys = qw( district city county state country );
+ my $location = $cust_pkg->tax_location;
+ my %taxhash = map { $_ => $location->$_ } @loc_keys;
- } while ( !scalar(@taxes) && scalar(@elim) );
+ $taxhash{'taxclass'} = $part_pkg->taxclass;
- 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);
- }
+ warn "taxhash:\n". Dumper(\%taxhash) if $DEBUG > 2;
- $taxes{''} = [ @taxes ];
- $taxes{'setup'} = [ @taxes ];
- $taxes{'recur'} = [ @taxes ];
- $taxes{$_} = [ @taxes ] foreach (@classes);
-
- # # maybe eliminate this entirely, along with all the 0% records
- # unless ( @taxes ) {
- # return
- # "fatal: can't find tax rate for state/county/country/taxclass ".
- # join('/', map $taxhash{$_}, qw(state county country taxclass) );
- # }
-
- } #if $conf->exists('enable_taxproducts') ...
-
- } # if $self->payby eq 'COMP'
-
- #what's this doing in the middle of _handle_taxes? probably should split
- #this into three parts above in _make_lines
- $cust_bill_pkg->set_display( part_pkg => $part_pkg,
- real_pkgpart => $real_pkgpart,
- );
-
- my %tax_cust_bill_pkg = $cust_bill_pkg->disintegrate;
- foreach my $key (keys %tax_cust_bill_pkg) {
- # $key is "setup", "recur", or a usage class name. ('' is a usage class.)
- # $tax_cust_bill_pkg{$key} is a cust_bill_pkg for that component of
- # the line item.
- # $taxes{$key} is an arrayref of cust_main_county or tax_rate objects that
- # apply to $key-class charges.
- my @taxes = @{ $taxes{$key} || [] };
- my $tax_cust_bill_pkg = $tax_cust_bill_pkg{$key};
-
- my %localtaxlisthash = ();
- foreach my $tax ( @taxes ) {
-
- # this is the tax identifier, not the taxname
- my $taxname = ref( $tax ). ' '. $tax->taxnum;
- $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.
-
- # $taxlisthash: keys are "setup", "recur", and usage classes.
- # Values are arrayrefs, first the tax object (cust_main_county
- # or tax_rate) and then any cust_bill_pkg objects that the
- # tax applies to.
- $taxlisthash->{ $taxname } ||= [ $tax ];
- push @{ $taxlisthash->{ $taxname } }, $tax_cust_bill_pkg;
-
- $localtaxlisthash{ $taxname } ||= [ $tax ];
- push @{ $localtaxlisthash{ $taxname } }, $tax_cust_bill_pkg;
+ my @taxes = (); # entries are cust_main_county objects
+ my %taxhash_elim = %taxhash;
+ my @elim = qw( district city county state );
+ do {
- }
+ #first try a match with taxclass
+ @taxes = qsearch( 'cust_main_county', \%taxhash_elim );
- warn "finding taxed taxes...\n" if $DEBUG > 2;
- foreach my $tax ( keys %localtaxlisthash ) {
- my $tax_object = shift @{ $localtaxlisthash{$tax} };
- warn "found possible taxed tax ". $tax_object->taxname. " we call $tax\n"
- if $DEBUG > 2;
- next unless $tax_object->can('tax_on_tax');
+ if ( !scalar(@taxes) && $taxhash_elim{'taxclass'} ) {
+ #then try a match without taxclass
+ my %no_taxclass = %taxhash_elim;
+ $no_taxclass{ 'taxclass' } = '';
+ @taxes = qsearch( 'cust_main_county', \%no_taxclass );
+ }
- foreach my $tot ( $tax_object->tax_on_tax( $self ) ) {
- my $totname = ref( $tot ). ' '. $tot->taxnum;
+ $taxhash_elim{ shift(@elim) } = '';
- warn "checking $totname which we call ". $tot->taxname. " as applicable\n"
- if $DEBUG > 2;
- next unless exists( $localtaxlisthash{ $totname } ); # only increase
- # existing taxes
- warn "adding $totname to taxed taxes\n" if $DEBUG > 2;
- my $hashref_or_error =
- $tax_object->taxline( $localtaxlisthash{$tax},
- 'custnum' => $self->custnum,
- 'invoice_time' => $invoice_time,
- );
- return $hashref_or_error
- unless ref($hashref_or_error);
-
- $taxlisthash->{ $totname } ||= [ $tot ];
- push @{ $taxlisthash->{ $totname } }, $hashref_or_error->{amount};
+ } while ( !scalar(@taxes) && scalar(@elim) );
- }
+ foreach (@taxes) {
+ my $tax_id = 'cust_main_county '.$_->taxnum;
+ $taxlisthash->{$tax_id} ||= [ $_ ];
+ push @{ $taxlisthash->{$tax_id} }, $cust_bill_pkg;
}
}
-
'';
}
diff --git a/FS/FS/cust_main_county.pm b/FS/FS/cust_main_county.pm
index 87c1ca7..db6be75 100644
--- a/FS/FS/cust_main_county.pm
+++ b/FS/FS/cust_main_county.pm
@@ -258,10 +258,15 @@ sub _list_sql {
=item taxline TAXABLES_ARRAYREF, [ OPTION => VALUE ... ]
-Returns an hashref of a name and an amount of tax calculated for the
-line items (L<FS::cust_bill_pkg> objects) in TAXABLES_ARRAYREF. The line
-items must come from the same invoice. Returns a scalar error message
-on error.
+Takes an arrayref of L<FS::cust_bill_pkg> objects representing taxable
+line items, and returns a new L<FS::cust_bill_pkg> object representing
+the tax on them under this tax rate.
+
+This will have a pseudo-field, "cust_bill_pkg_tax_location", containing
+an arrayref of L<FS::cust_bill_pkg_tax_location> objects. Each of these
+will in turn have a "taxable_cust_bill_pkg" pseudo-field linking it to one
+of the taxable items. All of these links must be resolved as the objects
+are inserted.
In addition to calculating the tax for the line items, this will calculate
any appropriate tax exemptions and attach them to the line items.
@@ -275,8 +280,7 @@ tax exemption limit if there is one.
=cut
-# XXX this should just return a cust_bill_pkg object for the tax,
-# but that requires changing stuff in tax_rate.pm also.
+# XXX change tax_rate.pm to work like this
sub taxline {
my( $self, $taxables, %opt ) = @_;
@@ -294,7 +298,8 @@ sub taxline {
my $dbh = dbh;
my $name = $self->taxname || 'Tax';
- my $amount = 0;
+ my $taxable_cents = 0;
+ my $tax_cents = 0;
my $cust_bill = $taxables->[0]->cust_bill;
my $custnum = $cust_bill ? $cust_bill->custnum : $opt{'custnum'};
@@ -325,6 +330,15 @@ sub taxline {
push @existing_exemptions, @{ $_->cust_tax_exempt_pkg }
for @$taxables;
+ my $tax_item = FS::cust_bill_pkg->new({
+ 'pkgnum' => 0,
+ 'recur' => 0,
+ 'sdate' => '',
+ 'edate' => '',
+ 'itemdesc' => $name,
+ });
+ my @tax_location;
+
foreach my $cust_bill_pkg (@$taxables) {
my $cust_pkg = $cust_bill_pkg->cust_pkg;
@@ -472,37 +486,47 @@ sub taxline {
$_->taxnum($self->taxnum) foreach @new_exemptions;
- #if ( $cust_bill_pkg->billpkgnum ) {
-
- #no, need to do this to e.g. calculate tax credit amounts
- #die "tried to calculate tax exemptions on a previously billed line item\n";
-
- # this is unnecessary
-# foreach my $cust_tax_exempt_pkg (@new_exemptions) {
-# my $error = $cust_tax_exempt_pkg->insert;
-# if ( $error ) {
-# $dbh->rollback if $oldAutoCommit;
-# return "can't insert cust_tax_exempt_pkg: $error";
-# }
-# }
- #}
-
# attach them to the line item
push @{ $cust_bill_pkg->cust_tax_exempt_pkg }, @new_exemptions;
push @existing_exemptions, @new_exemptions;
- # If we were smart, we'd also generate a cust_bill_pkg_tax_location
- # record at this point, but that would require redesigning more stuff.
$taxable_charged = sprintf( "%.2f", $taxable_charged);
-
- $amount += $taxable_charged * $self->tax / 100;
+ next if $taxable_charged == 0;
+
+ my $this_tax_cents = int($taxable_charged * $self->tax);
+ my $location = FS::cust_bill_pkg_tax_location->new({
+ 'taxnum' => $self->taxnum,
+ 'taxtype' => ref($self),
+ 'cents' => $this_tax_cents,
+ 'taxable_cust_bill_pkg' => $cust_bill_pkg,
+ 'tax_cust_bill_pkg' => $tax_item,
+ });
+ push @tax_location, $location;
+
+ $taxable_cents += $taxable_charged;
+ $tax_cents += $this_tax_cents;
} #foreach $cust_bill_pkg
-
- return {
- 'name' => $name,
- 'amount' => $amount,
- };
-
+
+ # now round and distribute
+ my $extra_cents = sprintf('%.2f', $taxable_cents * $self->tax / 100) * 100
+ - $tax_cents;
+ if ( $extra_cents < 0 ) {
+ die "nonsense extra_cents value $extra_cents"; # because seriously, wtf
+ }
+ $tax_cents += $extra_cents;
+ my $i = 0;
+ foreach (@tax_location) { # can never require more than a single pass, yes?
+ my $cents = $_->get('cents');
+ if ( $extra_cents > 0 ) {
+ $cents++;
+ $extra_cents--;
+ }
+ $_->set('amount', sprintf('%.2f', $cents/100));
+ }
+ $tax_item->set('setup' => sprintf('%.2f', $tax_cents / 100));
+ $tax_item->set('cust_bill_pkg_tax_location', \@tax_location);
+
+ return $tax_item;
}
=back
commit bce3e7d93335196a137f6dbfc75335668842ecff
Author: Mark Wells <mark at freeside.biz>
Date: Sun Dec 30 15:36:25 2012 -0800
set default location on one-time charges, related to #940
diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index 4ea4a6b..45d57cd 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -3403,6 +3403,8 @@ New-style, with a hashref of options:
'setuptax' => '', # or 'Y' for tax exempt
+ 'locationnum'=> 1234, # optional
+
#internal taxation
'taxclass' => 'Tax class',
@@ -3434,6 +3436,7 @@ sub charge {
my $no_auto = '';
my $cust_pkg_ref = '';
my ( $bill_now, $invoice_terms ) = ( 0, '' );
+ my $locationnum;
if ( ref( $_[0] ) ) {
$amount = $_[0]->{amount};
$quantity = exists($_[0]->{quantity}) ? $_[0]->{quantity} : 1;
@@ -3451,6 +3454,7 @@ sub charge {
$cust_pkg_ref = exists($_[0]->{cust_pkg_ref}) ? $_[0]->{cust_pkg_ref} : '';
$bill_now = exists($_[0]->{bill_now}) ? $_[0]->{bill_now} : '';
$invoice_terms = exists($_[0]->{invoice_terms}) ? $_[0]->{invoice_terms} : '';
+ $locationnum = $_[0]->{locationnum} || $self->ship_locationnum;
} else {
$amount = shift;
$quantity = 1;
@@ -3517,6 +3521,7 @@ sub charge {
'quantity' => $quantity,
'start_date' => $start_date,
'no_auto' => $no_auto,
+ 'locationnum'=> $locationnum,
} );
$error = $cust_pkg->insert;
commit 49e08fcef54f4c4a3463d91f1cfcb69f121b2c5a
Author: Mark Wells <mark at freeside.biz>
Date: Sun Dec 30 15:35:43 2012 -0800
generate E911 charges in IPifony download, #18333
diff --git a/FS/bin/freeside-ipifony-download b/FS/bin/freeside-ipifony-download
index 64905e1..837cc33 100644
--- a/FS/bin/freeside-ipifony-download
+++ b/FS/bin/freeside-ipifony-download
@@ -12,7 +12,18 @@ use FS::Conf;
use Text::CSV;
my %opt;
-getopts('va:P:C:T:', \%opt);
+getopts('va:P:C:e:', \%opt);
+
+# Product codes that are subject to flat rate E911 charges. For these
+# products, the'quantity' field represents the number of lines.
+my @E911_CODES = ( 'V-HPBX', 'V-TRUNK' );
+
+# Map TAXNONVOICE/TAXVOICE to Freeside taxclass names
+my %TAXCLASSES = (
+ 'TAXNONVOICE' => 'Other',
+ 'TAXVOICE' => 'VoIP',
+);
+
#$Net::SFTP::Foreign::debug = -1;
sub HELP_MESSAGE { '
@@ -22,7 +33,7 @@ sub HELP_MESSAGE { '
[ -a archivedir ]
[ -P port ]
[ -C category ]
- [ -T taxclass ]
+ [ -e pkgpart ]
freesideuser sftpuser at hostname[:path]
' }
@@ -30,12 +41,14 @@ my @fields = (
'custnum',
'date_desc',
'quantity',
- 'amount',
+ 'unit_price',
'classname',
+ 'taxclass',
);
my $user = shift or die &HELP_MESSAGE;
-adminsuidsetup $user;
+my $dbh = adminsuidsetup $user;
+$FS::UID::AutoCommit = 0;
# for statistics
my $num_charges = 0;
@@ -51,6 +64,16 @@ if ( $opt{a} ) {
unless -w $opt{a};
}
+my $e911_part_pkg;
+if ( $opt{e} ) {
+ $e911_part_pkg = FS::part_pkg->by_key($opt{e})
+ or die "E911 pkgpart $opt{e} not found.\n";
+
+ if ( $e911_part_pkg->base_recur > 0 or $e911_part_pkg->freq ) {
+ die "E911 pkgpart $opt{e} must be a one-time charge.\n";
+ }
+}
+
my $categorynum = '';
if ( $opt{C} ) {
# find this category (don't auto-create it, it should exist already)
@@ -61,8 +84,6 @@ if ( $opt{C} ) {
$categorynum = $category->categorynum;
}
-my $taxclass = $opt{T} || '';
-
#my $tmpdir = File::Temp->newdir();
my $tmpdir = tempdir( CLEANUP => 1 ); #DIR=>somewhere?
@@ -88,7 +109,7 @@ my $sftp = Net::SFTP::Foreign->new(
port => $port,
# for now we don't support passwords. use authorized_keys.
timeout => 30,
- more => ($opt{v} ? '-v' : ''),
+ #more => ($opt{v} ? '-v' : ''),
);
die "failed to connect to '$sftpuser\@$host'\n(".$sftp->error.")\n"
if $sftp->error;
@@ -100,6 +121,12 @@ if (!@$files) {
print STDERR "No charge files found.\n" if $opt{v};
exit(-1);
}
+
+my %cust_main; # cache
+my %e911_qty; # custnum => sum of E911-subject quantity
+
+my %is_e911 = map {$_ => 1} @E911_CODES;
+
FILE: foreach my $filename (@$files) {
print STDERR "Retrieving $filename\n" if $opt{v};
$sftp->get("$filename", "$tmpdir/$filename");
@@ -133,7 +160,7 @@ FILE: foreach my $filename (@$files) {
open my $fh, "<$tmpdir/$filename";
my $header = <$fh>;
- if ($header !~ /^cust_id/) {
+ if ($header !~ /^"cust_id"/) {
warn "warning: $filename has incorrect header row:\n$header\n";
# but try anyway
}
@@ -145,7 +172,8 @@ FILE: foreach my $filename (@$files) {
next FILE;
};
@hash{@fields} = $csv->fields();
- my $cust_main = FS::cust_main->by_key($hash{custnum});
+ my $cust_main =
+ $cust_main{$hash{custnum}} ||= FS::cust_main->by_key($hash{custnum});
if (!$cust_main) {
warn "customer #$hash{custnum} not found\n";
next;
@@ -153,13 +181,14 @@ FILE: foreach my $filename (@$files) {
print STDERR "Found customer #$hash{custnum}: ".$cust_main->name."\n"
if $opt{v};
+ my $amount = sprintf('%.2f',$hash{quantity} * $hash{unit_price});
# construct arguments for $cust_main->charge
- my %opt = (
- amount => $hash{amount},
+ my %charge_opt = (
+ amount => $amount,
quantity => $hash{quantity},
start_date => $cust_main->next_bill_date,
pkg => $hash{date_desc},
- taxclass => $taxclass,
+ taxclass => $TAXCLASSES{ $hash{taxclass} },
);
if (my $classname = $hash{classname}) {
if (!exists($classnum_of{$classname}) ) {
@@ -182,11 +211,11 @@ FILE: foreach my $filename (@$files) {
$classnum_of{$classname} = $pkg_class->classnum;
}
- $opt{classnum} = $classnum_of{$classname};
+ $charge_opt{classnum} = $classnum_of{$classname};
}
- print STDERR " Charging $hash{amount}\n"
+ print STDERR " Charging $hash{unit_price} * $hash{quantity}\n"
if $opt{v};
- my $error = $cust_main->charge(\%opt);
+ my $error = $cust_main->charge(\%charge_opt);
if ($error) {
warn "Error creating charge: $error" if $error;
$num_errors++;
@@ -194,48 +223,94 @@ FILE: foreach my $filename (@$files) {
$num_charges++;
$sum_charges += $hash{amount};
}
+
+ if ( $opt{e} and $is_e911{$hash{classname}} ) {
+ $e911_qty{$hash{custnum}} ||= 0;
+ $e911_qty{$hash{custnum}} += $hash{quantity};
+ }
} #while $line
close $fh;
} #FILE
+# Order E911 packages
+my $num_e911 = 0;
+my $num_lines = 0;
+foreach my $custnum ( keys (%e911_qty) ) {
+ my $cust_main = $cust_main{$custnum};
+ my $quantity = $e911_qty{$custnum};
+ next if $quantity == 0;
+ my $cust_pkg = FS::cust_pkg->new({
+ pkgpart => $opt{e},
+ custnum => $custnum,
+ start_date => $cust_main->next_bill_date,
+ quantity => $quantity,
+ });
+ my $error = $cust_main->order_pkg({ cust_pkg => $cust_pkg });
+ if ( $error ) {
+ warn "Error creating e911 charge for customer $custnum: $error\n";
+ $num_errors++;
+ } else {
+ $num_e911++;
+ $num_lines += $quantity;
+ }
+}
+
+$dbh->commit;
+
if ($opt{v}) {
print STDERR "
Finished!
Processed files: @$files
Created charges: $num_charges
Sum of charges: \$".sprintf('%0.2f', $sum_charges)."
+ E911 charges: $num_e911
+ E911 lines: $num_lines
Errors: $num_errors
";
}
=head1 NAME
-freeside-eftca-download - Retrieve payment batch responses from EFT Canada.
+freeside-ipifony-download - Download and import invoice items from IPifony.
=head1 SYNOPSIS
- freeside-eftca-download [ -v ] [ -a archivedir ] user
+ freeside-ipifony-download
+ [ -v ]
+ [ -a archivedir ]
+ [ -P port ]
+ [ -C category ]
+ [ -T taxclass ]
+ [ -e pkgpart ]
+ freesideuser sftpuser at hostname[:path]
-=head1 DESCRIPTION
+=head1 REQUIRED PARAMETERS
-Command line tool to download returned payment reports from the EFT Canada
-gateway and void the returned payments. Uses the login and password from
-'batchconfig-eft_canada'.
+I<freesideuser>: the Freeside user to run as.
--v: Be verbose.
+I<sftpuser>: the SFTP user to connect as. The 'freeside' system user should
+have an authorization key to connect as that user.
--a directory: Archive response files in the provided directory.
+I<hostname>: the SFTP server.
+
+=head1 OPTIONAL PARAMETERS
+
+-v: Be verbose.
-user: freeside username
+-a I<archivedir>: Save a copy of the downloaded file to I<archivedir>.
-=head1 BUGS
+-P I<port>: Connect to that TCP port.
-You need to manually SFTP to ftp.eftcanada.com from the freeside account
-and accept their key before running this script.
+-C I<category>: The name of a package category to use when creating package
+classes.
-=head1 SEE ALSO
+-e I<pkgpart>: The pkgpart (L<FS::part_pkg>) to use for E911 charges. A
+package of this type will be ordered for each invoice that has E911-subject
+line items. The 'quantity' field on this package will be set to the total
+quantity of those line items.
-L<FS::pay_batch>
+The E911 package must be a one-time package (flat rate, no frequency, no
+recurring fee) with setup fee equal to the fee per line.
=cut
-----------------------------------------------------------------------
Summary of changes:
FS/FS/cust_bill_pkg.pm | 48 ++++-
FS/FS/cust_main.pm | 5 +
FS/FS/cust_main/Billing.pm | 443 ++++++++++++++++++--------------------
FS/FS/cust_main_county.pm | 88 +++++---
FS/bin/freeside-ipifony-download | 133 +++++++++---
5 files changed, 419 insertions(+), 298 deletions(-)
More information about the freeside-commits
mailing list