[freeside-commits] branch master updated. 786d21e09d1f11c8976a8d45ef734d2d0a100ee7

Mark Wells mark at 420.am
Sun May 5 16:45:35 PDT 2013


The branch, master has been updated
       via  786d21e09d1f11c8976a8d45ef734d2d0a100ee7 (commit)
      from  1daf1a670d3cdfb307271fb7c7c98c83fb1fb464 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 786d21e09d1f11c8976a8d45ef734d2d0a100ee7
Author: Mark Wells <mark at freeside.biz>
Date:   Sun May 5 16:44:26 2013 -0700

    allow certain minor location edits without moving packages, #940

diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index d8cbf59..0c8c0bb 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 b12a161..4560716 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<hash> 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<insert>
-(or a method such as C<move_to>) to insert it, and check for errors at that
-point.
+If an existing location is not found, this one I<will> be inserted.  (This is a
+change from the "new_or_existing" method that this replaces.)
+
+The following fields are considered "essential" and I<must> 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<empty> 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 1cf0365..f21932c 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(',', at 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 41ef228..8484df5 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 4464aa5..c49007c 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 3cc1adc..bab8537 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 77f261d..c893f13 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 56c3968..fd1b874 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);
 
 </%init>
diff --git a/httemplate/edit/process/cust_main.cgi b/httemplate/edit/process/cust_main.cgi
index c1f8155..d295ed3 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;
 
 </%once>
 <%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 0cc17d3..14dbda1 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 9983ea2..09398fd 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 )
     });

-----------------------------------------------------------------------

Summary of changes:
 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(-)




More information about the freeside-commits mailing list