fix whitespace and case correctness of city names, #71501
authorMark Wells <mark@freeside.biz>
Thu, 21 Jul 2016 20:47:42 +0000 (13:47 -0700)
committerMark Wells <mark@freeside.biz>
Thu, 21 Jul 2016 20:47:42 +0000 (13:47 -0700)
FS/FS/Record.pm
FS/FS/TaxEngine/internal.pm
FS/FS/Upgrade.pm
FS/FS/cust_location.pm
FS/FS/cust_main_county.pm

index 7f76d99..7ec8117 100644 (file)
@@ -3220,6 +3220,22 @@ sub ut_agentnum_acl {
 
 }
 
 
 }
 
+=item trim_whitespace FIELD[, FIELD ... ]
+
+Strip leading and trailing spaces from the value in the named FIELD(s).
+
+=cut
+
+sub trim_whitespace {
+  my $self = shift;
+  foreach my $field (@_) {
+    my $value = $self->get($field);
+    $value =~ s/^\s+//;
+    $value =~ s/\s+$//;
+    $self->set($field, $value);
+  }
+}
+
 =item fields [ TABLE ]
 
 This is a wrapper for real_fields.  Code that called
 =item fields [ TABLE ]
 
 This is a wrapper for real_fields.  Code that called
index db7010c..3e3e7e5 100644 (file)
@@ -28,8 +28,10 @@ sub add_sale {
 
   push @{ $self->{items} }, $cust_bill_pkg;
 
 
   push @{ $self->{items} }, $cust_bill_pkg;
 
-  my @loc_keys = qw( district city county state country );
-  my %taxhash = map { $_ => $location->get($_) } @loc_keys;
+  my %taxhash = map { $_ => $location->get($_) }
+                qw( district county state country );
+  # city names in cust_main_county are uppercase
+  $taxhash{'city'} = uc($location->get('city'));
 
   $taxhash{'taxclass'} = $part_item->taxclass;
 
 
   $taxhash{'taxclass'} = $part_item->taxclass;
 
index 6f14cd2..65b83bd 100644 (file)
@@ -478,8 +478,12 @@ sub upgrade_data {
     #populate tax statuses
     'tax_status' => [],
 
     #populate tax statuses
     'tax_status' => [],
 
-    #mark certain taxes as system-maintained
+    #mark certain taxes as system-maintained,
+    # and fix whitespace
     'cust_main_county' => [],
     'cust_main_county' => [],
+
+    #fix whitespace
+    'cust_location' => [],
   ;
 
   \%hash;
   ;
 
   \%hash;
index 9040098..fdc2cf8 100644 (file)
@@ -2,7 +2,7 @@ package FS::cust_location;
 use base qw( FS::geocode_Mixin FS::Record );
 
 use strict;
 use base qw( FS::geocode_Mixin FS::Record );
 
 use strict;
-use vars qw( $import $DEBUG $conf $label_prefix );
+use vars qw( $import $DEBUG $conf $label_prefix $allow_location_edit );
 use Data::Dumper;
 use Date::Format qw( time2str );
 use FS::UID qw( dbh driver_name );
 use Data::Dumper;
 use Date::Format qw( time2str );
 use FS::UID qw( dbh driver_name );
@@ -171,6 +171,10 @@ sub find_or_insert {
   delete $nonempty{'locationnum'};
 
   my %hash = map { $_ => $self->get($_) } @essential;
   delete $nonempty{'locationnum'};
 
   my %hash = map { $_ => $self->get($_) } @essential;
+  foreach (values %hash) {
+    s/^\s+//;
+    s/\s+$//;
+  }
   my @matches = qsearch('cust_location', \%hash);
 
   # we no longer reject matches for having different values in nonessential
   my @matches = qsearch('cust_location', \%hash);
 
   # we no longer reject matches for having different values in nonessential
@@ -292,7 +296,7 @@ sub replace {
   # it's a prospect location, then there are no active packages, no billing
   # history, no taxes, and in general no reason to keep the old location
   # around.
   # it's a prospect location, then there are no active packages, no billing
   # history, no taxes, and in general no reason to keep the old location
   # around.
-  if ( $self->custnum ) {
+  if ( !$allow_location_edit and $self->custnum ) {
     foreach (qw(address1 address2 city state zip country)) {
       if ( $self->$_ ne $old->$_ ) {
         return "can't change cust_location field $_";
     foreach (qw(address1 address2 city state zip country)) {
       if ( $self->$_ ne $old->$_ ) {
         return "can't change cust_location field $_";
@@ -347,6 +351,10 @@ sub check {
 
   return '' if $self->disabled; # so that disabling locations never fails
 
 
   return '' if $self->disabled; # so that disabling locations never fails
 
+  # maybe should just do all fields in the table?
+  # or in every table?
+  $self->trim_whitespace(qw(district city county state country));
+
   my $error = 
     $self->ut_numbern('locationnum')
     || $self->ut_foreign_keyn('prospectnum', 'prospect_main', 'prospectnum')
   my $error = 
     $self->ut_numbern('locationnum')
     || $self->ut_foreign_keyn('prospectnum', 'prospect_main', 'prospectnum')
@@ -887,6 +895,35 @@ sub process_standardize {
   close $log;
 }
 
   close $log;
 }
 
+sub _upgrade_data {
+  my $class = shift;
+
+  # are we going to need to update tax districts?
+  my $use_districts = $conf->config('tax_district_method') ? 1 : 0;
+
+  # trim whitespace on records that need it
+  local $allow_location_edit = 1;
+  foreach my $field (qw(city county state country district)) {
+    foreach my $location (qsearch({
+      table => 'cust_location',
+      extra_sql => " WHERE $field LIKE ' %' OR $field LIKE '% '"
+    })) {
+      my $error = $location->replace;
+      die "$error (fixing whitespace in $field, locationnum ".$location->locationnum.')'
+        if $error;
+
+      if ( $use_districts ) {
+        my $queue = new FS::queue {
+          'job' => 'FS::geocode_Mixin::process_district_update'
+        };
+        $error = $queue->insert( 'FS::cust_location' => $location->locationnum );
+        die $error if $error;
+      }
+    } # foreach $location
+  } # foreach $field
+  '';
+}
+
 =head1 BUGS
 
 =head1 SEE ALSO
 =head1 BUGS
 
 =head1 SEE ALSO
index 3c355e8..a1233d0 100644 (file)
@@ -122,6 +122,9 @@ methods.
 sub check {
   my $self = shift;
 
 sub check {
   my $self = shift;
 
+  $self->trim_whitespace(qw(district city county state country));
+  $self->set('city', uc($self->get('city'))); # also county?
+
   $self->exempt_amount(0) unless $self->exempt_amount;
 
   $self->ut_numbern('taxnum')
   $self->exempt_amount(0) unless $self->exempt_amount;
 
   $self->ut_numbern('taxnum')
@@ -701,6 +704,49 @@ sub _upgrade_data {
     }
     FS::upgrade_journal->set_done($journal);
   }
     }
     FS::upgrade_journal->set_done($journal);
   }
+  # trim whitespace and convert to uppercase in the 'city' field.
+  foreach my $record (qsearch({
+    table => 'cust_main_county',
+    extra_sql => " WHERE city LIKE ' %' OR city LIKE '% ' OR city != UPPER(city)",
+  })) {
+    # any with-trailing-space records probably duplicate other records
+    # from the same city, and if we just fix the record in place, we'll
+    # create an exact duplicate.
+    # so find the record this one would duplicate, and merge them.
+    $record->check; # trims whitespace
+    my %match = map { $_ => $record->get($_) }
+      qw(city county state country district taxname taxclass);
+    my $other = qsearchs('cust_main_county', \%match);
+    if ($other) {
+      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;
+          }
+        }
+        my $error = $record->delete;
+        die $error if $error;
+      }
+    } else {
+      # else there is no record this one duplicates, so just fix it
+      my $error = $record->replace;
+      die $error if $error;
+    }
+  } # foreach $record
   '';
 }
 
   '';
 }