[freeside-commits] branch master updated. 6d337d9a42b43898d48e45f13060d5285ddfdaa2

Mitch Jackson mitch at freeside.biz
Sat May 4 18:09:06 PDT 2019


The branch, master has been updated
       via  6d337d9a42b43898d48e45f13060d5285ddfdaa2 (commit)
       via  9cb0274535a95a45a8f5796c8edafefb074f57c8 (commit)
      from  f6ee9a26cb655615c239b13c334b3accb92c6d0f (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 6d337d9a42b43898d48e45f13060d5285ddfdaa2
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat May 4 17:04:55 2019 -0400

    RT# 83122 fix dupes on wa_sales tax table updates

diff --git a/FS/FS/Cron/tax_rate_update.pm b/FS/FS/Cron/tax_rate_update.pm
index ef529c4a5..bb9d4d13d 100755
--- a/FS/FS/Cron/tax_rate_update.pm
+++ b/FS/FS/Cron/tax_rate_update.pm
@@ -332,17 +332,64 @@ sub wa_sales_update_cust_main_county {
   for my $taxclass ( FS::part_pkg_taxclass->taxclass_names ) {
     $taxclass ||= undef; # trap empty string when taxclasses are disabled
 
-    my %cust_main_county =
-      map { $_->district => $_ }
+    # Dupe detection/remediation:
+    #
+    # Previous code for washington state tax district was creating
+    # duplicate entries for tax districts.  This could lead to customers
+    # being double-taxed
+    #
+    # The following code detects and eliminates duplicates that
+    # were created by wa_sales district code (source=wa_sales)
+    # before updating the tax table with the newly downloaded
+    # data
+
+    my %cust_main_county;
+    my %cust_main_county_dupe;
+
+    for my $row (
       qsearch(
         cust_main_county => {
-          district => { op => '!=', value => undef },
-          state    => 'WA',
-          country  => 'US',
-          source   => 'wa_sales',
-          taxclass => $taxclass,
+          source    => 'wa_sales',
+          district  => { op => '!=', value => undef },
+          tax_class => $taxclass,
         }
-      );
+      )
+    ) {
+      my $district = $row->district;
+
+      # Row belongs to a known dupe group of districts
+      if ( $cust_main_county_dupe{$district} ) {
+        push @{ $cust_main_county_dupe{$district} }, $row;
+        next;
+      }
+
+      # Row is the first seen dupe for the given district
+      if ( $cust_main_county{$district} ) {
+        $cust_main_county_dupe{$district} = [
+          delete $cust_main_county{$district},
+          $row
+        ];
+        next;
+      }
+
+      # Row is the first seen with this district
+      $cust_main_county{$district} = $row;
+    }
+
+    # Merge any dupes, place resulting non-dupe row in %cust_main_county
+    #  Merge, even if one of the dupes has a $0 tax, or some other
+    #  variation on tax row data.  Data for this row will get corrected
+    #  during the following tax import
+    for my $dupe_district_aref ( values %cust_main_county_dupe ) {
+      my $row_to_keep = shift @$dupe_district_aref;
+      while ( my $row_to_merge = shift @$dupe_district_aref ) {
+        $row_to_merge->_merge_into(
+          $row_to_keep,
+          { identical_record_check => 0 },
+        );
+      }
+      $cust_main_county{$row_to_keep->district} = $row_to_keep;
+    }
 
     for my $district ( @{ $args->{tax_districts} } ) {
       if ( my $row = $cust_main_county{ $district->{district} } ) {

commit 9cb0274535a95a45a8f5796c8edafefb074f57c8
Author: Mitch Jackson <mitch at freeside.biz>
Date:   Sat May 4 16:44:15 2019 -0400

    RT# 83122 Update method to merge dupe tax records
    
    FS::cust_main_county::_merge_into
    - Add application logging
    - Add optional param hash

diff --git a/FS/FS/cust_main_county.pm b/FS/FS/cust_main_county.pm
index 5325fa562..2bd7342ca 100644
--- a/FS/FS/cust_main_county.pm
+++ b/FS/FS/cust_main_county.pm
@@ -4,6 +4,7 @@ use base qw( FS::Record );
 use strict;
 use vars qw( @EXPORT_OK $conf
              @cust_main_county %cust_main_county $countyflag ); # $cityflag );
+use Carp qw( croak );
 use Exporter;
 use FS::Record qw( qsearch qsearchs dbh );
 use FS::cust_bill_pkg;
@@ -12,6 +13,7 @@ use FS::cust_pkg;
 use FS::part_pkg;
 use FS::cust_tax_exempt;
 use FS::cust_tax_exempt_pkg;
+use FS::Log;
 use FS::upgrade_journal;
 
 @EXPORT_OK = qw( regionselector );
@@ -683,33 +685,74 @@ END
 }
 
 sub _merge_into {
-  # for internal use: takes another cust_main_county object, transfers
-  # all existing references to this record to that one, and deletes this
-  # one.
-  my $record = shift;
-  my $other = shift or die "record to merge into must be provided";
-  my $new_taxnum = $other->taxnum;
-  my $old_taxnum = $record->taxnum;
-  if ($other->tax != $record->tax or
-      $other->exempt_amount != $record->exempt_amount) {
-    # don't assume these are the same.
-    warn "Found duplicate taxes (#$new_taxnum and #$old_taxnum) but they have different rates and can't be merged.\n";
-  } else {
-    warn "Merging tax #$old_taxnum into #$new_taxnum\n";
-    foreach my $table (qw(
-      cust_bill_pkg_tax_location
-      cust_bill_pkg_tax_location_void
-      cust_tax_exempt_pkg
-      cust_tax_exempt_pkg_void
-    )) {
-      foreach my $row (qsearch($table, { 'taxnum' => $old_taxnum })) {
-        $row->set('taxnum' => $new_taxnum);
-        my $error = $row->replace;
-        die $error if $error;
+  # For internal use:
+  #
+  # When given two cust_main_county row objects, rewrite all database foreign
+  # key references referring to $row_to_merge->taxnum as references to
+  # $row_to_keep->taxnum, so $row_to_merge can be safely deleted from
+  # cust_main_county
+  #
+  # Usage (class method):
+  #    $row_to_merge->_merge_into( $row_to_keep )
+  #
+  # Usage (package function):
+  #    FS::cust_main_county::_merge_into( $row_to_merge, $row_to_keep )
+  #
+  # Optionally, allow merge when records don't match
+  #      (useful during tax table update routines)
+  #     $row_to_merge->_merge_info(
+  #       $row_to_keep,
+  #       { identical_record_check => 0 }
+  #     );
+
+  my $row_to_merge = shift;
+  my $row_to_keep  = shift
+    or croak 'record to merge into must be provided';
+
+  my $args = shift || { identical_record_check => 1 };
+  croak 'invalid arguments hashref' unless ref $args;
+
+  my $log = FS::Log->new('FS::cust_main_county');
+
+  my $keep_taxnum  = $row_to_keep->taxnum;
+  my $merge_taxnum = $row_to_merge->taxnum;
+
+  if (
+    $args->{identical_record_check}
+    && (
+      $row_to_keep->tax != $row_to_merge->tax
+      || $row_to_keep->exempt_amount != $row_to_merge->exempt_amount
+    )
+  ) {
+    my $msg = "Found duplicate taxes (#$keep_taxnum and #$merge_taxnum) "
+            . "but they have different rates and can't be merged.";
+    $log->warn( $msg );
+    warn "$msg\n";
+    return;
+  }
+
+  my $msg = "Merging tax #$merge_taxnum into #$keep_taxnum";
+  $log->warn( $msg );
+  warn "$msg\n";
+
+  foreach my $table (qw(
+    cust_bill_pkg_tax_location
+    cust_bill_pkg_tax_location_void
+    cust_tax_exempt_pkg
+    cust_tax_exempt_pkg_void
+  )) {
+    foreach my $row (qsearch($table, { 'taxnum' => $merge_taxnum })) {
+      $row->set('taxnum' => $keep_taxnum);
+      if ( my $error = $row->replace ) {
+        $log->error( $error );
+        die $error;
       }
     }
-    my $error = $record->delete;
-    die $error if $error;
+  }
+
+  if ( my $error = $row_to_merge->delete ) {
+    $log->error( $error );
+    die $error;
   }
 }
 

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/Cron/tax_rate_update.pm | 63 +++++++++++++++++++++++++----
 FS/FS/cust_main_county.pm     | 93 +++++++++++++++++++++++++++++++------------
 2 files changed, 123 insertions(+), 33 deletions(-)




More information about the freeside-commits mailing list