correct invoice package address display and reduce false laziness

5 files changed, 107 insertions, 66 deletions

diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm
index ca81c03..0f08aaa 100644
--- a/FS/FS/cust_bill.pm
+++ b/FS/FS/cust_bill.pm
@@ -3956,9 +3956,12 @@ sub _items_cust_bill_pkg {
           {
             push @d, map &{$escape_function}($_),
                          $cust_pkg->h_labels_short($self->_date);
-            push @d, map &{$escape_function}($_),
-                         $cust_pkg->location_label_short
-              if $multilocation;
+            if ( $multilocation ) {
+              my $loc = $cust_pkg->location_label;
+              $loc = substr($desc, 0, 50). '...'
+                if $format eq 'latex' && length($loc) > 50;
+              push @d, &{$escape_function}($loc);
+            }
           }
           push @d, $cust_bill_pkg->details(%details_opt)
             if $cust_bill_pkg->recur == 0;
@@ -4013,9 +4016,12 @@ sub _items_cust_bill_pkg {
                                                    #$cust_bill_pkg->edate,
                                                    #$cust_bill_pkg->sdate)
             ;
-            push @d, map &{$escape_function}($_),
-                         $cust_pkg->location_label_short
-              if $multilocation;
+            if ( $multilocation ) {
+              my $loc = $cust_pkg->location_label;
+              $loc = substr($desc, 0, 50). '...'
+                if $format eq 'latex' && length($loc) > 50;
+              push @d, &{$escape_function}($loc);
+            }
           }
 
           push @d, $cust_bill_pkg->details(%details_opt)
diff --git a/FS/FS/cust_location.pm b/FS/FS/cust_location.pm
index 3a9cc7f..997532c 100644
--- a/FS/FS/cust_location.pm
+++ b/FS/FS/cust_location.pm
@@ -158,58 +158,71 @@ sub country_full {
   code2country($self->country);
 }
 
-=item line
+=item location_label [ OPTION => VALUE ... ]
 
-Returns this location on one line
+Returns the label of the service location for this customer.
 
-=cut
+Options are
 
-sub line {
-  my $self = shift;
-  my $cydefault = FS::conf->new->config('countrydefault') || 'US';
+=over 4
 
-  my $line =       $self->address1;
-  $line   .= ', '. $self->address2              if $self->address2;
-  $line   .= ', '. $self->city;
-  $line   .= ' ('. $self->county. ' county)'    if $self->county;
-  $line   .= ', '. $self->state                 if $self->state;
-  $line   .= '  '. $self->zip                   if $self->zip;
-  $line   .= '  '. code2country($self->country) if $self->country ne $cydefault;
+=item join_string
 
-  $line;
-}
+used to separate the address elements (defaults to ', ')
+
+=item escape_function
 
-=item line_short
 
-Returns this location on one line in a shortened form
+a callback used for escaping the text of the address elements
+
+=back
 
 =cut
 
-# configurable?
+# false laziness with FS::cust_main::location_label
 
-sub line_short {
+sub location_label {
   my $self = shift;
-  my $cydefault = FS::conf->new->config('countrydefault') || 'US';
+  my %opt = @_;
 
-  my $line =       $self->address1;
-  #$line   .= ', '. $self->address2              if $self->address2;
-  $line   .= ', '. $self->city;
-  $line   .= ', '. $self->state                 if $self->state;
-  $line   .= '  '. $self->zip                   if $self->zip;
-  $line   .= '  '. code2country($self->country) if $self->country ne $cydefault;
+  my $separator = $opt{join_string} || ', ';
+  my $escape = $opt{escape_function} || sub{ shift };
+  my $line = '';
+  my $cydefault = FS::conf->new->config('countrydefault') || 'US';
+  my $prefix = '';
+
+  my $notfirst = 0;
+  foreach (qw ( address1 address2 ) ) {
+    my $method = "$prefix$_";
+    $line .= ($notfirst ? $separator : ''). &$escape($self->$method)
+      if $self->$method;
+    $notfirst++;
+  }
+  $notfirst = 0;
+  foreach (qw ( city county state zip ) ) {
+    my $method = "$prefix$_";
+    if ( $self->$method ) {
+      $line .= '(' if $method eq 'county';
+      $line .= ($notfirst ? ' ' : $separator). &$escape($self->$method);
+      $line .= ')' if $method eq 'county';
+      $notfirst++;
+    }
+  }
+  $line .= $separator. &$escape(code2country($self->country))
+    if $self->country ne $cydefault;
 
   $line;
 }
 
-=item location_label_short
+=item line
 
-Synonym for line_short
+Synonym for location_label
 
 =cut
 
-sub location_label_short {
+sub line {
   my $self = shift;
-  $self->line_short;
+  $self->location_label;
 }
 
 =back
diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index 83487e4..17dda9c 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -1955,24 +1955,57 @@ sub cust_location {
   qsearch('cust_location', { 'custnum' => $self->custnum } );
 }
 
-=item location_label_short
+=item location_label [ OPTION => VALUE ... ]
 
-Returns the short label of the service location (see analog in L<FS::cust_location>) for this customer.
+Returns the label of the service location (see analog in L<FS::cust_location>) for this customer.
+
+Options are
+
+=over 4
+
+=item join_string
+
+used to separate the address elements (defaults to ', ')
+
+=item escape_function
+
+a callback used for escaping the text of the address elements
+
+=back
 
 =cut
 
-# false laziness with FS::cust_location::line_short
+# false laziness with FS::cust_location::line
 
-sub location_label_short {
+sub location_label {
   my $self = shift;
-  my $cydefault = FS::conf->new->config('countrydefault') || 'US';
+  my %opt = @_;
 
-  my $line =       $self->address1;
-  #$line   .= ', '. $self->address2              if $self->address2;
-  $line   .= ', '. $self->city;
-  $line   .= ', '. $self->state                 if $self->state;
-  $line   .= '  '. $self->zip                   if $self->zip;
-  $line   .= '  '. code2country($self->country) if $self->country ne $cydefault;
+  my $separator = $opt{join_string} || ', ';
+  my $escape = $opt{escape_function} || sub{ shift };
+  my $line = '';
+  my $cydefault = FS::conf->new->config('countrydefault') || 'US';
+  my $prefix = length($self->ship_last) ? 'ship_' : '';
+
+  my $notfirst = 0;
+  foreach (qw ( address1 address2 ) ) {
+    my $method = "$prefix$_";
+    $line .= ($notfirst ? $separator : ''). &$escape($self->$method)
+      if $self->$method;
+    $notfirst++;
+  }
+  $notfirst = 0;
+  foreach (qw ( city county state zip ) ) {
+    my $method = "$prefix$_";
+    if ( $self->$method ) {
+      $line .= '(' if $method eq 'county';
+      $line .= ($notfirst ? ' ' : $separator). &$escape($self->$method);
+      $line .= ')' if $method eq 'county';
+      $notfirst++;
+    }
+  }
+  $line .= $separator. &$escape(code2country($self->country))
+    if $self->country ne $cydefault;
 
   $line;
 }
diff --git a/FS/FS/cust_pkg.pm b/FS/FS/cust_pkg.pm
index 8110d73..fa0fa69 100644
--- a/FS/FS/cust_pkg.pm
+++ b/FS/FS/cust_pkg.pm
@@ -1946,16 +1946,16 @@ sub cust_location_or_main {
   $self->cust_location || $self->cust_main;
 }
 
-=item location_label_short
+=item location_label [ OPTION => VALUE ... ]
 
-Returns the short label of the location object (see L<FS::cust_location>).
+Returns the label of the location object (see L<FS::cust_location>).
 
 =cut
 
-sub location_label_short {
+sub location_label {
   my $self = shift;
   my $object = $self->cust_location_or_main;
-  $object->location_label_short;
+  $object->location_label(@_);
 }
 
 =item seconds_since TIMESTAMP
diff --git a/httemplate/view/cust_main/packages/location.html b/httemplate/view/cust_main/packages/location.html
index 59efce1..9f348e5 100644
--- a/httemplate/view/cust_main/packages/location.html
+++ b/httemplate/view/cust_main/packages/location.html
@@ -4,18 +4,10 @@
   <I><FONT SIZE=-1>(default service address)</FONT><BR>
 % }
 
-  <% $loc->get($prefix.'address1') |h %><BR>
-
-% if ( $loc->get($prefix.'address2') !~ /^\s*$/ ) {
-    <% $loc->get($prefix.'address2') |h %><BR>
-% }
-
-  <% $loc->get($prefix.'city') |h %><% $loc->get($prefix.'county') ? ' ('.$loc->get($prefix.'county').' county)' : '' |h %>,
-  <% $loc->get($prefix.'state') |h %> &nbsp; <% $loc->get($prefix.'zip') |h %><BR>
-
-% if ( $loc->get($prefix.'country') ne $countrydefault ) {
-  <% code2country( $loc->get($prefix.'country') ) %>
-% }
+  <% $loc->location_label( 'join_string' => '<BR>',
+                           'escape_function' => \&encode_entities,
+                         )
+  %>
 
   </I>
 
@@ -35,14 +27,11 @@ my %opt = @_;
 
 my $bgcolor        = $opt{'bgcolor'};
 my $cust_pkg       = $opt{'cust_pkg'};
-my $part_pkg       = $opt{'part_pkg'};
 my $countrydefault = $opt{'countrydefault'} || 'US';
 my $statedefault   = $opt{'statedefault'}
                      || ($countrydefault eq 'US' ? 'CA' : '');
 
 my $loc = $cust_pkg->cust_location_or_main;
-my $prefix =
-  ( $loc->table eq 'cust_main' && length($loc->ship_last) ) ? 'ship_' : ''; #doh
 
 sub pkg_change_location_link {
   my $cust_pkg = shift;