Merge branch 'master' of https://github.com/jgoodman/Freeside
[freeside.git] / FS / FS / Template_Mixin.pm
index dd0801c..131a236 100644 (file)
@@ -2,7 +2,7 @@ package FS::Template_Mixin;
 
 use strict;
 use vars qw( $DEBUG $me
-             $money_char $date_format $rdate_format $date_format_long );
+             $money_char );
              # but NOT $conf
 use vars qw( $invoice_lines @buf ); #yuck
 use List::Util qw(sum);
@@ -15,9 +15,11 @@ use Locale::Country;
 use Cwd;
 use FS::UID;
 use FS::Record qw( qsearch qsearchs );
+use FS::Conf;
 use FS::Misc qw( generate_ps generate_pdf );
 use FS::pkg_category;
 use FS::pkg_class;
+use FS::invoice_mode;
 use FS::L10N;
 
 $DEBUG = 0;
@@ -25,17 +27,53 @@ $me = '[FS::Template_Mixin]';
 FS::UID->install_callback( sub { 
   my $conf = new FS::Conf; #global
   $money_char       = $conf->config('money_char')       || '$';  
-  $date_format      = $conf->config('date_format')      || '%x'; #/YY
-  $rdate_format     = $conf->config('date_format')      || '%m/%d/%Y';  #/YYYY
-  $date_format_long = $conf->config('date_format_long') || '%b %o, %Y';
 } );
 
-=item print_text HASHREF | [ TIME [ , TEMPLATE [ , OPTION => VALUE ... ] ] ]
+=item conf [ MODE ]
+
+Returns a configuration handle (L<FS::Conf>) set to the customer's locale.
+
+If the "mode" pseudo-field is set on the object, the configuration handle
+will be an L<FS::invoice_conf> for that invoice mode (and the customer's
+locale).
+
+=cut
+
+sub conf {
+  my $self = shift;
+  my $mode = $self->get('mode');
+  if ($self->{_conf} and !defined($mode)) {
+    return $self->{_conf};
+  }
+
+  my $cust_main = $self->cust_main;
+  my $locale = $cust_main ? $cust_main->locale : '';
+  my $conf;
+  if ( $mode ) {
+    if ( ref $mode and $mode->isa('FS::invoice_mode') ) {
+      $mode = $mode->modenum;
+    } elsif ( $mode =~ /\D/ ) {
+      die "invalid invoice mode $mode";
+    }
+    $conf = qsearchs('invoice_conf', { modenum => $mode, locale => $locale });
+    if (!$conf) {
+      $conf = qsearchs('invoice_conf', { modenum => $mode, locale => '' });
+      # it doesn't have a locale, but system conf still might
+      $conf->set('locale' => $locale) if $conf;
+    }
+  }
+  # if $mode is unspecified, or if there is no invoice_conf matching this mode
+  # and locale, then use the system config only (but with the locale)
+  $conf ||= FS::Conf->new({ 'locale' => $locale });
+  # cache it
+  return $self->{_conf} = $conf;
+}
+
+=item print_text OPTIONS
 
 Returns an text invoice, as a list of lines.
 
-Options can be passed as a hashref (recommended) or as a list of time, template
-and then any key/value pairs for any other options.
+Options can be passed as a hash.
 
 I<time>, if specified, is used to control the printing of overdue messages.  The
 default is now.  It isn't the date of the invoice; that's the `_date' field.
@@ -50,25 +88,19 @@ I<notice_name>, if specified, overrides "Invoice" as the name of the sent docume
 
 sub print_text {
   my $self = shift;
-  my( $today, $template, %opt );
+  my %params;
   if ( ref($_[0]) ) {
-    %opt = %{ shift() };
-    $today = delete($opt{'time'}) || '';
-    $template = delete($opt{template}) || '';
+    %params = %{ shift() };
   } else {
-    ( $today, $template, %opt ) = @_;
+    %params = @_;
   }
 
-  my %params = ( 'format' => 'template' );
-  $params{'time'} = $today if $today;
-  $params{'template'} = $template if $template;
-  $params{$_} = $opt{$_} 
-    foreach grep $opt{$_}, qw( unsquelch_cdr notice_name );
+  $params{'format'} = 'template'; # for some reason
 
   $self->print_generic( %params );
 }
 
-=item print_latex HASHREF | [ TIME [ , TEMPLATE [ , OPTION => VALUE ... ] ] ]
+=item print_latex HASHREF
 
 Internal method - returns a filename of a filled-in LaTeX template for this
 invoice (Note: add ".tex" to get the actual filename), and a filename of
@@ -76,15 +108,16 @@ an associated logo (with the .eps extension included).
 
 See print_ps and print_pdf for methods that return PostScript and PDF output.
 
-Options can be passed as a hashref (recommended) or as a list of time, template
-and then any key/value pairs for any other options.
+Options can be passed as a hash.
 
 I<time>, if specified, is used to control the printing of overdue messages.  The
 default is now.  It isn't the date of the invoice; that's the `_date' field.
 It is specified as a UNIX timestamp; see L<perlfunc/"time">.  Also see
 L<Time::Local> and L<Date::Parse> for conversion functions.
 
-I<template>, if specified, is the name of a suffix for alternate invoices.
+I<template>, if specified, is the name of a suffix for alternate invoices.  
+This is strongly deprecated; see L<FS::invoice_conf> for the right way to
+customize invoice templates for different purposes.
 
 I<notice_name>, if specified, overrides "Invoice" as the name of the sent document (templates from 10/2009 or newer required)
 
@@ -92,22 +125,20 @@ I<notice_name>, if specified, overrides "Invoice" as the name of the sent docume
 
 sub print_latex {
   my $self = shift;
-  my $conf = $self->conf;
-  my( $today, $template, %opt );
+  my %params;
+
   if ( ref($_[0]) ) {
-    %opt = %{ shift() };
-    $today = delete($opt{'time'}) || '';
-    $template = delete($opt{template}) || '';
+    %params = %{ shift() };
   } else {
-    ( $today, $template, %opt ) = @_;
+    %params = @_;
   }
 
-  my %params = ( 'format' => 'latex' );
-  $params{'time'} = $today if $today;
-  $params{'template'} = $template if $template;
-  $params{$_} = $opt{$_} 
-    foreach grep $opt{$_}, qw( unsquelch_cdr notice_name no_date no_number );
+  $params{'format'} = 'latex';
+  my $conf = $self->conf;
 
+  # this needs to go away
+  my $template = $params{'template'};
+  # and this especially
   $template ||= $self->_agent_template
     if $self->can('_agent_template');
 
@@ -191,7 +222,8 @@ Non optional options include
 
 Optional options include
 
-template - a value used as a suffix for a configuration template
+template - a value used as a suffix for a configuration template.  Please 
+don't use this.
 
 time - a value used to control the printing of overdue messages.  The
 default is now.  It isn't the date of the invoice; that's the `_date' field.
@@ -214,6 +246,7 @@ locale - override customer's locale
 sub print_generic {
   my( $self, %params ) = @_;
   my $conf = $self->conf;
+
   my $today = $params{today} ? $params{today} : time;
   warn "$me print_generic called on $self with suffix $params{template}\n"
     if $DEBUG;
@@ -227,6 +260,8 @@ sub print_generic {
     unless $cust_main->payname
         && $cust_main->payby !~ /^(CARD|DCRD|CHEK|DCHK)$/;
 
+  my $locale = $params{'locale'} || $cust_main->locale;
+
   my %delimiters = ( 'latex'    => [ '[@--', '--@]' ],
                      'html'     => [ '<%=', '%>' ],
                      'template' => [ '{', '}' ],
@@ -235,11 +270,18 @@ sub print_generic {
   warn "$me print_generic creating template\n"
     if $DEBUG > 1;
 
+  # set the notice name here, and nowhere else.
+  my $notice_name =  $params{notice_name}
+                  || $conf->config('notice_name')
+                  || $self->notice_name;
+
   #create the template
   my $template = $params{template} ? $params{template} : $self->_agent_template;
   my $templatefile = $self->template_conf. $format;
   $templatefile .= "_$template"
     if length($template) && $conf->exists($templatefile."_$template");
+
+  # the base template
   my @invoice_template = map "$_\n", $conf->config($templatefile)
     or die "cannot load config data $templatefile";
 
@@ -361,14 +403,6 @@ sub print_generic {
   my $escape_function_nonbsp = ($format eq 'html')
                                  ? \&_html_escape : $escape_function;
 
-  my %date_formats = ( 'latex'    => $date_format_long,
-                       'html'     => $date_format_long,
-                       'template' => '%s',
-                     );
-  $date_formats{'html'} =~ s/ /&nbsp;/g;
-
-  my $date_format = $date_formats{$format};
-
   my %newline_tokens = (  'latex'     => '\\\\',
                           'html'      => '<br>',
                           'template'  => "\n",
@@ -380,6 +414,7 @@ sub print_generic {
 
   # generate template variables
   my $returnaddress;
+
   if (
          defined( $conf->config_orbase( "invoice_${format}returnaddress",
                                         $template
@@ -452,14 +487,14 @@ sub print_generic {
     '_date'           => ( $params{'no_date'} ? '' : $self->_date ),
     'date'            => ( $params{'no_date'}
                              ? ''
-                             : time2str($date_format, $self->_date)
+                             : $self->time2str_local('long', $self->_date, $format)
                          ),
-    'today'           => time2str($date_format_long, $today),
+    'today'           => $self->time2str_local('long', $today, $format),
     'terms'           => $self->terms,
     'template'        => $template, #params{'template'},
-    'notice_name'     => ($params{'notice_name'} || $self->notice_name),#escape_function?
+    'notice_name'     => $notice_name, # escape?
     'current_charges' => sprintf("%.2f", $self->charged),
-    'duedate'         => $self->due_date2str($rdate_format), #date_format?
+    'duedate'         => $self->due_date2str('rdate'), #date_format?
 
     #customer info
     'custnum'         => $cust_main->display_custnum,
@@ -499,15 +534,9 @@ sub print_generic {
   );
  
   #localization
-  my $lh = FS::L10N->get_handle( $params{'locale'} || $cust_main->locale );
   $invoice_data{'emt'} = sub { &$escape_function($self->mt(@_)) };
-  my %info = FS::Locales->locale_info($cust_main->locale || 'en_US');
-  # eval to avoid death for unimplemented languages
-  my $dh = eval { Date::Language->new($info{'name'}) } ||
-           Date::Language->new(); # fall back to English
   # prototype here to silence warnings
-  $invoice_data{'time2str'} = sub ($;$$) { $dh->time2str(@_) };
-  # eventually use this date handle everywhere in here, too
+  $invoice_data{'time2str'} = sub ($;$$) { $self->time2str_local(@_, $format) };
 
   my $min_sdate = 999999999999;
   my $max_edate = 0;
@@ -520,8 +549,10 @@ sub print_generic {
   }
 
   $invoice_data{'bill_period'} = '';
-  $invoice_data{'bill_period'} = time2str('%e %h', $min_sdate) 
-    . " to " . time2str('%e %h', $max_edate)
+  $invoice_data{'bill_period'} =
+      $self->time2str_local('%e %h', $min_sdate, $format) 
+      . " to " .
+      $self->time2str_local('%e %h', $max_edate, $format)
     if ($max_edate != 0 && $min_sdate != 999999999999);
 
   $invoice_data{finance_section} = '';
@@ -536,11 +567,14 @@ sub print_generic {
   my $countrydefault = $conf->config('countrydefault') || 'US';
   foreach ( qw( address1 address2 city state zip country fax) ){
     my $method = 'ship_'.$_;
-    $invoice_data{"ship_$_"} = _latex_escape($cust_main->$method);
+    $invoice_data{"ship_$_"} = $escape_function->($cust_main->$method);
   }
-  foreach ( qw( contact company ) ) { #compatibility
-    $invoice_data{"ship_$_"} = _latex_escape($cust_main->$_);
+  if ( length($cust_main->ship_company) ) {
+    $invoice_data{'ship_company'} = $escape_function->($cust_main->ship_company);
+  } else {
+    $invoice_data{'ship_company'} = $escape_function->($cust_main->company);
   }
+  $invoice_data{'ship_contact'} = $escape_function->($cust_main->contact);
   $invoice_data{'ship_country'} = ''
     if ( $invoice_data{'ship_country'} eq $countrydefault );
   
@@ -608,23 +642,12 @@ sub print_generic {
   # summary formats
   $invoice_data{'last_bill'} = {};
 
-  # returns the last unpaid bill, not the last bill
-  #my $last_bill = $pr_cust_bill[-1];
-
   if ( $self->custnum && $self->invnum ) {
 
-    # THIS returns the customer's last bill before  this one
-    my $last_bill = qsearchs({
-        'table'   => 'cust_bill',
-        'hashref' => { 'custnum' => $self->custnum,
-                       'invnum'  => { op => '<', value => $self->invnum },
-                     },
-        'order_by'  => ' ORDER BY invnum DESC LIMIT 1'
-    });
-    if ( $last_bill ) {
+    if ( $self->previous_bill ) {
+      my $last_bill = $self->previous_bill;
       $invoice_data{'last_bill'} = {
         '_date'     => $last_bill->_date, #unformatted
-        # all we need for now
       };
       my (@payments, @credits);
       # for formats that itemize previous payments
@@ -637,7 +660,7 @@ sub print_generic {
         next if $cust_pay->_date > $self->_date;
         push @payments, {
             '_date'       => $cust_pay->_date,
-            'date'        => time2str($date_format, $cust_pay->_date),
+            'date'        => $self->time2str_local('long', $cust_pay->_date, $format),
             'payinfo'     => $cust_pay->payby_payinfo_pretty,
             'amount'      => sprintf('%.2f', $cust_pay->paid),
         };
@@ -652,7 +675,7 @@ sub print_generic {
         next if $cust_credit->_date > $self->_date;
         push @credits, {
             '_date'       => $cust_credit->_date,
-            'date'        => time2str($date_format, $cust_credit->_date),
+            'date'        => $self->time2str_local('long', $cust_credit->_date, $format),
             'creditreason'=> $cust_credit->reason,
             'amount'      => sprintf('%.2f', $cust_credit->amount),
         };
@@ -766,6 +789,7 @@ sub print_generic {
   my $taxtotal = 0;
   my $tax_section = { 'description' => $self->mt('Taxes, Surcharges, and Fees'),
                       'subtotal'    => $taxtotal,   # adjusted below
+                      'tax_section' => 1,
                     };
   my $tax_weight = _pkg_category($tax_section->{description})
                         ? _pkg_category($tax_section->{description})->weight
@@ -912,7 +936,7 @@ sub print_generic {
       my $detail = {
         ref             => $line_item->{'pkgnum'},
         pkgpart         => $line_item->{'pkgpart'},
-        quantity        => 1,
+        #quantity        => 1, # not really correct
         section         => $previous_section, # which might be $default_section
         description     => &$escape_function($line_item->{'description'}),
         ext_description => [ map { &$escape_function($_) } 
@@ -952,9 +976,6 @@ sub print_generic {
 
   foreach my $section (@sections, @$late_sections) {
 
-    warn "$me adding section \n". Dumper($section)
-      if $DEBUG > 1;
-
     # begin some normalization
     $section->{'subtotal'} = $section->{'amount'}
       if $multisection
@@ -1167,7 +1188,7 @@ sub print_generic {
         $adjust_section->{'pretotal'} = $self->mt('New charges total').' '.
           $other_money_char.  sprintf('%.2f', $self->charged );
       } 
-    }else{
+    } else {
       push @total_items, $total;
     }
     push @buf,['','-----------'];
@@ -1187,7 +1208,9 @@ sub print_generic {
     
       # credits
       my $credittotal = 0;
-      foreach my $credit ( $self->_items_credits('trim_len'=>60) ) {
+      foreach my $credit (
+        $self->_items_credits( 'template' => $template, 'trim_len' => 60 )
+      ) {
 
         my $total;
         $total->{'total_item'} = &$escape_function($credit->{'description'});
@@ -1213,13 +1236,17 @@ sub print_generic {
       $invoice_data{'credittotal'} = sprintf('%.2f', $credittotal);
 
       #credits (again)
-      foreach my $credit ( $self->_items_credits('trim_len'=>32) ) {
+      foreach my $credit (
+        $self->_items_credits( 'template' => $template, 'trim_len'=>32 )
+      ) {
         push @buf, [ $credit->{'description'}, $money_char.$credit->{'amount'} ];
       }
 
       # payments
       my $paymenttotal = 0;
-      foreach my $payment ( $self->_items_payments ) {
+      foreach my $payment (
+        $self->_items_payments( 'template' => $template )
+      ) {
         my $total = {};
         $total->{'total_item'} = &$escape_function($payment->{'description'});
         $paymenttotal += $payment->{'amount'};
@@ -1554,12 +1581,9 @@ sub print_html {
   my %params;
   if ( ref($_[0]) ) {
     %params = %{ shift() }; 
-  }else{
-    $params{'time'} = shift;
-    $params{'template'} = shift;
-    $params{'cid'} = shift;
+  } else {
+    %params = @_;
   }
-
   $params{'format'} = 'html';
   
   $self->print_generic( %params );
@@ -1682,7 +1706,7 @@ sub due_date {
 
 sub due_date2str {
   my $self = shift;
-  $self->due_date ? time2str(shift, $self->due_date) : '';
+  $self->due_date ? $self->time2str_local(shift, $self->due_date) : '';
 }
 
 sub balance_due_msg {
@@ -1691,7 +1715,7 @@ sub balance_due_msg {
   return $msg unless $self->terms;
   if ( $self->due_date ) {
     $msg .= ' - ' . $self->mt('Please pay by'). ' '.
-      $self->due_date2str($date_format);
+      $self->due_date2str('short');
   } elsif ( $self->terms ) {
     $msg .= ' - '. $self->terms;
   }
@@ -1704,7 +1728,7 @@ sub balance_due_date {
   my $duedate = '';
   if (    $conf->exists('invoice_default_terms') 
        && $conf->config('invoice_default_terms')=~ /^\s*Net\s*(\d+)\s*$/ ) {
-    $duedate = time2str($rdate_format, $self->_date + ($1*86400) );
+    $duedate = $self->time2str_local('rdate', $self->_date + ($1*86400) );
   }
   $duedate;
 }
@@ -1722,7 +1746,7 @@ Returns a string with the date, for example: "3/20/2008"
 
 sub _date_pretty {
   my $self = shift;
-  time2str($date_format, $self->_date);
+  $self->time2str_local('short', $self->_date);
 }
 
 =item _items_sections OPTIONS
@@ -2428,6 +2452,8 @@ sub _items_cust_bill_pkg {
 
         warn "$me _items_cust_bill_pkg cust_bill_pkg is quotation_pkg\n"
           if $DEBUG > 1;
+        # quotation_pkgs are never fees, so don't worry about the case where
+        # part_pkg is undefined
 
         if ( $cust_bill_pkg->setup != 0 ) {
           my $description = $desc;
@@ -2447,12 +2473,13 @@ sub _items_cust_bill_pkg {
           };
         }
 
-      } elsif ( $cust_bill_pkg->pkgnum > 0 ) {
+      } elsif ( $cust_bill_pkg->pkgnum > 0 ) { # and it's not a quotation_pkg
 
         warn "$me _items_cust_bill_pkg cust_bill_pkg is non-tax\n"
           if $DEBUG > 1;
  
         my $cust_pkg = $cust_bill_pkg->cust_pkg;
+        my $part_pkg = $cust_pkg->part_pkg;
 
         # which pkgpart to show for display purposes?
         my $pkgpart = $cust_bill_pkg->pkgpart_override || $cust_pkg->pkgpart;
@@ -2461,7 +2488,7 @@ sub _items_cust_bill_pkg {
         # things with them
         my %item_dates = ();
         %item_dates = map { $_ => $cust_bill_pkg->$_ } ('sdate', 'edate')
-          unless $cust_pkg->part_pkg->option('disable_line_item_date_ranges',1);
+          unless $part_pkg->option('disable_line_item_date_ranges',1);
 
         if (    (!$type || $type eq 'S')
              && (    $cust_bill_pkg->setup != 0
@@ -2479,6 +2506,14 @@ sub _items_cust_bill_pkg {
             || $discount_show_always
             || $cust_bill_pkg->recur_show_zero;
 
+          $description .= $cust_bill_pkg->time_period_pretty( $part_pkg,
+                                                              $self->agentnum )
+            if $part_pkg->is_prepaid #for prepaid, "display the validity period
+                                     # triggered by the recurring charge freq
+                                     # (RT#26274)
+            && $cust_bill_pkg->recur == 0
+            && ! $cust_bill_pkg->recur_show_zero;
+
           my @d = ();
           my $svc_label;
           unless ( $cust_pkg->part_pkg->hide_svc_detail
@@ -2542,42 +2577,16 @@ sub _items_cust_bill_pkg {
             if $DEBUG > 1;
 
           my $is_summary = $display->summary;
-          my $description = ($is_summary && $type && $type eq 'U')
-                            ? "Usage charges" : $desc;
+          my $description = $desc;
+          if ( $type eq 'U' and defined($r) ) {
+            # don't just show the same description as the recur line
+            $description = $self->mt('Usage charges');
+          }
 
           my $part_pkg = $cust_pkg->part_pkg;
 
-          #pry be a bit more efficient to look some of this conf stuff up
-          # outside the loop
-          unless (
-            $conf->exists('disable_line_item_date_ranges')
-              || $part_pkg->option('disable_line_item_date_ranges',1)
-              || ! $cust_bill_pkg->sdate
-              || ! $cust_bill_pkg->edate
-          ) {
-            my $time_period;
-            my $date_style = '';
-            $date_style = $conf->config( 'cust_bill-line_item-date_style-non_monhtly',
-                                         $self->agentnum
-                                       )
-              if $part_pkg && $part_pkg->freq !~ /^1m?$/;
-            $date_style ||= $conf->config( 'cust_bill-line_item-date_style',
-                                            $self->agentnum
-                                         );
-            if ( defined($date_style) && $date_style eq 'month_of' ) {
-              $time_period = time2str('The month of %B', $cust_bill_pkg->sdate);
-            } elsif ( defined($date_style) && $date_style eq 'X_month' ) {
-              my $desc = $conf->config( 'cust_bill-line_item-date_description',
-                                         $self->agentnum
-                                      );
-              $desc .= ' ' unless $desc =~ /\s$/;
-              $time_period = $desc. time2str('%B', $cust_bill_pkg->sdate);
-            } else {
-              $time_period =      time2str($date_format, $cust_bill_pkg->sdate).
-                           " - ". time2str($date_format, $cust_bill_pkg->edate);
-            }
-            $description .= " ($time_period)";
-          }
+          $description .= $cust_bill_pkg->time_period_pretty( $part_pkg,
+                                                              $self->agentnum );
 
           my @d = ();
           my @seconds = (); # for display of usage info
@@ -2590,10 +2599,19 @@ sub _items_cust_bill_pkg {
           push @dates, $prev->sdate if $prev;
           push @dates, undef if !$prev;
 
+          # show service labels, unless...
+                    # the package is set not to display them
           unless ( $part_pkg->hide_svc_detail
+                    # or this is a tax-like line item
                 || $cust_bill_pkg->itemdesc
+                    # or this is a hidden (bundled) line item
                 || $cust_bill_pkg->hidden
+                    # or this is a usage summary line
                 || $is_summary && $type && $type eq 'U'
+                    # or this is a usage line and there's a recurring line
+                    # for the package in the same section (which will 
+                    # have service labels already)
+                || ($type eq 'U' and defined($r))
               )
           {
 
@@ -2601,7 +2619,7 @@ sub _items_cust_bill_pkg {
               if $DEBUG > 1;
 
             my @svc_labels = map &{$escape_function}($_),
-                        $cust_pkg->h_labels_short($self->_date, undef, 'I');
+                        $cust_pkg->h_labels_short(@dates, 'I');
             push @d, @svc_labels
               unless $cust_bill_pkg->pkgpart_override; #don't redisplay services
             $svc_label = $svc_labels[0];
@@ -2641,7 +2659,7 @@ sub _items_cust_bill_pkg {
               }
             } #if svc_acct-usage_seconds
 
-          }
+          } # if we are showing service labels
 
           unless ( $is_summary ) {
             warn "$me _items_cust_bill_pkg adding details\n"
@@ -2667,15 +2685,15 @@ sub _items_cust_bill_pkg {
             $amount = $cust_bill_pkg->usage;
           }
   
-          my $unit_amount =
-            ( $cust_bill_pkg->unitrecur > 0 ) ? $cust_bill_pkg->unitrecur
-                                              : $amount;
-
           if ( !$type || $type eq 'R' ) {
 
             warn "$me _items_cust_bill_pkg adding recur\n"
               if $DEBUG > 1;
 
+            my $unit_amount =
+              ( $cust_bill_pkg->unitrecur > 0 ) ? $cust_bill_pkg->unitrecur
+                                                : $amount;
+
             if ( $cust_bill_pkg->hidden ) {
               $r->{amount}      += $amount;
               $r->{unit_amount} += $unit_amount;
@@ -2701,50 +2719,43 @@ sub _items_cust_bill_pkg {
             warn "$me _items_cust_bill_pkg adding usage\n"
               if $DEBUG > 1;
 
-            if ( $cust_bill_pkg->hidden ) {
+            if ( $cust_bill_pkg->hidden and defined($u) ) {
+              # if this is a hidden package and there's already a usage
+              # line for the bundle, add this package's total amount and
+              # usage details to it
               $u->{amount}      += $amount;
-              $u->{unit_amount} += $unit_amount,
               push @{ $u->{ext_description} }, @d;
-            } else {
+            } elsif ( $amount ) {
+              # create a new usage line
               $u = {
                 description     => $description,
                 pkgpart         => $pkgpart,
                 pkgnum          => $cust_bill_pkg->pkgnum,
                 amount          => $amount,
                 recur_show_zero => $cust_bill_pkg->recur_show_zero,
-                unit_amount     => $unit_amount,
-                quantity        => $cust_bill_pkg->quantity,
                 %item_dates,
                 ext_description => \@d,
               };
-            }
+            } # else this has no usage, so don't create a usage section
           }
 
         } # recurring or usage with recurring charge
 
-      } else { #pkgnum tax or one-shot line item (??)
+      } else { # taxes and fees
 
         warn "$me _items_cust_bill_pkg cust_bill_pkg is tax\n"
           if $DEBUG > 1;
 
-        if ( $cust_bill_pkg->setup != 0 ) {
-          push @b, {
-            'description' => $desc,
-            'amount'      => sprintf("%.2f", $cust_bill_pkg->setup),
-          };
-        }
-        if ( $cust_bill_pkg->recur != 0 ) {
-          push @b, {
-            'description' => "$desc (".
-                             time2str($date_format, $cust_bill_pkg->sdate). ' - '.
-                             time2str($date_format, $cust_bill_pkg->edate). ')',
-            'amount'      => sprintf("%.2f", $cust_bill_pkg->recur),
-          };
-        }
+        # items of this kind should normally not have sdate/edate.
+        push @b, {
+          'description' => $desc,
+          'amount'      => sprintf('%.2f', $cust_bill_pkg->setup 
+                                           + $cust_bill_pkg->recur)
+        };
 
-      }
+      } # if quotation / package line item / other line item
 
-    }
+    } # foreach $display
 
     $discount_show_always = ($cust_bill_pkg->cust_bill_pkg_discount
                                 && $conf->exists('discount-show-always'));
@@ -2754,8 +2765,11 @@ sub _items_cust_bill_pkg {
   foreach ( $s, $r, ($opt{skip_usage} ? () : $u ) ) {
     if ( $_  ) {
       $_->{amount}      = sprintf( "%.2f", $_->{amount} ),
+        if exists($_->{amount});
       $_->{amount}      =~ s/^\-0\.00$/0.00/;
-      $_->{unit_amount} = sprintf( "%.2f", $_->{unit_amount} ),
+      $_->{unit_amount} = sprintf('%.2f', $_->{unit_amount})
+        if exists($_->{unit_amount});
+
       push @b, { %$_ }
         if $_->{amount} != 0
         || $discount_show_always