From: Mark Wells Date: Mon, 31 Dec 2012 01:03:58 +0000 (-0800) Subject: fix linking of cust_bill_pkg_tax_location records during billing, #20629 X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=commitdiff_plain;h=a14db43cc4ec18badd0aff4fbc3e6738f4f63f6c 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 5cff140bb..716c0983e 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 6d3ff9146..cd46c7332 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) for this customer. Usually used internally by bill method B. @@ -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 87c1ca730..db6be751d 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 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 objects representing taxable +line items, and returns a new L 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 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