[freeside-commits] branch master updated. cf69a36faa57cdb544948c905059cf1e1ac73e07

Mark Wells mark at 420.am
Mon Feb 24 19:12:03 PST 2014


The branch, master has been updated
       via  cf69a36faa57cdb544948c905059cf1e1ac73e07 (commit)
       via  cc3a43f7d4386297a8babebfdd49646f836db127 (commit)
      from  04220e7ef18314883ad1cec05c552f13d8d5f7e4 (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 cf69a36faa57cdb544948c905059cf1e1ac73e07
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Feb 24 19:11:34 2014 -0800

    fix deletion of objects with options, fallout from #13971

diff --git a/FS/FS/option_Common.pm b/FS/FS/option_Common.pm
index c1dda22..74adbed 100644
--- a/FS/FS/option_Common.pm
+++ b/FS/FS/option_Common.pm
@@ -134,13 +134,7 @@ sub delete {
   my $oldAutoCommit = $FS::UID::AutoCommit;
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
-
-  my $error = $self->SUPER::delete;
-  if ( $error ) {
-    $dbh->rollback if $oldAutoCommit;
-    return $error;
-  }
-  
+ 
   my $pkey = $self->primary_key;
   #my $option_table = $self->option_table;
 
@@ -152,6 +146,12 @@ sub delete {
     }
   }
 
+  my $error = $self->SUPER::delete;
+  if ( $error ) {
+    $dbh->rollback if $oldAutoCommit;
+    return $error;
+  }
+ 
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   '';
diff --git a/FS/FS/part_export.pm b/FS/FS/part_export.pm
index 8e10ea7..9d261f0 100644
--- a/FS/FS/part_export.pm
+++ b/FS/FS/part_export.pm
@@ -161,6 +161,10 @@ sub delete {
     'link_table'    => 'export_nas',
     'target_table'  => 'nas',
     'params'        => [],
+  ) || $self->process_m2m(
+    'link_table'    => 'export_svc',
+    'target_table'  => 'part_svc',
+    'params'        => [],
   ) || $self->SUPER::delete;
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
diff --git a/bin/test_scrub_sql b/bin/test_scrub_sql
index fb26fe9..fe66805 100755
--- a/bin/test_scrub_sql
+++ b/bin/test_scrub_sql
@@ -15,11 +15,11 @@
 
 foreach my $table (qw(
   part_export_option
-  payment_gateway
   payment_gateway_option
+  payment_gateway
   agent_payment_gateway
-  queue
   queue_arg
+  queue
   cust_pay_batch
 )) {
   print "DELETE FROM $table;\n";

commit cc3a43f7d4386297a8babebfdd49646f836db127
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Feb 24 15:45:44 2014 -0800

    non-package fees, fixes for tax calculation and sales reports, #25899

diff --git a/FS/FS/Report/Table.pm b/FS/FS/Report/Table.pm
index 7f59384..17b12ae 100644
--- a/FS/FS/Report/Table.pm
+++ b/FS/FS/Report/Table.pm
@@ -141,7 +141,7 @@ sub payments {
 sub credits {
   my( $self, $speriod, $eperiod, $agentnum, %opt ) = @_;
   $self->scalar_sql("
-    SELECT SUM(amount)
+    SELECT SUM(cust_credit.amount)
       FROM cust_credit
         LEFT JOIN cust_main USING ( custnum )
       WHERE ". $self->in_time_period_and_agent($speriod, $eperiod, $agentnum).
@@ -390,9 +390,6 @@ unspecified, defaults to all three.
 'use_override': for line items generated by an add-on package, use the class
 of the add-on rather than the base package.
 
-'freq': limit to packages with this frequency.  Currently uses the part_pkg 
-frequency, so term discounted packages may give odd results.
-
 'distribute': for non-monthly recurring charges, ignore the invoice 
 date.  Instead, consider the line item's starting/ending dates.  Determine 
 the fraction of the line item duration that falls within the specified 
@@ -421,7 +418,8 @@ my $cust_bill_pkg_join = '
     LEFT JOIN cust_main USING ( custnum )
     LEFT JOIN cust_pkg USING ( pkgnum )
     LEFT JOIN part_pkg USING ( pkgpart )
-    LEFT JOIN part_pkg AS override ON pkgpart_override = override.pkgpart';
+    LEFT JOIN part_pkg AS override ON pkgpart_override = override.pkgpart
+    LEFT JOIN part_fee USING ( feepart )';
 
 sub cust_bill_pkg_setup {
   my $self = shift;
@@ -434,7 +432,7 @@ sub cust_bill_pkg_setup {
   $agentnum ||= $opt{'agentnum'};
 
   my @where = (
-    'pkgnum != 0',
+    '(pkgnum != 0 OR feepart IS NOT NULL)',
     $self->with_classnum($opt{'classnum'}, $opt{'use_override'}),
     $self->with_report_option(%opt),
     $self->in_time_period_and_agent($speriod, $eperiod, $agentnum),
@@ -461,7 +459,7 @@ sub cust_bill_pkg_recur {
   my $cust_bill_pkg = $opt{'project'} ? 'v_cust_bill_pkg' : 'cust_bill_pkg';
 
   my @where = (
-    'pkgnum != 0',
+    '(pkgnum != 0 OR feepart IS NOT NULL)',
     $self->with_classnum($opt{'classnum'}, $opt{'use_override'}),
     $self->with_report_option(%opt),
   );
@@ -476,13 +474,14 @@ sub cust_bill_pkg_recur {
     $item_usage = 'usage'; #already calculated
   }
   else {
-    $item_usage = '( SELECT COALESCE(SUM(amount),0)
+    $item_usage = '( SELECT COALESCE(SUM(cust_bill_pkg_detail.amount),0)
       FROM cust_bill_pkg_detail
       WHERE cust_bill_pkg_detail.billpkgnum = cust_bill_pkg.billpkgnum )';
   }
   my $recur_fraction = '';
 
   if ( $opt{'distribute'} ) {
+    $where[0] = 'pkgnum != 0'; # specifically exclude fees
     push @where, "cust_main.agentnum = $agentnum" if $agentnum;
     push @where,
       "$cust_bill_pkg.sdate <  $eperiod",
@@ -521,7 +520,8 @@ Arguments as for C<cust_bill_pkg>, plus:
 sub cust_bill_pkg_detail {
   my( $self, $speriod, $eperiod, $agentnum, %opt ) = @_;
 
-  my @where = ( "cust_bill_pkg.pkgnum != 0" );
+  my @where = 
+    ( "(cust_bill_pkg.pkgnum != 0 OR cust_bill_pkg.feepart IS NOT NULL)" );
 
   push @where, 'cust_main.refnum = '. $opt{'refnum'} if $opt{'refnum'};
 
@@ -536,7 +536,9 @@ sub cust_bill_pkg_detail {
     ;
 
   if ( $opt{'distribute'} ) {
-    # then limit according to the usage time, not the billing date
+    # exclude fees
+    $where[0] = 'cust_bill_pkg.pkgnum != 0';
+    # and limit according to the usage time, not the billing date
     push @where, $self->in_time_period_and_agent($speriod, $eperiod, $agentnum,
       'cust_bill_pkg_detail.startdate'
     );
@@ -547,7 +549,7 @@ sub cust_bill_pkg_detail {
     );
   }
 
-  my $total_sql = " SELECT SUM(amount) ";
+  my $total_sql = " SELECT SUM(cust_bill_pkg_detail.amount) ";
 
   $total_sql .=
     " / CASE COUNT(cust_pkg.*) WHEN 0 THEN 1 ELSE COUNT(cust_pkg.*) END "
@@ -561,6 +563,7 @@ sub cust_bill_pkg_detail {
         LEFT JOIN cust_pkg ON cust_bill_pkg.pkgnum = cust_pkg.pkgnum
         LEFT JOIN part_pkg USING ( pkgpart )
         LEFT JOIN part_pkg AS override ON pkgpart_override = override.pkgpart
+        LEFT JOIN part_fee USING ( feepart )
       WHERE ".join( ' AND ', grep $_, @where );
 
   $self->scalar_sql($total_sql);
@@ -683,14 +686,14 @@ sub with_classnum {
   @$classnum = grep /^\d+$/, @$classnum;
   my $in = 'IN ('. join(',', @$classnum). ')';
 
-  if ( $use_override ) {
-    "(
+  my $expr = "
          ( COALESCE(part_pkg.classnum, 0) $in AND pkgpart_override IS NULL)
-      OR ( COALESCE(override.classnum, 0) $in AND pkgpart_override IS NOT NULL )
-    )";
-  } else {
-    "COALESCE(part_pkg.classnum, 0) $in";
+      OR ( COALESCE(part_fee.classnum, 0) $in AND feepart IS NOT NULL )";
+  if ( $use_override ) {
+    $expr .= "
+      OR ( COALESCE(override.classnum, 0) $in AND pkgpart_override IS NOT NULL )";
   }
+  "( $expr )";
 }
 
 sub with_usageclass {
@@ -834,7 +837,8 @@ sub init_projection {
       # sdate/edate overlapping the ROI, for performance
       "INSERT INTO v_cust_bill_pkg ( 
         SELECT cust_bill_pkg.*,
-          (SELECT COALESCE(SUM(amount),0) FROM cust_bill_pkg_detail 
+          (SELECT COALESCE(SUM(cust_bill_pkg_detail.amount),0)
+          FROM cust_bill_pkg_detail 
           WHERE cust_bill_pkg_detail.billpkgnum = cust_bill_pkg.billpkgnum),
           cust_bill._date,
           cust_pkg.expire
diff --git a/FS/FS/Template_Mixin.pm b/FS/FS/Template_Mixin.pm
index c4c2d7f..131a236 100644
--- a/FS/FS/Template_Mixin.pm
+++ b/FS/FS/Template_Mixin.pm
@@ -2452,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;
@@ -2471,7 +2473,7 @@ 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;
@@ -2739,29 +2741,21 @@ sub _items_cust_bill_pkg {
 
         } # 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 (".
-                             $self->time2str_local('short', $cust_bill_pkg->sdate). ' - '.
-                             $self->time2str_local('short', $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'));
diff --git a/FS/FS/cust_bill_pkg.pm b/FS/FS/cust_bill_pkg.pm
index a943921..066ddf1 100644
--- a/FS/FS/cust_bill_pkg.pm
+++ b/FS/FS/cust_bill_pkg.pm
@@ -968,6 +968,31 @@ sub tax_locationnum {
   }
 }
 
+sub tax_location {
+  my $self = shift;
+  FS::cust_location->by_key($self->tax_locationnum);
+}
+
+=item part_X
+
+Returns the L<FS::part_pkg> or L<FS::part_fee> object that defines this
+charge.  If called on a tax line, returns nothing.
+
+=cut
+
+sub part_X {
+  my $self = shift;
+  if ( $self->override_pkgpart ) {
+    return FS::part_pkg->by_key($self->override_pkgpart);
+  } elsif ( $self->pkgnum ) {
+    return $self->cust_pkg->part_pkg;
+  } elsif ( $self->feepart ) {
+    return $self->part_fee;
+  } else {
+    return;
+  }
+}
+
 =back
 
 =head1 CLASS METHODS
diff --git a/FS/FS/cust_credit.pm b/FS/FS/cust_credit.pm
index 7ae6c97..1890845 100644
--- a/FS/FS/cust_credit.pm
+++ b/FS/FS/cust_credit.pm
@@ -910,14 +910,9 @@ sub credit_lineitems {
 
     # recalculate taxes with new amounts
     $taxlisthash{$invnum} ||= {};
-    my $part_pkg = $cust_bill_pkg->part_pkg;
-    $cust_main->_handle_taxes( $part_pkg,
-                               $taxlisthash{$invnum},
-                               $cust_bill_pkg,
-                               $cust_bill_pkg->cust_pkg,
-                               $cust_bill_pkg->cust_bill->_date, #invoice time
-                               $cust_bill_pkg->cust_pkg->pkgpart,
-                             );
+    my $part_pkg = $cust_bill_pkg->part_pkg
+      if $cust_bill_pkg->pkgpart_override;
+    $cust_main->_handle_taxes( $taxlisthash{$invnum}, $cust_bill_pkg );
   }
 
   ###
@@ -1013,12 +1008,12 @@ sub credit_lineitems {
 
       # we still have to deal with the possibility that the tax links don't
       # cover the whole amount of tax because of an incomplete upgrade...
-      if ($amount > 0) {
+      if ($amount > 0.005) {
         $cust_credit_bill{$invnum} += $amount;
         push @{ $cust_credit_bill_pkg{$invnum} },
           new FS::cust_credit_bill_pkg {
             'billpkgnum' => $tax_item->billpkgnum,
-            'amount'     => $amount,
+            'amount'     => sprintf('%.2f', $amount),
             'setuprecur' => 'setup',
           };
 
diff --git a/FS/FS/cust_main/Billing.pm b/FS/FS/cust_main/Billing.pm
index 6bd82d1..f4c30ce 100644
--- a/FS/FS/cust_main/Billing.pm
+++ b/FS/FS/cust_main/Billing.pm
@@ -596,12 +596,9 @@ sub bill {
       my $fee_location = $self->ship_location; # I think?
 
       my $error = $self->_handle_taxes(
-        $part_fee,
         $taxlisthash{$pass},
         $fee_item,
-        $fee_location,
-        $options{invoice_time},
-        {} # no options
+        location => $fee_location
       );
       return $error if $error;
 
@@ -1319,14 +1316,7 @@ sub _make_lines {
       # handle taxes
       ###
 
-      my $error = $self->_handle_taxes(
-        $part_pkg,
-        $taxlisthash,
-        $cust_bill_pkg,
-        $cust_location,
-        $options{invoice_time},
-        \%options # I have serious objections to this
-      );
+      my $error = $self->_handle_taxes( $taxlisthash, $cust_bill_pkg );
       return $error if $error;
 
       $cust_bill_pkg->set_display(
@@ -1423,15 +1413,13 @@ sub _transfer_balance {
   return @transfers;
 }
 
-=item _handle_taxes PART_ITEM TAXLISTHASH CUST_BILL_PKG CUST_LOCATION TIME [ OPTIONS ]
+=item handle_taxes TAXLISTHASH CUST_BILL_PKG [ OPTIONS ]
 
 This is _handle_taxes.  It's called once for each cust_bill_pkg generated
-from _make_lines, along with the part_pkg (or part_fee), cust_location,
-invoice time, a flag indicating whether the package is being canceled, and a 
-partridge in a pear tree.
+from _make_lines.
 
-The most important argument is 'taxlisthash'.  This is shared across the 
-entire invoice.  It looks like this:
+TAXLISTHASH is a hashref shared across the entire invoice.  It looks like 
+this:
 {
   'cust_main_county 1001' => [ [FS::cust_main_county], ... ],
   'cust_main_county 1002' => [ [FS::cust_main_county], ... ],
@@ -1444,16 +1432,27 @@ That "..." is a list of FS::cust_bill_pkg objects that will be fed to
 the 'taxline' method to calculate the amount of the tax.  This doesn't
 happen until calculate_taxes, though.
 
+OPTIONS may include:
+- part_item: a part_pkg or part_fee object to be used as the package/fee 
+  definition.
+- location: a cust_location to be used as the billing location.
+
+If not supplied, part_item will be inferred from the pkgnum or feepart of the
+cust_bill_pkg, and location from the pkgnum (or, for fees, the invnum and 
+the customer's default service location).
+
 =cut
 
 sub _handle_taxes {
   my $self = shift;
-  my $part_item = shift;
   my $taxlisthash = shift;
   my $cust_bill_pkg = shift;
-  my $location = shift;
-  my $invoice_time = shift;
-  my $options = shift;
+  my %options = @_;
+
+  # at this point I realize that we have enough information to infer all this
+  # stuff, instead of passing around giant honking argument lists
+  my $location = $options{location} || $cust_bill_pkg->tax_location;
+  my $part_item = $options{part_item} || $cust_bill_pkg->part_X;
 
   local($DEBUG) = $FS::cust_main::DEBUG if $FS::cust_main::DEBUG > $DEBUG;
 
@@ -1473,9 +1472,8 @@ sub _handle_taxes {
     my @classes;
     #push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->type eq 'U';
     push @classes, $cust_bill_pkg->usage_classes if $cust_bill_pkg->usage;
-    # debatable
-    push @classes, 'setup' if ($cust_bill_pkg->setup && !$options->{cancel});
-    push @classes, 'recur' if ($cust_bill_pkg->recur && !$options->{cancel});
+    push @classes, 'setup' if $cust_bill_pkg->setup;
+    push @classes, 'recur' if $cust_bill_pkg->recur;
 
     my $exempt = $conf->exists('cust_class-tax_exempt')
                    ? ( $self->cust_class ? $self->cust_class->tax : '' )
@@ -1543,10 +1541,7 @@ sub _handle_taxes {
           warn "adding $totname to taxed taxes\n" if $DEBUG > 2;
           # calculate the tax amount that the tax_on_tax will apply to
           my $hashref_or_error = 
-            $tax_object->taxline( $localtaxlisthash{$tax},
-                                  'custnum'      => $self->custnum,
-                                  'invoice_time' => $invoice_time,
-                                );
+            $tax_object->taxline( $localtaxlisthash{$tax} );
           return $hashref_or_error
             unless ref($hashref_or_error);
           
diff --git a/FS/FS/part_event/Action/fee.pm b/FS/FS/part_event/Action/fee.pm
index c2b4673..f1d5891 100644
--- a/FS/FS/part_event/Action/fee.pm
+++ b/FS/FS/part_event/Action/fee.pm
@@ -1,5 +1,7 @@
 package FS::part_event::Action::fee;
 
+# DEPRECATED; will most likely be removed in 4.x
+
 use strict;
 use base qw( FS::part_event::Action );
 
@@ -53,11 +55,9 @@ sub _calc_fee {
       my $part_pkg = FS::part_pkg->new({
           taxclass => $self->option('taxclass')
       });
-      my $error = $cust_main->_handle_taxes(
-        FS::part_pkg->new({ taxclass => ($self->option('taxclass') || '') }),
-        $taxlisthash,
-        $charge,
-        FS::cust_pkg->new({custnum => $cust_main->custnum}),
+      my $error = $cust_main->_handle_taxes( $taxlisthash, $charge,
+        location  => $cust_main->ship_location,
+        part_item => $part_pkg,
       );
       if ( $error ) {
         warn "error estimating taxes for breakage charge: custnum ".$cust_main->custnum."\n";
diff --git a/FS/FS/part_fee.pm b/FS/FS/part_fee.pm
index 67da245..9605d61 100644
--- a/FS/FS/part_fee.pm
+++ b/FS/FS/part_fee.pm
@@ -126,6 +126,8 @@ and replace methods.
 sub check {
   my $self = shift;
 
+  $self->set('amount', 0) unless $self->amount;
+
   my $error = 
     $self->ut_numbern('feepart')
     || $self->ut_textn('comment')
@@ -219,11 +221,17 @@ representing the invoice line item for the fee, with linked
 L<FS::cust_bill_pkg_fee> record(s) allocating the fee to the invoice or 
 its line items, as appropriate.
 
+If the fee is going to be charged on the upcoming invoice (credit card 
+processing fees, postal invoice fees), INVOICE should be an uninserted
+L<FS::cust_bill> object where the 'cust_bill_pkg' property is an arrayref
+of the non-fee line items that will appear on the invoice.
+
 =cut
 
 sub lineitem {
   my $self = shift;
   my $cust_bill = shift;
+  my $cust_main = $cust_bill->cust_main;
 
   my $amount = 0 + $self->get('amount');
   my $total_base;  # sum of base line items
@@ -273,14 +281,15 @@ sub lineitem {
 
   my $maximum = $self->maximum;
   if ( $self->limit_credit ) {
-    my $balance = $cust_bill->cust_main;
+    my $balance = $cust_bill->cust_main->balance;
     if ( $balance >= 0 ) {
-      $maximum = 0;
+      warn "Credit balance is zero, so fee is zero" if $DEBUG;
+      return; # don't bother doing estimated tax, etc.
     } elsif ( -1 * $balance < $maximum ) {
       $maximum = -1 * $balance;
     }
   }
-  if ( $maximum ne '' and $amount > $maximum ) {
+  if ( $maximum ne '' ) {
     warn "Applying maximum fee\n" if $DEBUG;
     $amount = $maximum;
   }
@@ -296,8 +305,35 @@ sub lineitem {
       setup       => 0,
       recur       => 0,
   });
+
+  if ( $maximum and $self->taxable ) {
+    warn "Estimating taxes on fee.\n";
+    # then we need to estimate tax to respect the maximum
+    # XXX currently doesn't work with external (tax_rate) taxes
+    # or batch taxes, obviously
+    my $taxlisthash = {};
+    my $error = $cust_main->_handle_taxes(
+      $taxlisthash,
+      $cust_bill_pkg,
+      location => $cust_main->ship_location
+    );
+    my $total_rate = 0;
+    # $taxlisthash: tax identifier => [ cust_main_county, cust_bill_pkg... ]
+    my @taxes = map { $_->[0] } values %$taxlisthash;
+    foreach (@taxes) {
+      $total_rate += $_->tax;
+    }
+    if ($total_rate > 0) {
+      my $max_cents = $maximum * 100;
+      my $charge_cents = sprintf('%0.f', $max_cents * 100/(100 + $total_rate));
+      $maximum = sprintf('%.2f', $charge_cents / 100.00);
+      $amount = $maximum if $amount > $maximum;
+    }
+  } # if $maximum and $self->taxable
+
+  # set the amount that we'll charge
   $cust_bill_pkg->set( $self->setuprecur, $amount );
-  
+
   if ( $self->classnum ) {
     my $pkg_category = $self->pkg_class->pkg_category;
     $cust_bill_pkg->set('section' => $pkg_category->categoryname)
diff --git a/FS/FS/tax_rate.pm b/FS/FS/tax_rate.pm
index 3d37677..4516004 100644
--- a/FS/FS/tax_rate.pm
+++ b/FS/FS/tax_rate.pm
@@ -371,7 +371,7 @@ sub passtype_name {
   $tax_passtypes{$self->passtype};
 }
 
-=item taxline TAXABLES, [ OPTIONSHASH ]
+=item taxline TAXABLES
 
 Returns a listref of a name and an amount of tax calculated for the list
 of packages/amounts referenced by TAXABLES.  If an error occurs, a message
@@ -381,13 +381,13 @@ is returned as a scalar.
 
 sub taxline {
   my $self = shift;
+  # this used to accept a hash of options but none of them did anything
+  # so it's been removed.
 
   my $taxables;
-  my %opt = ();
 
   if (ref($_[0]) eq 'ARRAY') {
     $taxables = shift;
-    %opt = @_;
   }else{
     $taxables = [ @_ ];
     #exemptions would be broken in this case
diff --git a/httemplate/browse/part_fee.html b/httemplate/browse/part_fee.html
index 0370fe0..482c692 100644
--- a/httemplate/browse/part_fee.html
+++ b/httemplate/browse/part_fee.html
@@ -1,16 +1,16 @@
 <& elements/browse.html,
-  'title'         => 'Fee definitions',
-  'name_singular' => 'fee definition',
-  'query'         => $query,
-  'count_query'   => $count_query,
-  'header'        => [  '#',
+  title           => 'Fee definitions',
+  name_singular   => 'fee definition',
+  query           => $query,
+  count_query     => $count_query,
+  header          => [  '#',
                         'Description',
                         'Comment',
                         'Class',
                         'Amount',
                         'Tax status',
                      ],
-  'fields'        => [  'feepart',
+  fields          => [  'feepart',
                         'itemdesc',
                         'comment',
                         'classname',
@@ -27,6 +27,7 @@
                         $link,
                      ],
   align           => 'cllccc',
+  menubar         => \@menubar,
 &>
 <%init>
 my $curuser = $FS::CurrentUser::CurrentUser;
@@ -64,4 +65,7 @@ my $sub_tax = sub {
 };
 
 my $link = [ $p.'edit/part_fee.html?', 'feepart' ];
+
+my @menubar = ( 'Add a new fee definition',
+                $p.'edit/part_fee.html' );
 </%init>
diff --git a/httemplate/edit/credit-cust_bill_pkg.html b/httemplate/edit/credit-cust_bill_pkg.html
index a5ecb69..40faddc 100644
--- a/httemplate/edit/credit-cust_bill_pkg.html
+++ b/httemplate/edit/credit-cust_bill_pkg.html
@@ -269,7 +269,8 @@ my @cust_bill_pkg = qsearch({
   'select'    => 'cust_bill_pkg.*',
   'table'     => 'cust_bill_pkg',
   'addl_from' => 'LEFT JOIN cust_bill USING (invnum)',
-  'extra_sql' => "WHERE custnum = $custnum AND pkgnum != 0",
+  'extra_sql' => "WHERE custnum = $custnum ".
+                 "AND (pkgnum != 0 or feepart IS NOT NULL)",
   'order_by'  => 'ORDER BY invnum ASC, billpkgnum ASC',
 });
 
diff --git a/httemplate/misc/xmlhttp-calculate_taxes.html b/httemplate/misc/xmlhttp-calculate_taxes.html
index ed7bd01..2bb1f4c 100644
--- a/httemplate/misc/xmlhttp-calculate_taxes.html
+++ b/httemplate/misc/xmlhttp-calculate_taxes.html
@@ -62,14 +62,7 @@ if ( $sub eq 'calculate_taxes' ) {
 
     my $taxlisthash = {};
     foreach my $cust_bill_pkg (values %cust_bill_pkg) {
-      my $part_pkg = $cust_bill_pkg->part_pkg;
-      $cust_main->_handle_taxes( $part_pkg,
-                                 $taxlisthash,
-                                 $cust_bill_pkg,
-                                 $cust_bill_pkg->cust_pkg,
-                                 $cust_bill_pkg->cust_bill->_date,
-                                 $cust_bill_pkg->cust_pkg->pkgpart,
-                               );
+      $cust_main->_handle_taxes( $taxlisthash, $cust_bill_pkg );
     }
     my $listref_or_error = 
       $cust_main->calculate_taxes( [ values %cust_bill_pkg ], $taxlisthash, [ values %cust_bill_pkg ]->[0]->cust_bill->_date );
diff --git a/httemplate/misc/xmlhttp-cust_bill_pkg-calculate_taxes.html b/httemplate/misc/xmlhttp-cust_bill_pkg-calculate_taxes.html
index c0db3e2..4558682 100644
--- a/httemplate/misc/xmlhttp-cust_bill_pkg-calculate_taxes.html
+++ b/httemplate/misc/xmlhttp-cust_bill_pkg-calculate_taxes.html
@@ -62,15 +62,7 @@ if ( $sub eq 'calculate_taxes' ) {
 
       push @cust_bill_pkg, $cust_bill_pkg;
 
-      my $part_pkg = $cust_bill_pkg->part_pkg;
-      $cust_main->_handle_taxes( $part_pkg,
-                                 $taxlisthash,
-                                 $cust_bill_pkg,
-                                 $cust_bill_pkg->cust_pkg,
-                                 $cust_bill_pkg->cust_bill->_date,
-                                 $cust_bill_pkg->cust_pkg->pkgpart,
-                               );
-
+      $cust_main->_handle_taxes( $taxlisthash, $cust_bill_pkg );
     }
 
     if ( @cust_bill_pkg ) {
@@ -89,7 +81,10 @@ if ( $sub eq 'calculate_taxes' ) {
       foreach my $taxline ( @$listref_or_error ) {
         my $amount = $taxline->setup;
         my $desc = $taxline->desc;
-        foreach my $location ( @{$taxline->cust_bill_pkg_tax_location}, @{$taxline->cust_bill_pkg_tax_rate_location} ) {
+        foreach my $location (
+          @{$taxline->get('cust_bill_pkg_tax_location')},
+          @{$taxline->get('cust_bill_pkg_tax_rate_location')} )
+        {
           my $taxlocnum = $location->locationnum || '';
           my $taxratelocnum = $location->taxratelocationnum || '';
           $location->cust_bill_pkg_desc($taxline->desc); #ugh @ that kludge
diff --git a/httemplate/search/cust_bill_pkg.cgi b/httemplate/search/cust_bill_pkg.cgi
index 6b7a5e6..440ab15 100644
--- a/httemplate/search/cust_bill_pkg.cgi
+++ b/httemplate/search/cust_bill_pkg.cgi
@@ -137,9 +137,9 @@ Filtering parameters:
 - use_override: Apply "classnum" and "taxclass" filtering based on the 
   override (bundle) pkgpart, rather than always using the true pkgpart.
 
-- nottax: Limit to items that are not taxes (pkgnum > 0).
+- nottax: Limit to items that are not taxes (pkgnum > 0 or feepart > 0).
 
-- istax: Limit to items that are taxes (pkgnum == 0).
+- istax: Limit to items that are taxes (pkgnum == 0 and feepart = null).
 
 - taxnum: Limit to items whose tax definition matches this taxnum.
   With "nottax" that means items that are subject to that tax;
@@ -305,7 +305,8 @@ if ( $cgi->param('custnum') =~ /^(\d+)$/ ) {
 # we want the package and its definition if available
 my $join_pkg = 
 ' LEFT JOIN cust_pkg      USING (pkgnum) 
-  LEFT JOIN part_pkg      USING (pkgpart)';
+  LEFT JOIN part_pkg      USING (pkgpart)
+  LEFT JOIN part_fee      USING (feepart)';
 
 my $part_pkg = 'part_pkg';
 # "Separate sub-packages from parents"
@@ -319,12 +320,16 @@ if ( $use_override ) {
   $part_pkg = 'override';
 }
 push @select, "$part_pkg.pkgpart", "$part_pkg.pkg";
-push @select, "$part_pkg.taxclass" if $conf->exists('enable_taxclasses');
+push @select, "COALESCE($part_pkg.taxclass, part_fee.taxclass) AS taxclass"
+  if $conf->exists('enable_taxclasses');
 
 # the non-tax case
 if ( $cgi->param('nottax') ) {
 
-  push @where, 'cust_bill_pkg.pkgnum > 0';
+  push @select, "part_fee.itemdesc";
+
+  push @where,
+    '(cust_bill_pkg.pkgnum > 0 OR cust_bill_pkg.feepart IS NOT NULL)';
 
   my @tax_where; # will go into a subquery
   my @exempt_where; # will also go into a subquery
@@ -335,7 +340,7 @@ if ( $cgi->param('nottax') ) {
   # N: classnum
   if ( grep { $_ eq 'classnum' } $cgi->param ) {
     my @classnums = grep /^\d*$/, $cgi->param('classnum');
-    push @where, "COALESCE($part_pkg.classnum, 0) IN ( ".
+    push @where, "COALESCE(part_fee.classnum, $part_pkg.classnum, 0) IN ( ".
                      join(',', @classnums ).
                  ' )'
       if @classnums;
@@ -360,7 +365,7 @@ if ( $cgi->param('nottax') ) {
     # effective taxclass, not the real one
     push @tax_where, 'cust_main_county.taxclass IS NULL'
   } elsif ( $cgi->param('taxclass') ) {
-    push @tax_where, "$part_pkg.taxclass IN (" .
+    push @tax_where, "COALESCE(part_fee.taxclass, $part_pkg.taxclass) IN (" .
                  join(', ', map {dbh->quote($_)} $cgi->param('taxclass') ).
                  ')';
   }
@@ -681,7 +686,7 @@ if ( $cgi->param('salesnum') =~ /^(\d+)$/ ) {
     'paid'            => ($cgi->param('paid') ? 1 : 0),
     'classnum'        => scalar($cgi->param('classnum'))
   );
-  $join_pkg .= " JOIN sales_pkg_class ON ( COALESCE(sales_pkg_class.classnum, 0) = COALESCE( part_pkg.classnum, 0) )";
+  $join_pkg .= " JOIN sales_pkg_class ON ( COALESCE(sales_pkg_class.classnum, 0) = COALESCE( part_fee.classnum, part_pkg.classnum, 0) )";
 
   my $extra_sql = $subsearch->{extra_sql};
   $extra_sql =~ s/^WHERE//;

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

Summary of changes:
 FS/FS/Report/Table.pm                              |   40 ++++++++-------
 FS/FS/Template_Mixin.pm                            |   30 +++++-------
 FS/FS/cust_bill_pkg.pm                             |   25 +++++++++
 FS/FS/cust_credit.pm                               |   15 ++----
 FS/FS/cust_main/Billing.pm                         |   53 +++++++++-----------
 FS/FS/option_Common.pm                             |   14 +++---
 FS/FS/part_event/Action/fee.pm                     |   10 ++--
 FS/FS/part_export.pm                               |    4 ++
 FS/FS/part_fee.pm                                  |   44 +++++++++++++++--
 FS/FS/tax_rate.pm                                  |    6 +-
 bin/test_scrub_sql                                 |    4 +-
 httemplate/browse/part_fee.html                    |   16 ++++--
 httemplate/edit/credit-cust_bill_pkg.html          |    3 +-
 httemplate/misc/xmlhttp-calculate_taxes.html       |    9 +---
 .../xmlhttp-cust_bill_pkg-calculate_taxes.html     |   15 ++----
 httemplate/search/cust_bill_pkg.cgi                |   21 +++++---
 16 files changed, 180 insertions(+), 129 deletions(-)




More information about the freeside-commits mailing list