From dd9a0ea1c8351841c8d41ab46e94abbdb0c75db4 Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Thu, 21 Jul 2016 13:47:42 -0700 Subject: [PATCH] fix whitespace and case correctness of city names, #71501 --- FS/FS/Record.pm | 16 ++++++++++++++++ FS/FS/Upgrade.pm | 6 +++++- FS/FS/cust_location.pm | 41 +++++++++++++++++++++++++++++++++++++++-- FS/FS/cust_main/Billing.pm | 6 ++++-- FS/FS/cust_main_county.pm | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 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 ''; } -- 2.11.0