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

9 files changed, 348 insertions, 140 deletions

diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm
index 9c9c6aaaf..a7a0b45c5 100644
--- a/FS/FS/Conf.pm
+++ b/FS/FS/Conf.pm
@@ -1948,6 +1948,13 @@ worry that config_items is freeside-specific and icky.
   },
 
   {
+    'key'         => 'cust_pkg-always_show_location',
+    'section'     => 'UI',
+    'description' => "Always display package locations, even when they're all the default service address.",
+    'type'        => 'checkbox',
+  },
+
+  {
     'key'         => 'svc_acct-edit_uid',
     'section'     => 'shell',
     'description' => 'Allow UID editing.',
diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index fb6808107..94280a4db 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -697,9 +697,7 @@ sub order_pkgs {
   my $cust_pkgs = shift;
   my $seconds = shift;
   my %options = @_;
-  my %svc_options = ();
-  $svc_options{'depend_jobnum'} = $options{'depend_jobnum'}
-    if exists $options{'depend_jobnum'};
+
   warn "$me order_pkgs called with options ".
        join(', ', map { "$_: $options{$_}" } keys %options ). "\n"
     if $DEBUG;
@@ -718,36 +716,125 @@ sub order_pkgs {
   local $FS::svc_Common::noexport_hack = 1 if $options{'noexport'};
 
   foreach my $cust_pkg ( keys %$cust_pkgs ) {
-    $cust_pkg->custnum( $self->custnum );
-    my $error = $cust_pkg->insert;
+
+    my $error = $self->order_pkg( 'cust_pkg'      => $cust_pkg,
+                                  'svcs'          => $cust_pkgs->{$cust_pkg},
+                                  'seconds'       => $seconds,
+                                  'depend_jobnum' => $options{'depend_jobnum'},
+                                );
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
-      return "inserting cust_pkg (transaction rolled back): $error";
+      return $error;
     }
-    foreach my $svc_something ( @{$cust_pkgs->{$cust_pkg}} ) {
-      if ( $svc_something->svcnum ) {
-        my $old_cust_svc = $svc_something->cust_svc;
-        my $new_cust_svc = new FS::cust_svc { $old_cust_svc->hash };
-        $new_cust_svc->pkgnum( $cust_pkg->pkgnum);
-        $error = $new_cust_svc->replace($old_cust_svc);
-      } else {
-        $svc_something->pkgnum( $cust_pkg->pkgnum );
-        if ( $seconds && $$seconds && $svc_something->isa('FS::svc_acct') ) {
-          $svc_something->seconds( $svc_something->seconds + $$seconds );
-          $$seconds = 0;
-        }
-        $error = $svc_something->insert(%svc_options);
-      }
-      if ( $error ) {
-        $dbh->rollback if $oldAutoCommit;
-        #return "inserting svc_ (transaction rolled back): $error";
-        return $error;
+
+  }
+
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+  ''; #no error
+}
+
+=item order_pkg HASHREF | OPTION => VALUE ... 
+
+Orders a single package.  This is the preferred and most flexible method for
+ordering a single package, including the ability to set a (new or existing)
+location as well as insert services.
+
+Options may be passed as a list of key/value pairs or as a hash reference.
+Options are:
+
+=over 4
+
+=item cust_pkg
+
+FS::cust_pkg object
+
+=item cust_location
+
+Optional FS::cust_location object
+
+=item svcs
+
+Optional arryaref of FS::svc_* service objects.
+
+=item depend_jobnum
+
+If this option is set to a job queue jobnum (see L<FS::queue), all provisioning
+jobs will have a dependancy on the supplied job (they will not run until the
+specific job completes).  This can be used to defer provisioning until some
+action completes (such as running the customer's credit card successfully).
+
+=back
+
+=cut
+
+sub order_pkg {
+  my $self = shift;
+  my $opt = ref($_[0]) ? shift : { @_ };
+
+  warn "$me order_pkg called with options ".
+       join(', ', map { "$_: $opt->{$_}" } keys %$opt ). "\n"
+    if $DEBUG;
+
+  my $cust_pkg = $opt->{'cust_pkg'};
+  my $seconds  = $opt->{'seconds'};
+  my $svcs     = $opt->{'svcs'} || [];
+
+  my %svc_options = ();
+  $svc_options{'depend_jobnum'} = $opt->{'depend_jobnum'}
+    if exists($opt->{'depend_jobnum'}) && $opt->{'depend_jobnum'};
+
+  local $SIG{HUP} = 'IGNORE';
+  local $SIG{INT} = 'IGNORE';
+  local $SIG{QUIT} = 'IGNORE';
+  local $SIG{TERM} = 'IGNORE';
+  local $SIG{TSTP} = 'IGNORE';
+  local $SIG{PIPE} = 'IGNORE';
+
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
+
+  if ( $opt->{'cust_location'} &&
+       ( ! $cust_pkg->locationnum || $cust_pkg->locationnum == -1 ) ) {
+    my $error = $opt->{'cust_location'}->insert;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "inserting cust_location (transaction rolled back): $error";
+    }
+    $cust_pkg->locationnum($opt->{'cust_location'}->locationnum);
+  }
+
+  $cust_pkg->custnum( $self->custnum );
+
+  my $error = $cust_pkg->insert;
+  if ( $error ) {
+    $dbh->rollback if $oldAutoCommit;
+    return "inserting cust_pkg (transaction rolled back): $error";
+  }
+
+  foreach my $svc_something ( @{ $opt->{'svcs'} } ) {
+    if ( $svc_something->svcnum ) {
+      my $old_cust_svc = $svc_something->cust_svc;
+      my $new_cust_svc = new FS::cust_svc { $old_cust_svc->hash };
+      $new_cust_svc->pkgnum( $cust_pkg->pkgnum);
+      $error = $new_cust_svc->replace($old_cust_svc);
+    } else {
+      $svc_something->pkgnum( $cust_pkg->pkgnum );
+      if ( $seconds && $$seconds && $svc_something->isa('FS::svc_acct') ) {
+        $svc_something->seconds( $svc_something->seconds + $$seconds );
+        $$seconds = 0;
       }
+      $error = $svc_something->insert(%svc_options);
+    }
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "inserting svc_ (transaction rolled back): $error";
     }
   }
 
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   ''; #no error
+
 }
 
 =item recharge_prepay IDENTIFIER | PREPAY_CREDIT_OBJ [ , AMOUNTREF, SECONDSREF, UPBYTEREF, DOWNBYTEREF ]
diff --git a/FS/FS/cust_pkg.pm b/FS/FS/cust_pkg.pm
index 03cec75b8..25985ce1f 100644
--- a/FS/FS/cust_pkg.pm
+++ b/FS/FS/cust_pkg.pm
@@ -436,11 +436,13 @@ replace methods.
 sub check {
   my $self = shift;
 
+  $self->locationnum('') if $self->locationnum == 0 || $self->locationnum == -1;
+
   my $error = 
     $self->ut_numbern('pkgnum')
     || $self->ut_foreign_key('custnum', 'cust_main', 'custnum')
     || $self->ut_numbern('pkgpart')
-    || $self->ut_foreign_keyn('locationnum', 'location', 'locationnum')
+    || $self->ut_foreign_keyn('locationnum', 'cust_location', 'locationnum')
     || $self->ut_numbern('setup')
     || $self->ut_numbern('bill')
     || $self->ut_numbern('susp')
@@ -1584,7 +1586,7 @@ Returns the location object, if any (see L<FS::cust_location>).
 sub cust_location {
   my $self = shift;
   return '' unless $self->locationnum;
-  qsearchs( 'cust_main', { 'locationnum' => $self->locationnum } );
+  qsearchs( 'cust_location', { 'locationnum' => $self->locationnum } );
 }
 
 =item cust_location_or_main
diff --git a/httemplate/edit/process/quick-cust_pkg.cgi b/httemplate/edit/process/quick-cust_pkg.cgi
index 2aec344c5..9c2474330 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 bf340894a..f21b8ad01 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 000000000..b62c65c03
--- /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 3c3a85545..419c59f2e 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 f91143154..2c8335154 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 9a4997be5..afd9941f6 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 {