revise process for updating WA sales taxes, #73185 and #73226

7 files changed, 443 insertions, 58 deletions

diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm
index 0808c38..0d45ace 100644
--- a/FS/FS/Conf.pm
+++ b/FS/FS/Conf.pm
@@ -4728,7 +4728,7 @@ and customer address. Include units.',
 
   {
     'key'         => 'tax_district_method',
-    'section'     => 'UI',
+    'section'     => 'billing', # 'UI', #???
     'description' => 'The method to use to look up tax district codes.',
     'type'        => 'select',
     #'select_hash' => [ FS::Misc::Geo::get_district_methods() ],
@@ -4740,6 +4740,13 @@ and customer address. Include units.',
   },
 
   {
+    'key'         => 'tax_district_taxname',
+    'section'     => 'billing',
+    'description' => 'The tax name to display on the invoice for district sales taxes. Defaults to "Tax".',
+    'type'        => 'text',
+  },
+
+  {
     'key'         => 'company_latitude',
     'section'     => 'UI',
     'description' => '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 0d62209..7a59ea7 100644
--- a/FS/FS/log_context.pm
+++ b/FS/FS/log_context.pm
@@ -14,6 +14,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::upload
   spool_upload
diff --git a/FS/FS/part_pkg_taxclass.pm b/FS/FS/part_pkg_taxclass.pm
index 824fd17..f7e332f 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);
@@ -211,6 +211,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 4d432ef..e21569d 100755
--- a/FS/bin/freeside-daily
+++ b/FS/bin/freeside-daily
@@ -35,6 +35,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
+@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');