[freeside-commits] branch master updated. dc4e882662ac72279c008d47903a3978cf227f72
Mark Wells
mark at 420.am
Mon Nov 7 16:26:49 PST 2016
The branch, master has been updated
via dc4e882662ac72279c008d47903a3978cf227f72 (commit)
from 52c4764eafbb02a1ee74f983ece53b07306e1dde (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 dc4e882662ac72279c008d47903a3978cf227f72
Author: Mark Wells <mark at freeside.biz>
Date: Mon Nov 7 16:24:16 2016 -0800
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 e4f5819..31759ed 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 0000000..e345964
--- /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<cust_main_county>.
+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 09b1131..46f8128 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 809237d..51aa79d 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 055c778..d8ddb15 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 03d2350..ee95c14 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 0000000..473c9a7
--- /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
+ at 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');
-----------------------------------------------------------------------
Summary of changes:
FS/FS/Conf.pm | 7 ++
FS/FS/Cron/tax_rate_update.pm | 111 ++++++++++++++++++++
FS/FS/geocode_Mixin.pm | 122 ++++++++++++----------
FS/FS/log_context.pm | 1 +
FS/FS/part_pkg_taxclass.pm | 22 +++-
FS/bin/freeside-daily | 4 +
FS/t/suite/12-wa_sales_tax.t | 232 +++++++++++++++++++++++++++++++++++++++++
7 files changed, 442 insertions(+), 57 deletions(-)
create mode 100755 FS/FS/Cron/tax_rate_update.pm
create mode 100755 FS/t/suite/12-wa_sales_tax.t
More information about the freeside-commits
mailing list