remove duplicate cust_bill_pkg creation RT#3919

9 files changed, 98 insertions, 151 deletions

diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm
index 08b299fba..11286a89a 100644
--- a/FS/FS/Conf.pm
+++ b/FS/FS/Conf.pm
@@ -866,6 +866,13 @@ worry that config_items is freeside-specific and icky.
     'type'        => 'checkbox',
   },
 
+  { 
+    'key'         => 'separate_usage',
+    'section'     => 'billing',
+    'description' => 'Split the rated call usage into a separate line from the recurring charges.',
+    'type'        => 'checkbox',
+  },
+
   {
     'key'         => 'payment_receipt_email',
     'section'     => 'billing',
diff --git a/FS/FS/Report/Table/Monthly.pm b/FS/FS/Report/Table/Monthly.pm
index 0b8fca975..d75f0be79 100644
--- a/FS/FS/Report/Table/Monthly.pm
+++ b/FS/FS/Report/Table/Monthly.pm
@@ -329,7 +329,6 @@ sub cust_bill_pkg {
         LEFT JOIN cust_pkg USING ( pkgnum )
         LEFT JOIN part_pkg USING ( pkgpart )
       WHERE pkgnum != 0
-        AND ( duplicate IS NULL OR duplicate = '' )
         AND $where
         AND ". $self->in_time_period_and_agent($speriod, $eperiod, $agentnum)
   );
diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm
index 3274d3875..6b2bcd675 100644
--- a/FS/FS/cust_bill.pm
+++ b/FS/FS/cust_bill.pm
@@ -2555,6 +2555,15 @@ sub _items_sections {
     if ( $cust_bill_pkg->pkgnum > 0 ) {
 
       my $desc = $cust_bill_pkg->section;
+      my $dup_desc = $cust_bill_pkg->duplicate_section;
+
+      if ($cust_bill_pkg->duplicate) {
+        $s{$dup_desc} += $cust_bill_pkg->setup
+          if ( $cust_bill_pkg->setup != 0 );
+
+        $s{$dup_desc} += $cust_bill_pkg->recur
+          if ( $cust_bill_pkg->recur != 0 );
+      }
 
       if ( $cust_bill_pkg->post_total ) {
         $l{$desc} += $cust_bill_pkg->setup
@@ -2634,11 +2643,12 @@ sub _items_previous {
 sub _items_pkg {
   my $self = shift;
   my %options = @_;
-  my $section = delete $options{'section'};
+  my $section = $options{'section'};
+  my $desc = $section->{'description'};
   my @cust_bill_pkg =
     grep { $_->pkgnum &&
            ( defined($section)
-               ? $_->section eq $section->{'description'}
+               ? ( $_->section eq $desc || $_->duplicate_section eq $desc )
                : 1
            )
          } $self->cust_bill_pkg;
@@ -2671,6 +2681,7 @@ sub _items_cust_bill_pkg {
   my $unsquelched = $opt{unsquelched} || '';
 
   my @b = ();
+  my $last_pkgnum = '';
   foreach my $cust_bill_pkg ( grep { $unsquelched ? 1 : ! $_->separate_cdr }
                               @$cust_bill_pkg
                             )
@@ -2706,11 +2717,19 @@ sub _items_cust_bill_pkg {
           quantity        => $cust_bill_pkg->quantity,
           ext_description => \@d,
         };
+
+        $last_pkgnum = '';
+
       }
 
       if ( $cust_bill_pkg->recur != 0 ) {
 
-        my $description = $desc;
+        my $is_summary =
+          ( $cust_bill_pkg->duplicate && 
+            $opt{section}->{description} ne $cust_bill_pkg->section
+          );
+        my $description = $is_summary ? "Usage charges" : $desc;
+
         unless ( $conf->exists('disable_line_item_date_ranges') ) {
           $description .= " (" . time2str("%x", $cust_bill_pkg->sdate).
                           " - ". time2str("%x", $cust_bill_pkg->edate). ")";
@@ -2718,23 +2737,42 @@ sub _items_cust_bill_pkg {
 
         #at least until cust_bill_pkg has "past" ranges in addition to
         #the "future" sdate/edate ones... see #3032
-        my @d = map &{$escape_function}($_),
-                    $cust_pkg->h_labels_short($self->_date);
+        my @d = ();
+        push @d, map &{$escape_function}($_),
+                       $cust_pkg->h_labels_short($self->_date)
                                               #$cust_bill_pkg->edate,
                                               #$cust_bill_pkg->sdate),
-        @d = () if $cust_bill_pkg->itemdesc;
-        push @d, $cust_bill_pkg->details(%details_opt);
+          unless ($cust_bill_pkg->pkgnum eq $last_pkgnum);
 
-        push @b, {
-          description     => $description,
-          #pkgpart         => $part_pkg->pkgpart,
-          pkgnum          => $cust_bill_pkg->pkgnum,
-          amount          => sprintf("%.2f", $cust_bill_pkg->recur),
-          unit_amount     => sprintf("%.2f", $cust_bill_pkg->unitrecur),
-          quantity        => $cust_bill_pkg->quantity,
-          ext_description => \@d,
-        };
+        @d = () if ($cust_bill_pkg->itemdesc || $is_summary);
+        push @d, $cust_bill_pkg->details(%details_opt)
+          unless $is_summary;
+
+        if ($cust_bill_pkg->pkgnum eq $last_pkgnum) {
+
+          $b[$#b]->{amount} =
+            sprintf("%.2f", $b[$#b]->{amount} + $cust_bill_pkg->recur);
+          push @{$b[$#b]->{ext_description}}, @d;
 
+        }else{
+
+          push @b, {
+            description     => $description,
+            #pkgpart         => $part_pkg->pkgpart,
+            pkgnum          => $cust_bill_pkg->pkgnum,
+            amount          => sprintf("%.2f", $cust_bill_pkg->recur),
+            unit_amount     => sprintf("%.2f", $cust_bill_pkg->unitrecur),
+            quantity        => $cust_bill_pkg->quantity,
+            ext_description => \@d,
+          };
+
+        }
+
+        if ($conf->exists('separate_usage') && $cust_bill_pkg->type ne 'U') {
+          $last_pkgnum = '';
+        }else{
+          $last_pkgnum = $cust_bill_pkg->pkgnum;
+        }
       }
 
     } else { #pkgnum tax or one-shot line item (??)
@@ -2754,6 +2792,8 @@ sub _items_cust_bill_pkg {
         };
       }
 
+      $last_pkgnum = '';
+
     }
 
   }
diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index 6925de8b3..aa37ac4b5 100644
--- a/FS/FS/cust_bill_pkg.pm
+++ b/FS/FS/cust_bill_pkg.pm
@@ -61,8 +61,7 @@ supported:
 
 =item section - Invoice section (overrides normal package section)
 
-=duplicate - Indicates this item appears elsewhere on the invoice
-             (and should not be retaxed or reincluded in totals)
+=duplicate - Indicates this item is a candidate for summarizing and duplicating at print time
 
 =post_total - A hint that this item should appear after invoice totals
 
@@ -77,6 +76,11 @@ sub section {
   }
 }
 
+sub duplicate_section {
+  my $self = shift;
+  $self->duplicate ? $self->part_pkg->categoryname : '';
+}
+
 =item quantity - If not set, defaults to 1
 
 =item unitsetup - If not set, defaults to setup
@@ -375,7 +379,7 @@ sub owed_recur {
 # modeled after cust_bill::owed...
 sub owed {
   my( $self, $field ) = @_;
-  my $balance = $self->duplicate ? 0 : $self->$field();
+  my $balance = $self->$field();
   $balance -= $_->amount foreach ( $self->cust_bill_pay_pkg($field) );
   $balance -= $_->amount foreach ( $self->cust_credit_bill_pkg($field) );
   $balance = sprintf( '%.2f', $balance );
diff --git a/FS/FS/cust_main.pm b/FS/FS/cust_main.pm
index 718fccf2f..e9e21b80b 100644
--- a/FS/FS/cust_main.pm
+++ b/FS/FS/cust_main.pm
@@ -2068,7 +2068,6 @@ sub bill {
   $self->select_for_update; #mutex
 
   my @cust_bill_pkg = ();
-  my @appended_cust_bill_pkg = ();
 
   ###
   # find the packages which are due for billing, find out how much they are
@@ -2107,7 +2106,6 @@ sub bill {
                             'cust_pkg'            => $cust_pkg,
                             'precommit_hooks'     => \@precommit_hooks,
                             'line_items'          => \@cust_bill_pkg,
-                            'appended_line_items' => \@appended_cust_bill_pkg,
                             'setup'               => \$total_setup,
                             'recur'               => \$total_recur,
                             'tax_matrix'          => \%taxlisthash,
@@ -2123,8 +2121,6 @@ sub bill {
 
   } #foreach my $cust_pkg
 
-  push @cust_bill_pkg, @appended_cust_bill_pkg;
-
   unless ( @cust_bill_pkg ) { #don't create an invoice w/o line items
     #but do commit any package date cycling that happened
     $dbh->commit or die $dbh->errstr if $oldAutoCommit;
@@ -2144,7 +2140,6 @@ sub bill {
                             'cust_pkg'            => $postal_pkg,
                             'precommit_hooks'     => \@precommit_hooks,
                             'line_items'          => \@cust_bill_pkg,
-                            'appended_line_items' => \@appended_cust_bill_pkg,
                             'setup'               => \$total_setup,
                             'recur'               => \$total_recur,
                             'tax_matrix'          => \%taxlisthash,
@@ -2296,8 +2291,6 @@ sub _make_lines {
   my $cust_pkg = $params{cust_pkg} or die "no cust_pkg specified";
   my $precommit_hooks = $params{precommit_hooks} or die "no package specified";
   my $cust_bill_pkgs = $params{line_items} or die "no line buffer specified";
-  my $appended_cust_bill_pkg = $params{appended_line_items}
-    or die "no appended line buffer specified";
   my $total_setup = $params{setup} or die "no setup accumulator specified";
   my $total_recur = $params{recur} or die "no recur accumulator specified";
   my $taxlisthash = $params{tax_matrix} or die "no tax accumulator specified";
@@ -2448,6 +2441,7 @@ sub _make_lines {
 
       warn "    charges (setup=$setup, recur=$recur); adding line items\n"
         if $DEBUG > 1;
+
       my $cust_bill_pkg = new FS::cust_bill_pkg {
         'pkgnum'    => $cust_pkg->pkgnum,
         'setup'     => $setup,
@@ -2469,61 +2463,19 @@ sub _make_lines {
       # handle taxes
       ###
 
-      unless ( $self->tax =~ /Y/i || $self->payby eq 'COMP' ) {
-
-        #some garbage disappears on cust_bill_pkg refactor
-        my $err_or_cust_bill_pkg =
-          $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg);
-
-        return $err_or_cust_bill_pkg
-          unless ( ref($err_or_cust_bill_pkg) );
+      my $err_or_cust_bill_pkg =
+        $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg, $cust_pkg);
 
-        push @$cust_bill_pkgs, @$err_or_cust_bill_pkg;
+      return $err_or_cust_bill_pkg
+        unless ( ref($err_or_cust_bill_pkg) );
 
-      } #unless $self->tax =~ /Y/i || $self->payby eq 'COMP'
+      push @$cust_bill_pkgs, @$err_or_cust_bill_pkg;
 
     } #if $setup != 0 || $recur != 0
       
   } #if $line_items
 
-  if ( $part_pkg->can('append_cust_bill_pkgs') ) {
-    my %param = ( 'precommit_hooks' => $precommit_hooks, );
-    my ($more_cust_bill_pkgs) =
-      eval { $part_pkg->append_cust_bill_pkgs( $cust_pkg, \$sdate, \%param ) };
-
-    return "$@ running append_cust_bill_pkgs for $cust_pkg\n"
-      if ( $@ );
-    return "$more_cust_bill_pkgs"
-      unless ( ref($more_cust_bill_pkgs) );
-
-    foreach my $cust_bill_pkg ( @{$more_cust_bill_pkgs} ) {
-
-      $cust_bill_pkg->pkgpart_override($part_pkg->pkgpart)
-        unless $part_pkg->pkgpart == $real_pkgpart;
-
-      unless ($cust_bill_pkg->duplicate) {
-        $$total_setup += $cust_bill_pkg->setup;
-        $$total_recur += $cust_bill_pkg->recur;
-
-        ###
-        # handle taxes
-        ###
-
-        unless ( $self->tax =~ /Y/i || $self->payby eq 'COMP' ) {
-
-          #some garbage disappears on cust_bill_pkg refactor
-          my $err_or_cust_bill_pkg =
-            $self->_handle_taxes($part_pkg, $taxlisthash, $cust_bill_pkg);
-
-          return $err_or_cust_bill_pkg
-            unless ( ref($err_or_cust_bill_pkg) );
-
-          push @$appended_cust_bill_pkg, @$err_or_cust_bill_pkg;
-
-        } #unless $self->tax =~ /Y/i || $self->payby eq 'COMP'
-      }
-    }
-  }
+  '';
 
 }
 
@@ -2532,6 +2484,7 @@ sub _handle_taxes {
   my $part_pkg = shift;
   my $taxlisthash = shift;
   my $cust_bill_pkg = shift;
+  my $cust_pkg = shift;
 
   my %cust_bill_pkg = ();
   my %taxes = ();
@@ -2549,6 +2502,7 @@ sub _handle_taxes {
 
   if ( $conf->exists('enable_taxproducts')
        && (scalar($part_pkg->part_pkg_taxoverride) || $part_pkg->has_taxproduct)
+       && ( $self->tax !~ /Y/i && $self->payby ne 'COMP' )
      )
   { 
 
@@ -2564,7 +2518,7 @@ sub _handle_taxes {
       $taxes{''} = $err_or_ref;
     }
 
-  }else{
+  }elsif ( $self->tax !~ /Y/i && $self->payby ne 'COMP' ) {
 
     my %taxhash = map { $_ => $self->get("$prefix$_") }
                       qw( state county country );
@@ -2599,10 +2553,16 @@ sub _handle_taxes {
                   $part_pkg->taxclass ). "\n";
     }
 
-  } #if $conf->exists('enable_taxproducts') 
-
-  # XXX all this goes away with cust_bill_pay refactor
+  } #if $conf->exists('enable_taxproducts') ...
+ 
+  my $section = $cust_pkg->part_pkg->option('usage_section', 'Hush!')
+    if $cust_pkg->part_pkg->option('separate_usage');
+  my $want_duplicate =
+    $cust_pkg->part_pkg->option('summarize_usage', 'Hush!') &&
+    $cust_pkg->part_pkg->option('usage_section', 'Hush!');
 
+  # XXX this mostly goes away with cust_bill_pkg refactor
+ 
   $cust_bill_pkg{setup} = $cust_bill_pkg if $cust_bill_pkg->setup;
   $cust_bill_pkg{recur} = $cust_bill_pkg if $cust_bill_pkg->recur;
     
@@ -2626,6 +2586,9 @@ sub _handle_taxes {
         new FS::cust_bill_pkg { $cust_bill_pkg{recur}->hash };
     $cust_bill_pkg_usage->recur( $usage );
     $cust_bill_pkg_usage->type( 'U' );
+    $cust_bill_pkg_usage->duplicate( $want_duplicate ? 'Y' :  '' );
+    $cust_bill_pkg_usage->section( $section );
+    $cust_bill_pkg_usage->post_total( $want_duplicate ? 'Y' :  '' );
     my $recur = sprintf( "%.2f", $cust_bill_pkg{recur}->recur - $usage );
     $cust_bill_pkg{recur}->recur( $recur );
     $cust_bill_pkg{recur}->type( '' );
diff --git a/FS/FS/part_pkg/voip_cdr.pm b/FS/FS/part_pkg/voip_cdr.pm
index 10257056d..bd73ea604 100644
--- a/FS/FS/part_pkg/voip_cdr.pm
+++ b/FS/FS/part_pkg/voip_cdr.pm
@@ -85,10 +85,6 @@ tie my %rating_method, 'Tie::IxHash',
                          'select_options' => { FS::cdr::invoice_formats() },
                        },
 
-    'separate_usage' => { 'name' => 'Separate usage charges from recurring charges',
-                          'type' => 'checkbox',
-                        },
-
     'usage_section' => { 'name' => 'Section in which to place separate usage charges',
                        },
 
@@ -127,7 +123,7 @@ tie my %rating_method, 'Tie::IxHash',
                        disable_src
                        domestic_prefix international_prefix
                        use_amaflags use_disposition output_format
-                       separate_usage summarize_usage usage_section
+                       summarize_usage usage_section
                      )
                   ],
   'weight' => 40,
@@ -138,16 +134,8 @@ sub calc_setup {
   $self->option('setup_fee');
 }
 
-sub calc_recur {
-  my $self = shift;
-  my $charges = 0;
-  $charges = $self->calc_usage(@_)
-    unless $self->option('separate_usage', 'Hush!');
-  $self->option('recur_fee') + $charges;
-}
-
 #false laziness w/voip_sqlradacct calc_recur resolve it if that one ever gets used again
-sub calc_usage {
+sub calc_recur {
   my($self, $cust_pkg, $sdate, $details, $param ) = @_;
 
   my $last_bill = $cust_pkg->last_bill;
@@ -452,7 +440,7 @@ sub calc_usage {
 
   } #if ( $spool_cdr && length($downstream_cdr) )
 
-  $charges;
+  $self->option('recur_fee') + $charges;
 
 }
 
@@ -472,55 +460,5 @@ sub calc_units {
   scalar(grep { $_->part_svc->svcdb eq 'svc_phone' } $cust_pkg->cust_svc);
 }
 
-sub append_cust_bill_pkgs {
-  my $self = shift;
-  my($cust_pkg, $sdate, $details, $param ) = @_;
-  return []
-    unless $self->option('separate_usage', 'Hush!');
-
-  my @details = ();
-  my $charges = $self->calc_usage($cust_pkg, $sdate, \@details, $param);
-
-  return []
-    unless $charges;  # unless @details?
-
-  my @cust_bill_pkg = ();
-
-  my $want_summary = $self->option('summarize_usage', 'Hush!') &&
-                     $self->option('usage_section', 'Hush!');
-
-  push @cust_bill_pkg, new FS::cust_bill_pkg {
-    'pkgnum'    => $cust_pkg->pkgnum,
-    'setup'     => 0,
-    'unitsetup' => 0,
-    'recur'     => sprintf( "%.2f", $charges),  # hmmm
-    'unitrecur' => 0,
-    'quantity'  => $cust_pkg->quantity,
-    'sdate'     => $$sdate,
-    'edate'     => $cust_pkg->bill,             # already fiddled
-    'itemdesc'  => 'Usage charges',             # configurable?
-    'duplicate' => 'Y',
-  }
-    if $want_summary;
-
-  push @cust_bill_pkg, new FS::cust_bill_pkg {
-    'pkgnum'     => $cust_pkg->pkgnum,
-    'setup'      => 0,
-    'unitsetup ' => 0,
-    'recur'      => sprintf( "%.2f", $charges),  # hmmm
-    'unitrecur ' => 0,
-    'quantity'   => $cust_pkg->quantity,
-    'sdate'      => $$sdate,
-    'edate'      => $cust_pkg->bill,             # already fiddled
-    'itemdesc'   => 'Usage charges',             # configurable?
-    'section'    => $self->option('usage_section', 'Hush!'),
-    'details'    => \@details,
-    'post_total' => ( $want_summary ? 'Y' : '' ),
-  };
-
-
-  return [ @cust_bill_pkg ];
-}
-
 1;
 
diff --git a/httemplate/search/cust_bill_pkg.cgi b/httemplate/search/cust_bill_pkg.cgi
index 2354be953..74efe4f7e 100644
--- a/httemplate/search/cust_bill_pkg.cgi
+++ b/httemplate/search/cust_bill_pkg.cgi
@@ -70,8 +70,6 @@ my $agentnums_sql =
 
 my @where = ( $agentnums_sql );
 
-push @where, "(duplicate IS NULL OR duplicate = '' )";
-
 my($beginning, $ending) = FS::UI::Web::parse_beginning_ending($cgi);
 push @where, "_date >= $beginning",
              "_date <= $ending";
diff --git a/httemplate/search/report_prepaid_income.cgi b/httemplate/search/report_prepaid_income.cgi
index 3694c7aae..27dbcbf9f 100644
--- a/httemplate/search/report_prepaid_income.cgi
+++ b/httemplate/search/report_prepaid_income.cgi
@@ -43,9 +43,8 @@ my( $total, $total_legacy ) = ( 0, 0 );
 my @cust_bill_pkg =
   grep { $_->cust_pkg && $_->cust_pkg->part_pkg->freq !~ /^([01]|\d+[dw])$/ }
     qsearch( 'cust_bill_pkg', {
-                                'recur'     => { op=>'!=', value=>0 },
-                                'edate'     => { op=>'>', value=>$now },
-                                'duplicate' => '',
+                                'recur' => { op=>'!=', value=>0 },
+                                'edate' => { op=>'>', value=>$now },
                               }, );
 
 my @cust_pkg = 
diff --git a/httemplate/search/report_tax.cgi b/httemplate/search/report_tax.cgi
index dd6779a61..3d37c47e4 100755
--- a/httemplate/search/report_tax.cgi
+++ b/httemplate/search/report_tax.cgi
@@ -188,8 +188,7 @@ my $join_pkg = "
     LEFT JOIN part_pkg USING ( pkgpart )
 ";
 
-my $where = "WHERE _date >= $beginning AND _date <= $ending ".
-            "AND (duplicate IS NULL or duplicate = '')";
+my $where = "WHERE _date >= $beginning AND _date <= $ending ";
 my @base_param = qw( county county state state country );
 if ( $conf->exists('tax-ship_address') ) {