From 786d21e09d1f11c8976a8d45ef734d2d0a100ee7 Mon Sep 17 00:00:00 2001 From: Mark Wells Date: Sun, 5 May 2013 16:44:26 -0700 Subject: [PATCH] allow certain minor location edits without moving packages, #940 --- FS/FS/cust_bill_pkg.pm | 15 ++-- FS/FS/cust_location.pm | 100 +++++++++++++++++++++------ FS/FS/cust_main.pm | 38 ++-------- FS/FS/cust_main/Packages.pm | 11 ++- FS/FS/cust_pkg.pm | 16 ++--- FS/FS/svc_phone.pm | 5 +- httemplate/edit/process/change-cust_pkg.html | 2 +- httemplate/edit/process/cust_location.cgi | 6 +- httemplate/edit/process/cust_main.cgi | 4 +- httemplate/edit/process/quick-cust_pkg.cgi | 2 +- httemplate/edit/process/svc_phone.html | 2 +- 11 files changed, 111 insertions(+), 90 deletions(-) diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm index d8cbf5915..0c8c0bbbf 100644 --- a/FS/FS/cust_bill_pkg.pm +++ b/FS/FS/cust_bill_pkg.pm @@ -1104,15 +1104,12 @@ sub upgrade_tax_location { delete @hash{qw(censustract censusyear latitude longitude coord_auto)}; $hash{custnum} = $h_cust_main->custnum; - my $tax_loc = FS::cust_location->new_or_existing(\%hash); - if ( !$tax_loc->locationnum ) { - $tax_loc->disabled('Y'); - my $error = $tax_loc->insert; - if ( $error ) { - warn "couldn't create historical location record for cust#". - $h_cust_main->custnum.": $error\n"; - next INVOICE; - } + my $tax_loc = FS::cust_location->new(\%hash); + my $error = $tax_loc->find_or_insert || $tax_loc->disable_if_unused; + if ( $error ) { + warn "couldn't create historical location record for cust#". + $h_cust_main->custnum.": $error\n"; + next INVOICE; } my $exempt_cust = 1 if $h_cust_main->tax; diff --git a/FS/FS/cust_location.pm b/FS/FS/cust_location.pm index b12a161db..4560716d5 100644 --- a/FS/FS/cust_location.pm +++ b/FS/FS/cust_location.pm @@ -104,33 +104,93 @@ points to. You can ask the object for a copy with the I method. sub table { 'cust_location'; } -=item new_or_existing HASHREF +=item find_or_insert -Returns an existing location matching the customer and address fields in -HASHREF, if one exists; otherwise returns a new location containing those -fields. The following fields must match: address1, address2, city, county, -state, zip, country, geocode, disabled. Other fields are only required -to match if they're specified in HASHREF. +Finds an existing location matching the customer and address values in this +location, if one exists, and sets the contents of this location equal to that +one (including its locationnum). -The new location will not be inserted; the calling code must call C -(or a method such as C) to insert it, and check for errors at that -point. +If an existing location is not found, this one I be inserted. (This is a +change from the "new_or_existing" method that this replaces.) + +The following fields are considered "essential" and I match: custnum, +address1, address2, city, county, state, zip, country, location_number, +location_type, location_kind. Disabled locations will be found only if this +location is set to disabled. + +If 'coord_auto' is null, and latitude and longitude are not null, then +latitude and longitude are also essential fields. + +All other fields are considered "non-essential". If a non-essential field is +empty in this location, it will be ignored in determining whether an existing +location matches. + +If a non-essential field is non-empty in this location, existing locations +that contain a different non-empty value for that field will not match. An +existing location in which the field is I will match, but will be +updated in-place with the value of that field. + +Returns an error string if inserting or updating a location failed. + +It is unfortunately hard to determine if this created a new location or not. =cut -sub new_or_existing { - my $class = shift; - my %hash = ref($_[0]) ? %{$_[0]} : @_; - # if coords are empty, then it doesn't matter if they're auto or not - if ( !$hash{'latitude'} and !$hash{'longitude'} ) { - delete $hash{'coord_auto'}; +sub find_or_insert { + my $self = shift; + + my @essential = (qw(custnum address1 address2 city county state zip country + location_number location_type location_kind disabled)); + + if ( !$self->coord_auto and $self->latitude and $self->longitude ) { + push @essential, qw(latitude longitude); + # but NOT coord_auto; if the latitude and longitude match the geocoded + # values then that's good enough } - foreach ( qw(address1 address2 city county state zip country geocode - disabled ) ) { - # empty fields match only empty fields - $hash{$_} = '' if !defined($hash{$_}); + + # put nonempty, nonessential fields/values into this hash + my %nonempty = map { $_ => $self->get($_) } + grep {$self->get($_)} $self->fields; + delete @nonempty{@essential}; + delete $nonempty{'locationnum'}; + + my %hash = map { $_ => $self->get($_) } @essential; + my @matches = qsearch('cust_location', \%hash); + + # consider candidate locations + MATCH: foreach my $old (@matches) { + my $reject = 0; + foreach my $field (keys %nonempty) { + my $old_value = $old->get($field); + if ( length($old_value) > 0 ) { + if ( $field eq 'latitude' or $field eq 'longitude' ) { + # special case, because these are decimals + if ( abs($old_value - $nonempty{$field}) > 0.000001 ) { + $reject = 1; + } + } elsif ( $old_value ne $nonempty{$field} ) { + $reject = 1; + } + } else { + # it's empty in $old, has a value in $self + $old->set($field, $nonempty{$field}); + } + next MATCH if $reject; + } # foreach $field + + if ( $old->modified ) { + my $error = $old->replace; + return $error if $error; + } + # set $self equal to $old + foreach ($self->fields) { + $self->set($_, $old->get($_)); + } + return ""; } - return qsearchs('cust_location', \%hash) || $class->new(\%hash); + + # didn't find a match + return $self->insert; } =item insert diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm index 1cf036551..f21932cf6 100644 --- a/FS/FS/cust_main.pm +++ b/FS/FS/cust_main.pm @@ -1509,43 +1509,17 @@ sub replace { my $old_loc = $old->$l; my $new_loc = $self->$l; - if ( !$new_loc->locationnum ) { - # changing location - # If the new location is all empty fields, or if it's identical to - # the old location in all fields, don't replace. - my @nonempty = grep { $new_loc->$_ } $self->location_fields; - next if !@nonempty; - my @unlike = grep { $new_loc->$_ ne $old_loc->$_ } $self->location_fields; - - if ( @unlike or $old_loc->disabled ) { - warn " changed $l fields: ".join(',',@unlike)."\n" - if $DEBUG; - $new_loc->set(custnum => $self->custnum); - - # insert it--the old location will be disabled later - my $error = $new_loc->insert; - if ( $error ) { - $dbh->rollback if $oldAutoCommit; - return $error; - } - - } else { - # no fields have changed and $old_loc isn't disabled, so don't change it - next; - } - - } - elsif ( $new_loc->custnum ne $self->custnum or $new_loc->prospectnum ) { + # find the existing location if there is one + $new_loc->set('custnum' => $self->custnum); + my $error = $new_loc->find_or_insert; + if ( $error ) { $dbh->rollback if $oldAutoCommit; - return "$l belongs to customer ".$new_loc->custnum; + return $error; } - # else the new location belongs to this customer so we're good - - # set the foo_locationnum now that we have one. $self->set($l.'num', $new_loc->locationnum); - } #for $l + # replace the customer record my $error = $self->SUPER::replace($old); if ( $error ) { diff --git a/FS/FS/cust_main/Packages.pm b/FS/FS/cust_main/Packages.pm index 41ef22894..8484df50e 100644 --- a/FS/FS/cust_main/Packages.pm +++ b/FS/FS/cust_main/Packages.pm @@ -130,13 +130,10 @@ sub order_pkg { } elsif ( $opt->{'cust_location'} ) { - if ( ! $opt->{'cust_location'}->locationnum ) { - # not inserted yet - my $error = $opt->{'cust_location'}->insert; - if ( $error ) { - $dbh->rollback if $oldAutoCommit; - return "inserting cust_location (transaction rolled back): $error"; - } + my $error = $opt->{'cust_location'}->find_or_insert; + if ( $error ) { + $dbh->rollback if $oldAutoCommit; + return "inserting cust_location (transaction rolled back): $error"; } $cust_pkg->locationnum($opt->{'cust_location'}->locationnum); diff --git a/FS/FS/cust_pkg.pm b/FS/FS/cust_pkg.pm index 4464aa5a2..c49007ce1 100644 --- a/FS/FS/cust_pkg.pm +++ b/FS/FS/cust_pkg.pm @@ -1773,19 +1773,13 @@ sub change { $hash{"change_$_"} = $self->$_() foreach qw( pkgnum pkgpart locationnum ); - if ( $opt->{'cust_location'} && - ( ! $opt->{'locationnum'} || $opt->{'locationnum'} == -1 ) ) { - - if ( ! $opt->{'cust_location'}->locationnum ) { - # not inserted yet - $error = $opt->{'cust_location'}->insert; - if ( $error ) { - $dbh->rollback if $oldAutoCommit; - return "inserting cust_location (transaction rolled back): $error"; - } + if ( $opt->{'cust_location'} ) { + $error = $opt->{'cust_location'}->find_or_insert; + if ( $error ) { + $dbh->rollback if $oldAutoCommit; + return "inserting cust_location (transaction rolled back): $error"; } $opt->{'locationnum'} = $opt->{'cust_location'}->locationnum; - } # whether to override pkgpart checking on the new package diff --git a/FS/FS/svc_phone.pm b/FS/FS/svc_phone.pm index 3cc1adc66..bab8537bb 100644 --- a/FS/FS/svc_phone.pm +++ b/FS/FS/svc_phone.pm @@ -288,9 +288,8 @@ sub insert { #false laziness w/cust_pkg.pm... move this to location_Mixin? that would #make it more of a base class than a mixin... :) - if ( $options{'cust_location'} - && ( ! $self->locationnum || $self->locationnum == -1 ) ) { - my $error = $options{'cust_location'}->insert; + if ( $options{'cust_location'} ) { + my $error = $options{'cust_location'}->find_or_insert; if ( $error ) { $dbh->rollback if $oldAutoCommit; return "inserting cust_location (transaction rolled back): $error"; diff --git a/httemplate/edit/process/change-cust_pkg.html b/httemplate/edit/process/change-cust_pkg.html index 77f261d56..c893f13a2 100644 --- a/httemplate/edit/process/change-cust_pkg.html +++ b/httemplate/edit/process/change-cust_pkg.html @@ -32,7 +32,7 @@ my %change = map { $_ => scalar($cgi->param($_)) } $change{'keep_dates'} = 1; if ( $cgi->param('locationnum') == -1 ) { - my $cust_location = FS::cust_location->new_or_existing({ + my $cust_location = FS::cust_location->new({ 'custnum' => $cust_pkg->custnum, map { $_ => scalar($cgi->param($_)) } qw( address1 address2 city county state zip country ) diff --git a/httemplate/edit/process/cust_location.cgi b/httemplate/edit/process/cust_location.cgi index 56c3968f6..fd1b8740e 100644 --- a/httemplate/edit/process/cust_location.cgi +++ b/httemplate/edit/process/cust_location.cgi @@ -28,12 +28,12 @@ my $cust_location = qsearchs({ }); die "unknown locationnum $locationnum" unless $cust_location; -my $new = FS::cust_location->new_or_existing({ +my $new = FS::cust_location->new({ custnum => $cust_location->custnum, prospectnum => $cust_location->prospectnum, map { $_ => scalar($cgi->param($_)) } FS::cust_main->location_fields }); - -my $error = $cust_location->move_to($new); +my $error = $new->find_or_insert; +$error ||= $cust_location->move_to($new); diff --git a/httemplate/edit/process/cust_main.cgi b/httemplate/edit/process/cust_main.cgi index c1f815550..d295ed317 100755 --- a/httemplate/edit/process/cust_main.cgi +++ b/httemplate/edit/process/cust_main.cgi @@ -11,7 +11,7 @@ <%once> my $me = '[edit/process/cust_main.cgi]'; -my $DEBUG = 0; +my $DEBUG = 1; <%init> @@ -83,7 +83,7 @@ for my $pre (qw(bill ship)) { } $hash{'custnum'} = $cgi->param('custnum'); warn Dumper \%hash if $DEBUG; - $locations{$pre} = FS::cust_location->new_or_existing(\%hash); + $locations{$pre} = FS::cust_location->new(\%hash); } if ( ($cgi->param('same') || '') eq 'Y' ) { diff --git a/httemplate/edit/process/quick-cust_pkg.cgi b/httemplate/edit/process/quick-cust_pkg.cgi index 0cc17d36b..14dbda166 100644 --- a/httemplate/edit/process/quick-cust_pkg.cgi +++ b/httemplate/edit/process/quick-cust_pkg.cgi @@ -155,7 +155,7 @@ if ( $quotationnum ) { } if ( $locationnum == -1 ) { - my $cust_location = FS::cust_location->new_or_existing({ + my $cust_location = FS::cust_location->new({ map { $_ => scalar($cgi->param($_)) } ('custnum', FS::cust_main->location_fields) }); diff --git a/httemplate/edit/process/svc_phone.html b/httemplate/edit/process/svc_phone.html index 9983ea2cb..09398fdfb 100644 --- a/httemplate/edit/process/svc_phone.html +++ b/httemplate/edit/process/svc_phone.html @@ -40,7 +40,7 @@ my $args_callback = sub { my %opt = (); if ( $cgi->param('locationnum') == -1 ) { - my $cust_location = FS::cust_location->new_or_existing({ + my $cust_location = FS::cust_location->new({ map { $_ => scalar($cgi->param($_)) } qw( custnum address1 address2 city county state zip country ) }); -- 2.11.0