optimize legacy CCH taxation, RT#74494
[freeside.git] / FS / FS / tax_rate.pm
index 48c0196..8c08f42 100644 (file)
@@ -10,6 +10,7 @@ use DateTime::Format::Strptime;
 use Storable qw( thaw nfreeze );
 use IO::File;
 use File::Temp;
+use Text::CSV_XS;
 use LWP::UserAgent;
 use HTTP::Request;
 use HTTP::Response;
@@ -17,6 +18,7 @@ use MIME::Base64;
 use DBIx::DBSchema;
 use DBIx::DBSchema::Table;
 use DBIx::DBSchema::Column;
+use List::Util 'sum';
 use FS::Record qw( qsearch qsearchs dbh dbdef );
 use FS::Conf;
 use FS::tax_class;
@@ -214,7 +216,7 @@ sub check {
   $self->ut_numbern('taxnum')
     || $self->ut_text('geocode')
     || $self->ut_textn('data_vendor')
-    || $self->ut_textn('location')
+    || $self->ut_cch_textn('location')
     || $self->ut_foreign_key('taxclassnum', 'tax_class', 'taxclassnum')
     || $self->ut_snumbern('effective_date')
     || $self->ut_float('tax')
@@ -244,6 +246,18 @@ sub check {
 
 }
 
+#ut_text / ut_textn w/ ` added cause now that's in the data
+sub ut_cch_textn {
+  my($self,$field)=@_;
+  $self->getfield($field)
+    =~ /^([\wô \!\@\#\$\%\&\(\)\-\+\;\:\'\"\,\.\?\/\=\[\]\<\>\`]*)$/
+      or return gettext('illegal_or_empty_text'). " $field: ".
+                 $self->getfield($field);
+  $self->setfield($field,$1);
+  '';
+
+}
+
 =item taxclass_description
 
 Returns the human understandable value associated with the related
@@ -275,16 +289,25 @@ sub unittype_name {
 
 =item maxtype_name
 
-Returns the human understandable value associated with the maxtype column
+Returns the human understandable value associated with the maxtype column.
 
 =cut
 
+# XXX these are non-functional, and most of them are horrible to implement
+# in our current model
+
 %tax_maxtypes = ( '0' => 'receipts per invoice',
                   '1' => 'receipts per item',
                   '2' => 'total utility charges per utility tax year',
                   '3' => 'total charges per utility tax year',
                   '4' => 'receipts per access line',
+                  '7' => 'total utility charges per calendar year',
                   '9' => 'monthly receipts per location',
+                  '10' => 'monthly receipts exceeds taxbase and total tax per month does not exceed maxtax', # wtf?
+                  '11' => 'receipts/units per access line',
+                  '14' => 'units per invoice',
+                  '15' => 'units per month',
+                  '18' => 'units per account',
 );
 
 sub maxtype_name {
@@ -360,69 +383,85 @@ sub passtype_name {
   $tax_passtypes{$self->passtype};
 }
 
-=item taxline TAXABLES, [ OPTIONSHASH ]
+#Returns a listref of a name and an amount of tax calculated for the list
+#of packages/amounts referenced by TAXABLES.  If an error occurs, a message
+#is returned as a scalar.
 
-Returns a listref of a name and an amount of tax calculated for the list
-of packages/amounts referenced by TAXABLES.  If an error occurs, a message
-is returned as a scalar.
+=item taxline TAXABLES_ARRAYREF, [ OPTION => VALUE ... ]
 
-=cut
+Takes an arrayref of L<FS::cust_bill_pkg> objects representing taxable
+line items, and returns some number of new L<FS::cust_bill_pkg> objects 
+representing the tax on them under this tax rate.  Each returned object
+will correspond to a single input line item.
 
-sub taxline {
-  my $self = shift;
+For accurate calculation of per-customer or per-location taxes, ALL items
+appearing on the invoice MUST be passed to this method together.
 
-  my $taxables;
-  my %opt = ();
+Optionally, any of the L<FS::cust_bill_pkg> objects may be followed in the
+array by a charge class: 'setup', 'recur', '' (for unclassified usage), or an
+integer denoting an L<FS::usage_class> number.  In this case, the tax will 
+only be charged on that portion of the line item.
 
-  if (ref($_[0]) eq 'ARRAY') {
-    $taxables = shift;
-    %opt = @_;
-  }else{
-    $taxables = [ @_ ];
-    #exemptions would be broken in this case
-  }
+Each returned object will have a pseudo-field,
+"cust_bill_pkg_tax_rate_location", containing a single
+L<FS::cust_bill_pkg_tax_rate_location> object.  This 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.
+
+If the tax is disabled, this method will return nothing.  Be prepared for 
+that.
+
+In addition to calculating the tax for the line items, this will calculate
+tax exemptions and attach them to the line items.  I<Currently this only
+supports customer exemptions.>
+
+Options may include 'custnum' and 'invoice_time' in case the cust_bill_pkg
+objects belong to an invoice that hasn't been inserted yet.
+
+The 'exemptions' option allowed in L<FS::cust_main_county::taxline> does 
+nothing here, since monthly exemptions aren't supported.
+
+=cut
+
+sub taxline {
+  my( $self, $taxables, %opt) = @_;
+  $taxables = [ $taxables ] unless ref($taxables) eq 'ARRAY';
 
   my $name = $self->taxname;
   $name = 'Other surcharges'
     if ($self->passtype == 2);
   my $amount = 0;
   
-  if ( $self->disabled ) { # we always know how to handle disabled taxes
-    return {
-      'name'   => $name,
-      'amount' => $amount,
-    };
+  return unless @$taxables; # nothing to do
+  return if $self->disabled; # tax is disabled, skip it
+  return if $self->passflag eq 'N'; # tax can't be passed to the customer
+    # but should probably still appear on the liability report--create a 
+    # cust_tax_exempt_pkg record for it?
+
+  # XXX a certain amount of false laziness with FS::cust_main_county
+  my $cust_bill = $taxables->[0]->cust_bill;
+  my $custnum = $cust_bill ? $cust_bill->custnum : $opt{'custnum'};
+  my $cust_main = FS::cust_main->by_key($custnum) if $custnum > 0;
+  if (!$cust_main) {
+    die "unable to calculate taxes for an unknown customer\n";
   }
 
-  my $taxable_charged = 0;
-  my @cust_bill_pkg = grep { $taxable_charged += $_ unless ref; ref; }
-                      @$taxables;
+  my $taxratelocationnum = $self->tax_rate_location->taxratelocationnum
+    or die "no tax_rate_location linked to tax_rate #".$self->taxnum."\n";
 
   warn "calculating taxes for ". $self->taxnum. " on ".
-    join (",", map { $_->pkgnum } @cust_bill_pkg)
+    join (",", map { $_->pkgnum } @$taxables)
     if $DEBUG;
 
-  if ($self->passflag eq 'N') {
-    # return "fatal: can't (yet) handle taxes not passed to the customer";
-    # until someone needs to track these in freeside
-    return {
-      'name'   => $name,
-      'amount' => 0,
-    };
-  }
-
   my $maxtype = $self->maxtype || 0;
-  if ($maxtype != 0 && $maxtype != 9) {
+  if ($maxtype != 0 && $maxtype != 1 
+      && $maxtype != 14 && $maxtype != 15
+      && $maxtype != 18 # sigh
+    ) {
     return $self->_fatal_or_null( 'tax with "'.
                                     $self->maxtype_name. '" threshold'
                                 );
-  }
-
-  if ($maxtype == 9) {
-    return
-      $self->_fatal_or_null( 'tax with "'. $self->maxtype_name. '" threshold' );
-                                                                # "texas" tax
-  }
+  } # I don't know why, it's not like there are maxtypes that we DO support
 
   # we treat gross revenue as gross receipts and expect the tax data
   # to DTRT (i.e. tax on tax rules)
@@ -435,69 +474,196 @@ sub taxline {
       $self->_fatal_or_null( 'tax with "'. $self->basetype_name. '" basis' );
   }
 
-  unless ($self->setuptax =~ /^Y$/i) {
-    $taxable_charged += $_->setup foreach @cust_bill_pkg;
-  }
-  unless ($self->recurtax =~ /^Y$/i) {
-    $taxable_charged += $_->recur foreach @cust_bill_pkg;
-  }
+  my @tax_locations;
+  my %seen; # locationnum or pkgnum => 1
 
+  my $taxable_cents = 0;
   my $taxable_units = 0;
-  unless ($self->recurtax =~ /^Y$/i) {
+  my $tax_cents = 0;
 
-    if (( $self->unittype || 0 ) == 0) { #access line
-      my %seen = ();
-      foreach (@cust_bill_pkg) {
-        $taxable_units += $_->units
-          unless $seen{$_->pkgnum}++;
-      }
+  while (@$taxables) {
+    my $cust_bill_pkg = shift @$taxables;
+    my $class = 'all';
+    if ( defined($taxables->[0]) and !ref($taxables->[0]) ) {
+      $class = shift @$taxables;
+    }
+
+    my %usage_map = map { $_ => $cust_bill_pkg->usage($_) }
+                    $cust_bill_pkg->usage_classes;
+    my $usage_total = sum( values(%usage_map), 0 );
 
-    } elsif ($self->unittype == 1) { #minute
-      return $self->_fatal_or_null( 'fee with minute unit type' );
+    # determine if the item has exemptions that apply to this tax def
+    my @exemptions = grep { $_->taxnum == $self->taxnum }
+      @{ $cust_bill_pkg->cust_tax_exempt_pkg };
 
-    } elsif ($self->unittype == 2) { #account
+    if ( $self->tax > 0 ) {
+
+      my $taxable_charged = 0;
+      if ($class eq 'all') {
+        $taxable_charged = $cust_bill_pkg->setup + $cust_bill_pkg->recur;
+      } elsif ($class eq 'setup') {
+        $taxable_charged = $cust_bill_pkg->setup;
+      } elsif ($class eq 'recur') {
+        $taxable_charged = $cust_bill_pkg->recur - $usage_total;
+      } else {
+        $taxable_charged = $usage_map{$class} || 0;
+      }
 
-      my $conf = new FS::Conf;
-      if ( $conf->exists('tax-pkg_address') ) {
-        #number of distinct locations
-        my %seen = ();
-        foreach (@cust_bill_pkg) {
-          $taxable_units++
-            unless $seen{$_->cust_pkg->locationnum}++;
+      foreach my $ex (@exemptions) {
+        # the only cases where the exemption doesn't apply:
+        # if it's a setup exemption and $class is not 'setup' or 'all'
+        # if it's a recur exemption and $class is 'setup'
+        if (   ( $ex->exempt_recur and $class eq 'setup' ) 
+            or ( $ex->exempt_setup and $class ne 'setup' and $class ne 'all' )
+        ) {
+          next;
         }
+
+        $taxable_charged -= $ex->amount;
+      }
+      # cust_main_county handles monthly capped exemptions; this doesn't.
+      #
+      # $taxable_charged can also be less than zero at this point 
+      # (recur exemption + usage class breakdown); treat that as zero.
+      next if $taxable_charged <= 0;
+
+      # yeah, some false laziness with cust_main_county
+      my $this_tax_cents = int(100 * $taxable_charged * $self->tax);
+      my $tax_location = FS::cust_bill_pkg_tax_rate_location->new({
+          'taxnum'                => $self->taxnum,
+          'taxtype'               => ref($self),
+          'cents'                 => $this_tax_cents, # not a real field
+          'locationtaxid'         => $self->location, # fundamentally silly
+          'taxable_cust_bill_pkg' => $cust_bill_pkg,
+          'taxratelocationnum'    => $taxratelocationnum,
+          'taxclass'              => $class,
+      });
+      push @tax_locations, $tax_location;
+
+      $taxable_cents += 100 * $taxable_charged;
+      $tax_cents += $this_tax_cents;
+
+    } elsif ( $self->fee > 0 ) {
+      # most CCH taxes are this type, because nearly every county has a 911
+      # fee
+      my $units = 0;
+
+      # since we don't support partial exemptions (except setup/recur), 
+      # if there's an exemption that applies to this package and taxrate, 
+      # don't charge ANY per-unit fees
+      next if @exemptions;
+
+      # don't apply fees to usage classes (maybe if we ever get per-minute
+      # fees?)
+      next unless $class eq 'setup'
+              or  $class eq 'recur'
+              or  $class eq 'all';
+      
+      if ( $self->unittype == 0 ) {
+        if ( !$seen{$cust_bill_pkg->pkgnum} ) {
+          # per access line
+          $units = $cust_bill_pkg->units;
+          $seen{$cust_bill_pkg->pkgnum} = 1;
+        } # else it's been seen, leave it at zero units
+
+      } elsif ($self->unittype == 1) { # per minute
+        # STILL not supported...fortunately these only exist if you happen
+        # to be in Idaho or Little Rock, Arkansas
+        #
+        # though a voip_cdr package could easily report minutes of usage...
+        return $self->_fatal_or_null( 'fee with minute unit type' );
+
+      } elsif ( $self->unittype == 2 ) {
+
+        my $locationnum = $cust_bill_pkg->tax_locationnum
+                       || $cust_main->ship_locationnum;
+        # per account
+        $units = 1 unless $seen{$locationnum};
+        $seen{$locationnum} = 1;
+
       } else {
-        $taxable_units = 1;
+        # Unittype 19 is used for prepaid wireless E911 charges in many states.
+        # Apparently "per retail purchase", which for us would mean per invoice.
+        # Unittype 20 is used for some 911 surcharges and I have no idea what 
+        # it means.
+        return $self->_fatal_or_null( 'unknown unit type in tax'. $self->taxnum );
       }
+      my $this_tax_cents = int($units * $self->fee * 100);
+      my $tax_location = FS::cust_bill_pkg_tax_rate_location->new({
+          'taxnum'                => $self->taxnum,
+          'taxtype'               => ref($self),
+          'cents'                 => $this_tax_cents,
+          'locationtaxid'         => $self->location,
+          'taxable_cust_bill_pkg' => $cust_bill_pkg,
+          'taxratelocationnum'    => $taxratelocationnum,
+          'taxclass'              => $class,
+      });
+      push @tax_locations, $tax_location;
+
+      $taxable_units += $units;
+      $tax_cents += $this_tax_cents;
 
-    } else {
-      return $self->_fatal_or_null( 'unknown unit type in tax'. $self->taxnum );
     }
-
+  } # foreach $cust_bill_pkg
+
+  # check bracket maxima; throw an error if we've gone over, because
+  # we don't really implement them
+
+  if ( ($self->taxmax > 0 and $taxable_cents > $self->taxmax*100 ) or
+       ($self->feemax > 0 and $taxable_units > $self->feemax) ) {
+    # throw an error
+    # (why not just cap taxable_charged/units at the taxmax/feemax? because
+    # it's way more complicated than that. this won't even catch every case
+    # where a bracket maximum should apply.)
+    return $self->_fatal_or_null( 'tax base > taxmax/feemax for tax'.$self->taxnum );
   }
 
-  #
-  # XXX insert exemption handling here
-  #
-  # the tax or fee is applied to taxbase or feebase and then
-  # the excessrate or excess fee is applied to taxmax or feemax
-  #
-
-  $amount += $taxable_charged * $self->tax;
-  $amount += $taxable_units * $self->fee;
-  
-  warn "calculated taxes as [ $name, $amount ]\n"
-    if $DEBUG;
+  # round and distribute
+  my $total_tax_cents = sprintf('%.0f',
+    ($taxable_cents * $self->tax) + ($taxable_units * $self->fee * 100)
+  );
+  my $extra_cents = sprintf('%.0f', $total_tax_cents - $tax_cents);
+  $tax_cents += $extra_cents;
+  my $i = 0;
+  foreach (@tax_locations) { # 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));
+  }
 
-  return {
-    'name'   => $name,
-    'amount' => $amount,
-  };
+  # just transform each CBPTRL record into a tax line item.
+  # calculate_taxes will consolidate them, but before that happens we have
+  # to do tax on tax calculation.
+  my @tax_items;
+  foreach (@tax_locations) {
+    next if $_->amount == 0;
+    my $tax_item = FS::cust_bill_pkg->new({
+        'pkgnum'        => 0,
+        'recur'         => 0,
+        'setup'         => $_->amount,
+        'sdate'         => '', # $_->sdate?
+        'edate'         => '',
+        'itemdesc'      => $name,
+        'cust_bill_pkg_tax_rate_location' => [ $_ ],
+        # Make the charge class easily accessible; we need it for tax-on-tax
+        # applicability. RT#36830.
+        '_class'        => $_->taxclass,
+    });
+    $_->set('tax_cust_bill_pkg' => $tax_item);
+    push @tax_items, $tax_item;
+  }
 
+  return @tax_items;
 }
 
 sub _fatal_or_null {
   my ($self, $error) = @_;
 
+  $DB::single = 1; # not a mistake
+
   my $conf = new FS::Conf;
 
   $error = "can't yet handle ". $error;
@@ -513,10 +679,10 @@ sub _fatal_or_null {
   }
 }
 
-=item tax_on_tax CUST_MAIN
+=item tax_on_tax CUST_LOCATION
 
 Returns a list of taxes which are candidates for taxing taxes for the
-given customer (see L<FS::cust_main>)
+given service location (see L<FS::cust_location>)
 
 =cut
 
@@ -524,13 +690,13 @@ given customer (see L<FS::cust_main>)
 sub tax_on_tax {
        #akshun
   my $self = shift;
-  my $cust_main = shift;
+  my $cust_location = shift;
 
   warn "looking up taxes on tax ". $self->taxnum. " for customer ".
-    $cust_main->custnum
+    $cust_location->custnum
     if $DEBUG;
 
-  my $geocode = $cust_main->geocode($self->data_vendor);
+  my $geocode = $cust_location->geocode($self->data_vendor);
 
   # CCH oddness in m2m
   my $dbh = dbh;
@@ -560,7 +726,9 @@ sub tax_on_tax {
     "AND (".  join(' OR ', map { "taxclassnum = $_" } @taxclassnums ). ")";
 
   qsearch({ 'table'     => 'tax_rate',
-            'hashref'   => { 'geocode' => $geocode, },
+            'hashref'   => { 'data_vendor' => $self->data_vendor,
+                             'geocode'     => $geocode,
+                           },
             'extra_sql' => $extra_sql,
          })
 
@@ -637,6 +805,7 @@ sub batch_import {
   $count *=2;
 
   if ( $format eq 'cch' || $format eq 'cch-update' ) {
+    #false laziness w/below (sub _perform_cch_diff)
     @fields = qw( geocode inoutcity inoutlocal tax location taxbase taxmax
                   excessrate effective_date taxauth taxtype taxcat taxname
                   usetax useexcessrate fee unittype feemax maxtype passflag
@@ -715,9 +884,6 @@ sub batch_import {
     die "unknown format $format";
   }
 
-  eval "use Text::CSV_XS;";
-  die $@ if $@;
-
   my $csv = new Text::CSV_XS;
 
   my $imported = 0;
@@ -758,9 +924,10 @@ sub batch_import {
     foreach my $field ( @fields ) {
       $tax_rate{$field} = shift @columns; 
     }
+
     if ( scalar( @columns ) ) {
       $dbh->rollback if $oldAutoCommit;
-      return "Unexpected trailing columns in line (wrong format?): $line";
+      return "Unexpected trailing columns in line (wrong format?) importing tax_rate: $line";
     }
 
     my $error = &{$hook}(\%tax_rate);
@@ -785,7 +952,8 @@ sub batch_import {
 
   }
 
-  for (grep { !exists($delete{$_}) } keys %insert) {
+  my @replace = grep { exists($delete{$_}) } keys %insert;
+  for (@replace) {
     if ( $job ) {  # progress bar
       if ( time - $min_sec > $last ) {
         my $error = $job->update_statustext(
@@ -799,20 +967,35 @@ sub batch_import {
       }
     }
 
-    my $tax_rate = new FS::tax_rate( $insert{$_} );
-    my $error = $tax_rate->insert;
+    my $old = qsearchs( 'tax_rate', $delete{$_} );
 
-    if ( $error ) {
-      $dbh->rollback if $oldAutoCommit;
-      my $hashref = $insert{$_};
-      $line = join(", ", map { "$_ => ". $hashref->{$_} } keys(%$hashref) );
-      return "can't insert tax_rate for $line: $error";
+    if ( $old ) {
+
+      my $new = new FS::tax_rate({ $old->hash, %{$insert{$_}}, 'manual' => ''  });
+      $new->taxnum($old->taxnum);
+      my $error = $new->replace($old);
+
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        my $hashref = $insert{$_};
+        $line = join(", ", map { "$_ => ". $hashref->{$_} } keys(%$hashref) );
+        return "can't replace tax_rate for $line: $error";
+      }
+
+      $imported++;
+
+    } else {
+
+      $old = delete $delete{$_};
+      warn "WARNING: can't find tax_rate to replace (inserting instead and continuing) for: ".
+        #join(" ", map { "$_ => ". $old->{$_} } @fields);
+        join(" ", map { "$_ => ". $old->{$_} } keys(%$old) );
     }
 
     $imported++;
   }
 
-  for (grep { exists($delete{$_}) } keys %insert) {
+  for (grep { !exists($delete{$_}) } keys %insert) {
     if ( $job ) {  # progress bar
       if ( time - $min_sec > $last ) {
         my $error = $job->update_statustext(
@@ -826,27 +1009,17 @@ sub batch_import {
       }
     }
 
-    my $old = qsearchs( 'tax_rate', $delete{$_} );
-    unless ($old) {
-      $dbh->rollback if $oldAutoCommit;
-      $old = $delete{$_};
-      return "can't find tax_rate to replace for: ".
-        #join(" ", map { "$_ => ". $old->{$_} } @fields);
-        join(" ", map { "$_ => ". $old->{$_} } keys(%$old) );
-    }
-    my $new = new FS::tax_rate({ $old->hash, %{$insert{$_}}, 'manual' => ''  });
-    $new->taxnum($old->taxnum);
-    my $error = $new->replace($old);
+    my $tax_rate = new FS::tax_rate( $insert{$_} );
+    my $error = $tax_rate->insert;
 
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
       my $hashref = $insert{$_};
       $line = join(", ", map { "$_ => ". $hashref->{$_} } keys(%$hashref) );
-      return "can't replace tax_rate for $line: $error";
+      return "can't insert tax_rate for $line: $error";
     }
 
     $imported++;
-    $imported++;
   }
 
   for (grep { !exists($insert{$_}) } keys %delete) {
@@ -864,20 +1037,22 @@ sub batch_import {
     }
 
     my $tax_rate = qsearchs( 'tax_rate', $delete{$_} );
-    unless ($tax_rate) {
+    if (!$tax_rate) {
       $dbh->rollback if $oldAutoCommit;
       $tax_rate = $delete{$_};
-      return "can't find tax_rate to delete for: ".
-        #join(" ", map { "$_ => ". $tax_rate->{$_} } @fields);
-        join(" ", map { "$_ => ". $tax_rate->{$_} } keys(%$tax_rate) );
-    }
-    my $error = $tax_rate->delete;
+      warn "WARNING: can't find tax_rate to delete for: ".
+        join(" ", map { "$_ => ". $tax_rate->{$_} } keys(%$tax_rate) ).
+        " (ignoring)\n";
+    } else {
+      my $error = $tax_rate->delete; #  XXX we really should not do this
+                                     # (it orphans CBPTRL records)
 
-    if ( $error ) {
-      $dbh->rollback if $oldAutoCommit;
-      my $hashref = $delete{$_};
-      $line = join(", ", map { "$_ => ". $hashref->{$_} } keys(%$hashref) );
-      return "can't delete tax_rate for $line: $error";
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        my $hashref = $delete{$_};
+        $line = join(", ", map { "$_ => ". $hashref->{$_} } keys(%$hashref) );
+        return "can't delete tax_rate for $line: $error";
+      }
     }
 
     $imported++;
@@ -961,7 +1136,7 @@ sub _perform_batch_import {
       my $file = lc($name). 'file';
 
       unless ($files{$file}) {
-        $error = "No $name supplied";
+        #$error = "No $name supplied";
         next;
       }
       next if $name eq 'DETAIL' && $format =~ /update/;
@@ -978,7 +1153,7 @@ sub _perform_batch_import {
         unlink $filename or warn "Can't delete $filename: $!"
           unless $keep_cch_files;
         push @insert_list, $name, $insertname, $import_sub, $format;
-        if ( $name eq 'GEOCODE' ) { #handle this whole ordering issue better
+        if ( $name eq 'GEOCODE' || $name eq 'CODE' ) { #handle this whole ordering issue better
           unshift @predelete_list, $name, $deletename, $import_sub, $format;
         } else {
           unshift @delete_list, $name, $deletename, $import_sub, $format;
@@ -993,13 +1168,20 @@ sub _perform_batch_import {
     }
 
     push @insert_list,
-      'DETAIL', "$dir/".$files{detail}, \&FS::tax_rate::batch_import, $format
+      'DETAIL', "$dir/".$files{detailfile}, \&FS::tax_rate::batch_import, $format
       if $format =~ /update/;
 
+    my %addl_param = ();
+    if ( $param->{'delete_only'} ) {
+      $addl_param{'delete_only'} = $param->{'delete_only'};
+      @insert_list = () 
+    }
+
     $error ||= _perform_cch_tax_import( $job,
                                         [ @predelete_list ],
                                         [ @insert_list ],
                                         [ @delete_list ],
+                                        \%addl_param,
     );
     
     
@@ -1024,7 +1206,8 @@ sub _perform_batch_import {
 
 
 sub _perform_cch_tax_import {
-  my ( $job, $predelete_list, $insert_list, $delete_list ) = @_;
+  my ( $job, $predelete_list, $insert_list, $delete_list, $addl_param ) = @_;
+  $addl_param ||= {};
 
   my $error = '';
   foreach my $list ($predelete_list, $insert_list, $delete_list) {
@@ -1033,7 +1216,11 @@ sub _perform_cch_tax_import {
       my $fmt = "$format-update";
       $fmt = $format. ( lc($name) eq 'zip' ? '-zip' : '' );
       open my $fh, "< $file" or $error ||= "Can't open $name file $file: $!";
-      $error ||= &{$method}({ 'filehandle' => $fh, 'format' => $fmt }, $job);
+      my $param = { 'filehandle' => $fh,
+                    'format'     => $fmt,
+                    %$addl_param,
+                  };
+      $error ||= &{$method}($param, $job);
       close $fh;
     }
   }
@@ -1115,8 +1302,26 @@ sub _perform_cch_diff {
   }
   close $newcsvfh;
 
-  for (keys %oldlines) {
-    print $dfh $_, ',"D"', "\n" if $oldlines{$_};
+  #false laziness w/above (sub batch_import)
+  my @fields = qw( geocode inoutcity inoutlocal tax location taxbase taxmax
+                   excessrate effective_date taxauth taxtype taxcat taxname
+                   usetax useexcessrate fee unittype feemax maxtype passflag
+                   passtype basetype );
+  my $numfields = scalar(@fields);
+
+  my $csv = new Text::CSV_XS { 'always_quote' => 1 };
+
+  for my $line (grep $oldlines{$_}, keys %oldlines) {
+
+    $csv->parse($line) or do {
+      #$dbh->rollback if $oldAutoCommit;
+      die "can't parse: ". $csv->error_input();
+    };
+    my @columns = $csv->fields();
+    
+    $csv->combine( splice(@columns, 0, $numfields) );
+
+    print $dfh $csv->string, ',"D"', "\n";
   }
 
   close $dfh;
@@ -1170,9 +1375,6 @@ sub _cch_fetch_and_unzip {
 sub _cch_extract_csv_from_dbf {
   my ( $job, $dir, $name ) = @_;
 
-  eval "use Text::CSV_XS;";
-  die $@ if $@;
-
   eval "use XBase;";
   die $@ if $@;
 
@@ -1635,16 +1837,16 @@ sub process_download_and_update {
 
     if (-d $dir) {
 
-      if (-d "$dir.4") {
-        opendir(my $dirh, "$dir.4") or die "failed to open $dir.4: $!\n";
+      if (-d "$dir.9") {
+        opendir(my $dirh, "$dir.9") or die "failed to open $dir.9: $!\n";
         foreach my $file (readdir($dirh)) {
-          unlink "$dir.4/$file" if (-f "$dir.4/$file");
+          unlink "$dir.9/$file" if (-f "$dir.9/$file");
         }
         closedir($dirh);
-        rmdir "$dir.4";
+        rmdir "$dir.9";
       }
 
-      for (3, 2, 1) {
+      for (8, 7, 6, 5, 4, 3, 2, 1) {
         if ( -e "$dir.$_" ) {
           rename "$dir.$_", "$dir.". ($_+1) or die "can't rename $dir.$_: $!\n";
         }
@@ -1783,6 +1985,12 @@ sub browse_queries {
 =item queue_liability_report PARAMS
 
 Launches a tax liability report.
+
+PARAMS needs to be a base64-encoded Storable hash containing:
+- beginning: the start date, as a I<user-readable string> (not a timestamp).
+- end: the end date of the report, likewise.
+- agentnum: the agent to limit the report to, if any.
+
 =cut
 
 sub queue_liability_report {
@@ -1806,8 +2014,12 @@ sub queue_liability_report {
 
 =item generate_liability_report PARAMS
 
-Generates a tax liability report.  Provide a hash including desired
-agentnum, beginning, and ending
+Generates a tax liability report.  PARAMS must include:
+
+- beginning, as a timestamp
+- ending, as a timestamp
+- p: the Freeside root URL, for generating links
+- agentnum (optional)
 
 =cut
 
@@ -1869,11 +2081,16 @@ sub generate_liability_report {
   my %taxes = ();
   my %basetaxes = ();
   my $calculated = 0;
+
+  # get all distinct tuples of (tax name, state, county, city, locationtaxid)
+  # for taxes that have been charged
+  # (state, county, city are from tax_rate_location, not from customer data)
   my @tax_and_location = qsearch({ table     => 'cust_bill_pkg',
                                    select    => $select,
                                    hashref   => { pkgpart => 0 },
                                    addl_from => $addl_from,
                                    extra_sql => $where,
+                                   debug     => 1,
                                 });
   $count = scalar(@tax_and_location);
   foreach my $t ( @tax_and_location ) {
@@ -1897,15 +2114,17 @@ sub generate_liability_report {
       $taxes{$label}->{'url_param'} =
         join(';', map { "$_=". uri_escape($t->$_) } @params);
 
-      my $payby_itemdesc_loc = 
-        "    payby != 'COMP' ".
-        "AND ( itemdesc = ? OR ? = '' AND itemdesc IS NULL ) ".
+      my $itemdesc_loc = 
+      # "    payby != 'COMP' ". # breaks the entire report under 4.x
+      #                         # and unnecessary since COMP accounts don't
+      #                         # get taxes calculated in the first place
+        "    ( itemdesc = ? OR ? = '' AND itemdesc IS NULL ) ".
         "AND ". FS::tax_rate_location->location_sql( map { $_ => $t->$_ }
                                                          @taxparams
                                                    );
 
       my $taxwhere =
-        "FROM cust_bill_pkg $addl_from $where AND $payby_itemdesc_loc";
+        "FROM cust_bill_pkg $addl_from $where AND $itemdesc_loc";
 
       my $sql = "SELECT SUM(amount) $taxwhere AND cust_bill_pkg.pkgnum = 0";
 
@@ -1916,7 +2135,7 @@ sub generate_liability_report {
       my $creditfrom =
        "JOIN cust_credit_bill_pkg USING (billpkgnum,billpkgtaxratelocationnum)";
       my $creditwhere =
-        "FROM cust_bill_pkg $addl_from $creditfrom $where AND $payby_itemdesc_loc";
+        "FROM cust_bill_pkg $addl_from $creditfrom $where AND $itemdesc_loc";
 
       $sql = "SELECT SUM(cust_credit_bill_pkg.amount) ".
              " $creditwhere AND cust_bill_pkg.pkgnum = 0";
@@ -1980,7 +2199,8 @@ sub generate_liability_report {
   my $dateagentlink = "begin=$args{beginning};end=$args{ending}";
   $dateagentlink .= ';agentnum='. $args{agentnum}
     if length($agentname);
-  my $baselink   = $args{p}. "search/cust_bill_pkg.cgi?$dateagentlink";
+  my $baselink   = $args{p}. "search/cust_bill_pkg.cgi?vendortax=1;" .
+                             $dateagentlink;
   my $creditlink = $args{p}. "search/cust_credit_bill_pkg.html?$dateagentlink";
 
   print $report <<EOF;
@@ -2069,7 +2289,7 @@ EOF
   my $dropstring = '%%%FREESIDE_CACHE%%%/cache.'. $FS::UID::datasrc. '/report.';
   $reportname =~ s/^$dropstring//;
 
-  my $reporturl = "%%%ROOTURL%%%/misc/queued_report?report=$reportname";
+  my $reporturl = "%%%ROOTURL%%%/misc/queued_report.html?report=$reportname";
   die "<a href=$reporturl>view</a>\n";
 
 }
@@ -2087,8 +2307,7 @@ EOF
 
 =head1 SEE ALSO
 
-L<FS::Record>, L<FS::cust_main>, L<FS::cust_bill>, schema.html from the base
-documentation.
+L<FS::Record>, L<FS::cust_location>, L<FS::cust_bill>
 
 =cut