cleanup for svc_broadband manual router feature, #14698

3 files changed, 68 insertions, 20 deletions

diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 1112f52..a36d2dc 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -2484,7 +2484,7 @@ sub tables_hashref {
         'routername', 'varchar', '', $char_d, '', '', 
         'svcnum', 'int', 'NULL', '', '', '', 
         'agentnum',   'int', 'NULL', '', '', '', 
-        'auto_addr', 'char', 'NULL', 1, '', '',
+        'manual_addr', 'char', 'NULL', 1, '', '',
       ],
       'primary_key' => 'routernum',
       'unique'      => [],
diff --git a/FS/FS/router.pm b/FS/FS/router.pm
index 99373e5..6fa44b4 100755
--- a/FS/FS/router.pm
+++ b/FS/FS/router.pm
@@ -40,8 +40,9 @@ fields are currently supported:
 
 =item svcnum - svcnum of the owning FS::svc_broadband, if appropriate
 
-=item auto_addr - flag to automatically assign IP addresses to services
-linked to this router ('Y' or null).
+=item manual_addr - set to 'Y' to allow services linked to this router 
+to have any IP address, rather than one in an address block belonging 
+to the router.
 
 =back
 
@@ -86,7 +87,7 @@ sub check {
   my $error =
     $self->ut_numbern('routernum')
     || $self->ut_text('routername')
-    || $self->ut_enum('auto_addr', [ '', 'Y' ])
+    || $self->ut_enum('manual_addr', [ '', 'Y' ])
     || $self->ut_agentnum_acl('agentnum', 'Broadband global configuration')
   ;
   return $error if $error;
@@ -146,7 +147,7 @@ sub addr_block {
 
 sub auto_addr_block {
   my $self = shift;
-  return () if !$self->auto_addr;
+  return () if $self->manual_addr;
   return qsearch('addr_block', { routernum => $self->routernum,
                                  manual_flag => '' });
 }
diff --git a/FS/FS/svc_broadband.pm b/FS/FS/svc_broadband.pm
index 1096200..212a4bf 100755
--- a/FS/FS/svc_broadband.pm
+++ b/FS/FS/svc_broadband.pm
@@ -135,7 +135,7 @@ sub table_info {
 
 sub table { 'svc_broadband'; }
 
-sub table_dupcheck_fields { ( 'mac_addr' ); }
+sub table_dupcheck_fields { ( 'ip_addr', 'mac_addr' ); }
 
 =item search HASHREF
 
@@ -406,7 +406,13 @@ sub check {
   }
   my $agentnum = $cust_pkg->cust_main->agentnum if $cust_pkg;
 
-  if ($self->routernum) {
+  if ( $conf->exists('auto_router') and $self->ip_addr and !$self->routernum ) {
+    # assign_router is guaranteed to provide a router that's legal
+    # for this agent and svcpart
+    my $error = $self->_check_ip_addr || $self->assign_router;
+    return $error if $error;
+  }
+  elsif ($self->routernum) {
     return "Router ".$self->routernum." does not provide this service"
       unless qsearchs('part_svc_router', { 
         svcpart => $svcpart,
@@ -417,16 +423,19 @@ sub check {
     return "Router ".$self->routernum." does not serve this customer"
       if $router->agentnum and $router->agentnum != $agentnum;
 
-    if ( $router->auto_addr ) {
+    if ( $router->manual_addr ) {
+      $self->blocknum('');
+    }
+    else {
       my $addr_block = $self->addr_block;
       unless ( $addr_block and $addr_block->manual_flag ) {
         my $error = $self->assign_ip_addr;
         return $error if $error;
       }
     }
-    else {
-      $self->blocknum('');
-    }
+ 
+    my $error = $self->_check_ip_addr;
+    return $error if $error;
   } # if $self->routernum
 
   if ( $cust_pkg && ! $self->latitude && ! $self->longitude ) {
@@ -440,15 +449,12 @@ sub check {
     }
   }
 
-  $error = $self->_check_ip_addr;
-  return $error if $error;
-
   $self->SUPER::check;
 }
 
 =item assign_ip_addr
 
-Assign an address block matching the selected router, and the selected block
+Assign an IP address matching the selected router, and the selected block
 if there is one.
 
 =cut
@@ -469,6 +475,7 @@ sub assign_ip_addr {
   else { 
     return '';
   }
+#warn "assigning ip address in blocks\n".join("\n",map{$_->cidr} @blocks)."\n";
 
   foreach my $block ( @blocks ) {
     if ( $self->ip_addr and $block->NetAddr->contains($self->NetAddr) ) {
@@ -487,6 +494,29 @@ sub assign_ip_addr {
   }
 }
 
+=item assign_router
+
+Assign an address block and router matching the selected IP address.
+Does nothing if IP address is null.
+
+=cut
+
+sub assign_router {
+  my $self = shift;
+  return '' if !$self->ip_addr;
+  #warn "assigning router/block for ".$self->ip_addr."\n";
+  foreach my $router ($self->allowed_routers) {
+    foreach my $block ($router->addr_block) {
+      if ( $block->NetAddr->contains($self->NetAddr) ) {
+        $self->blocknum($block->blocknum);
+        $self->routernum($block->routernum);
+        return '';
+      }
+    }
+  }
+  return $self->ip_addr.' is not in an allowed block.';
+}
+
 sub _check_ip_addr {
   my $self = shift;
 
@@ -494,6 +524,9 @@ sub _check_ip_addr {
     return '' if $conf->exists('svc_broadband-allow_null_ip_addr'); 
     return 'IP address required';
   }
+  else {
+    return 'Cannot parse address: '.$self->ip_addr unless $self->NetAddr;
+  }
 #  if (my $dup = qsearchs('svc_broadband', {
 #        ip_addr => $self->ip_addr,
 #        svcnum  => {op=>'!=', value => $self->svcnum}
@@ -506,10 +539,17 @@ sub _check_ip_addr {
 sub _check_duplicate {
   my $self = shift;
 
-  return "MAC already in use"
-    if ( $self->mac_addr &&
-         scalar( qsearch( 'svc_broadband', { 'mac_addr', $self->mac_addr } ) )
-       );
+  $self->lock_table;
+
+  my @dup;
+  @dup = $self->find_duplicates('global', 'ip_addr');
+  if ( @dup ) {
+    return "IP address in use (svcnum ".$dup[0]->svcnum.")";
+  }
+  @dup = $self->find_duplicates('global', 'mac_addr');
+  if ( @dup ) {
+    return "MAC address in use (svcnum ".$dup[0]->svcnum.")";
+  }
 
   '';
 }
@@ -558,8 +598,15 @@ Returns a list of allowed FS::router objects.
 sub allowed_routers {
   my $self = shift;
   my $svcpart = $self->svcnum ? $self->cust_svc->svcpart : $self->svcpart;
-  map { $_->router } qsearch('part_svc_router', 
+  my @r = map { $_->router } qsearch('part_svc_router', 
     { svcpart => $self->cust_svc->svcpart });
+  if ( $self->cust_main ) {
+    my $agentnum = $self->cust_main->agentnum;
+    return grep { !$_->agentnum or $_->agentnum == $agentnum } @r;
+  }
+  else {
+    return @r;
+  }
 }
 
 =back