fix whitespace and case correctness of city names, #71501

5 files changed, 110 insertions, 5 deletions

diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm
index 55cca08ca..7e9a2e090 100644
--- a/FS/FS/Record.pm
+++ b/FS/FS/Record.pm
@@ -3034,6 +3034,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
diff --git a/FS/FS/Upgrade.pm b/FS/FS/Upgrade.pm
index 377a0b17f..5b2750577 100644
--- a/FS/FS/Upgrade.pm
+++ b/FS/FS/Upgrade.pm
@@ -441,8 +441,12 @@ sub upgrade_data {
     #set default locations on quoted packages
     'quotation_pkg' => [],
 
-    #mark certain taxes as system-maintained
+    #mark certain taxes as system-maintained,
+    # and fix whitespace
     'cust_main_county' => [],
+
+    #fix whitespace
+    'cust_location' => [],
   ;
 
   \%hash;
diff --git a/FS/FS/cust_location.pm b/FS/FS/cust_location.pm
index 4960f746f..481ebb11a 100644
--- a/FS/FS/cust_location.pm
+++ b/FS/FS/cust_location.pm
@@ -2,7 +2,7 @@ package FS::cust_location;
 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 );
@@ -166,6 +166,10 @@ sub find_or_insert {
   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
@@ -287,7 +291,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.
-  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 $_";
@@ -342,6 +346,10 @@ sub check {
 
   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')
@@ -891,6 +899,35 @@ sub process_standardize {
   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
diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index d6a765a8e..b278fe410 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -1767,8 +1767,10 @@ sub _handle_taxes {
     # We fetch taxes even if the customer is completely exempt,
     # because we need to record that fact.
 
-    my @loc_keys = qw( district city county state country );
-    my %taxhash = map { $_ => $location->$_ } @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;
 
diff --git a/FS/FS/cust_main_county.pm b/FS/FS/cust_main_county.pm
index 6eadff244..40caabb13 100644
--- a/FS/FS/cust_main_county.pm
+++ b/FS/FS/cust_main_county.pm
@@ -122,6 +122,9 @@ methods.
 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')
@@ -660,6 +663,49 @@ sub _upgrade_data {
     }
     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
   '';
 }