correct invoice package address display and reduce false laziness

4 files changed, 103 insertions, 51 deletions

diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm
index ca81c03dc..0f08aaa51 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 3a9cc7f97..997532c90 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 83487e403..17dda9cf1 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 8110d7356..fa0fa6981 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