don't leave quotation side effects around, eek, RT#79310, RT#32489

[freeside.git] / FS / FS / quotation.pm

diff --git a/FS/FS/quotation.pm b/FS/FS/quotation.pm
index 60abd38..a3f0612 100644 (file)
--- a/FS/FS/quotation.pm
+++ b/FS/FS/quotation.pm
@@ -5,7 +5,7 @@ use base qw( FS::Template_Mixin FS::cust_main_Mixin FS::otaker_Mixin FS::Record
 use strict;
 use Tie::RefHash;
 use FS::CurrentUser;
 use strict;
 use Tie::RefHash;
 use FS::CurrentUser;

-use FS::UID qw( dbh );
+use FS::UID qw( dbh myconnect );

 use FS::Maketext qw( emt );
 use FS::Record qw( qsearch qsearchs );
 use FS::Conf;
 use FS::Maketext qw( emt );
 use FS::Record qw( qsearch qsearchs );
 use FS::Conf;


@@ -14,6 +14,10 @@ use FS::cust_pkg;
 use FS::quotation_pkg;
 use FS::quotation_pkg_tax;
 use FS::type_pkgs;
 use FS::quotation_pkg;
 use FS::quotation_pkg_tax;
 use FS::type_pkgs;

+use List::MoreUtils;
+
+our $DEBUG = 0;
+use Data::Dumper;

 
 =head1 NAME
 
 
 =head1 NAME
 


@@ -65,6 +69,13 @@ disabled
 
 usernum
 
 
 usernum
 

+=item close_date
+
+projected date when the quotation will be closed
+
+=item confidence
+
+projected confidence (expressed as integer) that quotation will close

 
 =back
 
 
 =back
 


@@ -84,6 +95,7 @@ points to.  You can ask the object for a copy with the I<hash> method.
 sub table { 'quotation'; }
 sub notice_name { 'Quotation'; }
 sub template_conf { 'quotation_'; }
 sub table { 'quotation'; }
 sub notice_name { 'Quotation'; }
 sub template_conf { 'quotation_'; }

+sub has_sections { 1; }

 
 =item insert
 
 
 =item insert
 


@@ -117,6 +129,8 @@ sub check {
     || $self->ut_numbern('_date')
     || $self->ut_enum('disabled', [ '', 'Y' ])
     || $self->ut_numbern('usernum')
     || $self->ut_numbern('_date')
     || $self->ut_enum('disabled', [ '', 'Y' ])
     || $self->ut_numbern('usernum')

+    || $self->ut_numbern('close_date')
+    || $self->ut_numbern('confidence')

   ;
   return $error if $error;
 
   ;
   return $error if $error;
 


@@ -124,6 +138,10 @@ sub check {
 
   $self->usernum($FS::CurrentUser::CurrentUser->usernum) unless $self->usernum;
 
 
   $self->usernum($FS::CurrentUser::CurrentUser->usernum) unless $self->usernum;
 

+  return 'confidence percentage must be an integer between 1 and 100'
+    if length($self->confidence)
+    && ( ($self->confidence < 1) || ($self->confidence > 100) );
+

   return 'prospectnum or custnum must be specified'
     if ! $self->prospectnum
     && ! $self->custnum;
   return 'prospectnum or custnum must be specified'
     if ! $self->prospectnum
     && ! $self->custnum;


@@ -149,7 +167,7 @@ sub cust_bill_pkg { #actually quotation_pkg objects
 
 sub total_setup {
   my $self = shift;
 
 sub total_setup {
   my $self = shift;

-  $self->_total('setup');
+  sprintf('%.2f', $self->_total('setup') + $self->_total('setup_tax'));

 }
 
 =item total_recur [ FREQ ]
 }
 
 =item total_recur [ FREQ ]


@@ -160,14 +178,14 @@ sub total_recur {
   my $self = shift;
 #=item total_recur [ FREQ ]
   #my $freq = @_ ? shift : '';
   my $self = shift;
 #=item total_recur [ FREQ ]
   #my $freq = @_ ? shift : '';

-  $self->_total('recur');
+  sprintf('%.2f', $self->_total('recur') + $self->_total('recur_tax'));

 }
 
 sub _total {
   my( $self, $method ) = @_;
 
   my $total = 0;
 }
 
 sub _total {
   my( $self, $method ) = @_;
 
   my $total = 0;

-  $total += $_->$method() for $self->cust_bill_pkg;
+  $total += $_->$method() for $self->quotation_pkg;

   sprintf('%.2f', $total);
 
 }
   sprintf('%.2f', $total);
 
 }


@@ -209,6 +227,11 @@ sub email_subject {
   eval qq("$subject");
 }
 
   eval qq("$subject");
 }
 

+sub pdf_filename {
+  my $self = shift;
+  'Quotation-'. $self->quotationnum. '.pdf';
+}
+

 =item cust_or_prosect
 
 =cut
 =item cust_or_prosect
 
 =cut


@@ -218,7 +241,7 @@ sub cust_or_prospect {
   $self->custnum ? $self->cust_main : $self->prospect_main;
 }
 
   $self->custnum ? $self->cust_main : $self->prospect_main;
 }
 

-=item cust_or_prospect_label_link P
+=item cust_or_prospect_label_link

 
 HTML links to either the customer or prospect.
 
 
 HTML links to either the customer or prospect.
 


@@ -250,87 +273,101 @@ sub cust_or_prospect_label_link {
 
 }
 
 
 }
 

-sub _items_tax {
-  ();
-}
+sub _items_sections {
+  my $self = shift;
+  my %opt = @_;
+  my $escape = $opt{escape}; # the only one we care about

 
 

-sub _items_nontax {
-  shift->cust_bill_pkg;
-}

 
 

-sub _items_total {
-  my $self = shift;
-  $self->quotationnum =~ /^(\d+)$/ or return ();
+  my %show; # package frequency => 1 if there's anything to display
+  my %subtotals = (); # package frequency => subtotal
+  my $prorate_total = 0;
+  foreach my $pkg ($self->quotation_pkg) {

 
 

-  my @items;
+    my $part_pkg = $pkg->part_pkg;

 
 

-  # show taxes in here also; the setup/recurring breakdown is different
-  # from what Template_Mixin expects
-  my @setup_tax = qsearch({
-      select      => 'itemdesc, SUM(setup_amount) as setup_amount',
-      table       => 'quotation_pkg_tax',
-      addl_from   => ' JOIN quotation_pkg USING (quotationpkgnum) ',
-      extra_sql   => ' WHERE quotationnum = '.$1,
-      order_by    => ' GROUP BY itemdesc HAVING SUM(setup_amount) > 0' .
-                     ' ORDER BY itemdesc',
-  });
-  # recurs need to be grouped by frequency, and to have a pkgpart
-  my @recur_tax = qsearch({
-      select      => 'freq, itemdesc, SUM(recur_amount) as recur_amount, MAX(pkgpart) as pkgpart',
-      table       => 'quotation_pkg_tax',
-      addl_from   => ' JOIN quotation_pkg USING (quotationpkgnum)'.
-                     ' JOIN part_pkg USING (pkgpart)',
-      extra_sql   => ' WHERE quotationnum = '.$1,
-      order_by    => ' GROUP BY freq, itemdesc HAVING SUM(recur_amount) > 0' .
-                     ' ORDER BY freq, itemdesc',
-  });
+    my $recur_freq = $part_pkg->freq;
+    $show{$recur_freq} = 1 if $pkg->unitrecur > 0 or $pkg->recur_show_zero;
+    $show{0} = 1 if $pkg->unitsetup > 0 or $pkg->setup_show_zero;
+    ($subtotals{0} ||= 0) += $pkg->setup + $pkg->setup_tax;
+    ($subtotals{$recur_freq} ||= 0) += $pkg->recur + $pkg->recur_tax;
+
+    #this is a shitty hack based on what's in part_pkg/ at the moment
+    # but its good enough for the 99% common case of preventing totals from
+    # displaying for prorate packages
+    $prorate_total = 1
+      if $part_pkg->plan =~ /^(prorate|torrus|agent$)/
+      || $part_pkg->option('recur_method') eq 'prorate'
+      || ( $part_pkg->option('sync_bill_date')
+             && $self->custnum
+             && $self->cust_main->billing_pkgs #num_billing_pkgs when we have it
+         );
+
+    #possible improvement: keep track of flat vs. prorate totals to make the
+    # bottom range more accurate when mixing flat and prorate packages

 
 

-  my $total_setup = $self->total_setup;
-  foreach my $pkg_tax (@setup_tax) {
-    if ($pkg_tax->setup_amount > 0) {
-      $total_setup += $pkg_tax->setup_amount;
-      push @items, {
-        'total_item'    => $pkg_tax->itemdesc . ' ' . $self->mt('(setup)'),
-        'total_amount'  => $pkg_tax->setup_amount,
-      };
-    }

   }
   }

+  my @pkg_freq_order = keys %{ FS::Misc->pkg_freqs };
+
+  my @sections;
+  my $no_recurring = 0;
+  foreach my $freq (keys %subtotals) {
+
+    #next if $subtotals{$freq} == 0;
+    next if !$show{$freq};
+
+    my $weight = 
+      List::MoreUtils::first_index { $_ eq $freq } @pkg_freq_order;
+    my $desc;
+    if ( $freq eq '0' ) {
+      if ( scalar(keys(%subtotals)) == 1 ) {
+        # there are no recurring packages
+        $no_recurring = 1;
+        $desc = $self->mt('Charges');
+      } else {
+        $desc = $self->mt('Setup Charges');
+      }
+    } else { # recurring
+      $desc = $self->mt('Recurring Charges') . ' - ' .
+              ucfirst($self->mt(FS::Misc->pkg_freqs->{$freq}))
+    }

 
 

-  if ( $total_setup > 0 ) {
-    push @items, {
-      'total_item'   => $self->mt( $self->total_recur > 0 ? 'Total Setup' : 'Total' ),
-      'total_amount' => sprintf('%.2f',$total_setup),
-      'break_after'  => ( scalar(@recur_tax) ? 1 : 0 )
+    push @sections, {
+      'description' => &$escape($desc),
+      'sort_weight' => $weight,
+      'category'    => $freq,
+      'subtotal'    => sprintf('%.2f',$subtotals{$freq}),

     };
   }
 
     };
   }
 

-  #could/should add up the different recurring frequencies on lines of their own
-  # but this will cover the 95% cases for now
-  my $total_recur = $self->total_recur;
-  # label these with the frequency
-  foreach my $pkg_tax (@recur_tax) {
-    if ($pkg_tax->recur_amount > 0) {
-      $total_recur += $pkg_tax->recur_amount;
-      # an arbitrary part_pkg, but with the right frequency
-      # XXX localization
-      my $part_pkg = qsearchs('part_pkg', { pkgpart => $pkg_tax->pkgpart });
-      push @items, {
-        'total_item'    => $pkg_tax->itemdesc . ' (' .  $part_pkg->freq_pretty . ')',
-        'total_amount'  => $pkg_tax->recur_amount,
+  unless ( $no_recurring ) {
+    my $total = 0;
+    $total += $_ for values %subtotals;
+    my %total = (
+      'sort_weight' => 0,
+      'category'    => 'Total category', #required but what's it used for?
+    );
+
+    if ( $prorate_total ) {
+
+      push @sections, {
+        %total,
+        'description' => 'First payment (depending on day of month)',
+        'subtotal'    => [ $subtotals{0}, $total ],
+      };
+
+    } else {
+
+      push @sections, {
+        %total,
+        'description' => 'First payment',
+        'subtotal'    => $total,

       };
     }
       };
     }

-  }

 
 

-  if ( $total_recur > 0 ) {
-    push @items, {
-      'total_item'   => $self->mt('Total Recurring'),
-      'total_amount' => sprintf('%.2f',$total_recur),
-      'break_after'  => 1,
-    };

   }
 
   }
 

-  return @items;
-
+  return \@sections, [];

 }
 
 =item enable_previous
 }
 
 =item enable_previous


@@ -339,7 +376,7 @@ sub _items_total {
 
 sub enable_previous { 0 }
 
 
 sub enable_previous { 0 }
 

-=item convert_cust_main
+=item convert_cust_main [ PARAMS ]

 
 If this quotation already belongs to a customer, then returns that customer, as
 an FS::cust_main object.
 
 If this quotation already belongs to a customer, then returns that customer, as
 an FS::cust_main object.


@@ -351,10 +388,13 @@ packages as real packages for the customer.
 If there is an error, returns an error message, otherwise, returns the
 newly-created FS::cust_main object.
 
 If there is an error, returns an error message, otherwise, returns the
 newly-created FS::cust_main object.
 

+Accepts the same params as L</order>.
+

 =cut
 
 sub convert_cust_main {
   my $self = shift;
 =cut
 
 sub convert_cust_main {
   my $self = shift;

+  my $params = shift || {};

 
   my $cust_main = $self->cust_main;
   return $cust_main if $cust_main; #already converted, don't again
 
   my $cust_main = $self->cust_main;
   return $cust_main if $cust_main; #already converted, don't again


@@ -371,7 +411,7 @@ sub convert_cust_main {
 
   $self->prospectnum('');
   $self->custnum( $cust_main->custnum );
 
   $self->prospectnum('');
   $self->custnum( $cust_main->custnum );

-  my $error = $self->replace || $self->order;
+  my $error = $self->replace || $self->order(undef,$params);

   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;


@@ -383,7 +423,7 @@ sub convert_cust_main {
 
 }
 
 
 }
 

-=item order
+=item order [ HASHREF ] [ PARAMS ]

 
 This method is for use with quotations which are already associated with a customer.
 
 
 This method is for use with quotations which are already associated with a customer.
 


@@ -391,28 +431,91 @@ Orders this quotation's packages as real packages for the customer.
 
 If there is an error, returns an error message, otherwise returns false.
 
 
 If there is an error, returns an error message, otherwise returns false.
 

+If HASHREF is passed, it will be filled with a hash mapping the 
+C<quotationpkgnum> of each quoted package to the C<pkgnum> of the package
+as ordered.
+
+If PARAMS hashref is passed, the following params are accepted:
+
+onhold - if true, suspends newly ordered packages
+

 =cut
 
 sub order {
   my $self = shift;
 =cut
 
 sub order {
   my $self = shift;

+  my $pkgnum_map = shift || {};
+  my $params = shift || {};
+  my $details_map = {};

 
   tie my %all_cust_pkg, 'Tie::RefHash';
   foreach my $quotation_pkg ($self->quotation_pkg) {
     my $cust_pkg = FS::cust_pkg->new;
 
   tie my %all_cust_pkg, 'Tie::RefHash';
   foreach my $quotation_pkg ($self->quotation_pkg) {
     my $cust_pkg = FS::cust_pkg->new;

+    $pkgnum_map->{ $quotation_pkg->quotationpkgnum } = $cust_pkg;
+
+    # details will be copied below, after package is ordered
+    $details_map->{ $quotation_pkg->quotationpkgnum } = [ 
+      map { $_->copy_on_order ? $_->detail : () } $quotation_pkg->quotation_pkg_detail
+    ];
+

     foreach (qw(pkgpart locationnum start_date contract_end quantity waive_setup)) {
       $cust_pkg->set( $_, $quotation_pkg->get($_) );
     }
 
     foreach (qw(pkgpart locationnum start_date contract_end quantity waive_setup)) {
       $cust_pkg->set( $_, $quotation_pkg->get($_) );
     }
 

-    # currently only one discount each
-    my ($pkg_discount) = $quotation_pkg->quotation_pkg_discount;
-    if ( $pkg_discount ) {
-      $cust_pkg->set('discountnum', $pkg_discount->discountnum);
+    # can now have two discounts each (setup and recur)
+    foreach my $pkg_discount ($quotation_pkg->quotation_pkg_discount) {
+      my $field = $pkg_discount->setuprecur . '_discountnum';
+      $cust_pkg->set($field, $pkg_discount->discountnum);

     }
 
     $all_cust_pkg{$cust_pkg} = []; # no services
   }
 
     }
 
     $all_cust_pkg{$cust_pkg} = []; # no services
   }
 

-  $self->cust_main->order_pkgs( \%all_cust_pkg );
+  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;
+
+  my $error = $self->cust_main->order_pkgs( \%all_cust_pkg );
+  
+  unless ($error) {
+    # copy details (copy_on_order filtering handled above)
+    foreach my $quotationpkgnum (keys %$details_map) {
+      next unless @{$details_map->{$quotationpkgnum}};
+      $error = $pkgnum_map->{$quotationpkgnum}->set_cust_pkg_detail(
+        'I',
+        @{$details_map->{$quotationpkgnum}}
+      );
+      last if $error;
+    }
+  }
+
+  if ($$params{'onhold'}) {
+    foreach my $quotationpkgnum (keys %$pkgnum_map) {
+      last if $error;
+      $error = $pkgnum_map->{$quotationpkgnum}->suspend();
+    }
+  }
+
+  if ($error) {
+    $dbh->rollback if $oldAutoCommit;
+    return $error;
+  }
+
+  $dbh->commit or die $dbh->errstr if $oldAutoCommit;
+
+  foreach my $quotationpkgnum (keys %$pkgnum_map) {
+    # convert the objects to just pkgnums
+    my $cust_pkg = $pkgnum_map->{$quotationpkgnum};
+    $pkgnum_map->{$quotationpkgnum} = $cust_pkg->pkgnum;
+  }
+
+  ''; #no error

 
 }
 
 
 }
 


@@ -433,6 +536,7 @@ sub charge {
   my $cust_pkg_ref = '';
   my ( $bill_now, $invoice_terms ) = ( 0, '' );
   my $locationnum;
   my $cust_pkg_ref = '';
   my ( $bill_now, $invoice_terms ) = ( 0, '' );
   my $locationnum;

+  my ( $discountnum, $discountnum_amount, $discountnum_percent ) = ( '','','' );

   if ( ref( $_[0] ) ) {
     $amount     = $_[0]->{amount};
     $setup_cost = $_[0]->{setup_cost};
   if ( ref( $_[0] ) ) {
     $amount     = $_[0]->{amount};
     $setup_cost = $_[0]->{setup_cost};


@@ -452,6 +556,9 @@ sub charge {
     $bill_now = exists($_[0]->{bill_now}) ? $_[0]->{bill_now} : '';
     $invoice_terms = exists($_[0]->{invoice_terms}) ? $_[0]->{invoice_terms} : '';
     $locationnum = $_[0]->{locationnum};
     $bill_now = exists($_[0]->{bill_now}) ? $_[0]->{bill_now} : '';
     $invoice_terms = exists($_[0]->{invoice_terms}) ? $_[0]->{invoice_terms} : '';
     $locationnum = $_[0]->{locationnum};

+    $discountnum = $_->{setup_discountnum};
+    $discountnum_amount = $_->{setup_discountnum_amount};
+    $discountnum_percent = $_->{setup_discountnum_percent};

   } else {
     $amount     = shift;
     $setup_cost = '';
   } else {
     $amount     = shift;
     $setup_cost = '';


@@ -522,12 +629,15 @@ sub charge {
   # of ordering a customer package, no "bill now")
 
   my $quotation_pkg = new FS::quotation_pkg ( {
   # of ordering a customer package, no "bill now")
 
   my $quotation_pkg = new FS::quotation_pkg ( {

-    'quotationnum'  => $self->quotationnum,
-    'pkgpart'       => $pkgpart,
-    'quantity'      => $quantity,
-    #'start_date' => $start_date,
-    #'no_auto'    => $no_auto,
-    'locationnum'=> $locationnum,
+    'quotationnum'              => $self->quotationnum,
+    'pkgpart'                   => $pkgpart,
+    'quantity'                  => $quantity,
+    #'start_date'                => $start_date,
+    #'no_auto'                   => $no_auto,
+    'locationnum'               => $locationnum,
+    'setup_discountnum'         => $discountnum,
+    'setup_discountnum_amount'  => $discountnum_amount,
+    'setup_discountnum_percent' => $discountnum_percent,

   } );
 
   $error = $quotation_pkg->insert;
   } );
 
   $error = $quotation_pkg->insert;


@@ -583,126 +693,253 @@ sub estimate {
   my $self = shift;
   my $conf = FS::Conf->new;
 
   my $self = shift;
   my $conf = FS::Conf->new;
 

-  my $dbh = dbh;
-  my $oldAutoCommit = $FS::UID::AutoCommit;
-  local $FS::UID::AutoCommit = 0;
+  my %pkgnum_of; # quotationpkgnum => temporary pkgnum

 
 

-  # bring individual items up to date (set setup/recur and discounts)
-  my @quotation_pkg = $self->quotation_pkg;
-  foreach my $pkg (@quotation_pkg) {
-    my $error = $pkg->estimate;
-    if ($error) {
-      $dbh->rollback if $oldAutoCommit;
-      die "error calculating estimate for pkgpart " . $pkg->pkgpart.": $error\n";
+  my $me = "[quotation #".$self->quotationnum."]"; # for debug messages
+
+  my @return_bill = ([]);
+  my $error;
+
+  ###### BEGIN TRANSACTION ######
+  local $@;
+  local $SIG{__DIE__};
+  eval {
+    my $temp_dbh = myconnect();
+    local $FS::UID::dbh = $temp_dbh;
+    local $FS::UID::AutoCommit = 0;
+
+    my $fake_self = FS::quotation->new({ $self->hash });
+
+    # if this is a prospect, make them into a customer for now
+    # XXX prospects currently can't have service locations
+    my $cust_or_prospect = $self->cust_or_prospect;
+    my $cust_main;
+    if ( $cust_or_prospect->isa('FS::prospect_main') ) {
+      $cust_main = $cust_or_prospect->convert_cust_main;
+      unless ( ref($cust_main) ) {
+        $temp_dbh->rollback;
+        die "$cust_main (simulating customer signup)\n";
+      }
+      $fake_self->set('prospectnum', '');
+      $fake_self->set('custnum', $cust_main->custnum);
+    } else {
+      $cust_main = $cust_or_prospect;
+    }
+
+    # order packages
+    local($FS::cust_pkg::disable_start_on_hold) = 1;
+    $error = $fake_self->order(\%pkgnum_of);
+    if ( $error ) {
+      $temp_dbh->rollback;
+      die "$error (simulating package order)\n";

     }
 
     }
 

-    # delete old tax records
-    foreach my $quotation_pkg_tax ($pkg->quotation_pkg_tax) {
-      $error = $quotation_pkg_tax->delete;
+    my @new_pkgs = map { FS::cust_pkg->by_key($_) } values(%pkgnum_of);
+
+    # simulate the first bill
+    my %bill_opt = (
+      'estimate'        => 1,
+      'pkg_list'        => \@new_pkgs,
+      'time'            => time, # an option to adjust this?
+      'return_bill'     => $return_bill[0],
+      'no_usage_reset'  => 1,
+    );
+    $error = $cust_main->bill(%bill_opt);
+    if ( $error ) {
+      $temp_dbh->rollback;
+      die "$error (simulating initial billing)\n" if $error;
+    }
+
+    # pick dates for future bills
+    my %next_bill_pkgs;
+    foreach (@new_pkgs) {
+      my $bill = $_->get('bill');
+      next if !$bill;
+      push @{ $next_bill_pkgs{$bill} ||= [] }, $_;
+    }
+
+    my $i = 1;
+    foreach my $next_bill (keys %next_bill_pkgs) {
+      $bill_opt{'time'} = $next_bill;
+      $bill_opt{'return_bill'} = $return_bill[$i] = [];
+      $bill_opt{'pkg_list'} = $next_bill_pkgs{$next_bill};
+      $error = $cust_main->bill(%bill_opt);

       if ( $error ) {
       if ( $error ) {

-        $dbh->rollback if $oldAutoCommit;
-        die "error flushing tax records for pkgpart ". $pkg->pkgpart.": $error\n";
+        $temp_dbh->rollback;
+        die "$error (simulating recurring billing cycle $i)\n";

       }
       }

+      $i++;

     }
     }

+
+    $temp_dbh->rollback;
+  };
+  return $@ if $@;
+  ###### END TRANSACTION ######
+  my %quotationpkgnum_of = reverse %pkgnum_of;
+
+  if ($DEBUG) {
+    warn "pkgnums:\n".Dumper(\%pkgnum_of);
+    warn Dumper(\@return_bill);

   }
 
   }
 

-  # annoyingly duplicates handle_taxes--fix this in 4.x 
-  if ( $conf->exists('enable_taxproducts') ) {
-    warn "can't calculate external taxes for quotations yet\n";
-    # then we're done
-    return;
+  # Careful: none of the foreign keys in here are correct outside the sandbox.
+  # We have a translation table for pkgnums; all others are total lies.
+
+  my %quotation_pkg; # quotationpkgnum => quotation_pkg
+  foreach my $qp ($self->quotation_pkg) {
+    $quotation_pkg{$qp->quotationpkgnum} = $qp;
+    $qp->set($_, 0) foreach qw(unitsetup unitrecur);
+    $qp->set('freq', '');
+    # flush old tax records
+    foreach ($qp->quotation_pkg_tax) {
+      $error = $_->delete;
+      return "$error (flushing tax records for pkgpart ".$qp->part_pkg->pkgpart.")" 
+        if $error;
+    }

   }
 
   }
 

-  my %taxnum_exemptions; # for monthly exemptions; as yet unused
-
-  foreach my $pkg (@quotation_pkg) {
-    my $location = $pkg->cust_location;
-
-    my $part_item = $pkg->part_pkg; # we don't have fees on these yet
-    my @loc_keys = qw( district city county state country);
-    my %taxhash = map { $_ => $location->$_ } @loc_keys;
-    $taxhash{'taxclass'} = $part_item->taxclass;
-    my @taxes;
-    my %taxhash_elim = %taxhash;
-    my @elim = qw( district city county state );
-    do {
-      @taxes = qsearch( 'cust_main_county', \%taxhash_elim );
-      if ( !scalar(@taxes) && $taxhash_elim{'taxclass'} ) {
-        #then try a match without taxclass
-        my %no_taxclass = %taxhash_elim;
-        $no_taxclass{ 'taxclass' } = '';
-        @taxes = qsearch( 'cust_main_county', \%no_taxclass );
+  my %quotation_pkg_tax; # quotationpkgnum => tax name => quotation_pkg_tax obj
+  my %quotation_pkg_discount; # quotationpkgnum => setuprecur => quotation_pkg_discount obj
+
+  for (my $i = 0; $i < scalar(@return_bill); $i++) {
+    my $this_bill = $return_bill[$i]->[0];
+    if (!$this_bill) {
+      warn "$me billing cycle $i produced no invoice\n";
+      next;
+    }
+
+    my @nonpkg_lines;
+    my %cust_bill_pkg;
+    foreach my $cust_bill_pkg (@{ $this_bill->get('cust_bill_pkg') }) {
+      my $pkgnum = $cust_bill_pkg->pkgnum;
+      $cust_bill_pkg{ $cust_bill_pkg->billpkgnum } = $cust_bill_pkg;
+      if ( !$pkgnum ) {
+        # taxes/fees; come back to it
+        push @nonpkg_lines, $cust_bill_pkg;
+        next;

       }
       }

-    
-      $taxhash_elim{ shift(@elim) } = '';
-    } while ( !scalar(@taxes) && scalar(@elim) );
-
-    foreach my $tax_def (@taxes) {
-      my $taxnum = $tax_def->taxnum;
-      $taxnum_exemptions{$taxnum} ||= [];
-
-      # XXX do some kind of equivalent to set_exemptions here
-      # but for now just declare that there are no exemptions,
-      # and then hack the taxable amounts if the package def
-      # excludes setup/recur
-      $pkg->set('cust_tax_exempt_pkg', []);
-
-      if ( $part_item->setuptax or $tax_def->setuptax ) {
-        $pkg->set('unitsetup', 0);
+      my $quotationpkgnum = $quotationpkgnum_of{$pkgnum};
+      my $qp = $quotation_pkg{$quotationpkgnum};
+      if (!$qp) {
+        # XXX supplemental packages could do this (they have separate pkgnums)
+        # handle that special case at some point
+        warn "$me simulated bill returned a package not on the quotation (pkgpart ".$cust_bill_pkg->pkgpart.")\n";
+        next;

       }
       }

-      if ( $part_item->recurtax or $tax_def->recurtax ) {
-        $pkg->set('unitrecur', 0);
+      if ( $i == 0 ) {
+        # then this is the first (setup) invoice
+        $qp->set('start_date', $cust_bill_pkg->sdate);
+        $qp->set('unitsetup', $qp->unitsetup + $cust_bill_pkg->unitsetup);
+        # pkgpart_override is a possibility
+      } else {
+        # recurring invoice (should be only one of these per package, though
+        # it may have multiple lineitems with the same pkgnum)
+        $qp->set('unitrecur', $qp->unitrecur + $cust_bill_pkg->unitrecur);

       }
 
       }
 

-      my %taxline;
-      foreach my $pass (qw(first recur)) {
-        if ($pass eq 'recur') {
-          $pkg->set('unitsetup', 0);
+      # discounts
+      if ( $cust_bill_pkg->get('discounts') ) {
+        # discount records are generated as (setup, recur).
+        # well, not always, sometimes it's just (recur), but fixing this
+        # is horribly invasive.
+        my $discount = $cust_bill_pkg->get('discounts')->[0];
+
+        if ( $discount ) {
+          # find the quotation_pkg_discount record for this billing pass...
+          my $setuprecur = $i ? 'recur' : 'setup';
+          my $qpd = $quotation_pkg_discount{$quotationpkgnum}{$setuprecur}
+                ||= qsearchs('quotation_pkg_discount', {
+                    'quotationpkgnum' => $quotationpkgnum,
+                    'setuprecur'      => $setuprecur,
+                    });
+
+          if (!$qpd) { #can't happen
+            warn "$me simulated bill returned a $setuprecur discount but no discount is in effect.\n";
+          }
+          if ($qpd) {
+            $qpd->set('amount', $discount->amount);
+          }

         }
         }

+      } # end of discount stuff

 
 

-        my $taxline = $tax_def->taxline(
-          [ $pkg ],
-          exemptions => $taxnum_exemptions{$taxnum}
-        );
-        if ($taxline and !ref($taxline)) {
-          $dbh->rollback if $oldAutoCommit;
-          die "error calculating '".$tax_def->taxname .
-              "' for pkgpart '".$pkg->pkgpart."': $taxline\n";
-        }
-        $taxline{$pass} = $taxline;
-      }
+    }

 
 

-      my $quotation_pkg_tax = FS::quotation_pkg_tax->new({
-          quotationpkgnum => $pkg->quotationpkgnum,
-          itemdesc        => ($tax_def->taxname || 'Tax'),
-          taxnum          => $taxnum,
-          taxtype         => ref($tax_def),
-      });
-      my $setup_amount = 0;
-      my $recur_amount = 0;
-      if ($taxline{first}) {
-        $setup_amount = $taxline{first}->setup; # "first cycle", not setup
-      }
-      if ($taxline{recur}) {
-        $recur_amount = $taxline{recur}->setup;
-        $setup_amount -= $recur_amount; # to get the actual setup amount
+    # create tax records
+    foreach my $cust_bill_pkg (@nonpkg_lines) {
+
+      my $itemdesc = $cust_bill_pkg->itemdesc;
+
+      if ($cust_bill_pkg->feepart) {
+        warn "$me simulated bill included a non-package fee (feepart ".
+          $cust_bill_pkg->feepart.")\n";
+        next;

       }
       }

-      if ( $recur_amount > 0 or $setup_amount > 0 ) {
-        $quotation_pkg_tax->set('setup_amount', sprintf('%.2f', $setup_amount));
-        $quotation_pkg_tax->set('recur_amount', sprintf('%.2f', $recur_amount));
-
-        my $error = $quotation_pkg_tax->insert;
-        if ($error) {
-          $dbh->rollback if $oldAutoCommit;
-          die "error recording '".$tax_def->taxname .
-              "' for pkgpart '".$pkg->pkgpart."': $error\n";
-        } # if $error
-      } # else there are no non-zero taxes; continue
-    } # foreach $tax_def
-  } # foreach $pkg
-
-  $dbh->commit if $oldAutoCommit;
-  '';
+      my $links = $cust_bill_pkg->get('cust_bill_pkg_tax_location') ||
+                  $cust_bill_pkg->get('cust_bill_pkg_tax_rate_location') ||
+                  [];
+      # breadth-first unrolled recursion:
+      # take each tax link and any tax-on-tax descendants, and merge them 
+      # into a single quotation_pkg_tax record for each pkgnum/taxname 
+      # combination
+      while (my $tax_link = shift @$links) {
+        my $target = $cust_bill_pkg{ $tax_link->taxable_billpkgnum }
+          or die "$me unable to resolve tax link\n";
+        if ($target->pkgnum) {
+          my $quotationpkgnum = $quotationpkgnum_of{$target->pkgnum};
+          # create this if there isn't one yet
+          my $qpt = $quotation_pkg_tax{$quotationpkgnum}{$itemdesc} ||=
+            FS::quotation_pkg_tax->new({
+              quotationpkgnum => $quotationpkgnum,
+              itemdesc        => $itemdesc,
+              setup_amount    => 0,
+              recur_amount    => 0,
+            });
+          if ( $i == 0 ) { # first invoice
+            $qpt->set('setup_amount', $qpt->setup_amount + $tax_link->amount);
+          } else { # subsequent invoices
+            # this isn't perfectly accurate, but that's why it's an estimate
+            $qpt->set('recur_amount', $qpt->recur_amount + $tax_link->amount);
+            $qpt->set('setup_amount', sprintf('%.2f', $qpt->setup_amount - $tax_link->amount));
+            $qpt->set('setup_amount', 0) if $qpt->setup_amount < 0;
+          }
+        } elsif ($target->feepart) {
+          # do nothing; we already warned for the fee itself
+        } else {
+          # tax on tax: the tax target is another tax item.
+          # since this is an estimate, I'm just going to assign it to the 
+          # first of the underlying packages. (RT#5243 is why we can't have
+          # nice things.)
+          my $sublinks = $target->cust_bill_pkg_tax_rate_location;
+          if ($sublinks and $sublinks->[0]) {
+            $tax_link->set('taxable_billpkgnum', $sublinks->[0]->taxable_billpkgnum);
+            push @$links, $tax_link; #try again
+          } else {
+            warn "$me unable to assign tax on tax; ignoring\n";
+          }
+        }
+      } # while my $tax_link
+
+    } # foreach my $cust_bill_pkg
+  }
+  foreach my $quotation_pkg (values %quotation_pkg) {
+    $error = $quotation_pkg->replace;
+    return "$error (recording estimate for ".$quotation_pkg->part_pkg->pkg.")"
+      if $error;
+  }
+  foreach (values %quotation_pkg_discount) {
+    # { setup => one, recur => another }
+    foreach my $quotation_pkg_discount (values %$_) {
+      $error = $quotation_pkg_discount->replace;
+      return "$error (recording estimated discount)"
+        if $error;
+    }
+  }
+  foreach my $quotation_pkg_tax (map { values %$_ } values %quotation_pkg_tax) {
+    $error = $quotation_pkg_tax->insert;
+    return "$error (recording estimated tax for ".$quotation_pkg_tax->itemdesc.")"
+    if $error;
+  }
+  return;

 }
 
 =back
 }
 
 =back


@@ -829,17 +1066,97 @@ sub search_sql_where {
 
 =item _items_pkg
 
 
 =item _items_pkg
 

-Return line item hashes for each package on this quotation. Differs from the
-base L<FS::Template_Mixin> version in that it recalculates each quoted package
-first, and doesn't implement the "condensed" option.
+Return line item hashes for each package on this quotation.

 
 =cut
 
 sub _items_pkg {
   my ($self, %options) = @_;
 
 =cut
 
 sub _items_pkg {
   my ($self, %options) = @_;

-  $self->estimate;
-  # run it through the Template_Mixin engine
-  return $self->_items_cust_bill_pkg([ $self->quotation_pkg ], %options);
+  my $escape = $options{'escape_function'};
+  my $locale = $self->cust_or_prospect->locale;
+
+  my $preref = $options{'preref_callback'};
+
+  my $section = $options{'section'};
+  my $freq = $section->{'category'};
+  my @pkgs = $self->quotation_pkg;
+  my @items;
+  die "_items_pkg called without section->{'category'}"
+    unless defined $freq;
+
+  my %tax_item; # taxname => hashref, will be aggregated AT DISPLAY TIME
+                # like we should have done in the first place
+
+  foreach my $quotation_pkg (@pkgs) {
+    my $part_pkg = $quotation_pkg->part_pkg;
+    my @details = $quotation_pkg->details;
+    my $setuprecur;
+    my $this_item = {
+      'pkgnum'          => $quotation_pkg->quotationpkgnum,
+      'description'     => $quotation_pkg->desc($locale),
+      'ext_description' => \@details,
+      'quantity'        => $quotation_pkg->quantity,
+    };
+    if ($freq eq '0') {
+      # setup/one-time
+      $setuprecur = 'setup';
+      if ($part_pkg->freq ne '0') {
+        # indicate that it's a setup fee on a recur package (cust_bill does 
+        # this too)
+        $this_item->{'description'} .= ' Setup';
+      }
+    } else {
+      # recur for this frequency
+      next if $freq ne $part_pkg->freq;
+      $setuprecur = 'recur';
+    }
+
+    $this_item->{'unit_amount'} = sprintf('%.2f', 
+      $quotation_pkg->get('unit'.$setuprecur));
+    $this_item->{'amount'} = sprintf('%.2f', $this_item->{'unit_amount'}
+                                             * $quotation_pkg->quantity);
+    next if $this_item->{'amount'} == 0 and !(
+      $setuprecur eq 'setup'
+      ? $quotation_pkg->setup_show_zero
+      : $quotation_pkg->recur_show_zero
+    );
+
+    if ( $preref ) {
+      $this_item->{'preref_html'} = &$preref($quotation_pkg);
+    }
+
+    push @items, $this_item;
+    my $discount = $quotation_pkg->_item_discount(setuprecur => $setuprecur);
+    if ($discount) {
+      $_ = &{$escape}($_) foreach @{ $discount->{ext_description} };
+      push @items, $discount;
+    }
+
+    # each quotation_pkg_tax has two amounts: the amount charged on the 
+    # setup invoice, and the amount on the recurring invoice.
+    foreach my $qpt ($quotation_pkg->quotation_pkg_tax) {
+      my $this_tax = $tax_item{$qpt->itemdesc} ||= {
+        'pkgnum'          => 0,
+        'description'     => $qpt->itemdesc,
+        'ext_description' => [],
+        'amount'          => 0,
+      };
+      $this_tax->{'amount'} += $qpt->get($setuprecur.'_amount');
+    }
+  } # foreach $quotation_pkg
+
+  foreach my $taxname ( sort { $a cmp $b } keys (%tax_item) ) {
+    my $this_tax = $tax_item{$taxname};
+    $this_tax->{'amount'} = sprintf('%.2f', $this_tax->{'amount'});
+    next if $this_tax->{'amount'} == 0;
+    push @items, $this_tax;
+  }
+
+  return @items;
+}
+
+sub _items_tax {
+  ();

 }
 
 =back
 }
 
 =back