From: Mark Wells Date: Tue, 8 Nov 2016 00:24:16 +0000 (-0800) Subject: revise process for updating WA sales taxes, #73185 and #73226 X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=commitdiff_plain;h=dc4e882662ac72279c008d47903a3978cf227f72 revise process for updating WA sales taxes, #73185 and #73226 Conflicts: FS/FS/Conf.pm --- diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm index e4f581961..31759ed62 100644 --- a/FS/FS/Conf.pm +++ b/FS/FS/Conf.pm @@ -4434,6 +4434,13 @@ and customer address. Include units.', }, { + 'key' => 'tax_district_taxname', + 'section' => 'taxation', + 'description' => 'The tax name to display on the invoice for district sales taxes. Defaults to "Tax".', + 'type' => 'text', + }, + + { 'key' => 'company_latitude', 'section' => 'taxation', 'description' => 'For Avalara taxation, your company latitude (-90 through 90)', diff --git a/FS/FS/Cron/tax_rate_update.pm b/FS/FS/Cron/tax_rate_update.pm new file mode 100755 index 000000000..e345964e8 --- /dev/null +++ b/FS/FS/Cron/tax_rate_update.pm @@ -0,0 +1,111 @@ +#!/usr/bin/perl + +=head1 NAME + +FS::Cron::tax_rate_update + +=head1 DESCRIPTION + +Cron routine to update city/district sales tax rates in I. +Currently supports sales tax in the state of Washington. + +=cut + +use strict; +use warnings; +use FS::Conf; +use FS::Record qw(qsearch qsearchs dbh); +use FS::cust_main_county; +use FS::part_pkg_taxclass; +use DateTime; +use LWP::UserAgent; +use File::Temp 'tempdir'; +use File::Slurp qw(read_file write_file); +use Text::CSV; +use Exporter; + +our @EXPORT_OK = qw(tax_rate_update); +our $DEBUG = 0; + +sub tax_rate_update { + my %opt = @_; + + my $oldAutoCommit = $FS::UID::AutoCommit; + $FS::UID::AutoCommit = 0; + my $dbh = dbh; + + my $conf = FS::Conf->new; + my $method = $conf->config('tax_district_method'); + return if !$method; + + my $taxname = $conf->config('tax_district_taxname') || ''; + + if ($method eq 'wa_sales') { + # download the update file + my $now = DateTime->now; + my $yr = $now->year; + my $qt = $now->quarter; + my $file = "Rates${yr}Q${qt}.zip"; + my $url = 'http://dor.wa.gov/downloads/Add_Data/'.$file; + my $dir = tempdir(); + chdir($dir); + my $ua = LWP::UserAgent->new; + warn "Downloading $url...\n" if $DEBUG; + my $response = $ua->get($url); + if ( ! $response->is_success ) { + die $response->status_line; + } + write_file($file, $response->decoded_content); + + # parse it + system('unzip', $file); + $file =~ s/\.zip$/.csv/; + if (! -f $file) { + die "$file not found in zip archive.\n"; + } + open my $fh, '<', $file + or die "couldn't open $file: $!\n"; + my $csv = Text::CSV->new; + my $header = $csv->getline($fh); + $csv->column_names(@$header); + # columns we care about are headed 'Code' and 'Rate' + + my $total_changed = 0; + my $total_skipped = 0; + while ( !$csv->eof ) { + my $line = $csv->getline_hr($fh); + my $district = $line->{Code} or next; + $district = sprintf('%04d', $district); + my $tax = sprintf('%.1f', $line->{Rate} * 100); + my $changed = 0; + my $skipped = 0; + # find rate(s) in this country+state+district+taxclass that have the + # wa_sales flag and the configured taxname, and haven't been disabled. + my @rates = qsearch('cust_main_county', { + country => 'US', + state => 'WA', # this is specific to WA + district => $district, + taxname => $taxname, + source => 'wa_sales', + tax => { op => '>', value => '0' }, + }); + foreach my $rate (@rates) { + if ( $rate->tax == $tax ) { + $skipped++; + } else { + $rate->set('tax', $tax); + my $error = $rate->replace; + die "error updating district $district: $error\n" if $error; + $changed++; + } + } # foreach $taxclass + print "$district: updated $changed, skipped $skipped\n" + if $DEBUG and ($changed or $skipped); + $total_changed += $changed; + $total_skipped += $skipped; + } + print "Updated $total_changed tax rates.\nSkipped $total_skipped unchanged rates.\n" if $DEBUG; + dbh->commit; + } # else $method isn't wa_sales, no other methods exist yet + ''; +} diff --git a/FS/FS/geocode_Mixin.pm b/FS/FS/geocode_Mixin.pm index 09b113112..46f8128aa 100644 --- a/FS/FS/geocode_Mixin.pm +++ b/FS/FS/geocode_Mixin.pm @@ -11,6 +11,7 @@ use FS::cust_pkg; use FS::cust_location; use FS::cust_tax_location; use FS::part_pkg; +use FS::part_pkg_taxclass; $DEBUG = 0; $me = '[FS::geocode_Mixin]'; @@ -253,8 +254,7 @@ Queueable function to update the tax district code using the selected method sub process_district_update { my $class = shift; my $id = shift; - - local $DEBUG = 1; + my $log = FS::Log->new('FS::cust_location::process_district_update'); eval "use FS::Misc::Geo qw(get_district); use FS::Conf; use $class;"; die $@ if $@; @@ -267,68 +267,78 @@ sub process_district_update { # dies on error, fine my $tax_info = get_district({ $self->location_hash }, $method); - - if ( $tax_info ) { - $self->set('district', $tax_info->{'district'} ); - my $error = $self->replace; - die $error if $error; + return unless $tax_info; + + $self->set('district', $tax_info->{'district'} ); + my $error = $self->replace; + die $error if $error; + + my %hash = map { $_ => uc( $tax_info->{$_} ) } + qw( district city county state country ); + $hash{'source'} = $method; # apply the update only to taxes we maintain + + my @classes = FS::part_pkg_taxclass->taxclass_names; + my $taxname = $conf->config('tax_district_taxname'); + # there must be exactly one cust_main_county for each district+taxclass. + # do NOT exclude taxes that are zero. + foreach my $taxclass (@classes) { + my @existing = qsearch('cust_main_county', { + %hash, + 'taxclass' => $taxclass + }); + + if ( scalar(@existing) == 0 ) { + + # then create one with the assigned tax name, and the tax rate from + # the lookup. + my $new = new FS::cust_main_county({ + %hash, + 'taxclass' => $taxclass, + 'taxname' => $taxname, + 'tax' => $tax_info->{tax}, + 'exempt_amount' => 0, + }); + $log->info("creating tax rate for district ".$tax_info->{'district'}); + $error = $new->insert; - my %hash = map { $_ => uc( $tax_info->{$_} ) } - qw( district city county state country ); - $hash{'source'} = $method; # apply the update only to taxes we maintain - - my @old = qsearch('cust_main_county', \%hash); - if ( @old ) { - # prune any duplicates rather than updating them - my %keep; # key => cust_main_county record - foreach my $cust_main_county (@old) { - my $key = join('.', $cust_main_county->city , - $cust_main_county->district , - $cust_main_county->taxclass - ); - if ( exists $keep{$key} ) { - my $disable_this = $cust_main_county; - # prefer records that have a tax name - if ( $cust_main_county->taxname and not $keep{$key}->taxname ) { - $disable_this = $keep{$key}; - $keep{$key} = $cust_main_county; + } else { + + my $to_update = $existing[0]; + # if there's somehow more than one, find the best candidate to be + # updated: + # - prefer tax > 0 over tax = 0 (leave disabled records disabled) + # - then, prefer taxname = the designated taxname + if ( scalar(@existing) > 1 ) { + $log->warning("tax district ".$tax_info->{district}." has multiple $method taxes."); + foreach (@existing) { + if ( $to_update->tax == 0 ) { + if ( $_->tax > 0 and $to_update->tax == 0 ) { + $to_update = $_; + } elsif ( $_->tax == 0 and $to_update->tax > 0 ) { + next; + } elsif ( $_->taxname eq $taxname and $to_update->tax ne $taxname ) { + $to_update = $_; + } } - # disable by setting the rate to zero, and setting source to null - # so it doesn't get auto-updated in the future. don't actually - # delete it, that produces orphan records - warn "disabling tax rate #" . - $disable_this->taxnum . - " because it's a duplicate for $key\n" - if $DEBUG; - # by setting its rate to zero, and never updating - # it again - $disable_this->set('tax' => 0); - $disable_this->set('source' => ''); - $error = $disable_this->replace; - die $error if $error; } - - $keep{$key} ||= $cust_main_county; - + # don't remove the excess records here; upgrade does that. } - foreach my $key (keys %keep) { - my $cust_main_county = $keep{$key}; - warn "updating tax rate #".$cust_main_county->taxnum. - " for $key" if $DEBUG; - # update the tax rate only - $cust_main_county->set('tax', $tax_info->{'tax'}); - $error ||= $cust_main_county->replace; + my $taxnum = $to_update->taxnum; + if ( $to_update->tax == 0 ) { + $log->debug("tax#$taxnum is set to zero; not updating."); + } elsif ( $to_update->tax == $tax_info->{tax} ) { + # do nothing, no need to update + } else { + $to_update->set('tax', $tax_info->{tax}); + $log->info("updating tax#$taxnum with new rate ($tax_info->{tax})."); + $error = $to_update->replace; } - } else { - # make a new tax record, and mark it so we can find it later - $tax_info->{'source'} = $method; - my $new = new FS::cust_main_county $tax_info; - warn "creating tax rate for district ".$tax_info->{'district'} if $DEBUG; - $error = $new->insert; } + die $error if $error; - } + } # foreach $taxclass + return; } diff --git a/FS/FS/log_context.pm b/FS/FS/log_context.pm index 809237d06..51aa79de5 100644 --- a/FS/FS/log_context.pm +++ b/FS/FS/log_context.pm @@ -16,6 +16,7 @@ my @contexts = ( qw( FS::Misc::Geo::standardize_uscensus FS::saved_search::send FS::saved_search::render + FS::cust_location::process_district_update Cron::bill Cron::backup Cron::upload diff --git a/FS/FS/part_pkg_taxclass.pm b/FS/FS/part_pkg_taxclass.pm index 055c778ba..d8ddb1512 100644 --- a/FS/FS/part_pkg_taxclass.pm +++ b/FS/FS/part_pkg_taxclass.pm @@ -4,7 +4,7 @@ use strict; use vars qw( @ISA ); use Scalar::Util qw( blessed ); use FS::UID qw( dbh ); -use FS::Record; # qw( qsearch qsearchs ); +use FS::Record qw(qsearch); # qsearchs ); use FS::cust_main_county; @ISA = qw(FS::Record); @@ -219,6 +219,26 @@ sub _upgrade_data { # class method } +=head1 CLASS METHODS + +=over 4 + +=item taxclass_names + +Returns a list of all the non-disabled tax classes. If tax classes aren't +enabled, returns a single empty string. + +=cut + +sub taxclass_names { + if ( FS::Conf->new->exists('enable_taxclasses') ) { + return map { $_->get('taxclass') } + qsearch('part_pkg_taxclass', { disabled => '' }); + } else { + return ( '' ); + } +} + =head1 BUGS Other tables (cust_main_county, part_pkg, agent_payment_gateway) have a text diff --git a/FS/bin/freeside-daily b/FS/bin/freeside-daily index 03d235061..ee95c14db 100755 --- a/FS/bin/freeside-daily +++ b/FS/bin/freeside-daily @@ -44,6 +44,10 @@ reconcile_breakage(%opt); use FS::Cron::upload qw(upload); upload(%opt); +#this only takes effect if WA sales taxes are enabled +use FS::Cron::tax_rate_update qw(tax_rate_update); +tax_rate_update(%opt); + use FS::Cron::set_lata_have_usage qw(set_lata_have_usage); set_lata_have_usage(%opt); diff --git a/FS/t/suite/12-wa_sales_tax.t b/FS/t/suite/12-wa_sales_tax.t new file mode 100755 index 000000000..473c9a7ba --- /dev/null +++ b/FS/t/suite/12-wa_sales_tax.t @@ -0,0 +1,232 @@ +#!/usr/bin/perl + +=head2 DESCRIPTION + +Tests automatic lookup of Washington sales tax districts and rates. + +This will set up two tax classes. One of them (class A) has only the sales +tax. The other (class B) will have an additional, manually created tax. + +This will test the following sequence of actions (running +process_district_update() after each one): + +1. Enter a customer in Washington for which there is not yet a district tax + entry. +2. Add a manual tax in class B. +3. Rename the sales taxes. +4. Delete the sales taxes. +5. Change the sales tax rates (to simulate a change in the actual rate). +6. Set the sales tax rate to zero. + +The correct result is always for there to be exactly one tax entry for this +district in each class, with the correct rate, except after step 6, when +the rate should remain at zero (because setting the rate to zero is a way +of manually disabling the tax). + +=cut + +use strict; +use Test::More tests => 6; +use FS::Test; +use FS::cust_main; +use FS::cust_location; +use FS::cust_main_county; +use FS::Misc; +use FS::Conf; +my $FS= FS::Test->new; + +my $error; + +# start clean +my @taxes = $FS->qsearch('cust_main_county', { city => 'SEATTLE' }); +my @classes = $FS->qsearch('part_pkg_taxclass'); +foreach (@taxes, @classes) { + $error = $_->delete; + BAIL_OUT("can't flush existing taxes: $error") if $error; + # we won't charge any of the taxes in this script so FK errors shouldn't + # happen. +} + +# configure stuff +@classes = map { FS::part_pkg_taxclass->new({ taxclass => $_ }) } + qw( ClassA ClassB ); +foreach (@classes) { + $error = $_->insert; + BAIL_OUT("can't create tax class: $error") if $error; +} + +# should be an FS::Test method to temporarily set this up +my $conf = FS::Conf->new; +$conf->set('tax_district_method', 'wa_sales'); +$conf->set('tax_district_taxname', 'Sales Tax'); +$conf->set('enable_taxclasses', 1); + +# create the customer +my $cust = $FS->new_customer('WA Taxes'); +# Sea-Tac International Airport +$cust->bill_location->address1('17801 International Blvd'); +$cust->bill_location->city('Seattle'); +$cust->bill_location->zip('98158'); +$cust->bill_location->state('WA'); +$cust->bill_location->country('US'); + +$error = $cust->insert; +BAIL_OUT("can't create test customer: $error") if $error; + +my $location = $cust->bill_location; +my @prev_taxes; + +# after each action, refresh the tax district (as if we'd added/edited a +# customer in that district) and then get the new list of defined taxes +sub reprocess { + # remember all the taxes from the last test + @prev_taxes = map { $_ && FS::cust_main_county->new({$_->hash}) } @taxes; + local $@; + eval { FS::geocode_Mixin::process_district_update( 'FS::cust_location', + $location->locationnum )}; + $error = $@; + BAIL_OUT("can't update tax district: $error") if $error; + + $location = $location->replace_old; + @taxes = $FS->qsearch({ + table => 'cust_main_county', + hashref => { city => 'SEATTLE' }, + order_by => 'ORDER BY taxclass ASC, taxname ASC', # make them easily findable + }); +} + +# and then we'll want to check that the total number of taxes is what we +# expect. +sub ok_num_taxes { + my $num = shift; + is( scalar(@taxes), $num, "Number of taxes" ) + or BAIL_OUT('Wrong number of tax records, can\'t continue.'); +} + +subtest 'Step 1: Initial tax lookup' => sub { + plan 'tests' => 4; + reprocess(); + ok( $location->district, 'Found district '.$location->district); + ok_num_taxes(2); + ok( ( $taxes[0] + and $taxes[0]->taxname eq 'Sales Tax' + and $taxes[0]->taxclass eq 'ClassA' + and $taxes[0]->district eq $location->district + and $taxes[0]->source eq 'wa_sales' + and $taxes[0]->tax > 0 + ), + 'ClassA tax = '.$taxes[0]->tax ) + or diag explain($taxes[0]); + ok( ( $taxes[1] + and $taxes[1]->taxname eq 'Sales Tax' + and $taxes[1]->taxclass eq 'ClassB' + and $taxes[1]->district eq $location->district + and $taxes[1]->source eq 'wa_sales' + and $taxes[1]->tax > 0 + ), + 'ClassB tax = '.$taxes[1]->tax ) + or diag explain($taxes[1]); +}; + +# "Sales Tax" sorts before "USF"; this is intentional. +subtest 'Step 2: Add manual tax ("USF") to ClassB' => sub { + plan tests => 4; + if ($taxes[1]) { + my $manual_tax = $taxes[1]->new({ + $taxes[1]->hash, + 'taxnum' => '', + 'taxname' => 'USF', + 'source' => '', + 'tax' => '17', + }); + $error = $manual_tax->insert; + BAIL_OUT("can't create manual tax: $error") if $error; + } + reprocess(); + ok_num_taxes(3); + is_deeply( $taxes[0], $prev_taxes[0], 'ClassA sales tax was not changed' ); + is_deeply( $taxes[1], $prev_taxes[1], 'ClassB sales tax was not changed' ); + ok( ( $taxes[2] + and $taxes[2]->taxname eq 'USF' + and $taxes[2]->taxclass eq 'ClassB' + and $taxes[2]->tax == 17 + and $taxes[2]->source eq '' + ), 'Manual tax was accepted') + or diag explain($taxes[2]); +}; + +subtest 'Step 3: Rename ClassB sales tax. Does it stay renamed?' => sub { + plan tests => 4; + if ($taxes[1]) { + $taxes[1]->set('taxname', 'Renamed Sales Tax'); + $error = $taxes[1]->replace; + BAIL_OUT("can't rename tax: $error") if $error; + } + + reprocess(); + ok_num_taxes(3); + is_deeply( $taxes[0], $prev_taxes[0], 'ClassA sales tax was not changed' ); + ok( ( $taxes[1] + and $taxes[1]->taxname eq 'Renamed Sales Tax' + and $taxes[1]->source eq 'wa_sales' + and $taxes[1]->tax == $prev_taxes[1]->tax + ), $taxes[1]->taxclass .' sales tax was renamed') + or diag explain($taxes[1]); + is_deeply( $taxes[2], $prev_taxes[2], 'ClassB manual tax was not changed' ); +}; + +subtest 'Step 4: Remove ClassB sales tax. Is it recreated?' => sub { + plan tests => 4; + if ($taxes[1]) { + $error = $taxes[1]->delete; + BAIL_OUT("can't delete tax: $error") if $error; + } + reprocess(); + ok_num_taxes(3); + is_deeply( $taxes[0], $prev_taxes[0], 'ClassA sales tax was not changed' ); + ok( ( $taxes[1] + and $taxes[1]->taxname eq 'Sales Tax' + and $taxes[1]->source eq 'wa_sales' + and $taxes[1]->tax == $prev_taxes[1]->tax + ), $taxes[1]->taxclass .' sales tax was deleted and recreated') + or diag explain($taxes[1]); + is_deeply( $taxes[2], $prev_taxes[2], 'ClassB manual tax was not changed' ); +}; + +subtest 'Step 5: Simulate a change in tax rate. Do the taxes update?' => sub { + plan tests => 3; + my $correct_rate = $taxes[0]->tax; + foreach (@taxes[0,1]) { + if ($_ and $_->source eq 'wa_sales') { + $_->tax( $correct_rate + 1 ); + $error = $_->replace; + BAIL_OUT("can't change tax rate: $error") if $error; + } + } + reprocess(); + ok_num_taxes(3); + ok( ( $taxes[0] and $taxes[0]->tax == $correct_rate + and $taxes[1] and $taxes[1]->tax == $correct_rate + ), 'Tax was reset to correct rate' ) + or diag("Tax rates: ".$taxes[0]->tax.', '.$taxes[1]->tax); + is_deeply( $taxes[2], $prev_taxes[2], 'ClassB manual tax was not changed' ); +}; + +subtest 'Step 6: Manually disable Class A sales tax. Does it stay disabled?' => sub { + plan tests => 4; + if ($taxes[0]) { + $taxes[0]->set('tax', 0); + $error = $taxes[0]->replace; + BAIL_OUT("can't change tax rate to zero: $error") if $error; + } + reprocess(); + ok_num_taxes(3); + ok( $taxes[0]->tax == 0, 'ClassA sales tax remains at zero') + or diag("Tax rate: ".$taxes[1]->tax); + is_deeply( $taxes[1], $prev_taxes[1], 'ClassB sales tax was not changed' ); + is_deeply( $taxes[2], $prev_taxes[2], 'ClassB manual tax was not changed' ); +}; + +$conf->delete('tax_district_method'); +$conf->delete('tax_district_taxname'); +$conf->delete('enable_taxclasses');