more work on package service addresses: hide locations when they're all the default, config to show them anyway / finish implementing package ordering, fix all the state/county weirdness when changing the location dropdown. RT#4499

6 files changed, 226 insertions, 114 deletions

diff --git a/httemplate/edit/process/quick-cust_pkg.cgi b/httemplate/edit/process/quick-cust_pkg.cgi
index 2aec344..9c24743 100644
--- a/httemplate/edit/process/quick-cust_pkg.cgi
+++ b/httemplate/edit/process/quick-cust_pkg.cgi
@@ -2,7 +2,7 @@
 %  $cgi->param('error', $error);
 <% $cgi->redirect(popurl(3). 'misc/order_pkg.html?'. $cgi->query_string ) %>
 %} else {
-%  my $frag = "cust_pkg". $cust_pkg[0]->pkgnum;
+%  my $frag = "cust_pkg". $cust_pkg->pkgnum;
 <% header('Package ordered') %>
   <SCRIPT TYPE="text/javascript">
     // XXX fancy ajax rebuild table at some point, but a page reload will do for now
@@ -19,15 +19,45 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Order customer package');
 
-#untaint custnum
+#untaint custnum (probably not necessary, searching for it is escape enough)
 $cgi->param('custnum') =~ /^(\d+)$/
   or die 'illegal custnum '. $cgi->param('custnum');
 my $custnum = $1;
+my $cust_main = qsearchs({
+  'table'     => 'cust_main',
+  'hashref'   => { 'custnum' => $custnum },
+  'extra_sql' => ' AND '. $FS::CurrentUser::CurrentUser->agentnums_sql,
+});
+die 'unknown custnum' unless $cust_main;
+
+#probably not necessary, taken care of by cust_pkg::check
 $cgi->param('pkgpart') =~ /^(\d+)$/
   or die 'illegal pkgpart '. $cgi->param('pkgpart');
 my $pkgpart = $1;
+$cgi->param('refnum') =~ /^(\d*)$/
+  or die 'illegal refnum '. $cgi->param('refnum');
+my $refnum = $1;
+$cgi->param('locationnum') =~ /^(\-?\d*)$/
+  or die 'illegal locationnum '. $cgi->param('locationnum');
+my $locationnum = $1;
+
+my $cust_pkg = new FS::cust_pkg {
+  'custnum'     => $custnum,
+  'pkgpart'     => $pkgpart,
+  'refnum'      => $refnum,
+  'locationnum' => $locationnum,
+};
+
+my %opt = ( 'cust_pkg' => $cust_pkg );
+
+if ( $locationnum == -1 ) {
+  my $cust_location = new FS::cust_location {
+    map { $_ => scalar($cgi->param($_)) }
+        qw( custnum address1 address2 city county state zip country )
+  };
+  $opt{'cust_location'} = $cust_location;
+}
 
-my @cust_pkg = ();
-my $error = FS::cust_pkg::order($custnum, [ $pkgpart ], [], \@cust_pkg, [ $cgi->param('refnum') ] );
+my $error = $cust_main->order_pkg( %opt );
 
 </%init>
diff --git a/httemplate/elements/location.html b/httemplate/elements/location.html
index bf34089..f21b8ad 100644
--- a/httemplate/elements/location.html
+++ b/httemplate/elements/location.html
@@ -20,6 +20,7 @@ Example:
   <TD COLSPAN=7>
     <INPUT TYPE     = "text"
            NAME     = "<%$pre%>address1"
+           ID       = "<%$pre%>address1"
            VALUE    = "<% $object->get($pre.'address1') |h %>"
            SIZE     = 70
            onChange = "<% $onchange %>"
@@ -34,6 +35,7 @@ Example:
   <TD COLSPAN=7>
     <INPUT TYPE     = "text"
            NAME     = "<%$pre%>address2"
+           ID       = "<%$pre%>address2"
            VALUE    = "<% $object->get($pre.'address2') |h %>"
            SIZE     = 70
            onChange = "<% $onchange %>"
@@ -48,6 +50,7 @@ Example:
   <TD>
     <INPUT TYPE     = "text"
            NAME     = "<%$pre%>city"
+           ID       = "<%$pre%>city"
            VALUE    = "<% $object->get($pre.'city') |h %>"
            onChange = "<% $onchange %>"
            <% $disabled %>
@@ -66,6 +69,7 @@ Example:
   <TD>
     <INPUT TYPE     = "text"
            NAME     = "<%$pre%>zip"
+           ID       = "<%$pre%>zip"
            VALUE    = "<% $object->get($pre.'zip') |h %>"
            SIZE     = 10
            onChange = "<% $onchange %>"
diff --git a/httemplate/elements/tr-select-cust_location.html b/httemplate/elements/tr-select-cust_location.html
new file mode 100644
index 0000000..b62c65c
--- /dev/null
+++ b/httemplate/elements/tr-select-cust_location.html
@@ -0,0 +1,176 @@
+<%doc>
+
+Example:
+
+  include('/elements/tr-select-cust_location.html',
+            'cgi'       => $cgi,
+            'cust_main' => $cust_main,
+         )
+
+</%doc>
+
+<% include('/elements/xmlhttp.html',
+              'url'  => $p.'misc/location.cgi',
+              'subs' => [ 'get_location' ],
+           )
+%>
+
+<SCRIPT TYPE="text/javascript">
+
+  function locationnum_changed(what) {
+    var locationnum = what.options[what.selectedIndex].value;
+    if ( locationnum == -1 ) {
+
+%     for (@location_fields) { 
+        what.form.<%$_%>.disabled = false;
+        what.form.<%$_%>.style.backgroundColor = '#ffffff';
+%     } 
+
+      what.form.address1.value = '';
+      what.form.address2.value = '';
+      what.form.city.value = '';
+      what.form.zip.value = '';
+
+      changeSelect(what.form.country, <% $countrydefault |js_string %>);
+
+      country_changed( what.form.country,
+                       fix_state_factory( <% $statedefault |js_string %>,
+                                          ''
+                                        )
+                     );
+
+    } else {
+
+      if ( locationnum == 0 ) {
+        what.form.address1.value = <% $cust_main->address1 |js_string %>;
+        what.form.address2.value = <% $cust_main->address2 |js_string %>;
+        what.form.city.value = <% $cust_main->city |js_string %>;
+        what.form.zip.value = <% $cust_main->zip |js_string %>;
+
+        changeSelect(what.form.country, <% $cust_main->country | js_string %> );
+
+        country_changed( what.form.country,
+                         fix_state_factory( <% $cust_main->state | js_string %>,
+                                            <% $cust_main->county | js_string %>
+                                          )
+                       );
+
+      } else {
+        get_location( locationnum, update_location );
+      } 
+
+%#sleep/wait until dropdowns are updated?
+%     for (@location_fields) { 
+        what.form.<%$_%>.disabled = true;
+        what.form.<%$_%>.style.backgroundColor = '#dddddd';
+%     } 
+
+    }
+  }
+
+  function fix_state_factory (state, county) {
+    function fix_state() {
+      var state_el = document.getElementById('state');
+      changeSelect(state_el, state);
+      state_changed(state_el, fix_county_factory(county) );
+    }
+    return fix_state;
+  }
+
+  function fix_county_factory(county) {
+    function fix_county() {
+      var county_el = document.getElementById('county');
+      if ( county.length > 0 ) {
+        changeSelect(county_el, county );
+      } else {
+        county_el.selectedIndex = 0;
+      }
+    }
+    return fix_county;
+  }
+
+  function changeSelect(what, value) {
+    for ( var i=0; i<what.length; i++) {
+      if ( what.options[i].value == value ) {
+        what.selectedIndex = i;
+      }
+    }
+  }
+
+  function update_location( string ) {
+    var hash = eval('('+string+')');
+    document.getElementById('address1').value = hash['address1'];
+    document.getElementById('address2').value = hash['address2'];
+    document.getElementById('city').value     = hash['city'];
+    document.getElementById('zip').value      = hash['zip'];
+
+    var country_el = document.getElementById('country');
+
+    changeSelect( country_el, hash['country'] );
+
+    country_changed( country_el,
+                     fix_state_factory( hash['state'],
+                                        hash['county']
+                                      )
+                   );
+  }
+
+</SCRIPT>
+
+<TR>
+  <TH ALIGN="right">Service location</TH>
+  <TD COLSPAN=7>
+    <SELECT NAME="locationnum" onChange="locationnum_changed(this);">
+      <OPTION VALUE="">(default service address)
+%     foreach my $loc ( $cust_main->cust_location ) {
+        <OPTION VALUE="<% $loc->locationnum %>"
+                <% $locationnum == $loc->locationnum ? 'SELECTED' : '' %>
+        ><% $loc->line |h %>
+%     }
+      <OPTION VALUE="-1"
+              <% $locationnum == -1 ? 'SELECTED' : '' %>
+      >Add new location
+    </SELECT>
+  </TD>
+</TR>
+
+<% include('/elements/location.html',
+             'object'       => $cust_location,
+             #'onchange' ?  probably not
+             'disabled'     => ( $locationnum == -1 ? '' : 'DISABLED' ),
+             'no_asterisks' => 1,
+          )
+%>
+
+<%once>
+
+my @location_fields = qw( address1 address2 city county state zip country );
+
+</%once>
+<%init>
+
+my $conf = new FS::Conf;
+my $countrydefault = $conf->config('countrydefault') || 'US';
+my $statedefault = $conf->config('statedefault')
+                   || ($countrydefault eq 'US' ? 'CA' : '');
+
+my %opt = @_;
+my $cgi       = $opt{'cgi'};
+my $cust_main = $opt{'cust_main'};
+
+$cgi->param('locationnum') =~ /^(\-?\d*)$/ or die "illegal locationnum";
+my $locationnum = $1;
+my $cust_location;
+if ( $locationnum && $locationnum != -1 ) {
+  $cust_location = qsearchs('cust_location', { 'locationnum' => $locationnum } )
+    or die "unknown locationnum";
+} else {
+  $cust_location = new FS::cust_location;
+  if ( $cgi->param('error') && $locationnum == -1 ) {
+    $cust_location->$_( $cgi->param($_) ) foreach @location_fields;
+  } else {
+    $cust_location->$_( $cust_main->$_() ) foreach @location_fields;
+  }
+}
+
+</%init>
diff --git a/httemplate/misc/location.cgi b/httemplate/misc/location.cgi
index 3c3a855..419c59f 100644
--- a/httemplate/misc/location.cgi
+++ b/httemplate/misc/location.cgi
@@ -4,6 +4,7 @@
 my $locationnum = $cgi->param('arg');
 
 my $cust_location = qsearchs({
+  'select'    => 'cust_location.*',
   'table'     => 'cust_location',
   'hashref'   => { 'locationnum' => $locationnum },
   'addl_from' => 'LEFT JOIN cust_main USING ( custnum )',
diff --git a/httemplate/misc/order_pkg.html b/httemplate/misc/order_pkg.html
index f911431..2c83351 100644
--- a/httemplate/misc/order_pkg.html
+++ b/httemplate/misc/order_pkg.html
@@ -1,71 +1,7 @@
 <% include('/elements/header-popup.html', 'Order new package' ) %>
 
-<% include('/elements/xmlhttp.html',
-              'url'  => $p.'misc/location.cgi',
-              'subs' => [ 'get_location' ],
-           )
-%>
-
 <SCRIPT TYPE="text/javascript">
 
-  function locationnum_changed(what) {
-    var locationnum = what.options[what.selectedIndex].value;
-    if ( locationnum == -1 ) {
-
-%     for (@location_fields) { 
-        what.form.<%$_%>.disabled = false;
-        what.form.<%$_%>.style.backgroundColor = '#ffffff';
-%     } 
-
-      what.form.address1.value = '';
-      what.form.address2.value = '';
-      what.form.city.value = '';
-      what.form.zip.value = '';
-      changeSelect(what.form.country, <% $countrydefault |js_string %>);
-%#shouldn't we sleep/wait here until the state dropdown is updated?
-%#(is it even triggered???)
-      changeSelect(what.form.state, <% $statedefault |js_string %>);
-      what.form.county.selectedIndex = 0;
-
-    } else {
-
-      if ( locationnum == 0 ) {
-        what.form.address1.value = <% $cust_main->address1 |js_string %>;
-        what.form.address2.value = <% $cust_main->address2 |js_string %>;
-        what.form.city.value = <% $cust_main->city |js_string %>;
-        what.form.zip.value = <% $cust_main->zip |js_string %>;
-        changeSelect(what.form.country, <% $cust_main->country | js_string %> );
-%#shouldn't we sleep/wait here until the state dropdown is updated?
-%#(is it even triggered???)
-        changeSelect(what.form.state, <% $cust_main->state | js_string %> );
-%#shouldn't we sleep/wait here until the county dropdown is updated?
-%#(is it even triggered???)
-        changeSelect(what.form.county, <% $cust_main->county | js_string %> );
-      } else {
-        get_location( locationnum, update_location );
-      } 
-
-%#sleep/wait until dropdowns are updated?
-%     for (@location_fields) { 
-        what.form.<%$_%>.disabled = true;
-        what.form.<%$_%>.style.backgroundColor = '#dddddd';
-%     } 
-
-    }
-  }
-
-  function changeSelect(what, value) {
-    for ( var i=0; i<what.length; i++) {
-      if ( what.options[i].value == value ) {
-        what.selectedIndex = i;
-      }
-    }
-  }
-
-  function update_location( hash ) {
-    alert(hash);
-  }
-
   function enable_order_pkg () {
     if ( document.OrderPkgForm.pkgpart.selectedIndex > 0 ) {
       document.OrderPkgForm.submit.disabled = false;
@@ -105,28 +41,9 @@
   %>
 % }
 
-<TR>
-  <TH ALIGN="right">Service location</TH>
-  <TD COLSPAN=7>
-    <SELECT NAME="locationnum" onChange="locationnum_changed(this);">
-      <OPTION VALUE="">(default service address)
-%     foreach my $loc ( $cust_main->cust_location ) {
-        <OPTION VALUE="<% $loc->locationnum %>"
-                <% $locationnum == $loc->locationnum ? 'SELECTED' : '' %>
-        ><% $loc->line |h %>
-%     }
-      <OPTION VALUE="-1"
-              <% $locationnum == -1 ? 'SELECTED' : '' %>
-      >Add new location
-    </SELECT>
-  </TD>
-</TR>
-
-<% include('/elements/location.html',
-             'object'       => $cust_location,
-             #'onchange' ?  probably not
-             'disabled'     => ( $locationnum == -1 ? '' : 'DISABLED' ),
-             'no_asterisks' => 1,
+<% include('/elements/tr-select-cust_location.html',
+             'cgi'       => $cgi,
+             'cust_main' => $cust_main,
           )
 %>
 
@@ -138,20 +55,12 @@
 </FORM>
 </BODY>
 </HTML>
-<%once>
-
-my @location_fields = qw( address1 address2 city county state zip country );
-
-</%once>
 <%init>
 
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Order customer package');
 
 my $conf = new FS::Conf;
-my $countrydefault = $conf->config('countrydefault') || 'US';
-my $statedefault = $conf->config('statedefault')
-                   || ($countrydefault eq 'US' ? 'CA' : '');
 
 $cgi->param('custnum') =~ /^(\d+)$/ or die "no custnum";
 my $custnum = $1;
@@ -163,19 +72,4 @@ my $cust_main = qsearchs({
 
 my $pkgpart = scalar($cgi->param('pkgpart'));
 
-$cgi->param('locationnum') =~ /^(\d*)$/ or die "illegal locationnum";
-my $locationnum = $1;
-my $cust_location;
-if ( $locationnum ) {
-  $cust_location = qsearchs('cust_location', { 'locationnum' => $locationnum } )
-    or die "unknown locationnum";
-} else {
-  $cust_location = new FS::cust_location;
-  if ( $cgi->param('error') && $locationnum == -1 ) {
-    $cust_location->$_( $cgi->param($_) ) foreach @location_fields;
-  } else {
-    $cust_location->$_( $cust_main->$_() ) foreach @location_fields;
-  }
-}
-
 </%init>
diff --git a/httemplate/view/cust_main/packages.html b/httemplate/view/cust_main/packages.html
index 9a4997b..afd9941 100755
--- a/httemplate/view/cust_main/packages.html
+++ b/httemplate/view/cust_main/packages.html
@@ -97,7 +97,7 @@ function taxoverridequickchargemagic() {
 
 
 <BR><BR>
-% if ( @$packages ) { 
+% if ( @$packages ) {
 
 Current packages
 % } 
@@ -131,7 +131,9 @@ Current packages
 <TR>
   <TH CLASS="grid" BGCOLOR="#cccccc">Package</TH>
   <TH CLASS="grid" BGCOLOR="#cccccc">Status</TH>
+% if ( $show_location ) {
   <TH CLASS="grid" BGCOLOR="#cccccc">Location</TH>
+% }
   <TH CLASS="grid" BGCOLOR="#cccccc">Services</TH>
 </TR>
 
@@ -153,7 +155,9 @@ Current packages
     <TR>
       <% include('packages/package.html',  %iopt) %>
       <% include('packages/status.html',   %iopt) %>
+% if ( $show_location ) {
       <% include('packages/location.html', %iopt) %>
+% }
       <% include('packages/services.html', %iopt) %>
     </TR>
 
@@ -183,6 +187,9 @@ my $curuser = $FS::CurrentUser::CurrentUser;
 
 my $packages = get_packages($cust_main, $conf);
 
+my $show_location = $conf->exists('cust_pkg-always_show_location')
+                        || ( grep $_->locationnum, @$packages ); # ? '1' : '0';
+
 #subroutines
 
 sub get_packages {