From 6262584c58bdc26ee7e7958e3a96483efe88500e 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/TaxEngine/internal.pm | 6 ++++-- FS/FS/Upgrade.pm | 6 +++++- FS/FS/cust_location.pm | 41 ++++++++++++++++++++++++++++++++++++++-- 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 7f76d9988..7ec811752 100644 --- a/FS/FS/Record.pm +++ b/FS/FS/Record.pm @@ -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 diff --git a/FS/FS/TaxEngine/internal.pm b/FS/FS/TaxEngine/internal.pm index db7010c18..3e3e7e520 100644 --- a/FS/FS/TaxEngine/internal.pm +++ b/FS/FS/TaxEngine/internal.pm @@ -28,8 +28,10 @@ sub add_sale { 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; diff --git a/FS/FS/Upgrade.pm b/FS/FS/Upgrade.pm index 6f14cd202..65b83bd5f 100644 --- a/FS/FS/Upgrade.pm +++ b/FS/FS/Upgrade.pm @@ -478,8 +478,12 @@ sub upgrade_data { #populate tax statuses 'tax_status' => [], - #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 90400984c..fdc2cf8da 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 ); @@ -171,6 +171,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 @@ -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. - 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 $_"; @@ -347,6 +351,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') @@ -887,6 +895,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_county.pm b/FS/FS/cust_main_county.pm index 3c355e823..a1233d083 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') @@ -701,6 +704,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