cleanup for svc_broadband manual router feature, #14698

9 files changed, 112 insertions, 92 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
diff --git a/httemplate/browse/router.cgi b/httemplate/browse/router.cgi
index 21047d7..ef8ad31 100644
--- a/httemplate/browse/router.cgi
+++ b/httemplate/browse/router.cgi
@@ -17,7 +17,7 @@
                                                                shift->addr_block
                                                  );
                                            },
-                                       sub { shift->auto_addr ? 'Automatic' : 'Manual' },
+                                       sub { shift->manual_addr ? 'Manual' : 'Automatic' },
                                        sub { 'Delete' },
                                      ],
                 'links'           => [ [ "${p2}edit/router.cgi?", 'routernum' ],
diff --git a/httemplate/edit/process/svc_broadband.cgi b/httemplate/edit/process/svc_broadband.cgi
index 4184f5f..31def25 100644
--- a/httemplate/edit/process/svc_broadband.cgi
+++ b/httemplate/edit/process/svc_broadband.cgi
@@ -13,6 +13,9 @@ die "access denied"
 
 sub precheck {
   my $cgi = shift;
+  if ( !defined($cgi->param('ip_addr')) ) {
+    $cgi->param('ip_addr', $cgi->param('prev_ip_addr') || '');
+  }
   $cgi->param("usergroup", [ $cgi->param('usergroup') ]);
   ''
 }
diff --git a/httemplate/edit/router.cgi b/httemplate/edit/router.cgi
index 6672d5d..fdcd7b3 100755
--- a/httemplate/edit/router.cgi
+++ b/httemplate/edit/router.cgi
@@ -7,13 +7,13 @@
                         'routername' => 'Name',
                         'svc_part'   => 'Service',
                         'agentnum'   => 'Agent',
-                        'auto_addr'  => 'Assign IP addresses automatically',
+                        'manual_addr'  => 'Assign IP addresses manually',
                       },
      'fields'      => [
                         { 'field'=>'routername', 'type'=>'text', 'size'=>32 },
                         { 'field'=>'agentnum',   'type'=>'select-agent' },
                         { 'field'=>'svcnum',     'type'=>'hidden' },
-                        { 'field'=>'auto_addr','type'=>'checkbox','value'=>'Y'},
+                        { 'field'=>'manual_addr','type'=>'checkbox','value'=>'Y'},
                       ],
      'error_callback' => $callback,
      'edit_callback'  => $callback,
diff --git a/httemplate/edit/svc_broadband.cgi b/httemplate/edit/svc_broadband.cgi
index 8fccb1f..b07c725 100644
--- a/httemplate/edit/svc_broadband.cgi
+++ b/httemplate/edit/svc_broadband.cgi
@@ -102,9 +102,9 @@ END
 my @fields = (
   qw( description speed_down speed_up ),
   { field=>'sectornum', type=>'select-tower_sector', },
-  { field=>'routernum', type=>'select-router_block_ip', },
-  qw( mac_addr latitude longitude altitude vlan_profile 
-      performance_profile authkey plan_id ),
+  { field=>'routernum', type=>'select-router_block_ip' },
+    qw( mac_addr latitude longitude altitude vlan_profile 
+    performance_profile authkey plan_id )
 );
 
 if ( $conf->exists('svc_broadband-radius') ) {
@@ -115,8 +115,6 @@ if ( $conf->exists('svc_broadband-radius') ) {
   }
 }
 
-my $fixedblock = '';
-
 my $part_svc;
 
 my $svc_edit_callback = sub {
@@ -124,8 +122,6 @@ my $svc_edit_callback = sub {
 
   $part_svc = $part_svc_x; #for field_callback to use
 
-  $opt->{'labels'}{'block_label'} = 'Block';
-
   my ($nas_export) = $part_svc->part_export('broadband_nas');
   #can we assume there's only one of these per part_svc?
   if ( $nas_export ) {
@@ -173,60 +169,13 @@ my $field_callback = sub {
                             ? 'fixed'
                             : 'hidden';
     $fieldref->{'value'} = $columndef->columnvalue;
-    $fixedblock = $fieldref->{value}
-      if $fieldref->{field} eq 'blocknum';
-
+    
     if ( $fieldref->{field} eq 'usergroup' ) {
       $fieldref->{'formatted_value'} = 
         [ $object->radius_groups('long_description') ];
     }
   }
 
-  if ($object->svcnum) { 
-
-    $fieldref->{type} = 'hidden'
-      if $fieldref->{field} eq 'blocknum';
-      
-    $fieldref->{value} = $object->addr_block->label
-      if $fieldref->{field} eq 'block_label' && $object->addr_block;
-
-  } else { 
-
-    if ($fieldref->{field} eq 'block_label') {
-      if ($fixedblock && $object->addr_block) {
-        $object->blocknum($fixedblock);
-        $fieldref->{value} = $object->addr_block->label;
-      }else{
-        $fieldref->{type} = 'hidden';
-      }
-    }
-
-    if ($fieldref->{field} eq 'blocknum') {
-      if ( $fixedblock or $conf->exists('auto_router') ) {
-        $fieldref->{type} = 'hidden';
-        $fieldref->{value} = $fixedblock;
-        return;
-      }
-
-      my $cust_pkg = qsearchs( 'cust_pkg', {pkgnum => $cgi->param('pkgnum')} );
-      die "No cust_pkg entry!" unless $cust_pkg;
-
-      $object->svcpart($part_svc->svcpart);
-      my @addr_block =
-        grep {  ! $_->agentnum
-               || $cust_pkg->cust_main->agentnum == $_->agentnum
-               && $FS::CurrentUser::CurrentUser->agentnum($_->agentnum)
-             }
-        map { $_->addr_block } $object->allowed_routers;
-      my @options = map { $_->blocknum } 
-                    sort { $a->label cmp $b->label } @addr_block;
-      my %option_labels = map { ( $_->blocknum => $_->label ) } @addr_block;
-      $fieldref->{type}    = 'select';
-      $fieldref->{options} = \@options;
-      $fieldref->{labels}  = \%option_labels;
-    }
-
-  }
 }; 
 
 </%init>
diff --git a/httemplate/elements/tr-select-router_block_ip.html b/httemplate/elements/tr-select-router_block_ip.html
index 45d1dac..ed8fe81 100644
--- a/httemplate/elements/tr-select-router_block_ip.html
+++ b/httemplate/elements/tr-select-router_block_ip.html
@@ -1,14 +1,22 @@
 <script type="text/javascript">
-var auto_addr_routernum = <% encode_json(\%auto_addr_routernum) %>;
-function hide_if_auto_addr(obj, i) {
+var manual_addr_routernum = <% encode_json(\%manual_addr_routernum) %>;
+var ip_addr_curr_value = <% $opt{'ip_addr'} |js_string %>;
+function lock_ip_addr(obj, i) {
   var routernum = obj.value;
   var select_blocknum = document.getElementsByName('blocknum')[0];
-  var label_auto_addr = document.getElementById('label_auto_addr');
   var input_ip_addr = document.getElementById('input_ip_addr');
-  var auto = ( auto_addr_routernum[routernum] == 'Y' );
-  select_blocknum.style.display = auto ? '' : 'none';
-  label_auto_addr.style.display = auto ? '' : 'none';
-  input_ip_addr.style.display = !auto ? '' : 'none';
+  if ( manual_addr_routernum[routernum] == 'Y' ) {
+%# enable ip_addr, default it to its previous value, and hide block selection
+    select_blocknum.style.display = 'none';
+    input_ip_addr.value = ip_addr_curr_value;
+    input_ip_addr.disabled = false;
+  }
+  else {
+%# the reverse
+    select_blocknum.style.display = '';
+    input_ip_addr.disabled = true;
+    input_ip_addr.value = '(automatic)';
+  }
 }
 </script>
 <& /elements/tr-td-label.html, label => ($opt{'label'} || 'Router') &>
@@ -19,7 +27,7 @@ function hide_if_auto_addr(obj, i) {
     records   => \@routers,
     name_col  => 'routername',
     value_col => 'routernum',
-    onchange  => 'hide_if_auto_addr',
+    onchange  => 'lock_ip_addr',
     curr_value=> $opt{'routernum'},
   },
   {
@@ -44,17 +52,18 @@ function hide_if_auto_addr(obj, i) {
 % }
 % else {
   <input type="text" id="input_ip_addr" name="ip_addr" 
-    style="display:none" value="<% $opt{'ip_addr'} |h%>">
+  value="<% $opt{'ip_addr'} |h%>">
 % }
-  <span id="label_auto_addr"><% $opt{'ip_addr'} || '' %> 
-  <i>(automatic)</i></span>
 </td> </tr>
+<input type="hidden" name="prev_ip_addr" value="<% $opt{'ip_addr'} |h%>">
 <script type="text/javascript">
-hide_if_auto_addr(document.getElementsByName('routernum')[0],0);
+lock_ip_addr(document.getElementsByName('routernum')[0],0);
 </script>
 <%init>
+
 my %opt = @_;
 my @routers;
+my $conf = FS::Conf->new;
 
 my $svc_x = $opt{'object'};
 if ( $svc_x ) {
@@ -91,5 +100,17 @@ else {
   @routers = qsearch('router', {});
 }
 
-my %auto_addr_routernum = map { $_->routernum, $_->auto_addr } @routers;
+my %manual_addr_routernum = map { $_->routernum, $_->manual_addr } @routers;
+
+if ( $conf->exists('auto_router') ) {
+  # Then show an "(automatic)" router, with no blocks.  manual_addr is on
+  # so that the ip_addr field will be unlocked.
+  unshift @routers, FS::router->new({
+      'routernum'   => '',
+      'routername'  => '(automatic)',
+      'manual_addr' => 'Y',
+  });
+  $manual_addr_routernum{''} = 'Y';
+}
+
 </%init>
diff --git a/httemplate/view/svc_broadband.cgi b/httemplate/view/svc_broadband.cgi
index 961374e..af2c575 100644
--- a/httemplate/view/svc_broadband.cgi
+++ b/httemplate/view/svc_broadband.cgi
@@ -17,7 +17,6 @@ my %labels = map { $_ => ( ref($fields->{$_})
 #my %labels = ();
 
 $labels{'description'} = emt('Description');
-$labels{'router'} = emt('Router');
 $labels{'speed_down'} = emt('Download Speed');
 $labels{'speed_up'} = emt('Upload Speed');
 $labels{'ip_addr'} = emt('IP Address');