[freeside-commits] branch master updated. c14803e37f1bfdcff882ee5cb35a9b10a93a88c9

Mark Wells mark at 420.am
Mon Apr 20 09:43:56 PDT 2015


The branch, master has been updated
       via  c14803e37f1bfdcff882ee5cb35a9b10a93a88c9 (commit)
       via  f2cf5c2843dcef5db0941a1673538eb922fd5a5a (commit)
      from  94f12d554f6f1038d63283a0962d726e2725773a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit c14803e37f1bfdcff882ee5cb35a9b10a93a88c9
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Apr 20 09:43:34 2015 -0700

    quotations + tax refactor, part 2

diff --git a/FS/FS/TaxEngine.pm b/FS/FS/TaxEngine.pm
index 54e305f..70f1f92 100644
--- a/FS/FS/TaxEngine.pm
+++ b/FS/FS/TaxEngine.pm
@@ -40,8 +40,12 @@ FS::TaxEngine - Base class for tax calculation engines.
 Creates an L<FS::TaxEngine> object.  The subclass will be chosen by the 
 'enable_taxproducts' configuration setting.
 
-CUST_MAIN and TIME are required.  OPTIONS can include "cancel" => 1 to 
-indicate that the package is being billed on cancellation.
+CUST_MAIN and TIME are required.  OPTIONS can include:
+
+"cancel" => 1 to indicate that the package is being billed on cancellation.
+
+"estimate" => 1 to indicate that this calculation is for tax estimation,
+and isn't an actual sale invoice, in case that matters.
 
 =cut
 
diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index 9e43c3c..32e3007 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -906,7 +906,8 @@ sub print_generic {
     if $DEBUG > 1;
 
   my $unsquelched = $params{unsquelch_cdr} || $cust_main->squelch_cdr ne 'Y';
-  my $multisection = $conf->exists($tc.'sections', $cust_main->agentnum) ||
+  my $multisection = $self->has_sections;
+  $conf->exists($tc.'sections', $cust_main->agentnum) ||
                      $conf->exists($tc.'sections_by_location', $cust_main->agentnum);
   $invoice_data{'multisection'} = $multisection;
   my $late_sections;
@@ -1215,7 +1216,6 @@ sub print_generic {
     List::Util::first { $_->{description} eq $tax_description } @sections;
   if (!$tax_section) {
     $tax_section = { 'description' => $tax_description };
-    push @sections, $tax_section if $multisection;
   }
   $tax_section->{tax_section} = 1; # mark this section as containing taxes
   # if this is an existing tax section, we're merging the tax items into it.
@@ -1231,6 +1231,8 @@ sub print_generic {
   #$tax_section->{'sort_weight'} = $tax_weight;
 
   my @items_tax = $self->_items_tax;
+  push @sections, $tax_section if $multisection and @items_tax > 0;
+
   foreach my $tax ( @items_tax ) {
 
     $taxtotal += $tax->{'amount'};
@@ -1647,7 +1649,10 @@ sub print_generic {
 
 sub notice_name { '('.shift->table.')'; }
 
-sub template_conf { 'invoice_'; }
+# this is not supposed to happen
+sub template_conf { warn "bare FS::Template_Mixin::template_conf";
+  'invoice_';
+}
 
 # helper routine for generating date ranges
 sub _prior_month30s {
diff --git a/FS/FS/cust_bill.pm b/FS/FS/cust_bill.pm
index b7b7367..95e7058 100644
--- a/FS/FS/cust_bill.pm
+++ b/FS/FS/cust_bill.pm
@@ -143,6 +143,16 @@ Invoices are normally created by calling the bill method of a customer object
 =cut
 
 sub table { 'cust_bill'; }
+sub template_conf { 'invoice_'; }
+
+sub has_sections {
+  my $self = shift;
+  my $agentnum = $self->cust_main->agentnum;
+  my $tc = $self->template_conf;
+
+  $self->conf->exists($tc.'sections', $agentnum) ||
+  $self->conf->exists($tc.'sections_by_location', $agentnum);
+}
 
 # should be the ONLY occurrence of "Invoice" in invoice rendering code.
 # (except email_subject and invnum_date_pretty)
diff --git a/FS/FS/cust_bill_void.pm b/FS/FS/cust_bill_void.pm
index b9db81b..f3dba90 100644
--- a/FS/FS/cust_bill_void.pm
+++ b/FS/FS/cust_bill_void.pm
@@ -108,7 +108,17 @@ points to.  You can ask the object for a copy with the I<hash> method.
 
 sub table { 'cust_bill_void'; }
 sub notice_name { 'VOIDED Invoice'; }
-#XXXsub template_conf { 'quotation_'; }
+sub template_conf { 'invoice_'; }
+
+sub has_sections {
+  my $self = shift;
+  my $agentnum = $self->cust_main->agentnum;
+  my $tc = $self->template_conf;
+
+  $self->conf->exists($tc.'sections', $agentnum) ||
+  $self->conf->exists($tc.'sections_by_location', $agentnum);
+}
+
 
 =item insert
 
diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index 29d5fa9..367745d 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -379,6 +379,12 @@ Do not save the generated bill in the database.  Useful with return_bill
 
 A list reference on which the generated bill(s) will be returned.
 
+=item estimate
+
+Boolean value; indicates that this is an estimate rather than a "tax invoice".
+This will be passed through to the tax engine, as online tax services 
+sometimes need to know it for reporting purposes. Otherwise it has no effect.
+
 =item invoice_terms
 
 Optional terms to be printed on this invoice.  Otherwise, customer-specific
@@ -474,7 +480,8 @@ sub bill {
   foreach (@passes) {
     $tax_engines{$_} = FS::TaxEngine->new(cust_main    => $self,
                                           invoice_time => $invoice_time,
-                                          cancel       => $options{cancel}
+                                          cancel       => $options{cancel},
+                                          estimate     => $options{estimate},
                                          );
     $tax_is_batch ||= $tax_engines{$_}->info->{batch};
   }
@@ -542,7 +549,8 @@ sub bill {
           $tax_engines{$pass} = FS::TaxEngine->new(
                                   cust_main    => $self,
                                   invoice_time => $invoice_time,
-                                  cancel       => $options{cancel}
+                                  cancel       => $options{cancel},
+                                  estimate     => $options{estimate},
                                 );
           $cust_bill_pkg{$pass} = [];
         }
diff --git a/FS/FS/quotation.pm b/FS/FS/quotation.pm
index 930083e..f2a9620 100644
--- a/FS/FS/quotation.pm
+++ b/FS/FS/quotation.pm
@@ -14,8 +14,9 @@ use FS::cust_pkg;
 use FS::quotation_pkg;
 use FS::quotation_pkg_tax;
 use FS::type_pkgs;
+use List::MoreUtils;
 
-our $DEBUG = 1;
+our $DEBUG = 0;
 use Data::Dumper;
 
 =head1 NAME
@@ -87,6 +88,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 has_sections { 1; }
 
 =item insert
 
@@ -152,7 +154,7 @@ sub cust_bill_pkg { #actually quotation_pkg objects
 
 sub total_setup {
   my $self = shift;
-  $self->_total('setup');
+  sprintf('%.2f', $self->_total('setup') + $self->_total('setup_tax'));
 }
 
 =item total_recur [ FREQ ]
@@ -163,14 +165,14 @@ sub total_recur {
   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;
-  $total += $_->$method() for $self->cust_bill_pkg;
+  $total += $_->$method() for $self->quotation_pkg;
   sprintf('%.2f', $total);
 
 }
@@ -221,7 +223,7 @@ sub cust_or_prospect {
   $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.
 
@@ -253,87 +255,47 @@ sub cust_or_prospect_label_link {
 
 }
 
-sub _items_tax {
-  ();
-}
-
-sub _items_nontax {
-  shift->cust_bill_pkg;
-}
-
-sub _items_total {
+sub _items_sections {
   my $self = shift;
-  $self->quotationnum =~ /^(\d+)$/ or return ();
-
-  my @items;
-
-  # 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 $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 %opt = @_;
+  my $escape = $opt{escape}; # the only one we care about
+
+  my %subtotals; # package frequency => subtotal
+  foreach my $pkg ($self->quotation_pkg) {
+    my $recur_freq = $pkg->part_pkg->freq;
+    ($subtotals{0} ||= 0) += $pkg->setup + $pkg->setup_tax;
+    ($subtotals{$recur_freq} ||= 0) += $pkg->recur + $pkg->recur_tax;
   }
+  my @pkg_freq_order = keys %{ FS::Misc->pkg_freqs };
 
-  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 )
-    };
-  }
+  my @sections;
+  foreach my $freq (keys %subtotals) {
 
-  #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,
-      };
+    next if $subtotals{$freq} == 0;
+
+    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
+        $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_recur > 0 ) {
-    push @items, {
-      'total_item'   => $self->mt('Total Recurring'),
-      'total_amount' => sprintf('%.2f',$total_recur),
-      'break_after'  => 1,
+    push @sections, {
+      'description' => &$escape($desc),
+      'sort_weight' => $weight,
+      'category'    => $freq,
+      'subtotal'    => sprintf('%.2f',$subtotals{$freq}),
     };
   }
-
-  return @items;
-
+  return \@sections, [];
 }
 
 =item enable_previous
@@ -637,6 +599,7 @@ sub estimate {
 
     # 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],
@@ -674,21 +637,24 @@ sub estimate {
     warn Dumper(\@return_bill);
   }
 
-  # careful: none of the pkgnums in here are correct outside the sandbox.
+  # 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, $qp->quotation_pkg_discount) {
+    foreach ($qp->quotation_pkg_tax) {
       $error = $_->delete;
       return "$error (flushing tax records for pkgpart ".$qp->part_pkg->pkgpart.")" 
         if $error;
     }
   }
 
-  my %quotation_pkg_tax; # quotationpkgnum => taxnum => quotation_pkg_tax obj
+  my %quotation_pkg_tax; # quotationpkgnum => tax name => quotation_pkg_tax obj
+  my %quotation_pkg_discount; # quotationpkgnum => quotation_pkg_discount obj
 
   for (my $i = 0; $i < scalar(@return_bill); $i++) {
     my $this_bill = $return_bill[$i]->[0];
@@ -725,8 +691,37 @@ sub estimate {
         # it may have multiple lineitems with the same pkgnum)
         $qp->set('unitrecur', $qp->unitrecur + $cust_bill_pkg->unitrecur);
       }
+
+      # discounts
+      if ( $cust_bill_pkg->get('discounts') ) {
+        my $discount = $cust_bill_pkg->get('discounts')->[0];
+        # discount records are generated as (setup, recur).
+        # well, not always, sometimes it's just (recur), but fixing this
+        # is horribly invasive.
+        my $qpd = $quotation_pkg_discount{$quotationpkgnum}
+              ||= qsearchs('quotation_pkg_discount', {
+                  'quotationpkgnum' => $quotationpkgnum
+                  });
+
+        if (!$qpd) { #can't happen
+          warn "$me simulated bill returned a discount but no discount is in effect.\n";
+        }
+        if ($discount and $qpd) {
+          if ( $i == 0 ) {
+            $qpd->set('setup_amount', $discount->amount);
+          } else {
+            $qpd->set('recur_amount', $discount->amount);
+          }
+        }
+      } # end of discount stuff
+
     }
+
+    # 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";
@@ -735,20 +730,20 @@ sub estimate {
       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
+      # 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 (taxnum ".$tax_link->taxnum.")\n";
+          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}{$tax_link->taxnum} ||=
+          my $qpt = $quotation_pkg_tax{$quotationpkgnum}{$itemdesc} ||=
             FS::quotation_pkg_tax->new({
               quotationpkgnum => $quotationpkgnum,
-              itemdesc        => $cust_bill_pkg->itemdesc,
-              taxnum          => $tax_link->taxnum,
-              taxtype         => $tax_link->taxtype,
+              itemdesc        => $itemdesc,
               setup_amount    => 0,
               recur_amount    => 0,
             });
@@ -763,9 +758,10 @@ sub estimate {
         } elsif ($target->feepart) {
           # do nothing; we already warned for the fee itself
         } else {
-          # tax on tax: the tax target is another tax item
+          # 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
+          # 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);
@@ -775,14 +771,19 @@ sub estimate {
           }
         }
       } # while my $tax_link
+
     } # foreach my $cust_bill_pkg
-    #XXX discounts
   }
   foreach my $quotation_pkg (values %quotation_pkg) {
     $error = $quotation_pkg->replace;
     return "$error (recording estimate for ".$quotation_pkg->part_pkg->pkg.")"
       if $error;
   }
+  foreach my $quotation_pkg_discount (values %quotation_pkg_discount) {
+    $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.")"
@@ -915,17 +916,92 @@ sub search_sql_where {
 
 =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) = @_;
-  $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 $setuprecur;
+    my $this_item = {
+      'pkgnum'          => $quotation_pkg->quotationpkgnum,
+      'description'     => $quotation_pkg->desc($locale),
+      'ext_description' => [],
+      '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;
+
+    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
diff --git a/FS/FS/quotation_pkg.pm b/FS/FS/quotation_pkg.pm
index 1674d2b..4c78be7 100644
--- a/FS/FS/quotation_pkg.pm
+++ b/FS/FS/quotation_pkg.pm
@@ -118,16 +118,11 @@ sub insert {
   my $error = $self->SUPER::insert;
 
   if ( !$error and $self->discountnum ) {
+    warn "inserting discount #".$self->discountnum."\n";
     $error = $self->insert_discount;
     $error .= ' (setting discount)' if $error;
   }
 
-  # update $self and any discounts with their amounts
-  if ( !$error ) {
-    $error = $self->estimate;
-    $error .= ' (calculating charges)' if $error;
-  }
-
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;
@@ -164,9 +159,9 @@ sub delete {
     return $error;
   } else {
     $dbh->commit if $oldAutoCommit;
-    return '';
   }
   
+  $self->quotation->estimate;
 }
 
 =item replace OLD_RECORD
@@ -232,99 +227,8 @@ sub desc {
   $self->part_pkg->pkg;
 }
 
-=item estimate
-
-Update the quotation_pkg record with the estimated setup and recurring 
-charges for the package. Returns nothing on success, or an error message
-on failure.
-
 =cut
 
-sub estimate {
-  my $self = shift;
-  my $part_pkg = $self->part_pkg;
-  my $quantity = $self->quantity || 1;
-  my ($unitsetup, $unitrecur);
-  # calculate base fees
-  if ( $self->waive_setup eq 'Y' || $self->{'_NO_SETUP_KLUDGE'} ) {
-    $unitsetup = '0.00';
-  } else {
-    $unitsetup = $part_pkg->base_setup;
-  }
-  if ( $self->{'_NO_RECUR_KLUDGE'} ) {
-    $unitrecur = '0.00';
-  } else {
-    $unitrecur = $part_pkg->base_recur;
-  }
-
-  #XXX add-on packages
-
-  $self->set('unitsetup', $unitsetup);
-  $self->set('unitrecur', $unitrecur);
-  my $error = $self->replace;
-  return $error if $error;
-
-  # semi-duplicates calc_discount
-  my $setup_discount = 0;
-  my $recur_discount = 0;
-
-  my %setup_discounts; # quotationpkgdiscountnum => amount
-  my %recur_discounts; # quotationpkgdiscountnum => amount
-
-  # XXX the order of applying discounts is ill-defined, which matters
-  # if there are percentage and amount discounts on the same package.
-  #
-  # but right now there can only be one discount on any package, so 
-  # it doesn't matter
-  foreach my $pkg_discount ($self->quotation_pkg_discount) {
-
-    my $discount = $pkg_discount->discount;
-    my $this_setup_discount = 0;
-    my $this_recur_discount = 0;
-
-    if ( $discount->percent > 0 ) {
-
-      if ( $discount->setup ) {
-        $this_setup_discount = ($discount->percent * $unitsetup / 100);
-      }
-      $this_recur_discount = ($discount->percent * $unitrecur / 100);
-
-    } elsif ( $discount->amount > 0 ) {
-
-      my $discount_left = $discount->amount;
-      if ( $discount->setup ) {
-        if ( $discount_left > $unitsetup - $setup_discount ) {
-          # then discount the setup to zero
-          $discount_left -= $unitsetup - $setup_discount;
-          $this_setup_discount = $unitsetup - $setup_discount;
-        } else {
-          # not enough discount to fully cover the setup
-          $this_setup_discount = $discount_left;
-          $discount_left = 0;
-        }
-      }
-      # same logic for recur
-      if ( $discount_left > $unitrecur - $recur_discount ) {
-        $this_recur_discount = $unitrecur - $recur_discount;
-      } else {
-        $this_recur_discount = $discount_left;
-      }
-
-    }
-
-    # increment the total discountage
-    $setup_discount += $this_setup_discount;
-    $recur_discount += $this_recur_discount;
-    # and update the pkg_discount object
-    $pkg_discount->set('setup_amount', sprintf('%.2f', $setup_discount));
-    $pkg_discount->set('recur_amount', sprintf('%.2f', $recur_discount));
-    my $error = $pkg_discount->replace;
-    return $error if $error;
-  }
-
-  '';
-}
-
 =item insert_discount
 
 Associates this package with a discount (see L<FS::cust_pkg_discount>,
@@ -353,6 +257,11 @@ sub insert_discount {
 
 sub _item_discount {
   my $self = shift;
+  my %options = @_;
+  my $setuprecur = $options{'setuprecur'};
+
+  # kind of silly treating this as multiple records, but it works, and will
+  # work if we allow multiple discounts at some point
   my @pkg_discounts = $self->pkg_discount;
   return if @pkg_discounts == 0;
   
@@ -360,20 +269,16 @@ sub _item_discount {
   my $d = {
     _is_discount    => 1,
     description     => $self->mt('Discount'),
-    setup_amount    => 0,
-    recur_amount    => 0,
     amount          => 0,
     ext_description => \@ext,
     # maybe should show quantity/unit discount?
   };
   foreach my $pkg_discount (@pkg_discounts) {
     push @ext, $pkg_discount->description;
-    $d->{setup_amount} -= $pkg_discount->setup_amount;
-    $d->{recur_amount} -= $pkg_discount->recur_amount;
+    my $amount = $pkg_discount->get($setuprecur.'_amount');
+    $d->{amount} -= $amount;
   }
-  $d->{setup_amount} *= $self->quantity || 1;
-  $d->{recur_amount} *= $self->quantity || 1;
-  $d->{amount} = $d->{setup_amount} + $d->{recur_amount};
+  $d->{amount} = sprintf('%.2f', $d->{amount} * $self->quantity);
   
   return $d;
 }
@@ -382,12 +287,23 @@ sub setup {
   my $self = shift;
   ($self->unitsetup - sum(0, map { $_->setup_amount } $self->pkg_discount))
     * ($self->quantity || 1);
+
+}
+
+sub setup_tax {
+  my $self = shift;
+  sum(0, map { $_->setup_amount } $self->quotation_pkg_tax);
 }
 
 sub recur {
   my $self = shift;
   ($self->unitrecur - sum(0, map { $_->recur_amount } $self->pkg_discount))
-    * ($self->quantity || 1);
+    * ($self->quantity || 1)
+}
+
+sub recur_tax {
+  my $self = shift;
+  sum(0, map { $_->recur_amount } $self->quotation_pkg_tax);
 }
 
 =item part_pkg_currency_option OPTIONNAME
@@ -480,7 +396,6 @@ sub tax_locationnum {
   $self->locationnum;
 }
 
-
 sub _upgrade_data {
   my $class = shift;
   my @quotation_pkg_without_location =
diff --git a/FS/FS/quotation_pkg_tax.pm b/FS/FS/quotation_pkg_tax.pm
index f459ed2..c6222ac 100644
--- a/FS/FS/quotation_pkg_tax.pm
+++ b/FS/FS/quotation_pkg_tax.pm
@@ -41,11 +41,6 @@ to.
 
 =item itemdesc - the name of the tax
 
-=item taxnum - the L<FS::cust_main_county> or L<FS::tax_rate> defining the 
-tax.
-
-=item taxtype - the class of the tax rate represented by C<taxnum>.
-
 =item setup_amount - the amount of tax calculated on one-time charges
 
 =item recur_amount - the amount of tax calculated on recurring charges
@@ -94,8 +89,6 @@ sub check {
     $self->ut_numbern('quotationtaxnum')
     || $self->ut_foreign_key('quotationpkgnum', 'quotation_pkg', 'quotationpkgnum')
     || $self->ut_text('itemdesc')
-    || $self->ut_number('taxnum')
-    || $self->ut_enum('taxtype', [ 'FS::cust_main_county', 'FS::tax_rate' ])
     || $self->ut_money('setup_amount')
     || $self->ut_money('recur_amount')
   ;
diff --git a/httemplate/edit/process/quick-cust_pkg.cgi b/httemplate/edit/process/quick-cust_pkg.cgi
index 34f5d12..f1d8c26 100644
--- a/httemplate/edit/process/quick-cust_pkg.cgi
+++ b/httemplate/edit/process/quick-cust_pkg.cgi
@@ -159,7 +159,7 @@ if ( $quotationnum ) {
   $quotation_pkg->prospectnum($prospect_main->prospectnum) if $prospect_main;
 
   #XXX handle new location
-  $error = $quotation_pkg->insert || $quotation_pkg->estimate;
+  $error = $quotation_pkg->insert;
 
 } else {
 
diff --git a/httemplate/view/quotation.html b/httemplate/view/quotation.html
index bd998bb..4abef9f 100755
--- a/httemplate/view/quotation.html
+++ b/httemplate/view/quotation.html
@@ -72,6 +72,9 @@ function areyousure(href, message) {
   <BR><BR>
 % }
 
+% if ( $error ) {
+<& /elements/error.html, $error &>
+% }
 
 % if ( $conf->exists('quotation_html') ) { 
     <% join('', $quotation->print_html( preref_callback=>$preref_callback )) %>
@@ -107,6 +110,8 @@ my $quotation = qsearchs({
 });
 die "Quotation #$quotationnum not found!" unless $quotation;
 
+my $error = $quotation->estimate;
+
 my $menubar = menubar( $quotation->cust_or_prospect_label_link($p) );
 
 my $link = "quotationnum=$quotationnum";

commit f2cf5c2843dcef5db0941a1673538eb922fd5a5a
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Apr 20 00:48:29 2015 -0700

    add Avalara tax status field to prospects, #25718

diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 29bbf78..7f28e11 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -1643,7 +1643,7 @@ sub tables_hashref {
         'message_noemail', 'char', 'NULL', 1, '', '',
         'bill_locationnum', 'int', 'NULL', '', '', '',
         'ship_locationnum', 'int', 'NULL', '', '', '',
-        'taxstatusnum',   'char', 'NULL',      32, '', '',
+        'taxstatusnum',   'int', 'NULL', '', '', '',
         'complimentary', 'char', 'NULL', 1, '', '',
         'po_number', 'varchar', 'NULL', $char_d, '', '',
         'invoice_attn', 'varchar', 'NULL', $char_d, '', '',
@@ -1885,6 +1885,7 @@ sub tables_hashref {
         'disabled',       'char', 'NULL',       1, '', '', 
         'custnum',         'int', 'NULL',      '', '', '',
         'refnum',          'int', 'NULL',      '', '', '', 
+        'taxstatusnum',    'int', 'NULL',      '', '', '',
       ],
       'primary_key'  => 'prospectnum',
       'unique'       => [],
@@ -1987,12 +1988,10 @@ sub tables_hashref {
         'quotationtaxnum',  'serial',     '',      '', '', '',
         'quotationpkgnum',     'int',     '',      '', '', '',
         'itemdesc',        'varchar',     '', $char_d, '', '',
-        'taxnum',              'int',     '',      '', '', '', 
-        'taxtype',         'varchar',     '', $char_d, '', '',
         'setup_amount',    @money_type,                '', '',
         'recur_amount',    @money_type,                '', '',
       ],
-      'primary_key' => 'quotationtaxnum',,
+      'primary_key' => 'quotationtaxnum',
       'unique' => [],
       'index'  => [ [ 'quotationpkgnum' ] ],
       'foreign_keys' => [
@@ -2000,7 +1999,7 @@ sub tables_hashref {
                             table      => 'quotation_pkg',
                           },
                         ],
-},
+    },
 
     'cust_location' => { #'location' now that its prospects too, but...
       'columns' => [
diff --git a/FS/FS/TaxEngine/avalara.pm b/FS/FS/TaxEngine/avalara.pm
index 183555d..d4a2360 100644
--- a/FS/FS/TaxEngine/avalara.pm
+++ b/FS/FS/TaxEngine/avalara.pm
@@ -11,7 +11,7 @@ use FS::tax_rate;
 use JSON;
 use Geo::StreetAddress::US;
 
-our $DEBUG = 2;
+our $DEBUG = 0;
 our $json = JSON->new->pretty(1);
 
 our $conf;
@@ -29,10 +29,6 @@ FS::UID->install_callback( sub {
 #}
 # Avalara address standardization would be nice but isn't necessary
 
-# XXX this is just here to avoid reworking the framework right now. By the
-# 4.0 release, ALL tax calculations should be done after the invoice has 
-# been inserted into the database.
-
 # nothing to do here
 sub add_sale {}
 
@@ -85,6 +81,8 @@ sub build_request {
     };
     push @lines, $line;
   }
+  # don't make the request unless there are some eligible line items
+  return '' if !@lines;
 
   # assemble address records for any cust_locations we used here, plus
   # the company address
@@ -141,6 +139,7 @@ sub build_request {
 
   # create the top level object
   my $date = DateTime->from_epoch(epoch => $self->{invoice_time});
+  my $doctype = $self->{estimate} ? 'SalesOrder' : 'SalesInvoice';
   return {
     'CustomerCode'      => $cust_main->custnum,
     'DocDate'           => $date->strftime('%Y-%m-%d'),
@@ -149,7 +148,7 @@ sub build_request {
     'DocCode'           => $cust_bill->invnum,
     'DetailLevel'       => 'Tax',
     'Commit'            => 'false',
-    'DocType'           => 'SalesInvoice', # ???
+    'DocType'           => $doctype,
     'CustomerUsageType' => $cust_main->taxstatus,
     # ExemptionNo, Discount, TaxOverride, PurchaseOrderNo,
     'Addresses'         => \@addrs,
@@ -196,6 +195,10 @@ account number, and license key.
 
   # assemble the request hash
   my $request = $self->build_request;
+  if (!$request) {
+    warn "no tax-eligible items on this invoice\n" if $DEBUG;
+    return [];
+  }
 
   warn "sending Avalara tax request\n" if $DEBUG;
   my $request_json = $json->encode($request);
@@ -247,6 +250,7 @@ account number, and license key.
       my $error = $tax_rate->find_or_insert;
       return "error inserting tax_rate record for '$taxname': $error\n"
         if $error;
+      $tax_rate = $tax_rate->replace_old; # get its taxnum if there wasn't one
 
       # create a tax_rate_location record
       my $tax_rate_location = FS::tax_rate_location->new({
@@ -266,7 +270,7 @@ account number, and license key.
 
       # create a link record
       my $tax_link = FS::cust_bill_pkg_tax_rate_location->new({
-          cust_bill_pkg       => $tax_item,
+          tax_cust_bill_pkg   => $tax_item,
           taxtype             => 'FS::tax_rate',
           taxnum              => $tax_rate->taxnum,
           taxratelocationnum  => $tax_rate_location->taxratelocationnum,
diff --git a/FS/FS/prospect_main.pm b/FS/FS/prospect_main.pm
index 7c58de3..79efa86 100644
--- a/FS/FS/prospect_main.pm
+++ b/FS/FS/prospect_main.pm
@@ -5,7 +5,7 @@ use strict;
 use vars qw( $DEBUG @location_fields );
 use Scalar::Util qw( blessed );
 use FS::Conf;
-use FS::Record qw( dbh qsearch ); # qsearchs );
+use FS::Record qw( dbh qsearch qsearchs );
 use FS::cust_location;
 use FS::cust_main;
 
@@ -246,6 +246,7 @@ sub check {
     || $self->ut_foreign_key(  'agentnum', 'agent',         'agentnum' )
     || $self->ut_foreign_keyn( 'refnum',   'part_referral', 'refnum'   )
     || $self->ut_textn('company')
+    || $self->ut_foreign_keyn( 'taxstatusnum', 'tax_status', 'taxstatusnum' )
   ;
   return $error if $error;
 
@@ -299,6 +300,36 @@ Returns the qualifications (see L<FS::qual>) associated with this prospect.
 
 Returns the agent (see L<FS::agent>) for this customer.
 
+=item tax_status
+
+Returns the external tax status, as an FS::tax_status object, or the empty 
+string if there is no tax status.
+
+=cut
+
+sub tax_status {
+  my $self = shift;
+  if ( $self->taxstatusnum ) {
+    qsearchs('tax_status', { 'taxstatusnum' => $self->taxstatusnum } );
+  } else { 
+    return '';
+  }
+}
+
+=item taxstatus
+
+Returns the tax status code if there is one.
+
+=cut
+
+sub taxstatus {
+  my $self = shift;
+  my $tax_status = $self->tax_status;
+  $tax_status
+    ? $tax_status->taxstatus
+    : '';
+}
+
 =item convert_cust_main
 
 Converts this prospect to a customer.
@@ -325,7 +356,7 @@ sub convert_cust_main {
   my $cust_main = new FS::cust_main {
     'bill_location' => $cust_location[0],
     'ship_location' => $cust_location[0],
-    ( map { $_ => $self->$_ } qw( agentnum refnum company ) ),
+    ( map { $_ => $self->$_ } qw( agentnum refnum company taxstatusnum ) ),
   };
 
   $cust_main->refnum( FS::Conf->new->config('referraldefault') || 1  )
@@ -410,6 +441,11 @@ sub cust_bill {
   return;
 }
 
+# XXX should have real localization here eventually
+sub locale {
+  FS::Conf->new->config('locale');
+}
+
 =back
 
 =head1 BUGS
diff --git a/httemplate/edit/prospect_main.html b/httemplate/edit/prospect_main.html
index da5c6ce..1269a84 100644
--- a/httemplate/edit/prospect_main.html
+++ b/httemplate/edit/prospect_main.html
@@ -7,6 +7,7 @@
                             'company'     => 'Company',
                             'contactnum'  => 'Contact',
                             'locationnum' => ' ',
+                            'taxstatusnum'=> 'Tax status',
                           },
      'fields'          => [
        { 'field'       => 'agentnum',
@@ -46,6 +47,10 @@
             'prospect_main' => shift
           },
        },
+       { 'field'    => 'taxstatusnum',
+         'type'     => 'select-tax_status',
+         'empty_label'   => ' ',
+       },
      ],
      'new_callback'    => $new_callback,
      'edit_callback'   => $edit_callback,
diff --git a/httemplate/view/prospect_main.html b/httemplate/view/prospect_main.html
index a1f14a3..b5ef64f 100644
--- a/httemplate/view/prospect_main.html
+++ b/httemplate/view/prospect_main.html
@@ -69,6 +69,14 @@
       &>
 %   }
 % }
+% if ( my $tax_status = $prospect_main->tax_status ) {
+  <TR>
+    <TD ALIGN="right">Tax status</TD>
+    <TD BGCOLOR="#FFFFFF">
+      <B><% $tax_status->taxstatus %>:</B> <% $tax_status->description %>
+    </TD>
+  </TR>
+% }
 
 </TABLE>
 

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/Schema.pm                            |    9 +-
 FS/FS/TaxEngine.pm                         |    8 +-
 FS/FS/TaxEngine/avalara.pm                 |   18 +-
 FS/FS/Template_Mixin.pm                    |   11 +-
 FS/FS/cust_bill.pm                         |   10 ++
 FS/FS/cust_bill_void.pm                    |   12 +-
 FS/FS/cust_main/Billing.pm                 |   12 +-
 FS/FS/prospect_main.pm                     |   40 ++++-
 FS/FS/quotation.pm                         |  270 ++++++++++++++++++----------
 FS/FS/quotation_pkg.pm                     |  129 +++----------
 FS/FS/quotation_pkg_tax.pm                 |    7 -
 httemplate/edit/process/quick-cust_pkg.cgi |    2 +-
 httemplate/edit/prospect_main.html         |    5 +
 httemplate/view/prospect_main.html         |    8 +
 httemplate/view/quotation.html             |    5 +
 15 files changed, 312 insertions(+), 234 deletions(-)




More information about the freeside-commits mailing list