slightly better error reporting for DID selector, from #39914

12 files changed, 171 insertions, 42 deletions

diff --git a/FS/FS/part_export.pm b/FS/FS/part_export.pm
index 182f476..daeff43 100644
--- a/FS/FS/part_export.pm
+++ b/FS/FS/part_export.pm
@@ -717,6 +717,8 @@ will return an array of actual DID numbers.
 Passing 'tollfree' with a true value will override the whole hierarchy and
 return an array of tollfree numbers.
 
+C<get_dids> methods should report errors via die().
+
 =cut
 
 # no stub; can('get_dids') should return false by default
diff --git a/FS/FS/part_export/bandwidth_com.pm b/FS/FS/part_export/bandwidth_com.pm
index 6c69fe3..4fbc2de 100644
--- a/FS/FS/part_export/bandwidth_com.pm
+++ b/FS/FS/part_export/bandwidth_com.pm
@@ -151,43 +151,46 @@ sub can_get_dids { 1 }
 sub get_dids_npa_select { 1 }
 
 sub get_dids {
-  local $SIG{__DIE__};
 
   my $self = shift;
   my %opt = @_;
 
   my ($exportnum) = $self->exportnum =~ /^(\d+)$/;
 
-  return [] if $opt{'tollfree'}; # we'll come back to this
+  try {
+    return [] if $opt{'tollfree'}; # we'll come back to this
 
-  my ($state, $npa, $nxx) = @opt{'state', 'areacode', 'exchange'};
+    my ($state, $npa, $nxx) = @opt{'state', 'areacode', 'exchange'};
 
-  if ( $nxx ) {
+    if ( $nxx ) {
 
-    die "areacode required\n" unless $npa;
-    my $limit = $self->option('num_dids') || 20;
-    my $result = $self->api_get('availableNumbers', [
-        'npaNxx'    => $npa.$nxx,
-        'quantity'  => $limit,
-        'LCA'       => 'false',
-        # find only those that match the NPA-NXX, not those thought to be in
-        # the same local calling area. though that might be useful.
-    ]);
-    return [ $result->findnodes('//TelephoneNumber')->to_literal_list ];
+      die "areacode required\n" unless $npa;
+      my $limit = $self->option('num_dids') || 20;
+      my $result = $self->api_get('availableNumbers', [
+          'npaNxx'    => $npa.$nxx,
+          'quantity'  => $limit,
+          'LCA'       => 'false',
+          # find only those that match the NPA-NXX, not those thought to be in
+          # the same local calling area. though that might be useful.
+      ]);
+      return [ $result->findnodes('//TelephoneNumber')->to_literal_list ];
 
-  } elsif ( $npa ) {
+    } elsif ( $npa ) {
 
-    return $self->npanxx_cache($npa);
+      return $self->npanxx_cache($npa);
 
-  } elsif ( $state ) {
+    } elsif ( $state ) {
 
-    return $self->npa_cache($state);
+      return $self->npa_cache($state);
 
-  } else { # something's wrong
+    } else { # something's wrong
 
-    warn "get_dids called with no arguments";
-    return [];
+      warn "get_dids called with no arguments";
+      return [];
 
+    }
+  } catch {
+    die "$me $_\n";
   }
 
 }
diff --git a/FS/FS/part_export/test.pm b/FS/FS/part_export/test.pm
index 126897c..392fc4f 100644
--- a/FS/FS/part_export/test.pm
+++ b/FS/FS/part_export/test.pm
@@ -18,6 +18,7 @@ tie %options, 'Tie::IxHash',
   'replace' => { label    => 'Replace',type => 'checkbox', default => 1, },
   'suspend' => { label    => 'Suspend',type => 'checkbox', default => 1, },
   'unsuspend'=>{ label => 'Unsuspend', type => 'checkbox', default => 1, },
+  'get_dids_npa_select' => { label => 'DIDs by NPA', type => 'checkbox' },
 ;
 
 %info = (
@@ -31,6 +32,8 @@ or always dies, according to the "Result" option.  It does nothing else; the
 purpose is purely to simulate success or failure within an export module.</P>
 <P>The checkbox options can be used to turn the export off for certain
 actions, if this is needed.</P>
+<P>This export will produce a small set of DIDs, in either Alabama (if the
+"DIDs by NPA" option is on) or California (if not).</P>
 END
 );
 
@@ -72,4 +75,79 @@ sub run {
   }
 }
 
+sub can_get_dids { 1 }
+
+sub get_dids_npa_select {
+  my $self = shift;
+  $self->option('get_dids_npa_select') ? 1 : 0;
+}
+
+# we don't yet have tollfree
+
+my $dids_by_npa = {
+  'states' => [ 'AK', 'AL' ],
+  # states
+  'AK' => [],
+  'AL' => [ '205', '998', '999' ],
+  # NPAs
+  '205' => [ 'ALABASTER (205-555-XXXX)', # an NPA-NXX
+             'EMPTY (205-998-XXXX)',
+             'INVALID (205-999-XXXX)',
+             'ALBERTVILLE, AL', # a ratecenter
+           ],
+  '998' => [],
+  '999' => undef,
+  # exchanges
+  '205555' => 
+    [
+      '2055550101',
+      '2055550102'
+    ],
+  '205998' => [],
+  '205999' => undef,
+  # ratecenters
+  'ALBERTVILLE' => [
+    '2055550111',
+    '2055550112',
+  ],
+},
+
+my $dids_by_region = {
+  'states' => [ 'CA', 'CO' ],
+  'CA' => [ 'CALIFORNIA',
+            'EMPTY',
+            'INVALID'
+          ],
+  'CO' => [],
+  # regions
+  'CALIFORNIA' => [
+    '4155550200',
+    '4155550201',
+  ],
+  'EMPTY' => [],
+  'INVALID' => undef,
+};
+
+sub get_dids {
+  my $self = shift;
+  my %opt = @_;
+  my $data = $self->get_dids_npa_select ? $dids_by_npa : $dids_by_region;
+
+  my $key;
+  if ( $opt{'exchange'} ) {
+    $key = $opt{'areacode'} . $opt{'exchange'};
+  } else {
+    $key =    $opt{'ratecenter'}
+          ||  $opt{'areacode'}
+          ||  $opt{'region'}
+          ||  $opt{'state'}
+          ||  'states';
+  }
+  if ( defined $data->{ $key } ) {
+    return $data->{ $key };
+  } else {
+    die "[test] '$key' is invalid\n";
+  }
+}
+
 1;
diff --git a/httemplate/REST/1.0/phone_avail b/httemplate/REST/1.0/phone_avail
index ef9d3e7..c6638f4 100644
--- a/httemplate/REST/1.0/phone_avail
+++ b/httemplate/REST/1.0/phone_avail
@@ -17,7 +17,8 @@ if ( scalar(@exports) > 1 ) {
   die "no DID providing export attached to svcpart $svcpart";
 }
 my $export = $exports[0];
-    
+
+# XXX no error handling
 my $phonenums = $export->get_dids( map { $_ => scalar($cgi->param($_)) }
                                      qw( ratecenter state areacode exchange )
                                  );
diff --git a/httemplate/elements/select-areacode.html b/httemplate/elements/select-areacode.html
index f0f56d5..612f03a 100644
--- a/httemplate/elements/select-areacode.html
+++ b/httemplate/elements/select-areacode.html
@@ -28,6 +28,7 @@
 
     function <% $opt{'prefix'} %>update_areacodes(areacodes) {
 
+      var reply = JSON.parse(areacodes);
       // blank the current areacode
       for ( var i = what.form.<% $opt{'prefix'} %>areacode.length; i >= 0; i-- )
           what.form.<% $opt{'prefix'} %>areacode.options[i] = null;
@@ -47,7 +48,7 @@
 %     }
 
       // add the new areacodes
-      var areacodeArray = eval('(' + areacodes + ')' );
+      var areacodeArray = reply.areacodes;
       for ( var s = 0; s < areacodeArray.length; s++ ) {
           var areacodeLabel = areacodeArray[s];
           if ( areacodeLabel == "" )
@@ -62,6 +63,11 @@
       } else {
         var areacodeerror = document.getElementById('<% $opt{'prefix'} %>areacodeerror');
         areacodeerror.style.display = 'inline';
+        if ( reply.error ) {
+          areacodeerror.textContent = reply.error;
+        } else {
+          areacodeerror.textContent = 'Select a different state';
+        }
       }
 
       //run the callback
@@ -78,7 +84,7 @@
 
 <DIV ID="areacodewait" STYLE="display:none"><IMG SRC="<%$fsurl%>images/wait-orange.gif"> <B>Finding area codes</B></DIV>
 
-<DIV ID="areacodeerror" STYLE="display:none"><IMG SRC="<%$fsurl%>images/cross.png"> <B>Select a different state</B></DIV>
+<DIV ID="areacodeerror" STYLE="display:none; font-weight: bold"><IMG SRC="<%$fsurl%>images/cross.png"></DIV>
 
 <SELECT NAME="<% $opt{'prefix'} %>areacode" onChange="<% $opt{'prefix'} %>areacode_changed(this); <% $opt{'onchange'} %>" <% $opt{'disabled'} %>>
   <OPTION VALUE="">Select area code</OPTION>
diff --git a/httemplate/elements/select-exchange.html b/httemplate/elements/select-exchange.html
index b967709..33def31 100644
--- a/httemplate/elements/select-exchange.html
+++ b/httemplate/elements/select-exchange.html
@@ -27,6 +27,7 @@
 
     function <% $opt{'prefix'} %>update_exchanges(exchanges) {
 
+      var reply = JSON.parse(exchanges);
       // blank the current exchange
       for ( var i = what.form.<% $opt{'prefix'} %>exchange.length; i >= 0; i-- )
           what.form.<% $opt{'prefix'} %>exchange.options[i] = null;
@@ -42,7 +43,7 @@
 %     }
 
       // add the new exchanges
-      var exchangeArray = eval('(' + exchanges + ')' );
+      var exchangeArray = reply.exchanges;
       for ( var s = 0; s < exchangeArray.length; s++ ) {
           var exchangeLabel = exchangeArray[s];
           if ( exchangeLabel == "" )
@@ -57,6 +58,12 @@
       } else {
         var exchangeerror = document.getElementById('<% $opt{'prefix'} %>exchangeerror');
         exchangeerror.style.display = 'inline';
+        if ( reply.error ) {
+          exchangeerror.textContent = reply.error;
+        } else {
+          exchangeerror.textContent = 'Select a different area code';
+        }
+
       }
 
       //run the callback
@@ -73,7 +80,7 @@
 
 <DIV ID="exchangewait" STYLE="display:none"><IMG SRC="<%$fsurl%>images/wait-orange.gif"> <B>Finding cities / exchanges</B></DIV>
 
-<DIV ID="exchangeerror" STYLE="display:none"><IMG SRC="<%$fsurl%>images/cross.png"> <B>Select a different area code</B></DIV>
+<DIV ID="exchangeerror" STYLE="display:none; font-weight: bold"><IMG SRC="<%$fsurl%>images/cross.png"></DIV>
 
 <SELECT NAME="<% $opt{'prefix'} %>exchange" onChange="<% $opt{'prefix'} %>exchange_changed(this); <% $opt{'onchange'} %>" <% $opt{'disabled'} %>>
   <OPTION VALUE="">Select city / exchange</OPTION>
diff --git a/httemplate/elements/select-phonenum.html b/httemplate/elements/select-phonenum.html
index 118fe49..dd1b847 100644
--- a/httemplate/elements/select-phonenum.html
+++ b/httemplate/elements/select-phonenum.html
@@ -56,6 +56,7 @@ passing the exchange (or region) and
 
     function <% $opt{'prefix'} %>update_phonenums(phonenums) {
 
+      var reply = JSON.parse(phonenums);
       // blank the current phonenum
       for ( var i = what.form.<% $opt{'prefix'} %>phonenum.length; i >= 0; i-- )
           what.form.<% $opt{'prefix'} %>phonenum.options[i] = null;
@@ -67,7 +68,7 @@ passing the exchange (or region) and
 %     }
 
       // add the new phonenums
-      var phonenumArray = eval('(' + phonenums + ')' );
+      var phonenumArray = reply.phonenums;
       for ( var s = 0; s < phonenumArray.length; s++ ) {
           var phonenumLabel = phonenumArray[s];
           if ( phonenumLabel == "" )
@@ -86,6 +87,11 @@ passing the exchange (or region) and
       } else {
         var phonenumerror = document.getElementById('<% $opt{'prefix'} %>phonenumerror');
         phonenumerror.style.display = 'inline';
+        if ( reply.error ) {
+          phonenumerror.textContent = reply.error;
+        } else {
+          phonenumerror.textContent = 'Select a different <% $opt{'region'} ? 'region' : 'city/exchange' %>';
+        }
       }
 
       //run the callback
@@ -157,7 +163,7 @@ passing the exchange (or region) and
 % unless ( $opt{'tollfree'} ) {
 <DIV ID="phonenumwait" STYLE="display:none"><IMG SRC="<%$fsurl%>images/wait-orange.gif"> <B>Finding phone numbers</B></DIV>
 
-<DIV ID="phonenumerror" STYLE="display:none"><IMG SRC="<%$fsurl%>images/cross.png"> <B>Select a different <% $opt{'region'} ? 'region' : 'city/exchange' %></B></DIV>
+<DIV ID="phonenumerror" STYLE="display:none; font-weight: bold"><IMG SRC="<%$fsurl%>images/cross.png"></DIV>
 % }
 
 <SELECT <% $opt{multiple} ? 'MULTIPLE SIZE=25' : '' %>
diff --git a/httemplate/elements/select-region.html b/httemplate/elements/select-region.html
index 7ed9592..46c37c9 100644
--- a/httemplate/elements/select-region.html
+++ b/httemplate/elements/select-region.html
@@ -27,6 +27,7 @@
 
     function <% $opt{'prefix'} %>update_regions(regions) {
 
+      var reply = JSON.parse(regions);
       // blank the current region
       for ( var i = what.form.<% $opt{'prefix'} %>region.length; i >= 0; i-- )
           what.form.<% $opt{'prefix'} %>region.options[i] = null;
@@ -42,7 +43,7 @@
 %     }
 
       // add the new regions
-      var regionArray = eval('(' + regions + ')' );
+      var regionArray = reply.regions;
       for ( var s = 0; s < regionArray.length; s++ ) {
           var regionLabel = regionArray[s];
           if ( regionLabel == "" )
@@ -57,6 +58,11 @@
       } else {
         var regionerror = document.getElementById('<% $opt{'prefix'} %>regionerror');
         regionerror.style.display = 'inline';
+        if ( reply.error ) {
+          regionerror.textContent = reply.error;
+        } else {
+          regionerror.textContent = 'Select a different state';
+        }
       }
 
       //run the callback
@@ -73,7 +79,7 @@
 
 <DIV ID="<% $opt{'prefix'} %>regionwait" STYLE="display:none"><IMG SRC="<%$fsurl%>images/wait-orange.gif"> <B>Finding regions</B></DIV>
 
-<DIV ID="<% $opt{'prefix'} %>regionerror" STYLE="display:none"><IMG SRC="<%$fsurl%>images/cross.png"> <B>Select a different state</B></DIV>
+<DIV ID="<% $opt{'prefix'} %>regionerror" STYLE="display:none; font-weight: bold"><IMG SRC="<%$fsurl%>images/cross.png"></DIV>
 
 <SELECT NAME="<% $opt{'prefix'} %>region" onChange="<% $opt{'prefix'} %>region_changed(this); <% $opt{'onchange'} %>" <% $opt{'disabled'} %>>
   <OPTION VALUE="">Select region</OPTION>
diff --git a/httemplate/misc/areacodes.cgi b/httemplate/misc/areacodes.cgi
index 4b31deb..afbe93e 100644
--- a/httemplate/misc/areacodes.cgi
+++ b/httemplate/misc/areacodes.cgi
@@ -1,4 +1,4 @@
-<% encode_json(\@areacodes) %>\
+<% encode_json({ error => $error, areacodes => \@areacodes}) %>\
 <%init>
 
 my( $state, $svcpart ) = $cgi->param('arg');
@@ -7,6 +7,8 @@ my $part_svc = qsearchs('part_svc', { 'svcpart'=>$svcpart } );
 die "unknown svcpart $svcpart" unless $part_svc;
 
 my @areacodes = ();
+my $error;
+
 if ( $state ) {
 
   my @exports = $part_svc->part_export_did;
@@ -17,9 +19,12 @@ if ( $state ) {
   }
   my $export = $exports[0];
 
-  my $something = $export->get_dids('state'=>$state);
+  local $@;
+  local $SIG{__DIE__};
+  my $something = eval { $export->get_dids('state'=>$state) };
+  $error = $@;
 
-  @areacodes = @{ $something };
+  @areacodes = @{ $something } if $something;
 
 }
 
diff --git a/httemplate/misc/exchanges.cgi b/httemplate/misc/exchanges.cgi
index 0de4ace..d626791 100644
--- a/httemplate/misc/exchanges.cgi
+++ b/httemplate/misc/exchanges.cgi
@@ -1,4 +1,4 @@
-<% encode_json(\@exchanges) %>\
+<% encode_json({ error => $error, exchanges => \@exchanges}) %>\
 <%init>
 
 my( $areacode, $svcpart ) = $cgi->param('arg');
@@ -7,6 +7,8 @@ my $part_svc = qsearchs('part_svc', { 'svcpart'=>$svcpart } );
 die "unknown svcpart $svcpart" unless $part_svc;
 
 my @exchanges = ();
+my $error;
+
 if ( $areacode ) {
 
   my @exports = $part_svc->part_export_did;
@@ -17,9 +19,12 @@ if ( $areacode ) {
   }
   my $export = $exports[0];
 
-  my $something = $export->get_dids('areacode'=>$areacode);
+  local $@;
+  local $SIG{__DIE__};
+  my $something = eval { $export->get_dids('areacode'=>$areacode) };
+  $error = $@;
 
-  @exchanges = @{ $something };
+  @exchanges = @{ $something } if $something;
 
 }
 
diff --git a/httemplate/misc/phonenums.cgi b/httemplate/misc/phonenums.cgi
index 62923ac..aae04f5 100644
--- a/httemplate/misc/phonenums.cgi
+++ b/httemplate/misc/phonenums.cgi
@@ -1,4 +1,4 @@
-<% encode_json(\@phonenums) %>\
+<% encode_json({ error => $error, phonenums => \@phonenums}) %>\
 <%init>
 
 my( $exchangestring, $svcpart ) = $cgi->param('arg');
@@ -7,6 +7,7 @@ my $part_svc = qsearchs('part_svc', { 'svcpart'=>$svcpart } );
 die "unknown svcpart $svcpart" unless $part_svc;
 
 my @phonenums = ();
+my $error;
 
 if ( $exchangestring ) {
 
@@ -35,8 +36,12 @@ if ( $exchangestring ) {
       $opts{'exchange'} = $exchange;
   }
 
-  my $something = $export->get_dids(%opts);
-  @phonenums = @{ $something };
+  local $@;
+  local $SIG{__DIE__};
+  my $something = eval { $export->get_dids(%opts) };
+  $error = $@;
+
+  @phonenums = @{ $something } if $something;
 
 }
 
diff --git a/httemplate/misc/regions.cgi b/httemplate/misc/regions.cgi
index 31538b0..882dd48 100644
--- a/httemplate/misc/regions.cgi
+++ b/httemplate/misc/regions.cgi
@@ -1,4 +1,4 @@
-<% encode_json(\@regions) %>\
+<% encode_json({ error => $error, regions => \@regions}) %>\
 <%init>
 
 my( $state, $svcpart ) = $cgi->param('arg');
@@ -7,6 +7,8 @@ my $part_svc = qsearchs('part_svc', { 'svcpart'=>$svcpart } );
 die "unknown svcpart $svcpart" unless $part_svc;
 
 my @regions = ();
+my $error;
+
 if ( $state ) {
 
   my @exports = $part_svc->part_export_did;
@@ -17,9 +19,12 @@ if ( $state ) {
   }
   my $export = $exports[0];
 
-  my $something = $export->get_dids('state'=>$state);
+  local $@;
+  local $SIG{__DIE__};
+  my $something = eval { $export->get_dids('state'=>$state) };
+  $error = $@;
 
-  @regions = @{ $something };
+  @regions = @{ $something } if $something;
 
 }