fix problems using inventory for UID (and other fields controlled by check in svc_acct and also svc_www, svc_domain and svc_forward), RT#6366

5 files changed, 61 insertions, 90 deletions

diff --git a/FS/FS/svc_Common.pm b/FS/FS/svc_Common.pm
index 8fd5d0d..a67504a 100644
--- a/FS/FS/svc_Common.pm
+++ b/FS/FS/svc_Common.pm
@@ -255,9 +255,11 @@ sub insert {
     $self->svcpart($cust_svc->svcpart);
   }
 
-  my $error =    $self->set_auto_inventory
+  my $error =    $self->preinsert_hook_first
+              || $self->set_auto_inventory
               || $self->check
               || $self->_check_duplicate
+              || $self->preinsert_hook
               || $self->SUPER::insert;
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
@@ -324,7 +326,9 @@ sub insert {
 }
 
 #fallbacks
+sub preinsert_hook_first { ''; }
 sub _check_duplcate { ''; }
+sub preinsert_hook { ''; }
 sub table_dupcheck_fields { (); }
 
 =item delete [ , OPTION => VALUE ... ]
diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm
index 1b12654..32dba25 100644
--- a/FS/FS/svc_acct.pm
+++ b/FS/FS/svc_acct.pm
@@ -528,42 +528,8 @@ sub insert {
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
 
-  my $error = $self->check;
-  return $error if $error;
-
-  if ( $self->svcnum && qsearchs('cust_svc',{'svcnum'=>$self->svcnum}) ) {
-    my $cust_svc = qsearchs('cust_svc',{'svcnum'=>$self->svcnum});
-    unless ( $cust_svc ) {
-      $dbh->rollback if $oldAutoCommit;
-      return "no cust_svc record found for svcnum ". $self->svcnum;
-    }
-    $self->pkgnum($cust_svc->pkgnum);
-    $self->svcpart($cust_svc->svcpart);
-  }
-
-  # set usage fields and thresholds if unset but set in a package def
-  if ( $self->pkgnum ) {
-    my $cust_pkg = qsearchs( 'cust_pkg', { 'pkgnum' => $self->pkgnum } );
-    my $part_pkg = $cust_pkg->part_pkg if $cust_pkg;
-    if ( $part_pkg && $part_pkg->can('usage_valuehash') ) {
-
-      my %values = $part_pkg->usage_valuehash;
-      my $multiplier = $conf->exists('svc_acct-usage_threshold') 
-                         ? 1 - $conf->config('svc_acct-usage_threshold')/100
-                         : 0.20; #doesn't matter
-
-      foreach ( keys %values ) {
-        next if $self->getfield($_);
-        $self->setfield( $_, $values{$_} );
-        $self->setfield( $_. '_threshold', int( $values{$_} * $multiplier ) )
-          if $conf->exists('svc_acct-usage_threshold');
-      }
-
-    }
-  }
-
   my @jobnums;
-  $error = $self->SUPER::insert(
+  my $error = $self->SUPER::insert(
     'jobnums'       => \@jobnums,
     'child_objects' => $self->child_objects,
     %options,
@@ -692,6 +658,31 @@ sub insert {
   ''; #no error
 }
 
+# set usage fields and thresholds if unset but set in a package def
+sub preinsert_hook_first {
+  my $self = shift;
+
+  return '' unless $self->pkgnum;
+
+  my $cust_pkg = qsearchs( 'cust_pkg', { 'pkgnum' => $self->pkgnum } );
+  my $part_pkg = $cust_pkg->part_pkg if $cust_pkg;
+  return '' unless $part_pkg && $part_pkg->can('usage_valuehash');
+
+  my %values = $part_pkg->usage_valuehash;
+  my $multiplier = $conf->exists('svc_acct-usage_threshold') 
+                     ? 1 - $conf->config('svc_acct-usage_threshold')/100
+                     : 0.20; #doesn't matter
+
+  foreach ( keys %values ) {
+    next if $self->getfield($_);
+    $self->setfield( $_, $values{$_} );
+    $self->setfield( $_. '_threshold', int( $values{$_} * $multiplier ) )
+      if $conf->exists('svc_acct-usage_threshold');
+  }
+
+  ''; #no error
+}
+
 =item delete
 
 Deletes this account from the database.  If there is an error, returns the
diff --git a/FS/FS/svc_domain.pm b/FS/FS/svc_domain.pm
index 3311ac5..8ca30c2 100644
--- a/FS/FS/svc_domain.pm
+++ b/FS/FS/svc_domain.pm
@@ -181,13 +181,6 @@ sub insert {
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
 
-  $error = $self->check;
-  return $error if $error;
-
-  return "Domain in use (here)"
-    if qsearchs( 'svc_domain', { 'domain' => $self->domain } );
-
-
   $error = $self->SUPER::insert(@_);
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
@@ -392,6 +385,18 @@ sub check {
 
 }
 
+sub _check_duplicate {
+  my $self = shift;
+
+  $self->lock_table;
+
+  if ( qsearchs( 'svc_domain', { 'domain' => $self->domain } ) ) {
+    return "Domain in use (here)";
+  } else {
+    return '';
+  }
+}
+
 =item domain_record
 
 =cut
diff --git a/FS/FS/svc_forward.pm b/FS/FS/svc_forward.pm
index 3250f8a..9e27a32 100644
--- a/FS/FS/svc_forward.pm
+++ b/FS/FS/svc_forward.pm
@@ -158,9 +158,6 @@ sub insert {
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
 
-  $error = $self->check;
-  return $error if $error;
-
   $error = $self->SUPER::insert(@_);
   if ($error) {
     $dbh->rollback if $oldAutoCommit;
diff --git a/FS/FS/svc_www.pm b/FS/FS/svc_www.pm
index 53225bb..7e02d81 100644
--- a/FS/FS/svc_www.pm
+++ b/FS/FS/svc_www.pm
@@ -113,57 +113,31 @@ If I<depend_jobnum> is set (to a scalar jobnum or an array reference of
 jobnums), all provisioning jobs will have a dependancy on the supplied
 jobnum(s) (they will not run until the specific job(s) complete(s)).
 
-
 =cut
 
-sub insert {
+sub preinsert_hook {
   my $self = shift;
 
-  my $error = $self->check;
-  return $error if $error;
-
-  local $SIG{HUP} = 'IGNORE';
-  local $SIG{INT} = 'IGNORE';
-  local $SIG{QUIT} = 'IGNORE';
-  local $SIG{TERM} = 'IGNORE';
-  local $SIG{TSTP} = 'IGNORE';
-  local $SIG{PIPE} = 'IGNORE';
-
-  my $oldAutoCommit = $FS::UID::AutoCommit;
-  local $FS::UID::AutoCommit = 0;
-  my $dbh = dbh;
-
-  #if ( $self->recnum =~ /^([\w\-]+|\@)\.(([\w\.\-]+\.)+\w+)$/ ) {
-  if ( $self->recnum =~ /^([\w\-]+|\@)\.(\d+)$/ ) {
-    my( $reczone, $domain_svcnum ) = ( $1, $2 );
-    unless ( $apacheip ) {
-      $dbh->rollback if $oldAutoCommit;
-      return "Configuration option apacheip not set; can't autocreate A record";
-             #"for $reczone". $svc_domain->domain;
-    }
-    my $domain_record = new FS::domain_record {
-      'svcnum'  => $domain_svcnum,
-      'reczone' => $reczone,
-      'recaf'   => 'IN',
-      'rectype' => 'A',
-      'recdata' => $apacheip,
-    };
-    $error = $domain_record->insert;
-    if ( $error ) {
-      $dbh->rollback if $oldAutoCommit;
-      return $error;
-    }
-    $self->recnum($domain_record->recnum);
-  }
+  #return '' unless $self->recnum =~ /^([\w\-]+|\@)\.(([\w\.\-]+\.)+\w+)$/;
+  return '' unless $self->recnum =~ /^([\w\-]+|\@)\.(\d+)$/;
 
-  $error = $self->SUPER::insert(@_);
-  if ( $error ) {
-    $dbh->rollback if $oldAutoCommit;
-    return $error;
+  my( $reczone, $domain_svcnum ) = ( $1, $2 );
+  unless ( $apacheip ) {
+    return "Configuration option apacheip not set; can't autocreate A record";
+           #"for $reczone". $svc_domain->domain;
   }
+  my $domain_record = new FS::domain_record {
+    'svcnum'  => $domain_svcnum,
+    'reczone' => $reczone,
+    'recaf'   => 'IN',
+    'rectype' => 'A',
+    'recdata' => $apacheip,
+  };
+  my $error = $domain_record->insert;
+  return $error if $error;
 
-  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
-  '';
+  $self->recnum($domain_record->recnum);
+  return '';
 }
 
 =item delete