fix new duplicate username checking

[freeside.git] / FS / FS / svc_acct.pm

diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm
index 7ea4c10..ce76fe5 100644 (file)
--- a/FS/FS/svc_acct.pm
+++ b/FS/FS/svc_acct.pm
@@ -25,6 +25,8 @@ use FS::svc_domain;
 use FS::raddb;
 use FS::queue;
 use FS::radius_usergroup;
+use FS::export_svc;
+use FS::part_export;
 use FS::Msgcat qw(gettext);
 
 @ISA = qw( FS::svc_Common );
@@ -191,10 +193,13 @@ sub insert {
   $error = $self->check;
   return $error if $error;
 
-  return gettext('username_in_use'). ": ". $self->username
-    if qsearchs( 'svc_acct', { 'username' => $self->username,
-                               'domsvc'   => $self->domsvc,
-                             } );
+  #no, duplicate checking just got a whole lot more complicated
+  #(perhaps keep this check with a config option to turn on?)
+
+  #return gettext('username_in_use'). ": ". $self->username
+  #  if qsearchs( 'svc_acct', { 'username' => $self->username,
+  #                             'domsvc'   => $self->domsvc,
+  #                           } );
 
   if ( $self->svcnum ) {
     my $cust_svc = qsearchs('cust_svc',{'svcnum'=>$self->svcnum});
@@ -206,12 +211,76 @@ sub insert {
     $self->svcpart($cust_svc->svcpart);
   }
 
+  #new duplicate username checking
+
+  my @dup_user = qsearch( 'svc_acct', { 'username' => $self->username } );
+  my @dup_userdomain = qsearchs( 'svc_acct', { 'username' => $self->username,
+                                               'domsvc'   => $self->domsvc } );
+
+  if ( @dup_user || @dup_userdomain ) {
+    my $exports = FS::part_export::export_info('svc_acct');
+    my( %conflict_user_svcpart, %conflict_userdomain_svcpart );
+
+    my $part_svc = qsearchs('part_svc', { 'svcpart' => $self->svcpart } );
+    unless ( $part_svc ) {
+      $dbh->rollback if $oldAutoCommit;
+      return 'unknown svcpart '. $self->svcpart;
+    }
+
+    foreach my $part_export ( $part_svc->part_export ) {
+
+      #this will catch to the same exact export
+      my @svcparts = map { $_->svcpart }
+        qsearch('export_svc', { 'exportnum' => $part_export->exportnum });
+
+      #this will catch to exports w/same exporthost+type ???
+      #my @other_part_export = qsearch('part_export', {
+      #  'machine'    => $part_export->machine,
+      #  'exporttype' => $part_export->exporttype,
+      #} );
+      #foreach my $other_part_export ( @other_part_export ) {
+      #  push @svcparts, map { $_->svcpart }
+      #    qsearch('export_svc', { 'exportnum' => $part_export->exportnum });
+      #}
+
+      my $nodomain = $exports->{$part_export->exporttype}{'nodomain'};
+      if ( $nodomain =~ /^Y/i ) {
+        $conflict_user_svcpart{$_} = $part_export->exportnum
+          foreach @svcparts;
+      } else {
+        $conflict_userdomain_svcpart{$_} = $part_export->exportnum
+          foreach @svcparts;
+      }
+    }
+
+    foreach my $dup_user ( @dup_user ) {
+      my $dup_svcpart = $dup_user->cust_svc->svcpart;
+      if ( exists($conflict_user_svcpart{$dup_svcpart}) ) {
+        return "duplicate username: conflicts with svcnum ". $dup_user->svcnum.
+               " via exportnum ". $conflict_user_svcpart{$dup_svcpart};
+      }
+    }
+
+    foreach my $dup_userdomain ( @dup_userdomain ) {
+      my $dup_svcpart = $dup_userdomain->cust_svc->svcpart;
+      if ( exists($conflict_user_svcpart{$dup_svcpart}) ) {
+        return "duplicate username\@domain: conflicts with svcnum ".
+               $dup_userdomain->svcnum. " via exportnum ".
+               $conflict_user_svcpart{$dup_svcpart};
+      }
+    }
+
+  }
+
+  #see?  i told you it was more complicated
+
   my $part_svc = qsearchs( 'part_svc', { 'svcpart' => $self->svcpart } );
   return "Unknown svcpart" unless $part_svc;
   return "uid in use"
     if $part_svc->part_svc_column('uid')->columnflag ne 'F'
       && qsearchs( 'svc_acct', { 'uid' => $self->uid } )
       && $self->username !~ /^(hyla)?fax$/
+      && $self->username !~ /^toor$/ #FreeBSD
     ;
 
   $error = $self->SUPER::insert;
@@ -234,18 +303,6 @@ sub insert {
     }
   }
 
-  #new-style exports!
-  unless ( $noexport_hack ) {
-    foreach my $part_export ( $self->cust_svc->part_svc->part_export ) {
-      my $error = $part_export->export_insert($self);
-      if ( $error ) {
-        $dbh->rollback if $oldAutoCommit;
-        return "exporting to ". $part_export->exporttype.
-               " (transaction rolled back): $error";
-      }
-    }
-  }
-
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   ''; #no error
 }
@@ -331,26 +388,12 @@ sub delete {
     }
   }
 
-  my $part_svc = $self->cust_svc->part_svc;
-
   my $error = $self->SUPER::delete;
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;
   }
 
-  #new-style exports!
-  unless ( $noexport_hack ) {
-    foreach my $part_export ( $part_svc->part_export ) {
-      my $error = $part_export->export_delete($self);
-      if ( $error ) {
-        $dbh->rollback if $oldAutoCommit;
-        return "exporting to ". $part_export->exporttype.
-               " (transaction rolled back): $error";
-      }
-    }
-  }
-
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   '';
 }
@@ -435,18 +478,6 @@ sub replace {
 
   }
 
-  #new-style exports!
-  unless ( $noexport_hack ) {
-    foreach my $part_export ( $new->cust_svc->part_svc->part_export ) {
-      my $error = $part_export->export_replace($new,$old);
-      if ( $error ) {
-        $dbh->rollback if $oldAutoCommit;
-        return "exporting to ". $part_export->exporttype.
-               " (transaction rolled back): $error";
-      }
-    }
-  }
-
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   ''; #no error
 }
@@ -468,10 +499,11 @@ sub suspend {
          ) {
     $hash{_password} = '*SUSPENDED* '.$hash{_password};
     my $new = new FS::svc_acct ( \%hash );
-    $new->replace($self);
-  } else {
-    ''; #no error (already suspended)
+    my $error = $new->replace($self);
+    return $error if $error;
   }
+
+  $self->SUPER::suspend;
 }
 
 =item unsuspend
@@ -489,10 +521,11 @@ sub unsuspend {
   if ( $hash{_password} =~ /^\*SUSPENDED\* (.*)$/ ) {
     $hash{_password} = $1;
     my $new = new FS::svc_acct ( \%hash );
-    $new->replace($self);
-  } else {
-    ''; #no error (already unsuspended)
+    my $error = $new->replace($self);
+    return $error if $error;
   }
+
+  $self->SUPER::unsuspend;
 }
 
 =item cancel
@@ -675,7 +708,9 @@ sub check {
     $recref->{_password} = '!!';
   } else {
     #return "Illegal password";
-    return gettext('illegal_password'). ": ". $recref->{_password};
+    return gettext('illegal_password'). "$passwordmin-$passwordmax".
+           FS::Msgcat::_gettext('illegal_password_characters').
+           ": ". $recref->{_password};
   }
 
   ''; #no error