fix package change vs. v4 discount refactor, RT#77513, RT#14092
[freeside.git] / FS / FS / cust_pkg.pm
index 01eaf62..c70a679 100644 (file)
@@ -1,18 +1,18 @@
 package FS::cust_pkg;
+use base qw( FS::cust_pkg::Search FS::cust_pkg::API
+             FS::otaker_Mixin FS::cust_main_Mixin FS::Sales_Mixin
+             FS::contact_Mixin FS::location_Mixin
+             FS::m2m_Common FS::option_Common
+           );
 
 use strict;
-use base qw( FS::otaker_Mixin FS::cust_main_Mixin
-             FS::contact_Mixin FS::location_Mixin
-             FS::m2m_Common FS::option_Common );
-use vars qw($disable_agentcheck $DEBUG $me);
 use Carp qw(cluck);
 use Scalar::Util qw( blessed );
-use List::Util qw(min max);
+use List::Util qw(min max sum);
 use Tie::IxHash;
 use Time::Local qw( timelocal timelocal_nocheck );
 use MIME::Entity;
 use FS::UID qw( dbh driver_name );
-use FS::Misc qw( send_email );
 use FS::Record qw( qsearch qsearchs fields );
 use FS::CurrentUser;
 use FS::cust_svc;
@@ -31,11 +31,18 @@ use FS::reg_code;
 use FS::part_svc;
 use FS::cust_pkg_reason;
 use FS::reason;
+use FS::cust_pkg_usageprice;
 use FS::cust_pkg_discount;
 use FS::discount;
-use FS::UI::Web;
+use FS::sales;
+# for modify_charge
+use FS::cust_credit;
+
 use Data::Dumper;
 
+# temporary fix; remove this once (un)suspend admin notices are cleaned up
+use FS::Misc qw(send_email);
+
 # need to 'use' these instead of 'require' in sub { cancel, suspend, unsuspend,
 # setup }
 # because they load configuration by setting FS::UID::callback (see TODO)
@@ -47,23 +54,33 @@ use FS::svc_forward;
 # for sending cancel emails in sub cancel
 use FS::Conf;
 
-$DEBUG = 0;
-$me = '[FS::cust_pkg]';
+our ($disable_agentcheck, $DEBUG, $me, $import) = (0, 0, '[FS::cust_pkg]', 0);
 
-$disable_agentcheck = 0;
+our $upgrade = 0; #go away after setup+start dates cleaned up for old customers
+
+our $cache_enabled = 0;
+
+our $disable_start_on_hold = 0;
+
+sub _simplecache {
+  my( $self, $hashref ) = @_;
+  if ( $cache_enabled && $hashref->{'pkg'} && $hashref->{'plan'} ) {
+    $self->{'_pkgpart'} = FS::part_pkg->new($hashref);
+  }
+}
 
 sub _cache {
   my $self = shift;
   my ( $hashref, $cache ) = @_;
-  #if ( $hashref->{'pkgpart'} ) {
-  if ( $hashref->{'pkg'} ) {
-    # #@{ $self->{'_pkgnum'} } = ();
-    # my $subcache = $cache->subcache('pkgpart', 'part_pkg');
-    # $self->{'_pkgpart'} = $subcache;
-    # #push @{ $self->{'_pkgnum'} },
-    #   FS::part_pkg->new_or_cached($hashref, $subcache);
-    $self->{'_pkgpart'} = FS::part_pkg->new($hashref);
-  }
+#  #if ( $hashref->{'pkgpart'} ) {
+#  if ( $hashref->{'pkg'} ) {
+#    # #@{ $self->{'_pkgnum'} } = ();
+#    # my $subcache = $cache->subcache('pkgpart', 'part_pkg');
+#    # $self->{'_pkgpart'} = $subcache;
+#    # #push @{ $self->{'_pkgnum'} },
+#    #   FS::part_pkg->new_or_cached($hashref, $subcache);
+#    $self->{'_pkgpart'} = FS::part_pkg->new($hashref);
+#  }
   if ( exists $hashref->{'svcnum'} ) {
     #@{ $self->{'_pkgnum'} } = ();
     my $subcache = $cache->subcache('svcnum', 'cust_svc', $hashref->{pkgnum});
@@ -104,6 +121,8 @@ FS::cust_pkg - Object methods for cust_pkg objects
 
   $seconds = $record->seconds_since($timestamp);
 
+  #bulk cancel+order... perhaps slightly deprecated, only used by the bulk
+  # cancel+order in the web UI and nowhere else (edit/process/cust_pkg.cgi)
   $error = FS::cust_pkg::order( $custnum, \@pkgparts );
   $error = FS::cust_pkg::order( $custnum, \@pkgparts, \@remove_pkgnums ] );
 
@@ -174,11 +193,6 @@ date
 
 order taker (see L<FS::access_user>)
 
-=item manual_flag
-
-If this field is set to 1, disables the automatic
-unsuspension of this package when using the B<unsuspendauto> config option.
-
 =item quantity
 
 If not set, defaults to 1
@@ -239,6 +253,74 @@ sub cust_unlinked_msg {
   ' (cust_pkg.pkgnum '. $self->pkgnum. ')';
 }
 
+=item set_initial_timers
+
+If required by the package definition, sets any automatic expire, adjourn,
+or contract_end timers to some number of months after the start date 
+(or setup date, if the package has already been setup). If the package has
+a delayed setup fee after a period of "free days", will also set the 
+start date to the end of that period.
+
+If the package has an automatic transfer rule (C<change_to_pkgnum>), then
+this will also order the package and set its start date.
+
+=cut
+
+sub set_initial_timers {
+  my $self = shift;
+  my $part_pkg = $self->part_pkg;
+  my $start = $self->start_date || $self->setup || time;
+
+  foreach my $action ( qw(expire adjourn contract_end) ) {
+    my $months = $part_pkg->get("${action}_months");
+    if($months and !$self->get($action)) {
+      $self->set($action, $part_pkg->add_freq($start, $months) );
+    }
+  }
+
+  # if this package has an expire date and a change_to_pkgpart, set automatic
+  # package transfer
+  # (but don't call change_later, as that would call $self->replace, and we're
+  # probably in the middle of $self->insert right now)
+  if ( $part_pkg->expire_months and $part_pkg->change_to_pkgpart ) {
+    if ( $self->change_to_pkgnum ) {
+      # this can happen if a package is ordered on hold, scheduled for a 
+      # future change _while on hold_, and then released from hold, causing
+      # the automatic transfer to schedule.
+      #
+      # what's correct behavior in that case? I think it's to disallow
+      # future-changing an on-hold package that has an automatic transfer.
+      # but if we DO get into this situation, let the manual package change
+      # win.
+      warn "pkgnum ".$self->pkgnum.": manual future package change blocks ".
+           "automatic transfer.\n";
+    } else {
+      my $change_to = FS::cust_pkg->new( {
+          start_date  => $self->get('expire'),
+          pkgpart     => $part_pkg->change_to_pkgpart,
+          map { $_ => $self->get($_) }
+            qw( custnum locationnum quantity refnum salesnum contract_end )
+      } );
+      my $error = $change_to->insert;
+
+      return $error if $error;
+      $self->set('change_to_pkgnum', $change_to->pkgnum);
+    }
+  }
+
+  # if this package has "free days" and delayed setup fee, then
+  # set start date that many days in the future.
+  # (this should have been set in the UI, but enforce it here)
+  if ( $part_pkg->option('free_days',1)
+       && $part_pkg->option('delay_setup',1)
+     )
+  {
+    $self->start_date( $part_pkg->default_start_date );
+  }
+
+  '';
+}
+
 =item insert [ OPTION => VALUE ... ]
 
 Adds this billing item to the database ("Orders" the item).  If there is an
@@ -254,6 +336,12 @@ setting I<refnum> to an array reference of refnums or a hash reference with
 refnums as keys.  If no I<refnum> is defined, a default FS::pkg_referral
 record will be created corresponding to cust_main.refnum.
 
+If the additional field I<cust_pkg_usageprice> is defined, it will be treated
+as an arrayref of FS::cust_pkg_usageprice objects, which will be inserted.
+(Note that this field cannot be set with a usual ->cust_pkg_usageprice method.
+It can be set as part of the hash when creating the object, or with the B<set>
+method.)
+
 The following options are available:
 
 =over 4
@@ -261,7 +349,8 @@ The following options are available:
 =item change
 
 If set true, supresses actions that should only be taken for new package
-orders.  (Currently this includes: intro periods when delay_setup is on.)
+orders.  (Currently this includes: intro periods when delay_setup is on,
+auto-adding a 1st start date, auto-adding expiration/adjourn/contract_end dates)
 
 =item options
 
@@ -288,56 +377,45 @@ a location change).
 sub insert {
   my( $self, %options ) = @_;
 
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
+
   my $error;
   $error = $self->check_pkgpart unless $options{'allow_pkgpart'};
-  return $error if $error;
 
   my $part_pkg = $self->part_pkg;
 
-  # if the package def says to start only on the first of the month:
-  if ( $part_pkg->option('start_1st', 1) && !$self->start_date ) {
-    my ($sec,$min,$hour,$mday,$mon,$year) = (localtime(time) )[0,1,2,3,4,5];
-    $mon += 1 unless $mday == 1;
-    until ( $mon < 12 ) { $mon -= 12; $year++; }
-    $self->start_date( timelocal_nocheck(0,0,0,1,$mon,$year) );
-  }
-
-  # set up any automatic expire/adjourn/contract_end timers
-  # based on the start date
-  foreach my $action ( qw(expire adjourn contract_end) ) {
-    my $months = $part_pkg->option("${action}_months",1);
-    if($months and !$self->$action) {
-      my $start = $self->start_date || $self->setup || time;
-      $self->$action( $part_pkg->add_freq($start, $months) );
-    }
-  }
-
-  # if this package has "free days" and delayed setup fee, tehn 
-  # set start date that many days in the future.
-  # (this should have been set in the UI, but enforce it here)
-  if (    ! $options{'change'}
-       && ( my $free_days = $part_pkg->option('free_days',1) )
-       && $part_pkg->option('delay_setup',1)
-       #&& ! $self->start_date
-     )
-  {
-    $self->start_date( $part_pkg->default_start_date );
-  }
+  if ( ! $import && ! $options{'change'} ) {
 
-  $self->order_date(time);
+    # set order date to now
+    $self->order_date(time) unless ($import && $self->order_date);
 
-  local $SIG{HUP} = 'IGNORE';
-  local $SIG{INT} = 'IGNORE';
-  local $SIG{QUIT} = 'IGNORE';
-  local $SIG{TERM} = 'IGNORE';
-  local $SIG{TSTP} = 'IGNORE';
-  local $SIG{PIPE} = 'IGNORE';
+    # if the package def says to start only on the first of the month:
+    if ( $part_pkg->option('start_1st', 1) && !$self->start_date ) {
+      my ($sec,$min,$hour,$mday,$mon,$year) = (localtime(time) )[0,1,2,3,4,5];
+      $mon += 1 unless $mday == 1;
+      until ( $mon < 12 ) { $mon -= 12; $year++; }
+      $self->start_date( timelocal_nocheck(0,0,0,1,$mon,$year) );
+    }
 
-  my $oldAutoCommit = $FS::UID::AutoCommit;
-  local $FS::UID::AutoCommit = 0;
-  my $dbh = dbh;
+    if ( $self->susp eq 'now'
+           or ( $part_pkg->start_on_hold && ! $disable_start_on_hold )
+       )
+    {
+      # if the package was ordered on hold:
+      # - suspend it
+      # - don't set the start date (it will be started manually)
+      $self->set('susp', $self->order_date);
+      $self->set('start_date', '');
+    } else {
+      # set expire/adjourn/contract_end timers, and free days, if appropriate
+      # and automatic package transfer, which can fail, so capture the result
+      $error = $self->set_initial_timers;
+    }
+  } # else this is a package change, and shouldn't have "new package" behavior
 
-  $error = $self->SUPER::insert($options{options} ? %{$options{options}} : ());
+  $error ||= $self->SUPER::insert($options{options} ? %{$options{options}} : ());
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
     return $error;
@@ -350,7 +428,18 @@ sub insert {
                       'params'       => $self->refnum,
                     );
 
-  if ( $self->discountnum ) {
+  if ( $self->hashref->{cust_pkg_usageprice} ) {
+    for my $cust_pkg_usageprice ( @{ $self->hashref->{cust_pkg_usageprice} } ) {
+      $cust_pkg_usageprice->pkgnum( $self->pkgnum );
+      my $error = $cust_pkg_usageprice->insert;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return $error;
+      }
+    }
+  }
+
+  if ( $self->setup_discountnum || $self->recur_discountnum ) {
     my $error = $self->insert_discount();
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
@@ -360,7 +449,22 @@ sub insert {
 
   my $conf = new FS::Conf;
 
-  if ( $conf->config('ticket_system') && $options{ticket_subject} ) {
+  if ($self->locationnum) {
+    my @part_export =
+      map qsearch( 'part_export', {exportnum=>$_} ),
+        $conf->config('cust_location-exports'); #, $agentnum
+
+    foreach my $part_export ( @part_export ) {
+      my $error = $part_export->export_pkg_location($self); #, @$export_args);
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return "exporting to ". $part_export->exporttype.
+               " (transaction rolled back): $error";
+      }
+    }
+  }
+
+  if ( ! $import && $conf->config('ticket_system') && $options{ticket_subject} ) {
 
     #this init stuff is still inefficient, but at least its limited to 
     # the small number (any?) folks using ticket emailing on pkg order
@@ -390,7 +494,7 @@ sub insert {
                );
   }
 
-  if ($conf->config('welcome_letter') && $self->cust_main->num_pkgs == 1) {
+  if (! $import && $conf->config('welcome_letter') && $self->cust_main->num_pkgs == 1) {
     my $queue = new FS::queue {
       'job'     => 'FS::cust_main::queueable_print',
     };
@@ -420,15 +524,26 @@ hide cancelled packages.
 
 =cut
 
+# this is still used internally to abort future package changes, so it 
+# does need to work
+
 sub delete {
   my $self = shift;
 
-  local $SIG{HUP} = 'IGNORE';
-  local $SIG{INT} = 'IGNORE';
-  local $SIG{QUIT} = 'IGNORE';
-  local $SIG{TERM} = 'IGNORE';
-  local $SIG{TSTP} = 'IGNORE';
-  local $SIG{PIPE} = 'IGNORE';
+  # The following foreign keys to cust_pkg are not cleaned up here, and will
+  # cause package deletion to fail:
+  #
+  # cust_credit.pkgnum and commission_pkgnum (and cust_credit_void)
+  # cust_credit_bill.pkgnum
+  # cust_pay_pending.pkgnum
+  # cust_pay.pkgnum (and cust_pay_void)
+  # cust_bill_pay.pkgnum (wtf, shouldn't reference pkgnum)
+  # cust_pkg_usage.pkgnum
+  # cust_pkg.uncancel_pkgnum, change_pkgnum, main_pkgnum, and change_to_pkgnum
+  # rt_field_charge.pkgnum
+
+  # cust_svc is handled by canceling the package before deleting it
+  # cust_pkg_option is handled via option_Common
 
   my $oldAutoCommit = $FS::UID::AutoCommit;
   local $FS::UID::AutoCommit = 0;
@@ -465,7 +580,13 @@ sub delete {
     }
   }
 
-  #pkg_referral?
+  foreach my $pkg_referral ( $self->pkg_referral ) {
+    my $error = $pkg_referral->delete;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return $error;
+    }
+  }
 
   my $error = $self->SUPER::delete(@_);
   if ( $error ) {
@@ -540,13 +661,6 @@ sub replace {
 
   local($disable_agentcheck) = 1 if $old->pkgpart == $new->pkgpart;
 
-  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;
@@ -605,6 +719,24 @@ sub replace {
     }
   }
 
+  # also run exports if removing locationnum?
+  #   doesn't seem to happen, and we don't export blank locationnum on insert...
+  if ($new->locationnum and ($new->locationnum != $old->locationnum)) {
+    my $conf = new FS::Conf;
+    my @part_export =
+      map qsearch( 'part_export', {exportnum=>$_} ),
+        $conf->config('cust_location-exports'); #, $agentnum
+
+    foreach my $part_export ( @part_export ) {
+      my $error = $part_export->export_pkg_location($new); #, @$export_args);
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return "exporting to ". $part_export->exporttype.
+               " (transaction rolled back): $error";
+      }
+    }
+  }
+
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   '';
 
@@ -631,6 +763,8 @@ sub check {
     || $self->ut_numbern('pkgpart')
     || $self->ut_foreign_keyn('contactnum',  'contact',       'contactnum' )
     || $self->ut_foreign_keyn('locationnum', 'cust_location', 'locationnum')
+    || $self->ut_foreign_keyn('salesnum', 'sales', 'salesnum')
+    || $self->ut_numbern('quantity')
     || $self->ut_numbern('start_date')
     || $self->ut_numbern('setup')
     || $self->ut_numbern('bill')
@@ -640,9 +774,10 @@ sub check {
     || $self->ut_numbern('resume')
     || $self->ut_numbern('expire')
     || $self->ut_numbern('dundate')
-    || $self->ut_enum('no_auto', [ '', 'Y' ])
-    || $self->ut_enum('waive_setup', [ '', 'Y' ])
-    || $self->ut_numbern('agent_pkgid')
+    || $self->ut_flag('no_auto', [ '', 'Y' ])
+    || $self->ut_flag('waive_setup', [ '', 'Y' ])
+    || $self->ut_flag('separate_bill')
+    || $self->ut_textn('agent_pkgid')
     || $self->ut_enum('recur_show_zero', [ '', 'Y', 'N', ])
     || $self->ut_enum('setup_show_zero', [ '', 'Y', 'N', ])
     || $self->ut_foreign_keyn('main_pkgnum', 'cust_pkg', 'pkgnum')
@@ -652,20 +787,13 @@ sub check {
   return $error if $error;
 
   return "A package with both start date (future start) and setup date (already started) will never bill"
-    if $self->start_date && $self->setup;
+    if $self->start_date && $self->setup && ! $upgrade;
 
   return "A future unsuspend date can only be set for a package with a suspend date"
     if $self->resume and !$self->susp and !$self->adjourn;
 
   $self->usernum($FS::CurrentUser::CurrentUser->usernum) unless $self->usernum;
 
-  if ( $self->dbdef_table->column('manual_flag') ) {
-    $self->manual_flag('') if $self->manual_flag eq ' ';
-    $self->manual_flag =~ /^([01]?)$/
-      or return "Illegal manual_flag ". $self->manual_flag;
-    $self->manual_flag($1);
-  }
-
   $self->SUPER::check;
 }
 
@@ -757,20 +885,35 @@ to a different pkgpart or location, and probably shouldn't be in any other
 case.  If it's not set, the 'unused_credit_cancel' part_pkg option will 
 be used.
 
+=item no_delay_cancel - prevents delay_cancel behavior
+no matter what other options say, for use when changing packages (or any
+other time you're really sure you want an immediate cancel)
+
 =back
 
 If there is an error, returns the error, otherwise returns false.
 
 =cut
 
+#NOT DOCUMENTING - this should only be used when calling recursively
+#=item delay_cancel - for internal use, to allow proper handling of
+#supplemental packages when the main package is flagged to suspend 
+#before cancelling, probably shouldn't be used otherwise (set the
+#corresponding package option instead)
+
 sub cancel {
   my( $self, %options ) = @_;
   my $error;
 
-  # pass all suspend/cancel actions to the main package
-  if ( $self->main_pkgnum and !$options{'from_main'} ) {
-    return $self->main_pkg->cancel(%options);
-  }
+  # supplemental packages can now be separately canceled, though the UI
+  # shouldn't permit it
+  #
+  ## pass all suspend/cancel actions to the main package
+  ## (unless the pkglinknum has been removed, then the link is defunct and
+  ## this package can be canceled on its own)
+  #if ( $self->main_pkgnum and $self->pkglinknum and !$options{'from_main'} ) {
+  #  return $self->main_pkg->cancel(%options);
+  #}
 
   my $conf = new FS::Conf;
 
@@ -778,13 +921,6 @@ sub cancel {
        join(', ', map { "$_: $options{$_}" } keys %options ). "\n"
     if $DEBUG;
 
-  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;
@@ -801,6 +937,21 @@ sub cancel {
   my $date = $options{'date'} if $options{'date'}; # expire/cancel later
   $date = '' if ($date && $date <= $cancel_time);      # complain instead?
 
+  my $delay_cancel = $options{'no_delay_cancel'} ? 0 : $options{'delay_cancel'};
+  if ( !$date && $self->part_pkg->option('delay_cancel',1)
+       && (($self->status eq 'active') || ($self->status eq 'suspended'))
+       && !$options{'no_delay_cancel'}
+  ) {
+    my $expdays = $conf->config('part_pkg-delay_cancel-days') || 1;
+    my $expsecs = 60*60*24*$expdays;
+    my $suspfor = $self->susp ? $cancel_time - $self->susp : 0;
+    $expsecs = $expsecs - $suspfor if $suspfor;
+    unless ($expsecs <= 0) { #if it's already been suspended long enough, don't re-suspend
+      $delay_cancel = 1;
+      $date = $cancel_time + $expsecs;
+    }
+  }
+
   #race condition: usage could be ongoing until unprovisioned
   #resolved by performing a change package instead (which unprovisions) and
   #later cancelling
@@ -849,13 +1000,28 @@ sub cancel {
     }
   }
 
+  # if a reasonnum was passed, get the actual reason object so we can check
+  # unused_credit
+
+  my $reason;
+  if ($options{'reason'} =~ /^\d+$/) {
+    $reason = FS::reason->by_key($options{'reason'});
+  }
+
   unless ($date) {
-    # credit remaining time if appropriate
+    # credit remaining time if any of these are true:
+    # - unused_credit => 1 was passed (this happens when canceling a package
+    #   for a package change when unused_credit_change is set)
+    # - no unused_credit option, and there is a cancel reason, and the cancel
+    #   reason says to credit the package
+    # - no unused_credit option, and the package definition says to credit the
+    #   package on cancellation
     my $do_credit;
     if ( exists($options{'unused_credit'}) ) {
       $do_credit = $options{'unused_credit'};
-    }
-    else {
+    } elsif ( defined($reason) && $reason->unused_credit ) {
+      $do_credit = 1;
+    } else {
       $do_credit = $self->part_pkg->option('unused_credit_cancel', 1);
     }
     if ( $do_credit ) {
@@ -865,22 +1031,38 @@ sub cancel {
         return $error;
       }
     }
-
   } #unless $date
 
   my %hash = $self->hash;
   if ( $date ) {
     $hash{'expire'} = $date;
+    if ($delay_cancel) {
+      # just to be sure these are clear
+      $hash{'adjourn'} = undef;
+      $hash{'resume'} = undef;
+    }
   } else {
     $hash{'cancel'} = $cancel_time;
   }
   $hash{'change_custnum'} = $options{'change_custnum'};
 
+  # if this is a supplemental package that's lost its part_pkg_link, and it's
+  # being canceled for real, unlink it completely
+  if ( !$date and ! $self->pkglinknum ) {
+    $hash{main_pkgnum} = '';
+  }
+
+  # if there is a future package change scheduled, unlink from it (like
+  # abort_change) first, then delete it.
+  $hash{'change_to_pkgnum'} = '';
+
+  # save the package state
   my $new = new FS::cust_pkg ( \%hash );
   $error = $new->replace( $self, options => { $self->options } );
+
   if ( $self->change_to_pkgnum ) {
     my $change_to = FS::cust_pkg->by_key($self->change_to_pkgnum);
-    $error ||= $change_to->cancel || $change_to->delete;
+    $error ||= $change_to->cancel('no_delay_cancel' => 1) || $change_to->delete;
   }
   if ( $error ) {
     $dbh->rollback if $oldAutoCommit;
@@ -888,43 +1070,69 @@ sub cancel {
   }
 
   foreach my $supp_pkg ( $self->supplemental_pkgs ) {
-    $error = $supp_pkg->cancel(%options, 'from_main' => 1);
+    $error = $supp_pkg->cancel(%options, 
+      'from_main' => 1, 
+      'date' => $date, #in case it got changed by delay_cancel
+      'delay_cancel' => $delay_cancel,
+    );
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
       return "canceling supplemental pkg#".$supp_pkg->pkgnum.": $error";
     }
   }
 
-  foreach my $usage ( $self->cust_pkg_usage ) {
-    $error = $usage->delete;
-    if ( $error ) {
-      $dbh->rollback if $oldAutoCommit;
-      return "deleting usage pools: $error";
+  if ($delay_cancel && !$options{'from_main'}) {
+    $error = $new->suspend(
+      'from_cancel' => 1,
+      'time'        => $cancel_time
+    );
+  }
+
+  unless ($date) {
+    foreach my $usage ( $self->cust_pkg_usage ) {
+      $error = $usage->delete;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return "deleting usage pools: $error";
+      }
     }
   }
 
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   return '' if $date; #no errors
 
-  my @invoicing_list = grep { $_ !~ /^(POST|FAX)$/ } $self->cust_main->invoicing_list;
-  if ( !$options{'quiet'} && 
-        $conf->exists('emailcancel', $self->cust_main->agentnum) && 
-        @invoicing_list ) {
-    my $msgnum = $conf->config('cancel_msgnum', $self->cust_main->agentnum);
-    my $error = '';
-    if ( $msgnum ) {
-      my $msg_template = qsearchs('msg_template', { msgnum => $msgnum });
-      $error = $msg_template->send( 'cust_main' => $self->cust_main,
-                                    'object'    => $self );
-    }
-    else {
-      $error = send_email(
-        'from'    => $conf->config('invoice_from', $self->cust_main->agentnum),
-        'to'      => \@invoicing_list,
-        'subject' => ( $conf->config('cancelsubject') || 'Cancellation Notice' ),
-        'body'    => [ map "$_\n", $conf->config('cancelmessage') ],
-      );
-    }
+  my $cust_main = $self->cust_main;
+
+  my @invoicing_list = $cust_main->invoicing_list_emailonly;
+  my $msgnum = $conf->config('cancel_msgnum', $cust_main->agentnum);
+  if (    !$options{'quiet'}
+       && $conf->config_bool('emailcancel', $cust_main->agentnum)
+       && @invoicing_list
+       && $msgnum
+     )
+  {
+    my $msg_template = qsearchs('msg_template', { msgnum => $msgnum });
+    my $error = $msg_template->send(
+      'cust_main' => $cust_main,
+      'object'    => $self,
+    );
+    #should this do something on errors?
+  }
+
+  my %pkg_class = map { $_=>1 }
+                    $conf->config('cancel_msgnum-referring_cust-pkg_class');
+  my $ref_msgnum = $conf->config('cancel_msgnum-referring_cust');
+  if (    !$options{'quiet'}
+       && $cust_main->referral_custnum
+       && $pkg_class{ $self->classnum } 
+       && $ref_msgnum
+     )
+  {
+    my $msg_template = qsearchs('msg_template', { msgnum => $ref_msgnum });
+    my $error = $msg_template->send( 
+      'cust_main' => $cust_main->referring_cust_main,
+      'object'    => $self,
+    );
     #should this do something on errors?
   }
 
@@ -950,6 +1158,166 @@ sub cancel_if_expired {
   '';
 }
 
+=item uncancel_svc_x
+
+For cancelled cust_pkg, returns a list of new, uninserted FS::svc_X records 
+for services that would be inserted by L</uncancel>.  Returned objects also
+include the field _h_svc_x, which contains the service history object.
+
+Set pkgnum before inserting.
+
+Accepts the following options:
+
+only_svcnum - arrayref of svcnum, only returns objects for these svcnum 
+(and only if they would otherwise be returned by this)
+
+=cut
+
+sub uncancel_svc_x {
+  my ($self, %opt) = @_;
+
+  die 'uncancel_svc_x called on a non-cancelled cust_pkg' unless $self->get('cancel');
+
+  #find historical services within this timeframe before the package cancel
+  # (incompatible with "time" option to cust_pkg->cancel?)
+  my $fuzz = 2 * 60; #2 minutes?  too much?   (might catch separate unprovision)
+                     #            too little? (unprovisioing export delay?)
+  my($end, $start) = ( $self->get('cancel'), $self->get('cancel') - $fuzz );
+  my @h_cust_svc = $self->h_cust_svc( $end, $start );
+
+  my @svc_x;
+  foreach my $h_cust_svc (@h_cust_svc) {
+    next if $opt{'only_svcnum'} && !(grep { $_ == $h_cust_svc->svcnum } @{$opt{'only_svcnum'}});
+    # filter out services that still exist on this package (ie preserved svcs)
+    # but keep services that have since been provisioned on another package (for informational purposes)
+    next if qsearchs('cust_svc',{ 'svcnum' => $h_cust_svc->svcnum, 'pkgnum' => $self->pkgnum });
+    my $h_svc_x = $h_cust_svc->h_svc_x( $end, $start );
+    next unless $h_svc_x; # this probably doesn't happen, but just in case
+    (my $table = $h_svc_x->table) =~ s/^h_//;
+    require "FS/$table.pm";
+    my $class = "FS::$table";
+    my $svc_x = $class->new( {
+      'svcpart' => $h_cust_svc->svcpart,
+      '_h_svc_x' => $h_svc_x,
+      map { $_ => $h_svc_x->get($_) } fields($table)
+    } );
+
+    # radius_usergroup
+    if ( $h_svc_x->isa('FS::h_svc_Radius_Mixin') ) {
+      $svc_x->usergroup( [ $h_svc_x->h_usergroup($end, $start) ] );
+    }
+
+    #these are pretty rare, but should handle them
+    # - dsl_device (mac addresses)
+    # - phone_device (mac addresses)
+    # - dsl_note (ikano notes)
+    # - domain_record (i.e. restore DNS information w/domains)
+    # - inventory_item(?) (inventory w/un-cancelling service?)
+    # - nas (svc_broaband nas stuff)
+    #this stuff is unused in the wild afaik
+    # - mailinglistmember
+    # - router.svcnum?
+    # - svc_domain.parent_svcnum?
+    # - acct_snarf (ancient mail fetching config)
+    # - cgp_rule (communigate)
+    # - cust_svc_option (used by our Tron stuff)
+    # - acct_rt_transaction (used by our time worked stuff)
+
+    push @svc_x, $svc_x;
+  }
+  return @svc_x;
+}
+
+=item uncancel_svc_summary
+
+Returns an array of hashrefs, one for each service that could 
+potentially be reprovisioned by L</uncancel>, with the following keys:
+
+svcpart
+
+svc
+
+uncancel_svcnum
+
+label - from history table if not currently calculable, undefined if it can't be loaded
+
+reprovisionable - 1 if test reprovision succeeded, otherwise 0
+
+num_cust_svc - number of svcs for this svcpart, only if summarizing (see below)
+
+Cannot be run from within a transaction.  Performs inserts
+to test the results, and then rolls back the transaction.
+Does not perform exports, so does not catch if export would fail.
+
+Also accepts the following options:
+
+no_test_reprovision - skip the test inserts (reprovisionable field will not exist)
+
+summarize_size - if true, returns a single summary record for svcparts with at
+least this many svcs, will have key num_cust_svc but not uncancel_svcnum, label or reprovisionable
+
+=cut
+
+sub uncancel_svc_summary {
+  my ($self, %opt) = @_;
+
+  die 'uncancel_svc_summary called on a non-cancelled cust_pkg' unless $self->get('cancel');
+  die 'uncancel_svc_summary called from within a transaction' unless $FS::UID::AutoCommit;
+
+  local $FS::svc_Common::noexport_hack = 1; # very important not to run exports!!!
+  local $FS::UID::AutoCommit = 0;
+
+  # sort by svcpart, to check summarize_size
+  my $uncancel_svc_x = {};
+  foreach my $svc_x (sort { $a->{'svcpart'} <=> $b->{'svcpart'} } $self->uncancel_svc_x) {
+    $uncancel_svc_x->{$svc_x->svcpart} = [] unless $uncancel_svc_x->{$svc_x->svcpart};
+    push @{$uncancel_svc_x->{$svc_x->svcpart}}, $svc_x;
+  }
+
+  my @out;
+  foreach my $svcpart (keys %$uncancel_svc_x) {
+    my @svcpart_svc_x = @{$uncancel_svc_x->{$svcpart}};
+    if ($opt{'summarize_size'} && (@svcpart_svc_x >= $opt{'summarize_size'})) {
+      my $svc_x = $svcpart_svc_x[0]; #grab first one for access to $part_svc
+      my $part_svc = $svc_x->part_svc;
+      push @out, {
+        'svcpart'      => $part_svc->svcpart,
+        'svc'          => $part_svc->svc,
+        'num_cust_svc' => scalar(@svcpart_svc_x),
+      };
+    } else {
+      foreach my $svc_x (@svcpart_svc_x) {
+        my $part_svc = $svc_x->part_svc;
+        my $out = {
+          'svcpart' => $part_svc->svcpart,
+          'svc'     => $part_svc->svc,
+          'uncancel_svcnum' => $svc_x->get('_h_svc_x')->svcnum,
+        };
+        $svc_x->pkgnum($self->pkgnum); # provisioning services on a canceled package, will be rolled back
+        my $insert_error;
+        unless ($opt{'no_test_reprovision'}) {
+          # avoid possibly fatal errors from missing linked records
+          eval { $insert_error = $svc_x->insert };
+          $insert_error ||= $@;
+        }
+        if ($opt{'no_test_reprovision'} or $insert_error) {
+          # avoid possibly fatal errors from missing linked records
+          eval { $out->{'label'} = $svc_x->label };
+          eval { $out->{'label'} = $svc_x->get('_h_svc_x')->label } unless defined($out->{'label'});
+          $out->{'reprovisionable'} = 0 unless $opt{'no_test_reprovision'};
+        } else {
+          $out->{'label'} = $svc_x->label;
+          $out->{'reprovisionable'} = 1;
+        }
+        push @out, $out;
+      }
+    }
+  }
+
+  dbh->rollback;
+  return @out;
+}
+
 =item uncancel
 
 "Un-cancels" this package: Orders a new package with the same custnum, pkgpart,
@@ -962,6 +1330,8 @@ svc_fatal: service provisioning errors are fatal
 
 svc_errors: pass an array reference, will be filled in with any provisioning errors
 
+only_svcnum: arrayref, only attempt to re-provision these cancelled services
+
 main_pkgnum: link the package as a supplemental package of this one.  For 
 internal use only.
 
@@ -981,13 +1351,6 @@ sub uncancel {
   # Transaction-alize
   ##
 
-  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;
@@ -1007,7 +1370,8 @@ sub uncancel {
       setup
       susp adjourn resume expire start_date contract_end dundate
       change_date change_pkgpart change_locationnum
-      manual_flag no_auto quantity agent_pkgid recur_show_zero setup_show_zero
+      no_auto separate_bill quantity agent_pkgid 
+      recur_show_zero setup_show_zero
     ),
   };
 
@@ -1024,32 +1388,12 @@ sub uncancel {
   # insert services
   ##
 
-  #find historical services within this timeframe before the package cancel
-  # (incompatible with "time" option to cust_pkg->cancel?)
-  my $fuzz = 2 * 60; #2 minutes?  too much?   (might catch separate unprovision)
-                     #            too little? (unprovisioing export delay?)
-  my($end, $start) = ( $self->get('cancel'), $self->get('cancel') - $fuzz );
-  my @h_cust_svc = $self->h_cust_svc( $end, $start );
-
   my @svc_errors;
-  foreach my $h_cust_svc (@h_cust_svc) {
-    my $h_svc_x = $h_cust_svc->h_svc_x( $end, $start );
-    #next unless $h_svc_x; #should this happen?
-    (my $table = $h_svc_x->table) =~ s/^h_//;
-    require "FS/$table.pm";
-    my $class = "FS::$table";
-    my $svc_x = $class->new( {
-      'pkgnum'  => $cust_pkg->pkgnum,
-      'svcpart' => $h_cust_svc->svcpart,
-      map { $_ => $h_svc_x->get($_) } fields($table)
-    } );
-
-    # radius_usergroup
-    if ( $h_svc_x->isa('FS::h_svc_Radius_Mixin') ) {
-      $svc_x->usergroup( [ $h_svc_x->h_usergroup($end, $start) ] );
-    }
+  foreach my $svc_x ($self->uncancel_svc_x('only_svcnum' => $options{'only_svcnum'})) {
 
+    $svc_x->pkgnum($cust_pkg->pkgnum);
     my $svc_error = $svc_x->insert;
+
     if ( $svc_error ) {
       if ( $options{svc_fatal} ) {
         $dbh->rollback if $oldAutoCommit;
@@ -1073,23 +1417,7 @@ sub uncancel {
         }
       } # svc_fatal
     } # svc_error
-  } #foreach $h_cust_svc
-
-  #these are pretty rare, but should handle them
-  # - dsl_device (mac addresses)
-  # - phone_device (mac addresses)
-  # - dsl_note (ikano notes)
-  # - domain_record (i.e. restore DNS information w/domains)
-  # - inventory_item(?) (inventory w/un-cancelling service?)
-  # - nas (svc_broaband nas stuff)
-  #this stuff is unused in the wild afaik
-  # - mailinglistmember
-  # - router.svcnum?
-  # - svc_domain.parent_svcnum?
-  # - acct_snarf (ancient mail fetching config)
-  # - cgp_rule (communigate)
-  # - cust_svc_option (used by our Tron stuff)
-  # - acct_rt_transaction (used by our time worked stuff)
+  } #foreach uncancel_svc_x
 
   ##
   # also move over any services that didn't unprovision at cancellation
@@ -1132,23 +1460,17 @@ sub uncancel {
 
 =item unexpire
 
-Cancels any pending expiration (sets the expire field to null).
+Cancels any pending expiration (sets the expire field to null)
+for this package and any supplemental packages.
 
 If there is an error, returns the error, otherwise returns false.
 
 =cut
 
 sub unexpire {
-  my( $self, %options ) = @_;
+  my( $self ) = @_;
   my $error;
 
-  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;
@@ -1176,6 +1498,14 @@ sub unexpire {
     return $error;
   }
 
+  foreach my $supp_pkg ( $self->supplemental_pkgs ) {
+    $error = $supp_pkg->unexpire;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "unexpiring supplemental pkg#".$supp_pkg->pkgnum.": $error";
+    }
+  }
+
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   ''; #no errors
@@ -1191,7 +1521,7 @@ Available options are:
 
 =over 4
 
-=item reason - can be set to a cancellation reason (see L<FS:reason>), 
+=item reason - can be set to a cancellation reason (see L<FS:reason>),
 either a reasonnum of an existing reason, or passing a hashref will create 
 a new reason.  The hashref should have the following keys: 
 - typenum - Reason type (see L<FS::reason_type>
@@ -1210,6 +1540,9 @@ separately.
 =item from_main - allows a supplemental package to be suspended, rather
 than redirecting the method call to its main package.  For internal use.
 
+=item from_cancel - used when suspending from the cancel method, forces
+this to skip everything besides basic suspension.  For internal use.
+
 =back
 
 If there is an error, returns the error, otherwise returns false.
@@ -1220,18 +1553,15 @@ sub suspend {
   my( $self, %options ) = @_;
   my $error;
 
-  # pass all suspend/cancel actions to the main package
+  # supplemental packages still can't be separately suspended, but silently
+  # exit instead of failing or passing the action to the main package (so
+  # that the "Suspend customer" action doesn't trip over the supplemental
+  # packages and die)
+
   if ( $self->main_pkgnum and !$options{'from_main'} ) {
-    return $self->main_pkg->suspend(%options);
+    return;
   }
 
-  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;
@@ -1259,7 +1589,7 @@ sub suspend {
   }
 
   # some false laziness with sub cancel
-  if ( !$options{nobill} && !$date &&
+  if ( !$options{nobill} && !$date && !$options{'from_cancel'} &&
        $self->part_pkg->option('bill_suspend_as_cancel',1) ) {
     # kind of a kludge--'bill_suspend_as_cancel' to avoid having to 
     # make the entire cust_main->bill path recognize 'suspend' and 
@@ -1276,6 +1606,7 @@ sub suspend {
       if $error;
   }
 
+  my $cust_pkg_reason;
   if ( $options{'reason'} ) {
     $error = $self->insert_reason( 'reason' => $options{'reason'},
                                    'action' => $date ? 'adjourn' : 'suspend',
@@ -1286,6 +1617,21 @@ sub suspend {
       dbh->rollback if $oldAutoCommit;
       return "Error inserting cust_pkg_reason: $error";
     }
+    $cust_pkg_reason = qsearchs('cust_pkg_reason', {
+        'date'    => $date ? $date : $suspend_time,
+        'action'  => $date ? 'A' : 'S',
+        'pkgnum'  => $self->pkgnum,
+    });
+  }
+
+  # if a reasonnum was passed, get the actual reason object so we can check
+  # unused_credit
+  # (passing a reason hashref is still allowed, but it can't be used with
+  # the fancy behavioral options.)
+
+  my $reason;
+  if ($options{'reason'} =~ /^\d+$/) {
+    $reason = FS::reason->by_key($options{'reason'});
   }
 
   my %hash = $self->hash;
@@ -1312,44 +1658,76 @@ sub suspend {
     return $error;
   }
 
-  unless ( $date ) {
-    # credit remaining time if appropriate
-    if ( $self->part_pkg->option('unused_credit_suspend', 1) ) {
-      my $error = $self->credit_remaining('suspend', $suspend_time);
-      if ($error) {
-        $dbh->rollback if $oldAutoCommit;
-        return $error;
+  unless ( $date ) { # then we are suspending now
+
+    unless ($options{'from_cancel'}) {
+      # credit remaining time if appropriate
+      # (if required by the package def, or the suspend reason)
+      my $unused_credit = $self->part_pkg->option('unused_credit_suspend',1)
+                          || ( defined($reason) && $reason->unused_credit );
+
+      if ( $unused_credit ) {
+        warn "crediting unused time on pkg#".$self->pkgnum."\n" if $DEBUG;
+        my $error = $self->credit_remaining('suspend', $suspend_time);
+        if ($error) {
+          $dbh->rollback if $oldAutoCommit;
+          return $error;
+        }
       }
     }
 
-    my @labels = ();
+    my @cust_svc = qsearch( 'cust_svc', { 'pkgnum' => $self->pkgnum } );
+
+    #attempt ordering ala cust_svc_suspend_cascade (without infinite-looping
+    # on the circular dep case)
+    #  (this is too simple for multi-level deps, we need to use something
+    #   to resolve the DAG properly when possible)
+    my %svcpart = ();
+    $svcpart{$_->svcpart} = 0 foreach @cust_svc;
+    foreach my $svcpart ( keys %svcpart ) {
+      foreach my $part_svc_link (
+        FS::part_svc_link->by_agentnum($self->cust_main->agentnum,
+                                         src_svcpart => $svcpart,
+                                         link_type => 'cust_svc_suspend_cascade'
+                                      )
+      ) {
+        $svcpart{$part_svc_link->dst_svcpart} = max(
+          $svcpart{$part_svc_link->dst_svcpart},
+          $svcpart{$part_svc_link->src_svcpart} + 1
+        );
+      }
+    }
+    @cust_svc = sort { $svcpart{ $a->svcpart } <=> $svcpart{ $b->svcpart } }
+                  @cust_svc;
 
-    foreach my $cust_svc (
-      qsearch( 'cust_svc', { 'pkgnum' => $self->pkgnum } )
-    ) {
-      my $part_svc = qsearchs( 'part_svc', { 'svcpart' => $cust_svc->svcpart } );
+    my @labels = ();
+    foreach my $cust_svc ( @cust_svc ) {
+      $cust_svc->suspend( 'labels_arrayref' => \@labels );
+    }
 
-      $part_svc->svcdb =~ /^([\w\-]+)$/ or do {
-        $dbh->rollback if $oldAutoCommit;
-        return "Illegal svcdb value in part_svc!";
-      };
-      my $svcdb = $1;
-      require "FS/$svcdb.pm";
+    # suspension fees: if there is a feepart, and it's not an unsuspend fee,
+    # and this is not a suspend-before-cancel
+    if ( $cust_pkg_reason ) {
+      my $reason_obj = $cust_pkg_reason->reason;
+      if ( $reason_obj->feepart and
+           ! $reason_obj->fee_on_unsuspend and
+           ! $options{'from_cancel'} ) {
 
-      my $svc = qsearchs( $svcdb, { 'svcnum' => $cust_svc->svcnum } );
-      if ($svc) {
-        $error = $svc->suspend;
-        if ( $error ) {
-          $dbh->rollback if $oldAutoCommit;
-          return $error;
-        }
-        my( $label, $value ) = $cust_svc->label;
-        push @labels, "$label: $value";
+        # register the need to charge a fee, cust_main->bill will do the rest
+        warn "registering suspend fee: pkgnum ".$self->pkgnum.", feepart ".$reason->feepart."\n"
+          if $DEBUG;
+        my $cust_pkg_reason_fee = FS::cust_pkg_reason_fee->new({
+            'pkgreasonnum'  => $cust_pkg_reason->num,
+            'pkgnum'        => $self->pkgnum,
+            'feepart'       => $reason->feepart,
+            'nextbill'      => $reason->fee_hold,
+        });
+        $error ||= $cust_pkg_reason_fee->insert;
       }
     }
 
     my $conf = new FS::Conf;
-    if ( $conf->config('suspend_email_admin') ) {
+    if ( $conf->config('suspend_email_admin') && !$options{'from_cancel'} ) {
  
       my $error = send_email(
         'from'    => $conf->config('invoice_from', $self->cust_main->agentnum),
@@ -1364,6 +1742,8 @@ sub suspend {
           'Package : #'. $self->pkgnum. " (". $self->part_pkg->pkg_comment. ")\n",
           ( map { "Service : $_\n" } @labels ),
         ],
+        'custnum' => $self->custnum,
+        'msgtype' => 'admin'
       );
 
       if ( $error ) {
@@ -1397,6 +1777,21 @@ are mandatory.
 
 =cut
 
+# Implementation note:
+#
+# If you pkgpart-change a package that has been billed, and it's set to give
+# credit on package change, then this method gets called and then the new
+# package will have no last_bill date. Therefore the customer will be credited
+# only once (per billing period) even if there are multiple package changes.
+#
+# If you location-change a package that has been billed, this method will NOT
+# be called and the new package WILL have the last bill date of the old
+# package.
+#
+# If the new package is then canceled within the same billing cycle, 
+# credit_remaining needs to run calc_remain on the OLD package to determine
+# the amount of unused time to credit.
+
 sub credit_remaining {
   # Add a credit for remaining service
   my ($self, $mode, $time) = @_;
@@ -1407,26 +1802,105 @@ sub credit_remaining {
   my $conf = FS::Conf->new;
   my $reason_type = $conf->config($mode.'_credit_type');
 
-  my $last_bill = $self->getfield('last_bill') || 0;
-  my $next_bill = $self->getfield('bill') || 0;
-  if ( $last_bill > 0         # the package has been billed
-      and $next_bill > 0      # the package has a next bill date
-      and $next_bill >= $time # which is in the future
-  ) {
-    my $remaining_value = $self->calc_remain('time' => $time);
-    if ( $remaining_value > 0 ) {
-      warn "Crediting for $remaining_value on package ".$self->pkgnum."\n"
-        if $DEBUG;
-      my $error = $self->cust_main->credit(
-        $remaining_value,
-        'Credit for unused time on '. $self->part_pkg->pkg,
-        'reason_type' => $reason_type,
-      );
-      return "Error crediting customer \$$remaining_value for unused time".
-        " on ". $self->part_pkg->pkg. ": $error"
-        if $error;
-    } #if $remaining_value
-  } #if $last_bill, etc.
+  $time ||= time;
+
+  my $remain_pkg = $self;
+  my (@billpkgnums, @amounts, @setuprecurs);
+  
+  # we may have to walk back past some package changes to get to the 
+  # one that actually has unused time. loop until that happens, or we
+  # reach the first package in the chain.
+  while (1) {
+    my $last_bill = $remain_pkg->get('last_bill') || 0;
+    my $next_bill = $remain_pkg->get('bill') || 0;
+    if ( $last_bill > 0         # the package has been billed
+        and $next_bill > 0      # the package has a next bill date
+        and $next_bill >= $time # which is in the future
+    ) {
+
+      # Find actual charges for the period ending on or after the cancel
+      # date.
+      my @charges = qsearch('cust_bill_pkg', {
+        pkgnum => $remain_pkg->pkgnum,
+        edate => {op => '>=', value => $time},
+        recur => {op => '>' , value => 0},
+      });
+
+      foreach my $cust_bill_pkg (@charges) {
+        # hack to deal with the weird behavior of edate on package
+        # cancellation
+        my $edate = $cust_bill_pkg->edate;
+        if ( $self->recur_temporality eq 'preceding' ) {
+          $edate = $self->add_freq($cust_bill_pkg->sdate);
+        }
+
+        # this will also get any package charges that are _entirely_ after
+        # the cancellation date (can happen with advance billing). in that
+        # case, use the entire recurring charge:
+        my $amount = $cust_bill_pkg->recur - $cust_bill_pkg->usage;
+        my $max_credit = $amount
+            - $cust_bill_pkg->credited('', '', setuprecur => 'recur') || 0;
+
+        # but if the cancellation happens during the interval, prorate it:
+        # (XXX obey prorate_round_day here?)
+        if ( $cust_bill_pkg->sdate < $time ) {
+          $amount = $amount *
+                      ($edate - $time) / ($edate - $cust_bill_pkg->sdate);
+        }
+
+        # if there are existing credits, don't let the sum of credits exceed
+        # the recurring charge
+        $amount = $max_credit if $amount > $max_credit;
+
+        $amount = sprintf('%.2f', $amount);
+
+        # if no time has been used and/or there are existing line item
+        # credits, we may end up not needing to credit anything.
+        if ( $amount > 0 ) {
+
+          push @billpkgnums, $cust_bill_pkg->billpkgnum;
+          push @amounts,     $amount;
+          push @setuprecurs, 'recur';
+
+          warn "Crediting for $amount on package ".$remain_pkg->pkgnum."\n"
+            if $DEBUG;
+        }
+
+      }
+
+      last if @charges;
+    }
+
+    if ( my $changed_from_pkgnum = $remain_pkg->change_pkgnum ) {
+      $remain_pkg = FS::cust_pkg->by_key($changed_from_pkgnum);
+    } else {
+      # the package has really never been billed
+      return;
+    }
+  }
+
+  # keep traditional behavior here. 
+  local $@;
+  my $reason = FS::reason->new_or_existing(
+    reason  => 'Credit for unused time on '. $self->part_pkg->pkg,
+    type    => $reason_type,
+    class   => 'R',
+  );
+  if ( $@ ) {
+    return "failed to set credit reason: $@";
+  }
+
+  my $error = FS::cust_credit->credit_lineitems(
+    'billpkgnums' => \@billpkgnums,
+    'setuprecurs' => \@setuprecurs,
+    'amounts'     => \@amounts,
+    'custnum'     => $self->custnum,
+    'date'        => time,
+    'reasonnum'   => $reason->reasonnum,
+    'apply'       => 1,
+    'set_source'  => 1,
+  );
+
   '';
 }
 
@@ -1450,10 +1924,8 @@ field).
 
 Can be set true to adjust the next bill date forward by
 the amount of time the account was inactive.  This was set true by default
-since 1.4.2 and 1.5.0pre6; however, starting with 1.7.0 this needs to be
-explicitly requested.  Price plans for which this makes sense (anniversary-date
-based than prorate or subscription) could have an option to enable this
-behaviour?
+in the past (from 1.4.2 and 1.5.0pre6 through 1.7.0), but now needs to be
+explicitly requested with this option or in the price plan.
 
 =back
 
@@ -1470,13 +1942,6 @@ sub unsuspend {
     return $self->main_pkg->unsuspend(%opt);
   }
 
-  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;
@@ -1494,6 +1959,8 @@ sub unsuspend {
     return "";  # no error                     # complain instead?
   }
 
+  # handle the case of setting a future unsuspend (resume) date
+  # and do not continue to actually unsuspend the package
   my $date = $opt{'date'};
   if ( $date and $date > time ) { # return an error if $date <= time?
 
@@ -1517,6 +1984,15 @@ sub unsuspend {
   
   } #if $date 
 
+  if (!$self->setup) {
+    # then this package is being released from on-hold status
+    $error = $self->set_initial_timers;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return $error;
+    }
+  }
+
   my @labels = ();
 
   foreach my $cust_svc (
@@ -1552,15 +2028,46 @@ sub unsuspend {
 
   my $conf = new FS::Conf;
 
-  if ( $inactive > 0 && 
-       ( $hash{'bill'} || $hash{'setup'} ) &&
-       ( $opt{'adjust_next_bill'} ||
-         $conf->exists('unsuspend-always_adjust_next_bill_date') ||
-         $self->part_pkg->option('unsuspend_adjust_bill', 1) )
-     ) {
+  #adjust the next bill date forward
+  # increment next bill date if certain conditions are met:
+  # - it was due to be billed at some point
+  # - either the global or local config says to do this
+  my $adjust_bill = 0;
+  if (
+       $inactive > 0
+    && ( $hash{'bill'} || $hash{'setup'} )
+    && (    $opt{'adjust_next_bill'}
+         || $conf->exists('unsuspend-always_adjust_next_bill_date')
+         || $self->part_pkg->option('unsuspend_adjust_bill', 1)
+       )
+  ) {
+    $adjust_bill = 1;
+  }
+
+  # but not if:
+  # - the package billed during suspension
+  # - or it was ordered on hold
+  # - or the customer was credited for the unused time
 
-    $hash{'bill'} = ( $hash{'bill'} || $hash{'setup'} ) + $inactive;
-  
+  if ( $self->option('suspend_bill',1)
+      or ( $self->part_pkg->option('suspend_bill',1)
+           and ! $self->option('no_suspend_bill',1)
+         )
+      or $hash{'order_date'} == $hash{'susp'}
+  ) {
+    $adjust_bill = 0;
+  }
+
+  if ( $adjust_bill ) {
+    if (    $self->part_pkg->option('unused_credit_suspend')
+         or ( ref($reason) and $reason->unused_credit ) ) {
+      # then the customer was credited for the unused time before suspending,
+      # so their next bill should be immediate 
+      $hash{'bill'} = time;
+    } else {
+      # add the length of time suspended to the bill date
+      $hash{'bill'} = ( $hash{'bill'} || $hash{'setup'} ) + $inactive;
+    }
   }
 
   $hash{'susp'} = '';
@@ -1575,23 +2082,39 @@ sub unsuspend {
 
   my $unsusp_pkg;
 
-  if ( $reason && $reason->unsuspend_pkgpart ) {
-    my $part_pkg = FS::part_pkg->by_key($reason->unsuspend_pkgpart)
-      or $error = "Unsuspend package definition ".$reason->unsuspend_pkgpart.
-                  " not found.";
-    my $start_date = $self->cust_main->next_bill_date 
-      if $reason->unsuspend_hold;
-
-    if ( $part_pkg ) {
-      $unsusp_pkg = FS::cust_pkg->new({
-          'custnum'     => $self->custnum,
-          'pkgpart'     => $reason->unsuspend_pkgpart,
-          'start_date'  => $start_date,
-          'locationnum' => $self->locationnum,
-          # discount? probably not...
+  if ( $reason ) {
+    if ( $reason->unsuspend_pkgpart ) {
+      warn "Suspend reason '".$reason->reason."' uses deprecated unsuspend_pkgpart feature.\n";
+      my $part_pkg = FS::part_pkg->by_key($reason->unsuspend_pkgpart)
+        or $error = "Unsuspend package definition ".$reason->unsuspend_pkgpart.
+                    " not found.";
+      my $start_date = $self->cust_main->next_bill_date 
+        if $reason->unsuspend_hold;
+
+      if ( $part_pkg ) {
+        $unsusp_pkg = FS::cust_pkg->new({
+            'custnum'     => $self->custnum,
+            'pkgpart'     => $reason->unsuspend_pkgpart,
+            'start_date'  => $start_date,
+            'locationnum' => $self->locationnum,
+            # discount? probably not...
+        });
+
+        $error ||= $self->cust_main->order_pkg( 'cust_pkg' => $unsusp_pkg );
+      }
+    }
+    # new way, using fees
+    if ( $reason->feepart and $reason->fee_on_unsuspend ) {
+      # register the need to charge a fee, cust_main->bill will do the rest
+      warn "registering unsuspend fee: pkgnum ".$self->pkgnum.", feepart ".$reason->feepart."\n"
+        if $DEBUG;
+      my $cust_pkg_reason_fee = FS::cust_pkg_reason_fee->new({
+          'pkgreasonnum'  => $cust_pkg_reason->num,
+          'pkgnum'        => $self->pkgnum,
+          'feepart'       => $reason->feepart,
+          'nextbill'      => $reason->fee_hold,
       });
-      
-      $error ||= $self->cust_main->order_pkg( 'cust_pkg' => $unsusp_pkg );
+      $error ||= $cust_pkg_reason_fee->insert;
     }
 
     if ( $error ) {
@@ -1619,6 +2142,8 @@ sub unsuspend {
           : ''
         ),
       ],
+      'custnum' => $self->custnum,
+      'msgtype' => 'admin',
     );
 
     if ( $error ) {
@@ -1643,23 +2168,17 @@ sub unsuspend {
 
 =item unadjourn
 
-Cancels any pending suspension (sets the adjourn field to null).
+Cancels any pending suspension (sets the adjourn field to null)
+for this package and any supplemental packages.
 
 If there is an error, returns the error, otherwise returns false.
 
 =cut
 
 sub unadjourn {
-  my( $self, %options ) = @_;
+  my( $self ) = @_;
   my $error;
 
-  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;
@@ -1694,6 +2213,14 @@ sub unadjourn {
     return $error;
   }
 
+  foreach my $supp_pkg ( $self->supplemental_pkgs ) {
+    $error = $supp_pkg->unadjourn;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "unadjourning supplemental pkg#".$supp_pkg->pkgnum.": $error";
+    }
+  }
+
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   ''; #no errors
@@ -1734,6 +2261,11 @@ New pkgpart (see L<FS::part_pkg>).
 
 New refnum (see L<FS::part_referral>).
 
+=item quantity
+
+New quantity; if unspecified, the new package will have the same quantity
+as the old.
+
 =item cust_pkg
 
 "New" (existing) FS::cust_pkg object.  The package's services and other 
@@ -1751,6 +2283,13 @@ can't be transferred (also see the I<cust_pkg-change_svcpart> config option).
 If unprotect_svcs is true, this method will transfer as many services as 
 it can and then unconditionally cancel the old package.
 
+=item contract_end
+
+If specified, sets this value for the contract_end date on the new package 
+(without regard for keep_dates or the usual date-preservation behavior.)
+Will throw an error if defined but false;  the UI doesn't allow editing 
+this unless it already exists, making removal impossible to undo.
+
 =back
 
 At least one of locationnum, cust_location, pkgpart, refnum, cust_main, or
@@ -1764,56 +2303,66 @@ For example:
 
 =cut
 
+#used by change and change_later
+#didn't put with documented check methods because it depends on change-specific opts
+#and it also possibly edits the value of opts
+sub _check_change {
+  my $self = shift;
+  my $opt = shift;
+  if ( defined($opt->{'contract_end'}) ) {
+    my $current_contract_end = $self->get('contract_end');
+    unless ($opt->{'contract_end'}) {
+      if ($current_contract_end) {
+        return "Cannot remove contract end date when changing packages";
+      } else {
+        #shouldn't even pass this option if there's not a current value
+        #but can be handled gracefully if the option is empty
+        warn "Contract end date passed unexpectedly";
+        delete $opt->{'contract_end'};
+        return '';
+      }
+    }
+    unless ($current_contract_end) {
+      #option shouldn't be passed, throw error if it's non-empty
+      return "Cannot add contract end date when changing packages " . $self->pkgnum;
+    }
+  }
+  return '';
+}
+
 #some false laziness w/order
 sub change {
   my $self = shift;
   my $opt = ref($_[0]) ? shift : { @_ };
 
-#  my ($custnum, $pkgparts, $remove_pkgnum, $return_cust_pkg, $refnum) = @_;
-#    
-
   my $conf = new FS::Conf;
 
-  # Transactionize this whole mess
-  local $SIG{HUP} = 'IGNORE';
-  local $SIG{INT} = 'IGNORE'; 
-  local $SIG{QUIT} = 'IGNORE';
-  local $SIG{TERM} = 'IGNORE';
-  local $SIG{TSTP} = 'IGNORE'; 
-  local $SIG{PIPE} = 'IGNORE'; 
+  # handle contract_end on cust_pkg same as passed option
+  if ( $opt->{'cust_pkg'} ) {
+    $opt->{'contract_end'} = $opt->{'cust_pkg'}->contract_end;
+    delete $opt->{'contract_end'} unless $opt->{'contract_end'};
+  }
+
+  # check contract_end, prevent adding/removing
+  my $error = $self->_check_change($opt);
+  return $error if $error;
 
+  # Transactionize this whole mess
   my $oldAutoCommit = $FS::UID::AutoCommit;
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
 
-  my $error;
-
-  my %hash = (); 
-
-  my $time = time;
-
-  #$hash{$_} = $self->$_() foreach qw( last_bill bill );
-    
-  #$hash{$_} = $self->$_() foreach qw( setup );
-
-  $hash{'setup'} = $time if $self->setup;
-
-  $hash{'change_date'} = $time;
-  $hash{"change_$_"}  = $self->$_()
-    foreach qw( pkgnum pkgpart locationnum );
-
   if ( $opt->{'cust_location'} ) {
     $error = $opt->{'cust_location'}->find_or_insert;
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
-      return "inserting cust_location (transaction rolled back): $error";
+      return "creating location record: $error";
     }
     $opt->{'locationnum'} = $opt->{'cust_location'}->locationnum;
   }
 
+  # figure out if we're changing pkgpart
   if ( $opt->{'cust_pkg'} ) {
-    # treat changing to a package with a different pkgpart as a 
-    # pkgpart change (because it is)
     $opt->{'pkgpart'} = $opt->{'cust_pkg'}->pkgpart;
   }
 
@@ -1823,26 +2372,108 @@ sub change {
     $same_pkgpart = 0;
   }
 
+  $self->set('waive_setup', $opt->{'waive_setup'}) if $opt->{'waive_setup'};
+
+  # Before going any further here: if the package is still in the pre-setup
+  # state, it's safe to modify it in place. No need to charge/credit for 
+  # partial period, transfer usage pools, copy invoice details, or change any
+  # dates. We DO need to "transfer" services (from the package to itself) to
+  # check their validity on the new pkgpart.
+  if ( ! $self->setup and ! $opt->{cust_pkg} and ! $opt->{cust_main} ) {
+    foreach ( qw( locationnum pkgpart quantity refnum salesnum waive_setup ) ) {
+      if ( length($opt->{$_}) ) {
+        $self->set($_, $opt->{$_});
+      }
+    }
+    # almost. if the new pkgpart specifies start/adjourn/expire timers, 
+    # apply those.
+    if ( !$same_pkgpart ) {
+      $error ||= $self->set_initial_timers;
+    }
+    # but if contract_end was explicitly specified, that overrides all else
+    $self->set('contract_end', $opt->{'contract_end'})
+      if $opt->{'contract_end'};
+
+    $error ||= $self->replace;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "modifying package: $error";
+    }
+
+    # check/convert services (only on pkgpart change, to avoid surprises
+    # when editing locations)
+    # (maybe do this if changing quantity?)
+    if ( !$same_pkgpart ) {
+
+      $error = $self->transfer($self);
+
+      if ( $error and $error == 0 ) {
+        $error = "transferring $error";
+      } elsif ( $error > 0 && $conf->exists('cust_pkg-change_svcpart') ) {
+        warn "trying transfer again with change_svcpart option\n" if $DEBUG;
+        $error = $self->transfer($self, 'change_svcpart'=>1 );
+        if ($error and $error == 0) {
+          $error = "converting $error";
+        }
+      }
+
+      if ($error > 0) {
+        $error = "unable to transfer all services";
+      }
+
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return $error;
+      }
+
+    } # done transferring services
+
+    $dbh->commit if $oldAutoCommit;
+    return $self;
+
+  }
+
+  my %hash = (); 
+
+  my $time = time;
+
+  $hash{'setup'} = $time if $self->get('setup');
+
+  $hash{'change_date'} = $time;
+  $hash{"change_$_"}  = $self->$_()
+    foreach qw( pkgnum pkgpart locationnum );
+
   my $unused_credit = 0;
   my $keep_dates = $opt->{'keep_dates'};
-  # Special case.  If the pkgpart is changing, and the customer is
-  # going to be credited for remaining time, don't keep setup, bill, 
-  # or last_bill dates, and DO pass the flag to cancel() to credit 
-  # the customer.
+
+  # Special case.  If the pkgpart is changing, and the customer is going to be
+  # credited for remaining time, don't keep setup, bill, or last_bill dates,
+  # and DO pass the flag to cancel() to credit the customer.  If the old
+  # package had a setup date, set the new package's setup to the package
+  # change date so that it has the same status as before.
   if ( $opt->{'pkgpart'} 
        and $opt->{'pkgpart'} != $self->pkgpart
        and $self->part_pkg->option('unused_credit_change', 1) ) {
     $unused_credit = 1;
     $keep_dates = 0;
-    $hash{$_} = '' foreach qw(setup bill last_bill);
+    $hash{'last_bill'} = '';
+    $hash{'bill'} = '';
   }
 
   if ( $keep_dates ) {
-    foreach my $date ( qw(setup bill last_bill susp adjourn cancel expire 
-                          resume start_date contract_end ) ) {
+    foreach my $date ( qw(setup bill last_bill) ) {
       $hash{$date} = $self->getfield($date);
     }
   }
+  # always keep the following dates
+  foreach my $date (qw(order_date susp adjourn cancel expire resume 
+                    start_date contract_end)) {
+    $hash{$date} = $self->getfield($date);
+  }
+  # but if contract_end was explicitly specified, that overrides all else
+  $hash{'contract_end'} = $opt->{'contract_end'}
+    if $opt->{'contract_end'};
+
   # allow $opt->{'locationnum'} = '' to specifically set it to null
   # (i.e. customer default location)
   $opt->{'locationnum'} = $self->locationnum if !exists($opt->{'locationnum'});
@@ -1853,14 +2484,17 @@ sub change {
   # 2. (more importantly) changing a package before it's billed
   $hash{'waive_setup'} = $self->waive_setup;
 
+  # if this package is scheduled for a future package change, preserve that
+  $hash{'change_to_pkgnum'} = $self->change_to_pkgnum;
+
   my $custnum = $self->custnum;
   if ( $opt->{cust_main} ) {
     my $cust_main = $opt->{cust_main};
     unless ( $cust_main->custnum ) { 
-      my $error = $cust_main->insert;
+      my $error = $cust_main->insert( @{ $opt->{cust_main_insert_args}||[] } );
       if ( $error ) {
         $dbh->rollback if $oldAutoCommit;
-        return "inserting cust_main (transaction rolled back): $error";
+        return "inserting customer record: $error";
       }
     }
     $custnum = $cust_main->custnum;
@@ -1874,19 +2508,25 @@ sub change {
     # changed from this package.
     $cust_pkg = $opt->{'cust_pkg'};
 
-    foreach ( qw( pkgnum pkgpart locationnum ) ) {
-      $cust_pkg->set("change_$_", $self->get($_));
+    # follow all the above rules for date changes, etc.
+    foreach (keys %hash) {
+      $cust_pkg->set($_, $hash{$_});
     }
-    $cust_pkg->set('change_date', $time);
+    # except those that implement the future package change behavior
+    foreach (qw(change_to_pkgnum start_date expire)) {
+      $cust_pkg->set($_, '');
+    }
+
     $error = $cust_pkg->replace;
 
   } else {
     # Create the new package.
     $cust_pkg = new FS::cust_pkg {
-      custnum        => $custnum,
-      pkgpart        => ( $opt->{'pkgpart'}     || $self->pkgpart      ),
-      refnum         => ( $opt->{'refnum'}      || $self->refnum       ),
-      locationnum    => ( $opt->{'locationnum'}                        ),
+      custnum     => $custnum,
+      locationnum => $opt->{'locationnum'},
+      ( map {  $_ => ( $opt->{$_} || $self->$_() )  }
+          qw( pkgpart quantity refnum salesnum )
+      ),
       %hash,
     };
     $error = $cust_pkg->insert( 'change' => 1,
@@ -1894,16 +2534,18 @@ sub change {
   }
   if ($error) {
     $dbh->rollback if $oldAutoCommit;
-    return $error;
+    return "inserting new package: $error";
   }
 
   # Transfer services and cancel old package.
-
+  # Enforce service limits only if this is a pkgpart change.
+  local $FS::cust_svc::ignore_quantity;
+  $FS::cust_svc::ignore_quantity = 1 if $same_pkgpart;
   $error = $self->transfer($cust_pkg);
   if ($error and $error == 0) {
     # $old_pkg->transfer failed.
     $dbh->rollback if $oldAutoCommit;
-    return $error;
+    return "transferring $error";
   }
 
   if ( $error > 0 && $conf->exists('cust_pkg-change_svcpart') ) {
@@ -1912,7 +2554,7 @@ sub change {
     if ($error and $error == 0) {
       # $old_pkg->transfer failed.
       $dbh->rollback if $oldAutoCommit;
-      return $error;
+      return "converting $error";
     }
   }
 
@@ -1924,7 +2566,7 @@ sub change {
     # Transfers were successful, but we still had services left on the old
     # package.  We can't change the package under this circumstances, so abort.
     $dbh->rollback if $oldAutoCommit;
-    return "Unable to transfer all services from package ". $self->pkgnum;
+    return "unable to transfer all services";
   }
 
   #reset usage if changing pkgpart
@@ -1939,7 +2581,7 @@ sub change {
 
     if ($error) {
       $dbh->rollback if $oldAutoCommit;
-      return "Error setting usage values: $error";
+      return "setting usage values: $error";
     }
   } else {
     # if NOT changing pkgpart, transfer any usage pools over
@@ -1948,7 +2590,29 @@ sub change {
       $error = $usage->replace;
       if ( $error ) {
         $dbh->rollback if $oldAutoCommit;
-        return "Error transferring usage pools: $error";
+        return "transferring usage pools: $error";
+      }
+    }
+  }
+
+  # transfer usage pricing add-ons, if we're not changing pkgpart or if they were specified
+  if ( $same_pkgpart || $opt->{'cust_pkg_usageprice'}) {
+    my @old_cust_pkg_usageprice;
+    if ($opt->{'cust_pkg_usageprice'}) {
+      @old_cust_pkg_usageprice = @{ $opt->{'cust_pkg_usageprice'} };
+    } else {
+      @old_cust_pkg_usageprice = $self->cust_pkg_usageprice;
+    }
+    foreach my $old_cust_pkg_usageprice (@old_cust_pkg_usageprice) {
+      my $new_cust_pkg_usageprice = new FS::cust_pkg_usageprice {
+        'pkgnum'         => $cust_pkg->pkgnum,
+        'usagepricepart' => $old_cust_pkg_usageprice->usagepricepart,
+        'quantity'       => $old_cust_pkg_usageprice->quantity,
+      };
+      $error = $new_cust_pkg_usageprice->insert;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return "Error transferring usage pricing add-on: $error";
       }
     }
   }
@@ -1958,14 +2622,14 @@ sub change {
     foreach my $old_discount ($self->cust_pkg_discount_active) {
       # don't remove the old discount, we may still need to bill that package.
       my $new_discount = new FS::cust_pkg_discount {
-        'pkgnum'      => $cust_pkg->pkgnum,
-        'discountnum' => $old_discount->discountnum,
-        'months_used' => $old_discount->months_used,
+        'pkgnum' => $cust_pkg->pkgnum,
+        map { $_ => $old_discount->$_() }
+          qw( discountnum months_used end_date usernum setuprecur ),
       };
       $error = $new_discount->insert;
       if ( $error ) {
         $dbh->rollback if $oldAutoCommit;
-        return "Error transferring discounts: $error";
+        return "transferring discounts: $error";
       }
     }
   }
@@ -1978,7 +2642,22 @@ sub change {
     $error = $new_detail->insert;
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
-      return "Error transferring package notes: $error";
+      return "transferring package notes: $error";
+    }
+  }
+
+  # transfer scheduled expire/adjourn reasons
+  foreach my $action ('expire', 'adjourn') {
+    if ( $cust_pkg->get($action) ) {
+      my $reason = $self->last_cust_pkg_reason($action);
+      if ( $reason ) {
+        $reason->set('pkgnum', $cust_pkg->pkgnum);
+        $error = $reason->replace;
+        if ( $error ) {
+          $dbh->rollback if $oldAutoCommit;
+          return "transferring $action reason: $error";
+        }
+      }
     }
   }
   
@@ -2054,10 +2733,24 @@ sub change {
     unused_credit  => $unused_credit,
     nobill         => $keep_dates,
     change_custnum => ( $self->custnum != $custnum ? $custnum : '' ),
+    no_delay_cancel => 1,
   );
   if ($error) {
     $dbh->rollback if $oldAutoCommit;
-    return $error;
+    return "canceling old package: $error";
+  }
+
+  # transfer rt_field_charge, if we're not changing pkgpart
+  # after billing of old package, before billing of new package
+  if ( $same_pkgpart ) {
+    foreach my $rt_field_charge ($self->rt_field_charge) {
+      $rt_field_charge->set('pkgnum', $cust_pkg->pkgnum);
+      $error = $rt_field_charge->replace;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return "transferring rt_field_charge: $error";
+      }
+    }
   }
 
   if ( $conf->exists('cust_pkg-change_pkgpart-bill_now') ) {
@@ -2067,7 +2760,7 @@ sub change {
     );
     if ( $error ) {
       $dbh->rollback if $oldAutoCommit;
-      return $error;
+      return "billing new package: $error";
     }
   }
 
@@ -2100,8 +2793,12 @@ The date for the package change.  Required, and must be in the future.
 
 =item locationnum
 
-The pkgpart and locationnum of the new package, with the same 
-meaning as in C<change>.
+=item quantity
+
+=item contract_end
+
+The pkgpart, locationnum, quantity and optional contract_end of the new 
+package, with the same meaning as in C<change>.
 
 =back
 
@@ -2111,6 +2808,10 @@ sub change_later {
   my $self = shift;
   my $opt = ref($_[0]) ? shift : { @_ };
 
+  # check contract_end, prevent adding/removing
+  my $error = $self->_check_change($opt);
+  return $error if $error;
+
   my $oldAutoCommit = $FS::UID::AutoCommit;
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
@@ -2124,7 +2825,15 @@ sub change_later {
     return "start_date $date is in the past";
   }
 
-  my $error;
+  # If the user entered a new location, set it up now.
+  if ( $opt->{'cust_location'} ) {
+    $error = $opt->{'cust_location'}->find_or_insert;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return "creating location record: $error";
+    }
+    $opt->{'locationnum'} = $opt->{'cust_location'}->locationnum;
+  }
 
   if ( $self->change_to_pkgnum ) {
     my $change_to = FS::cust_pkg->by_key($self->change_to_pkgnum);
@@ -2132,7 +2841,11 @@ sub change_later {
         if $opt->{'pkgpart'} and $opt->{'pkgpart'} != $change_to->pkgpart;
     my $new_locationnum = $opt->{'locationnum'}
         if $opt->{'locationnum'} and $opt->{'locationnum'} != $change_to->locationnum;
-    if ( $new_pkgpart or $new_locationnum ) {
+    my $new_quantity = $opt->{'quantity'}
+        if $opt->{'quantity'} and $opt->{'quantity'} != $change_to->quantity;
+    my $new_contract_end = $opt->{'contract_end'}
+        if $opt->{'contract_end'} and $opt->{'contract_end'} != $change_to->contract_end;
+    if ( $new_pkgpart or $new_locationnum or $new_quantity or $new_contract_end ) {
       # it hasn't been billed yet, so in principle we could just edit
       # it in place (w/o a package change), but that's bad form.
       # So change the package according to the new options...
@@ -2147,8 +2860,10 @@ sub change_later {
 
         $error = $self->replace       ||
                  $err_or_pkg->replace ||
-                 $change_to->cancel   ||
-                 $change_to->delete;
+                 #because change() might've edited existing scheduled change in place
+                 (($err_or_pkg->pkgnum == $change_to->pkgnum) ? '' :
+                  $change_to->cancel('no_delay_cancel' => 1) ||
+                  $change_to->delete);
       } else {
         $error = $err_or_pkg;
       }
@@ -2170,15 +2885,24 @@ sub change_later {
       if $opt->{'pkgpart'} and $opt->{'pkgpart'} != $self->pkgpart;
   my $new_locationnum = $opt->{'locationnum'}
       if $opt->{'locationnum'} and $opt->{'locationnum'} != $self->locationnum;
-  return '' unless $new_pkgpart or $new_locationnum; # wouldn't do anything
+  my $new_quantity = $opt->{'quantity'}
+      if $opt->{'quantity'} and $opt->{'quantity'} != $self->quantity;
+  my $new_contract_end = $opt->{'contract_end'}
+      if $opt->{'contract_end'} and $opt->{'contract_end'} != $self->contract_end;
 
-  my %hash = (
-    'custnum'     => $self->custnum,
-    'pkgpart'     => ($opt->{'pkgpart'}     || $self->pkgpart),
-    'locationnum' => ($opt->{'locationnum'} || $self->locationnum),
-    'start_date'  => $date,
-  );
-  my $new = FS::cust_pkg->new(\%hash);
+  return '' unless $new_pkgpart or $new_locationnum or $new_quantity or $new_contract_end; # wouldn't do anything
+
+  # allow $opt->{'locationnum'} = '' to specifically set it to null
+  # (i.e. customer default location)
+  $opt->{'locationnum'} = $self->locationnum if !exists($opt->{'locationnum'});
+
+  my $new = FS::cust_pkg->new( {
+    custnum     => $self->custnum,
+    locationnum => $opt->{'locationnum'},
+    start_date  => $date,
+    map   {  $_ => ( $opt->{$_} || $self->$_() )  }
+      qw( pkgpart quantity refnum salesnum contract_end )
+  } );
   $error = $new->insert('change' => 1, 
                         'allow_pkgpart' => ($new_pkgpart ? 0 : 1));
   if ( !$error ) {
@@ -2203,21 +2927,33 @@ Cancels a future package change scheduled by C<change_later>.
 
 sub abort_change {
   my $self = shift;
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+
   my $pkgnum = $self->change_to_pkgnum;
   my $change_to = FS::cust_pkg->by_key($pkgnum) if $pkgnum;
   my $error;
-  if ( $change_to ) {
-    $error = $change_to->cancel || $change_to->delete;
-    return $error if $error;
-  }
   $self->set('change_to_pkgnum', '');
   $self->set('expire', '');
-  $self->replace;
+  $error = $self->replace;
+  if ( $change_to ) {
+    $error ||= $change_to->cancel || $change_to->delete;
+  }
+
+  if ( $oldAutoCommit ) {
+    if ( $error ) {
+      dbh->rollback;
+    } else {
+      dbh->commit;
+    }
+  }
+
+  return $error;
 }
 
 =item set_quantity QUANTITY
 
-Change the package's quantity field.  This is the one package property
+Change the package's quantity field.  This is one of the few package properties
 that can safely be changed without canceling and reordering the package
 (because it doesn't affect tax eligibility).  Returns an error or an 
 empty string.
@@ -2227,37 +2963,253 @@ empty string.
 sub set_quantity {
   my $self = shift;
   $self = $self->replace_old; # just to make sure
-  my $qty = shift;
-  ($qty =~ /^\d+$/ and $qty > 0) or return "bad package quantity $qty";
-  $self->set('quantity' => $qty);
+  $self->quantity(shift);
   $self->replace;
 }
 
-use Storable 'thaw';
-use MIME::Base64;
-sub process_bulk_cust_pkg {
-  my $job = shift;
-  my $param = thaw(decode_base64(shift));
-  warn Dumper($param) if $DEBUG;
+=item set_salesnum SALESNUM
 
-  my $old_part_pkg = qsearchs('part_pkg', 
-                              { pkgpart => $param->{'old_pkgpart'} });
-  my $new_part_pkg = qsearchs('part_pkg',
-                              { pkgpart => $param->{'new_pkgpart'} });
-  die "Must select a new package type\n" unless $new_part_pkg;
-  #my $keep_dates = $param->{'keep_dates'} || 0;
-  my $keep_dates = 1; # there is no good reason to turn this off
+Change the package's salesnum (sales person) field.  This is one of the few
+package properties that can safely be changed without canceling and reordering
+the package (because it doesn't affect tax eligibility).  Returns an error or
+an empty string.
+
+=cut
+
+sub set_salesnum {
+  my $self = shift;
+  $self = $self->replace_old; # just to make sure
+  $self->salesnum(shift);
+  $self->replace;
+  # XXX this should probably reassign any credit that's already been given
+}
+
+=item modify_charge OPTIONS
+
+Change the properties of a one-time charge.  The following properties can
+be changed this way:
+- pkg: the package description
+- classnum: the package class
+- additional: arrayref of additional invoice details to add to this package
+
+and, I<if the charge has not yet been billed>:
+- start_date: the date when it will be billed
+- amount: the setup fee to be charged
+- quantity: the multiplier for the setup fee
+- separate_bill: whether to put the charge on a separate invoice
+
+If you pass 'adjust_commission' => 1, and the classnum changes, and there are
+commission credits linked to this charge, they will be recalculated.
 
-  local $SIG{HUP} = 'IGNORE';
-  local $SIG{INT} = 'IGNORE';
-  local $SIG{QUIT} = 'IGNORE';
-  local $SIG{TERM} = 'IGNORE';
-  local $SIG{TSTP} = 'IGNORE';
-  local $SIG{PIPE} = 'IGNORE';
+=cut
+
+sub modify_charge {
+  my $self = shift;
+  my %opt = @_;
+  my $part_pkg = $self->part_pkg;
+  my $pkgnum = $self->pkgnum;
 
+  my $dbh = dbh;
   my $oldAutoCommit = $FS::UID::AutoCommit;
   local $FS::UID::AutoCommit = 0;
-  my $dbh = dbh;
+
+  return "Can't use modify_charge except on one-time charges"
+    unless $part_pkg->freq eq '0';
+
+  if ( length($opt{'pkg'}) and $part_pkg->pkg ne $opt{'pkg'} ) {
+    $part_pkg->set('pkg', $opt{'pkg'});
+  }
+
+  my %pkg_opt = $part_pkg->options;
+  my $pkg_opt_modified = 0;
+
+  $opt{'additional'} ||= [];
+  my $i;
+  my @old_additional;
+  foreach (grep /^additional/, keys %pkg_opt) {
+    ($i) = ($_ =~ /^additional_info(\d+)$/);
+    $old_additional[$i] = $pkg_opt{$_} if $i;
+    delete $pkg_opt{$_};
+  }
+
+  for ( $i = 0; exists($opt{'additional'}->[$i]); $i++ ) {
+    $pkg_opt{ "additional_info$i" } = $opt{'additional'}->[$i];
+    if (!exists($old_additional[$i])
+        or $old_additional[$i] ne $opt{'additional'}->[$i])
+    {
+      $pkg_opt_modified = 1;
+    }
+  }
+  $pkg_opt_modified = 1 if scalar(@old_additional) != $i;
+  $pkg_opt{'additional_count'} = $i if $i > 0;
+
+  my $old_classnum;
+  if ( exists($opt{'classnum'}) and $part_pkg->classnum ne $opt{'classnum'} )
+  {
+    # remember it
+    $old_classnum = $part_pkg->classnum;
+    $part_pkg->set('classnum', $opt{'classnum'});
+  }
+
+  if ( !$self->get('setup') ) {
+    # not yet billed, so allow amount, setup_cost, quantity, start_date,
+    # and separate_bill
+
+    if ( exists($opt{'amount'}) 
+          and $part_pkg->option('setup_fee') != $opt{'amount'}
+          and $opt{'amount'} > 0 ) {
+
+      $pkg_opt{'setup_fee'} = $opt{'amount'};
+      $pkg_opt_modified = 1;
+    }
+
+    if ( exists($opt{'setup_cost'}) 
+          and $part_pkg->setup_cost != $opt{'setup_cost'}
+          and $opt{'setup_cost'} > 0 ) {
+
+      $part_pkg->set('setup_cost', $opt{'setup_cost'});
+    }
+
+    if ( exists($opt{'quantity'})
+          and $opt{'quantity'} != $self->quantity
+          and $opt{'quantity'} > 0 ) {
+        
+      $self->set('quantity', $opt{'quantity'});
+    }
+
+    if ( exists($opt{'start_date'})
+          and $opt{'start_date'} != $self->start_date ) {
+
+      $self->set('start_date', $opt{'start_date'});
+    }
+
+    if ( exists($opt{'separate_bill'})
+          and $opt{'separate_bill'} ne $self->separate_bill ) {
+
+      $self->set('separate_bill', $opt{'separate_bill'});
+    }
+
+
+  } # else simply ignore them; the UI shouldn't allow editing the fields
+
+  
+  if ( exists($opt{'taxclass'}) 
+          and $part_pkg->taxclass ne $opt{'taxclass'}) {
+    
+      $part_pkg->set('taxclass', $opt{'taxclass'});
+  }
+
+  my $error;
+  if ( $part_pkg->modified or $pkg_opt_modified ) {
+    # can we safely modify the package def?
+    # Yes, if it's not available for purchase, and this is the only instance
+    # of it.
+    if ( $part_pkg->disabled
+         and FS::cust_pkg->count('pkgpart = '.$part_pkg->pkgpart) == 1
+         and FS::quotation_pkg->count('pkgpart = '.$part_pkg->pkgpart) == 0
+       ) {
+      $error = $part_pkg->replace( options => \%pkg_opt );
+    } else {
+      # clone it
+      $part_pkg = $part_pkg->clone;
+      $part_pkg->set('disabled' => 'Y');
+      $error = $part_pkg->insert( options => \%pkg_opt );
+      # and associate this as yet-unbilled package to the new package def
+      $self->set('pkgpart' => $part_pkg->pkgpart);
+    }
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return $error;
+    }
+  }
+
+  if ($self->modified) { # for quantity or start_date change, or if we had
+                         # to clone the existing package def
+    my $error = $self->replace;
+    return $error if $error;
+  }
+  if (defined $old_classnum) {
+    # fix invoice grouping records
+    my $old_catname = $old_classnum
+                      ? FS::pkg_class->by_key($old_classnum)->categoryname
+                      : '';
+    my $new_catname = $opt{'classnum'}
+                      ? $part_pkg->pkg_class->categoryname
+                      : '';
+    if ( $old_catname ne $new_catname ) {
+      foreach my $cust_bill_pkg ($self->cust_bill_pkg) {
+        # (there should only be one...)
+        my @display = qsearch( 'cust_bill_pkg_display', {
+            'billpkgnum'  => $cust_bill_pkg->billpkgnum,
+            'section'     => $old_catname,
+        });
+        foreach (@display) {
+          $_->set('section', $new_catname);
+          $error = $_->replace;
+          if ( $error ) {
+            $dbh->rollback if $oldAutoCommit;
+            return $error;
+          }
+        }
+      } # foreach $cust_bill_pkg
+    }
+
+    if ( $opt{'adjust_commission'} ) {
+      # fix commission credits...tricky.
+      foreach my $cust_event ($self->cust_event) {
+        my $part_event = $cust_event->part_event;
+        foreach my $table (qw(sales agent)) {
+          my $class =
+            "FS::part_event::Action::Mixin::credit_${table}_pkg_class";
+          my $credit = qsearchs('cust_credit', {
+              'eventnum' => $cust_event->eventnum,
+          });
+          if ( $part_event->isa($class) ) {
+            # Yes, this results in current commission rates being applied 
+            # retroactively to a one-time charge.  For accounting purposes 
+            # there ought to be some kind of time limit on doing this.
+            my $amount = $part_event->_calc_credit($self);
+            if ( $credit and $credit->amount ne $amount ) {
+              # Void the old credit.
+              $error = $credit->void('Package class changed');
+              if ( $error ) {
+                $dbh->rollback if $oldAutoCommit;
+                return "$error (adjusting commission credit)";
+              }
+            }
+            # redo the event action to recreate the credit.
+            local $@ = '';
+            eval { $part_event->do_action( $self, $cust_event ) };
+            if ( $@ ) {
+              $dbh->rollback if $oldAutoCommit;
+              return $@;
+            }
+          } # if $part_event->isa($class)
+        } # foreach $table
+      } # foreach $cust_event
+    } # if $opt{'adjust_commission'}
+  } # if defined $old_classnum
+
+  $dbh->commit if $oldAutoCommit;
+  '';
+}
+
+sub process_bulk_cust_pkg {
+  my $job = shift;
+  my $param = shift;
+  warn Dumper($param) if $DEBUG;
+
+  my $old_part_pkg = qsearchs('part_pkg', 
+                              { pkgpart => $param->{'old_pkgpart'} });
+  my $new_part_pkg = qsearchs('part_pkg',
+                              { pkgpart => $param->{'new_pkgpart'} });
+  die "Must select a new package type\n" unless $new_part_pkg;
+  #my $keep_dates = $param->{'keep_dates'} || 0;
+  my $keep_dates = 1; # there is no good reason to turn this off
+
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
 
   my @cust_pkgs = qsearch('cust_pkg', { 'pkgpart' => $param->{'old_pkgpart'} } );
 
@@ -2491,13 +3443,6 @@ If there is an error, returns the error, otherwise returns false.
 sub set_cust_pkg_detail {
   my( $self, $detailtype, @details ) = @_;
 
-  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;
@@ -2531,7 +3476,7 @@ sub set_cust_pkg_detail {
 
 =item cust_event
 
-Returns the new-style customer billing events (see L<FS::cust_event>) for this invoice.
+Returns the customer billing events (see L<FS::cust_event>) for this invoice.
 
 =cut
 
@@ -2548,19 +3493,41 @@ sub cust_event {
 
 =item num_cust_event
 
-Returns the number of new-style customer billing events (see L<FS::cust_event>) for this invoice.
+Returns the number of customer billing events (see L<FS::cust_event>) for this package.
 
 =cut
 
 #false laziness w/cust_bill.pm
 sub num_cust_event {
   my $self = shift;
-  my $sql =
-    "SELECT COUNT(*) FROM cust_event JOIN part_event USING ( eventpart ) ".
-    "  WHERE tablenum = ? AND eventtable = 'cust_pkg'";
+  my $sql = "SELECT COUNT(*) ". $self->_from_cust_event_where;
+  $self->_prep_ex($sql, $self->pkgnum)->fetchrow_arrayref->[0];
+}
+
+=item exists_cust_event
+
+Returns true if there are customer billing events (see L<FS::cust_event>) for this package.  More efficient than using num_cust_event.
+
+=cut
+
+sub exists_cust_event {
+  my $self = shift;
+  my $sql = "SELECT 1 ". $self->_from_cust_event_where. " LIMIT 1";
+  my $row = $self->_prep_ex($sql, $self->pkgnum)->fetchrow_arrayref;
+  $row ? $row->[0] : '';
+}
+
+sub _from_cust_event_where {
+  #my $self = shift;
+  " FROM cust_event JOIN part_event USING ( eventpart ) ".
+  "  WHERE tablenum = ? AND eventtable = 'cust_pkg' ";
+}
+
+sub _prep_ex {
+  my( $self, $sql, @args ) = @_;
   my $sth = dbh->prepare($sql) or die  dbh->errstr. " preparing $sql"; 
-  $sth->execute($self->pkgnum) or die $sth->errstr. " executing $sql";
-  $sth->fetchrow_arrayref->[0];
+  $sth->execute(@args)         or die $sth->errstr. " executing $sql";
+  $sth;
 }
 
 =item part_pkg_currency_option OPTIONNAME
@@ -2587,16 +3554,32 @@ sub part_pkg_currency_option {
 
 =item cust_svc [ OPTION => VALUE ... ] (current usage)
 
+=item cust_svc_unsorted [ OPTION => VALUE ... ] 
+
 Returns the services for this package, as FS::cust_svc objects (see
 L<FS::cust_svc>).  Available options are svcpart and svcdb.  If either is
 spcififed, returns only the matching services.
 
+As an optimization, use the cust_svc_unsorted version if you are not displaying
+the results.
+
 =cut
 
 sub cust_svc {
   my $self = shift;
+  cluck "cust_pkg->cust_svc called" if $DEBUG > 2;
+  $self->_sort_cust_svc( $self->cust_svc_unsorted_arrayref(@_) );
+}
 
-  return () unless $self->num_cust_svc(@_);
+sub cust_svc_unsorted {
+  my $self = shift;
+  @{ $self->cust_svc_unsorted_arrayref(@_) };
+}
+
+sub cust_svc_unsorted_arrayref {
+  my $self = shift;
+
+  return [] unless $self->num_cust_svc(@_);
 
   my %opt = ();
   if ( @_ && $_[0] =~ /^\d+/ ) {
@@ -2608,24 +3591,17 @@ sub cust_svc {
   }
 
   my %search = (
-    'table'   => 'cust_svc',
-    'hashref' => { 'pkgnum' => $self->pkgnum },
+    'select'    => 'cust_svc.*, part_svc.*',
+    'table'     => 'cust_svc',
+    'hashref'   => { 'pkgnum' => $self->pkgnum },
+    'addl_from' => 'LEFT JOIN part_svc USING ( svcpart )',
   );
-  if ( $opt{svcpart} ) {
-    $search{hashref}->{svcpart} = $opt{'svcpart'};
-  }
-  if ( $opt{'svcdb'} ) {
-    $search{addl_from} = ' LEFT JOIN part_svc USING ( svcpart ) ';
-    $search{extra_sql} = ' AND svcdb = '. dbh->quote( $opt{'svcdb'} );
-  }
-
-  cluck "cust_pkg->cust_svc called" if $DEBUG > 2;
+  $search{hashref}->{svcpart} = $opt{svcpart}
+    if $opt{svcpart};
+  $search{extra_sql} = ' AND svcdb = '. dbh->quote( $opt{svcdb} )
+    if $opt{svcdb};
 
-  #if ( $self->{'_svcnum'} ) {
-  #  values %{ $self->{'_svcnum'}->cache };
-  #} else {
-    $self->_sort_cust_svc( [ qsearch(\%search) ] );
-  #}
+  [ qsearch(\%search) ];
 
 }
 
@@ -2658,12 +3634,16 @@ sub h_cust_svc {
     if $DEBUG;
 
   my ($end, $start, $mode) = @_;
+
+  local($FS::Record::qsearch_qualify_columns) = 0;
+
   my @cust_svc = $self->_sort_cust_svc(
     [ qsearch( 'h_cust_svc',
       { 'pkgnum' => $self->pkgnum, },  
       FS::h_cust_svc->sql_h_search(@_),  
     ) ]
   );
+
   if ( defined($mode) && $mode eq 'I' ) {
     my %hidden_svcpart = map { $_->svcpart => $_->hidden } $self->part_svc;
     return grep { !$hidden_svcpart{$_->svcpart} } @cust_svc;
@@ -2748,28 +3728,33 @@ Returns a list of FS::part_svc objects representing services included in this
 package but not yet provisioned.  Each FS::part_svc object also has an extra
 field, I<num_avail>, which specifies the number of available services.
 
+Accepts option I<provision_hold>;  if true, only returns part_svc for which the
+associated pkg_svc has the provision_hold flag set.
+
 =cut
 
 sub available_part_svc {
   my $self = shift;
+  my %opt  = @_;
 
   my $pkg_quantity = $self->quantity || 1;
 
   grep { $_->num_avail > 0 }
-    map {
-          my $part_svc = $_->part_svc;
-          $part_svc->{'Hash'}{'num_avail'} = #evil encapsulation-breaking
-            $pkg_quantity * $_->quantity - $self->num_cust_svc($_->svcpart);
-
-         # more evil encapsulation breakage
-         if($part_svc->{'Hash'}{'num_avail'} > 0) {
-           my @exports = $part_svc->part_export_did;
-           $part_svc->{'Hash'}{'can_get_dids'} = scalar(@exports);
-         }
-
-          $part_svc;
-        }
-      $self->part_pkg->pkg_svc;
+  map {
+    my $part_svc = $_->part_svc;
+    $part_svc->{'Hash'}{'num_avail'} = #evil encapsulation-breaking
+    $pkg_quantity * $_->quantity - $self->num_cust_svc($_->svcpart);
+
+    # more evil encapsulation breakage
+    if ($part_svc->{'Hash'}{'num_avail'} > 0) {
+      my @exports = $part_svc->part_export_did;
+      $part_svc->{'Hash'}{'can_get_dids'} = scalar(@exports);
+       }
+
+    $part_svc;
+  }
+  grep { $opt{'provision_hold'} ? $_->provision_hold : 1 }
+  $self->part_pkg->pkg_svc;
 }
 
 =item part_svc [ OPTION => VALUE ... ]
@@ -2780,17 +3765,35 @@ following extra fields:
 
 =over 4
 
-=item num_cust_svc  (count)
+=item num_cust_svc
+
+(count)
+
+=item num_avail
 
-=item num_avail     (quantity - count)
+(quantity - count)
 
-=item cust_pkg_svc (services) - array reference containing the provisioned services, as cust_svc objects
+=item cust_pkg_svc
+
+(services) - array reference containing the provisioned services, as cust_svc objects
 
 =back
 
-Accepts one option: summarize_size.  If specified and non-zero, will omit the
-extra cust_pkg_svc option for objects where num_cust_svc is this size or
-greater.
+Accepts two options:
+
+=over 4
+
+=item summarize_size
+
+If true, will omit the extra cust_pkg_svc option for objects where num_cust_svc
+is this size or greater.
+
+=item hide_discontinued
+
+If true, will omit looking for services that are no longer avaialble in the
+package definition.
+
+=back
 
 =cut
 
@@ -2819,16 +3822,18 @@ sub part_svc {
     $part_svc;
   } $self->part_pkg->pkg_svc;
 
-  #extras
-  push @part_svc, map {
-    my $part_svc = $_;
-    my $num_cust_svc = $self->num_cust_svc($part_svc->svcpart);
-    $part_svc->{'Hash'}{'num_cust_svc'} = $num_cust_svc; #speak no evail
-    $part_svc->{'Hash'}{'num_avail'}    = 0; #0-$num_cust_svc ?
-    $part_svc->{'Hash'}{'cust_pkg_svc'} =
-      $num_cust_svc ? [ $self->cust_svc($part_svc->svcpart) ] : [];
-    $part_svc;
-  } $self->extra_part_svc;
+  unless ( $opt{hide_discontinued} ) {
+    #extras
+    push @part_svc, map {
+      my $part_svc = $_;
+      my $num_cust_svc = $self->num_cust_svc($part_svc->svcpart);
+      $part_svc->{'Hash'}{'num_cust_svc'} = $num_cust_svc; #speak no evail
+      $part_svc->{'Hash'}{'num_avail'}    = 0; #0-$num_cust_svc ?
+      $part_svc->{'Hash'}{'cust_pkg_svc'} =
+        $num_cust_svc ? [ $self->cust_svc($part_svc->svcpart) ] : [];
+      $part_svc;
+    } $self->extra_part_svc;
+  }
 
   @part_svc;
 
@@ -2895,6 +3900,8 @@ Returns a short status string for this package, currently:
 
 =over 4
 
+=item on hold
+
 =item not yet billed
 
 =item one-time charge
@@ -2915,6 +3922,7 @@ sub status {
   my $freq = length($self->freq) ? $self->freq : $self->part_pkg->freq;
 
   return 'cancelled' if $self->get('cancel');
+  return 'on hold' if $self->susp && ! $self->setup;
   return 'suspended' if $self->susp;
   return 'not yet billed' unless $self->setup;
   return 'one-time charge' if $freq =~ /^(0|$)/;
@@ -2941,8 +3949,9 @@ Class method that returns the list of possible status strings for packages
 =cut
 
 tie my %statuscolor, 'Tie::IxHash', 
+  'on hold'         => 'FF00F5', #brighter purple!
   'not yet billed'  => '009999', #teal? cyan?
-  'one-time charge' => '000000',
+  'one-time charge' => '0000CC', #blue  #'000000',
   'active'          => '00CC00',
   'suspended'       => 'FF9900',
   'cancelled'       => 'FF0000',
@@ -2955,6 +3964,11 @@ sub statuses {
     keys %statuscolor;
 }
 
+sub statuscolors {
+  #my $self = shift;
+  \%statuscolor;
+}
+
 =item statuscolor
 
 Returns a hex triplet color string for this package's status.
@@ -2966,6 +3980,40 @@ sub statuscolor {
   $statuscolor{$self->status};
 }
 
+=item is_status_delay_cancel
+
+Returns true if part_pkg has option delay_cancel, 
+cust_pkg status is 'suspended' and expire is set
+to cancel package within the next day (or however
+many days are set in global config part_pkg-delay_cancel-days.
+
+Accepts option I<part_pkg-delay_cancel-days> which should be
+the value of the config setting, to avoid looking it up again.
+
+This is not a real status, this only meant for hacking display 
+values, because otherwise treating the package as suspended is 
+really the whole point of the delay_cancel option.
+
+=cut
+
+sub is_status_delay_cancel {
+  my ($self,%opt) = @_;
+  if ( $self->main_pkgnum and $self->pkglinknum ) {
+    return $self->main_pkg->is_status_delay_cancel;
+  }
+  return 0 unless $self->part_pkg->option('delay_cancel',1);
+  return 0 unless $self->status eq 'suspended';
+  return 0 unless $self->expire;
+  my $expdays = $opt{'part_pkg-delay_cancel-days'};
+  unless ($expdays) {
+    my $conf = new FS::Conf;
+    $expdays = $conf->config('part_pkg-delay_cancel-days') || 1;
+  }
+  my $expsecs = 60*60*24*$expdays;
+  return 0 unless $self->expire < time + $expsecs;
+  return 1;
+}
+
 =item pkg_label
 
 Returns a label for this package.  (Currently "pkgnum: pkg - comment" or
@@ -2975,7 +4023,7 @@ Returns a label for this package.  (Currently "pkgnum: pkg - comment" or
 
 sub pkg_label {
   my $self = shift;
-  my $label = $self->part_pkg->pkg_comment( 'nopkgpart' => 1 );
+  my $label = $self->part_pkg->pkg_comment( cust_pkg=>$self, nopkgpart=>1 );
   $label = $self->pkgnum. ": $label"
     if $FS::CurrentUser::CurrentUser->option('show_pkgnum');
   $label;
@@ -3046,23 +4094,27 @@ sub labels {
   map { [ $_->label ] } $self->cust_svc;
 }
 
-=item h_labels END_TIMESTAMP [ START_TIMESTAMP ] [ MODE ]
+=item h_labels END_TIMESTAMP [, START_TIMESTAMP [, MODE [, LOCALE ] ] ]
 
 Like the labels method, but returns historical information on services that
 were active as of END_TIMESTAMP and (optionally) not cancelled before
 START_TIMESTAMP.  If MODE is 'I' (for 'invoice'), services with the 
 I<pkg_svc.hidden> flag will be omitted.
 
-Returns a list of lists, calling the label method for all (historical) services
-(see L<FS::h_cust_svc>) of this billing item.
+If LOCALE is passed, service definition names will be localized.
+
+Returns a list of lists, calling the label method for all (historical)
+services (see L<FS::h_cust_svc>) of this billing item.
 
 =cut
 
 sub h_labels {
   my $self = shift;
-  warn "$me _h_labels called on $self\n"
+  my ($end, $start, $mode, $locale) = @_;
+  warn "$me h_labels\n"
     if $DEBUG;
-  map { [ $_->label(@_) ] } $self->h_cust_svc(@_);
+  map { [ $_->label($end, $start, $locale) ] }
+        $self->h_cust_svc($end, $start, $mode);
 }
 
 =item labels_short
@@ -3075,15 +4127,15 @@ individual services rather than individual items.
 =cut
 
 sub labels_short {
-  shift->_labels_short( 'labels', @_ );
+  shift->_labels_short( 'labels' ); # 'labels' takes no further arguments
 }
 
-=item h_labels_short END_TIMESTAMP [ START_TIMESTAMP ]
+=item h_labels_short END_TIMESTAMP [, START_TIMESTAMP [, MODE [, LOCALE ] ] ]
 
 Like h_labels, except returns a simple flat list, and shortens long
-(currently >5 or the cust_bill-max_same_services configuration value) lists of
-identical services to one line that lists the service label and the number of
-individual services rather than individual items.
+(currently >5 or the cust_bill-max_same_services configuration value) lists
+of identical services to one line that lists the service label and the
+number of individual services rather than individual items.
 
 =cut
 
@@ -3091,6 +4143,9 @@ sub h_labels_short {
   shift->_labels_short( 'h_labels', @_ );
 }
 
+# takes a method name ('labels' or 'h_labels') and all its arguments;
+# maybe should be "shorten($self->h_labels( ... ) )"
+
 sub _labels_short {
   my( $self, $method ) = ( shift, shift );
 
@@ -3154,13 +4209,6 @@ sub _labels_short {
 
 Returns the parent customer object (see L<FS::cust_main>).
 
-=cut
-
-sub cust_main {
-  my $self = shift;
-  qsearchs( 'cust_main', { 'custnum' => $self->custnum } );
-}
-
 =item balance
 
 Returns the balance for this specific package, when using
@@ -3222,7 +4270,16 @@ Returns the L<FS::cust_location> object for tax_locationnum.
 
 sub tax_location {
   my $self = shift;
-  FS::cust_location->by_key( $self->tax_locationnum )
+  my $conf = FS::Conf->new;
+  if ( $conf->exists('tax-pkg_address') and $self->locationnum ) {
+    return FS::cust_location->by_key($self->locationnum);
+  }
+  elsif ( $conf->exists('tax-ship_address') ) {
+    return $self->cust_main->ship_location;
+  }
+  else {
+    return $self->cust_main->bill_location;
+  }
 }
 
 =item seconds_since TIMESTAMP
@@ -3302,8 +4359,7 @@ sub attribute_since_sqlradacct {
   foreach my $cust_svc (
     grep {
       my $part_svc = $_->part_svc;
-      $part_svc->svcdb eq 'svc_acct'
-        && scalar($part_svc->part_export_usage);
+      scalar($part_svc->part_export_usage);
     } $self->cust_svc
   ) {
     $sum += $cust_svc->attribute_since_sqlradacct($start, $end, $attrib);
@@ -3364,11 +4420,13 @@ sub transfer {
   return ('Package does not exist: '.$dest_pkgnum) unless $dest;
 
   foreach my $pkg_svc ( $dest->part_pkg->pkg_svc ) {
-    $target{$pkg_svc->svcpart} = $pkg_svc->quantity;
+    $target{$pkg_svc->svcpart} = $pkg_svc->quantity * ( $dest->quantity || 1 );
   }
 
-  foreach my $cust_svc ($dest->cust_svc) {
-    $target{$cust_svc->svcpart}--;
+  unless ( $self->pkgnum == $dest->pkgnum ) {
+    foreach my $cust_svc ($dest->cust_svc) {
+      $target{$cust_svc->svcpart}--;
+    }
   }
 
   my %svcpart2svcparts = ();
@@ -3399,26 +4457,45 @@ sub transfer {
     }
   }
 
+  my $error;
   foreach my $cust_svc ($self->cust_svc) {
-    if($target{$cust_svc->svcpart} > 0
-       or $FS::cust_svc::ignore_quantity) { # maybe should be a 'force' option
+    my $svcnum = $cust_svc->svcnum;
+
+    if (    $target{$cust_svc->svcpart} > 0
+         or $FS::cust_svc::ignore_quantity # maybe should be a 'force' option
+       )
+    {
       $target{$cust_svc->svcpart}--;
+
+      local $FS::cust_svc::ignore_quantity = 1
+        if $self->pkgnum == $dest->pkgnum;
+
+      #why run replace at all in the $self->pkgnum == $dest->pkgnum case?
+      # we do want to trigger location and pkg_change exports, but 
+      # without pkgnum changing from an old to new package, cust_svc->replace
+      # doesn't know how to trigger those.  :/
+      # does this mean we scrap the whole idea of "safe to modify it in place",
+      # or do we special-case and pass the info needed to cust_svc->replace? :/
+
       my $new = new FS::cust_svc { $cust_svc->hash };
       $new->pkgnum($dest_pkgnum);
-      my $error = $new->replace($cust_svc);
-      return $error if $error;
+      $error = $new->replace($cust_svc);
+
     } elsif ( exists $opt{'change_svcpart'} && $opt{'change_svcpart'} ) {
+
       if ( $DEBUG ) {
         warn "looking for alternates for svcpart ". $cust_svc->svcpart. "\n";
         warn "alternates to consider: ".
              join(', ', @{$svcpart2svcparts{$cust_svc->svcpart}}). "\n";
       }
+
       my @alternate = grep {
                              warn "considering alternate svcpart $_: ".
                                   "$target{$_} available in new package\n"
                                if $DEBUG;
                              $target{$_} > 0;
                            } @{$svcpart2svcparts{$cust_svc->svcpart}};
+
       if ( @alternate ) {
         warn "alternate(s) found\n" if $DEBUG;
         my $change_svcpart = $alternate[0];
@@ -3426,14 +4503,20 @@ sub transfer {
         my $new = new FS::cust_svc { $cust_svc->hash };
         $new->svcpart($change_svcpart);
         $new->pkgnum($dest_pkgnum);
-        my $error = $new->replace($cust_svc);
-        return $error if $error;
+        $error = $new->replace($cust_svc);
       } else {
         $remaining++;
       }
+
     } else {
       $remaining++
     }
+
+    if ( $error ) {
+      my @label = $cust_svc->label;
+      return "$label[0] $label[1]: $error";
+    }
+
   }
   return $remaining;
 }
@@ -3449,13 +4532,6 @@ sub grab_svcnums {
   my $self = shift;
   my @svcnum = @_;
 
-  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;
@@ -3490,13 +4566,6 @@ order_pkgs methods in FS::cust_main for a better way to defer provisioning.
 sub reexport {
   my $self = shift;
 
-  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;
@@ -3527,13 +4596,6 @@ Calls the "pkg_change" export action for all services attached to this package.
 sub export_pkg_change {
   my( $self, $old )  = ( shift, shift );
 
-  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;
@@ -3609,7 +4671,7 @@ sub insert_reason {
     $reasonnum = $reason->reasonnum;
 
   } else {
-    return "Unparsable reason: ". $options{'reason'};
+    return "Unparseable reason: ". $options{'reason'};
   }
 
   my $cust_pkg_reason =
@@ -3630,13 +4692,10 @@ sub insert_reason {
 Associates this package with a discount (see L<FS::cust_pkg_discount>, possibly
 inserting a new discount on the fly (see L<FS::discount>).
 
-Available options are:
-
-=over 4
-
-=item discountnum
-
-=back
+This will look at the cust_pkg for a pseudo-field named "setup_discountnum",
+and if present, will create a setup discount. If the discountnum is -1,
+a new discount definition will be inserted using the value in
+"setup_discountnum_amount" or "setup_discountnum_percent". Likewise for recur.
 
 If there is an error, returns the error, otherwise returns false.
 
@@ -3646,21 +4705,29 @@ sub insert_discount {
   #my ($self, %options) = @_;
   my $self = shift;
 
-  my $cust_pkg_discount = new FS::cust_pkg_discount {
-    'pkgnum'      => $self->pkgnum,
-    'discountnum' => $self->discountnum,
-    'months_used' => 0,
-    'end_date'    => '', #XXX
-    #for the create a new discount case
-    '_type'       => $self->discountnum__type,
-    'amount'      => $self->discountnum_amount,
-    'percent'     => $self->discountnum_percent,
-    'months'      => $self->discountnum_months,
-    'setup'      => $self->discountnum_setup,
-    #'disabled'    => $self->discountnum_disabled,
-  };
+  foreach my $x (qw(setup recur)) {
+    if ( my $discountnum = $self->get("${x}_discountnum") ) {
+      my $cust_pkg_discount = FS::cust_pkg_discount->new( {
+        'pkgnum'      => $self->pkgnum,
+        'discountnum' => $discountnum,
+        'setuprecur'  => $x,
+        'months_used' => 0,
+        'end_date'    => '', #XXX
+        #for the create a new discount case
+        'amount'      => $self->get("${x}_discountnum_amount"),
+        'percent'     => $self->get("${x}_discountnum_percent"),
+        'months'      => $self->get("${x}_discountnum_months"),
+      } );
+      if ( $x eq 'setup' ) {
+        $cust_pkg_discount->setup('Y');
+        $cust_pkg_discount->months('');
+      }
+      my $error = $cust_pkg_discount->insert;
+      return $error if $error;
+    }
+  }
 
-  $cust_pkg_discount->insert;
+  '';
 }
 
 =item set_usage USAGE_VALUE_HASHREF 
@@ -3705,15 +4772,36 @@ sub recharge {
   }
 }
 
-=item cust_pkg_discount
+=item apply_usageprice 
 
 =cut
 
-sub cust_pkg_discount {
+sub apply_usageprice {
   my $self = shift;
-  qsearch('cust_pkg_discount', { 'pkgnum' => $self->pkgnum } );
+
+  my $oldAutoCommit = $FS::UID::AutoCommit;
+  local $FS::UID::AutoCommit = 0;
+  my $dbh = dbh;
+
+  my $error = '';
+
+  foreach my $cust_pkg_usageprice ( $self->cust_pkg_usageprice ) {
+    $error ||= $cust_pkg_usageprice->apply;
+  }
+
+  if ( $error ) {
+    $dbh->rollback if $oldAutoCommit;
+    die "error applying part_pkg_usageprice add-ons, pkgnum ". $self->pkgnum.
+        ": $error\n";
+  } else {
+    $dbh->commit if $oldAutoCommit;
+  }
+
+
 }
 
+=item cust_pkg_discount
+
 =item cust_pkg_discount_active
 
 =cut
@@ -3727,13 +4815,6 @@ sub cust_pkg_discount_active {
 
 Returns a list of all voice usage counters attached to this package.
 
-=cut
-
-sub cust_pkg_usage {
-  my $self = shift;
-  qsearch('cust_pkg_usage', { pkgnum => $self->pkgnum });
-}
-
 =item apply_usage OPTIONS
 
 Takes the following options:
@@ -3759,16 +4840,10 @@ sub apply_usage {
   my $pkgnum = $self->pkgnum;
   my $custnum = $self->custnum;
 
-  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 $order = FS::Conf->new->config('cdr-minutes_priority');
 
   my $is_classnum;
@@ -3802,7 +4877,7 @@ sub apply_usage {
         minutes     => min($cust_pkg_usage->minutes, $minutes),
     });
     $cust_pkg_usage->set('minutes',
-      sprintf('%.0f', $cust_pkg_usage->minutes - $cdr_cust_pkg_usage->minutes)
+      $cust_pkg_usage->minutes - $cdr_cust_pkg_usage->minutes
     );
     $error = $cust_pkg_usage->replace || $cdr_cust_pkg_usage->insert;
     $minutes -= $cdr_cust_pkg_usage->minutes;
@@ -4002,6 +5077,21 @@ sub inactive_sql { "
   AND ( cust_pkg.susp   IS NULL OR cust_pkg.susp   = 0 )
 "; }
 
+=item on_hold_sql
+
+Returns an SQL expression identifying on-hold packages.
+
+=cut
+
+sub on_hold_sql {
+  #$_[0]->recurring_sql(). ' AND '.
+  "
+        ( cust_pkg.cancel IS     NULL  OR cust_pkg.cancel  = 0 )
+    AND   cust_pkg.susp   IS NOT NULL AND cust_pkg.susp   != 0
+    AND ( cust_pkg.setup  IS     NULL  OR cust_pkg.setup   = 0 )
+  ";
+}
+
 =item susp_sql
 =item suspended_sql
 
@@ -4015,6 +5105,7 @@ sub susp_sql {
   "
         ( cust_pkg.cancel IS     NULL  OR cust_pkg.cancel = 0 )
     AND   cust_pkg.susp   IS NOT NULL AND cust_pkg.susp  != 0
+    AND   cust_pkg.setup  IS NOT NULL AND cust_pkg.setup != 0
   ";
 }
 
@@ -4031,6 +5122,17 @@ sub cancel_sql {
   "cust_pkg.cancel IS NOT NULL AND cust_pkg.cancel != 0";
 }
 
+=item ncancelled_recurring_sql
+
+Returns an SQL expression identifying un-cancelled, recurring packages.
+
+=cut
+
+sub ncancelled_recurring_sql {
+  $_[0]->recurring_sql().
+  " AND ( cust_pkg.cancel IS NULL OR cust_pkg.cancel = 0 ) ";
+}
+
 =item status_sql
 
 Returns an SQL expression to give the package status as a string.
@@ -4040,6 +5142,7 @@ Returns an SQL expression to give the package status as a string.
 sub status_sql {
 "CASE
   WHEN cust_pkg.cancel IS NOT NULL THEN 'cancelled'
+  WHEN ( cust_pkg.susp IS NOT NULL AND cust_pkg.setup IS NULL ) THEN 'on hold'
   WHEN cust_pkg.susp IS NOT NULL THEN 'suspended'
   WHEN cust_pkg.setup IS NULL THEN 'not yet billed'
   WHEN ".onetime_sql()." THEN 'one-time charge'
@@ -4047,450 +5150,6 @@ sub status_sql {
 END"
 }
 
-=item search HASHREF
-
-(Class method)
-
-Returns a qsearch hash expression to search for parameters specified in HASHREF.
-Valid parameters are
-
-=over 4
-
-=item agentnum
-
-=item magic
-
-active, inactive, suspended, cancel (or cancelled)
-
-=item status
-
-active, inactive, suspended, one-time charge, inactive, cancel (or cancelled)
-
-=item custom
-
- boolean selects custom packages
-
-=item classnum
-
-=item pkgpart
-
-pkgpart or arrayref or hashref of pkgparts
-
-=item setup
-
-arrayref of beginning and ending epoch date
-
-=item last_bill
-
-arrayref of beginning and ending epoch date
-
-=item bill
-
-arrayref of beginning and ending epoch date
-
-=item adjourn
-
-arrayref of beginning and ending epoch date
-
-=item susp
-
-arrayref of beginning and ending epoch date
-
-=item expire
-
-arrayref of beginning and ending epoch date
-
-=item cancel
-
-arrayref of beginning and ending epoch date
-
-=item query
-
-pkgnum or APKG_pkgnum
-
-=item cust_fields
-
-a value suited to passing to FS::UI::Web::cust_header
-
-=item CurrentUser
-
-specifies the user for agent virtualization
-
-=item fcc_line
-
-boolean; if true, returns only packages with more than 0 FCC phone lines.
-
-=item state, country
-
-Limit to packages with a service location in the specified state and country.
-For FCC 477 reporting, mostly.
-
-=back
-
-=cut
-
-sub search {
-  my ($class, $params) = @_;
-  my @where = ();
-
-  ##
-  # parse agent
-  ##
-
-  if ( $params->{'agentnum'} =~ /^(\d+)$/ and $1 ) {
-    push @where,
-      "cust_main.agentnum = $1";
-  }
-
-  ##
-  # parse custnum
-  ##
-
-  if ( $params->{'custnum'} =~ /^(\d+)$/ and $1 ) {
-    push @where,
-      "cust_pkg.custnum = $1";
-  }
-
-  ##
-  # custbatch
-  ##
-
-  if ( $params->{'pkgbatch'} =~ /^([\w\/\-\:\.]+)$/ and $1 ) {
-    push @where,
-      "cust_pkg.pkgbatch = '$1'";
-  }
-
-  ##
-  # parse status
-  ##
-
-  if (    $params->{'magic'}  eq 'active'
-       || $params->{'status'} eq 'active' ) {
-
-    push @where, FS::cust_pkg->active_sql();
-
-  } elsif (    $params->{'magic'}  =~ /^not[ _]yet[ _]billed$/
-            || $params->{'status'} =~ /^not[ _]yet[ _]billed$/ ) {
-
-    push @where, FS::cust_pkg->not_yet_billed_sql();
-
-  } elsif (    $params->{'magic'}  =~ /^(one-time charge|inactive)/
-            || $params->{'status'} =~ /^(one-time charge|inactive)/ ) {
-
-    push @where, FS::cust_pkg->inactive_sql();
-
-  } elsif (    $params->{'magic'}  eq 'suspended'
-            || $params->{'status'} eq 'suspended'  ) {
-
-    push @where, FS::cust_pkg->suspended_sql();
-
-  } elsif (    $params->{'magic'}  =~ /^cancell?ed$/
-            || $params->{'status'} =~ /^cancell?ed$/ ) {
-
-    push @where, FS::cust_pkg->cancelled_sql();
-
-  }
-
-  ###
-  # parse package class
-  ###
-
-  if ( exists($params->{'classnum'}) ) {
-
-    my @classnum = ();
-    if ( ref($params->{'classnum'}) ) {
-
-      if ( ref($params->{'classnum'}) eq 'HASH' ) {
-        @classnum = grep $params->{'classnum'}{$_}, keys %{ $params->{'classnum'} };
-      } elsif ( ref($params->{'classnum'}) eq 'ARRAY' ) {
-        @classnum = @{ $params->{'classnum'} };
-      } else {
-        die 'unhandled classnum ref '. $params->{'classnum'};
-      }
-
-
-    } elsif ( $params->{'classnum'} =~ /^(\d*)$/ && $1 ne '0' ) {
-      @classnum = ( $1 );
-    }
-
-    if ( @classnum ) {
-
-      my @c_where = ();
-      my @nums = grep $_, @classnum;
-      push @c_where, 'part_pkg.classnum IN ('. join(',',@nums). ')' if @nums;
-      my $null = scalar( grep { $_ eq '' } @classnum );
-      push @c_where, 'part_pkg.classnum IS NULL' if $null;
-
-      if ( scalar(@c_where) == 1 ) {
-        push @where, @c_where;
-      } elsif ( @c_where ) {
-        push @where, ' ( '. join(' OR ', @c_where). ' ) ';
-      }
-
-    }
-    
-
-  }
-
-  ###
-  # parse package report options
-  ###
-
-  my @report_option = ();
-  if ( exists($params->{'report_option'}) ) {
-    if ( ref($params->{'report_option'}) eq 'ARRAY' ) {
-      @report_option = @{ $params->{'report_option'} };
-    } elsif ( $params->{'report_option'} =~ /^([,\d]*)$/ ) {
-      @report_option = split(',', $1);
-    }
-
-  }
-
-  if (@report_option) {
-    # this will result in the empty set for the dangling comma case as it should
-    push @where, 
-      map{ "0 < ( SELECT count(*) FROM part_pkg_option
-                    WHERE part_pkg_option.pkgpart = part_pkg.pkgpart
-                    AND optionname = 'report_option_$_'
-                    AND optionvalue = '1' )"
-         } @report_option;
-  }
-
-  foreach my $any ( grep /^report_option_any/, keys %$params ) {
-
-    my @report_option_any = ();
-    if ( ref($params->{$any}) eq 'ARRAY' ) {
-      @report_option_any = @{ $params->{$any} };
-    } elsif ( $params->{$any} =~ /^([,\d]*)$/ ) {
-      @report_option_any = split(',', $1);
-    }
-
-    if (@report_option_any) {
-      # this will result in the empty set for the dangling comma case as it should
-      push @where, ' ( '. join(' OR ',
-        map{ "0 < ( SELECT count(*) FROM part_pkg_option
-                      WHERE part_pkg_option.pkgpart = part_pkg.pkgpart
-                      AND optionname = 'report_option_$_'
-                      AND optionvalue = '1' )"
-           } @report_option_any
-      ). ' ) ';
-    }
-
-  }
-
-  ###
-  # parse custom
-  ###
-
-  push @where,  "part_pkg.custom = 'Y'" if $params->{custom};
-
-  ###
-  # parse fcc_line
-  ###
-
-  push @where,  "(part_pkg.fcc_ds0s > 0 OR pkg_class.fcc_ds0s > 0)" 
-                                                        if $params->{fcc_line};
-
-  ###
-  # parse censustract
-  ###
-
-  if ( exists($params->{'censustract'}) ) {
-    $params->{'censustract'} =~ /^([.\d]*)$/;
-    my $censustract = "cust_location.censustract = '$1'";
-    $censustract .= ' OR cust_location.censustract is NULL' unless $1;
-    push @where,  "( $censustract )";
-  }
-
-  ###
-  # parse censustract2
-  ###
-  if ( exists($params->{'censustract2'})
-       && $params->{'censustract2'} =~ /^(\d*)$/
-     )
-  {
-    if ($1) {
-      push @where, "cust_location.censustract LIKE '$1%'";
-    } else {
-      push @where,
-        "( cust_location.censustract = '' OR cust_location.censustract IS NULL )";
-    }
-  }
-
-  ###
-  # parse country/state
-  ###
-  for (qw(state country)) { # parsing rules are the same for these
-  if ( exists($params->{$_}) 
-    && uc($params->{$_}) =~ /^([A-Z]{2})$/ )
-    {
-      # XXX post-2.3 only--before that, state/country may be in cust_main
-      push @where, "cust_location.$_ = '$1'";
-    }
-  }
-
-  ###
-  # parse part_pkg
-  ###
-
-  if ( ref($params->{'pkgpart'}) ) {
-
-    my @pkgpart = ();
-    if ( ref($params->{'pkgpart'}) eq 'HASH' ) {
-      @pkgpart = grep $params->{'pkgpart'}{$_}, keys %{ $params->{'pkgpart'} };
-    } elsif ( ref($params->{'pkgpart'}) eq 'ARRAY' ) {
-      @pkgpart = @{ $params->{'pkgpart'} };
-    } else {
-      die 'unhandled pkgpart ref '. $params->{'pkgpart'};
-    }
-
-    @pkgpart = grep /^(\d+)$/, @pkgpart;
-
-    push @where, 'pkgpart IN ('. join(',', @pkgpart). ')' if scalar(@pkgpart);
-
-  } elsif ( $params->{'pkgpart'} =~ /^(\d+)$/ ) {
-    push @where, "pkgpart = $1";
-  } 
-
-  ###
-  # parse dates
-  ###
-
-  my $orderby = '';
-
-  #false laziness w/report_cust_pkg.html
-  my %disable = (
-    'all'             => {},
-    'one-time charge' => { 'last_bill'=>1, 'bill'=>1, 'adjourn'=>1, 'susp'=>1, 'expire'=>1, 'cancel'=>1, },
-    'active'          => { 'susp'=>1, 'cancel'=>1 },
-    'suspended'       => { 'cancel' => 1 },
-    'cancelled'       => {},
-    ''                => {},
-  );
-
-  if( exists($params->{'active'} ) ) {
-    # This overrides all the other date-related fields
-    my($beginning, $ending) = @{$params->{'active'}};
-    push @where,
-      "cust_pkg.setup IS NOT NULL",
-      "cust_pkg.setup <= $ending",
-      "(cust_pkg.cancel IS NULL OR cust_pkg.cancel >= $beginning )",
-      "NOT (".FS::cust_pkg->onetime_sql . ")";
-  }
-  else {
-    foreach my $field (qw( setup last_bill bill adjourn susp expire contract_end change_date cancel )) {
-
-      next unless exists($params->{$field});
-
-      my($beginning, $ending) = @{$params->{$field}};
-
-      next if $beginning == 0 && $ending == 4294967295;
-
-      push @where,
-        "cust_pkg.$field IS NOT NULL",
-        "cust_pkg.$field >= $beginning",
-        "cust_pkg.$field <= $ending";
-
-      $orderby ||= "ORDER BY cust_pkg.$field";
-
-    }
-  }
-
-  $orderby ||= 'ORDER BY bill';
-
-  ###
-  # parse magic, legacy, etc.
-  ###
-
-  if ( $params->{'magic'} &&
-       $params->{'magic'} =~ /^(active|inactive|suspended|cancell?ed)$/
-  ) {
-
-    $orderby = 'ORDER BY pkgnum';
-
-    if ( $params->{'pkgpart'} =~ /^(\d+)$/ ) {
-      push @where, "pkgpart = $1";
-    }
-
-  } elsif ( $params->{'query'} eq 'pkgnum' ) {
-
-    $orderby = 'ORDER BY pkgnum';
-
-  } elsif ( $params->{'query'} eq 'APKG_pkgnum' ) {
-
-    $orderby = 'ORDER BY pkgnum';
-
-    push @where, '0 < (
-      SELECT count(*) FROM pkg_svc
-       WHERE pkg_svc.pkgpart =  cust_pkg.pkgpart
-         AND pkg_svc.quantity > ( SELECT count(*) FROM cust_svc
-                                   WHERE cust_svc.pkgnum  = cust_pkg.pkgnum
-                                     AND cust_svc.svcpart = pkg_svc.svcpart
-                                )
-    )';
-  
-  }
-
-  ##
-  # setup queries, links, subs, etc. for the search
-  ##
-
-  # here is the agent virtualization
-  if ($params->{CurrentUser}) {
-    my $access_user =
-      qsearchs('access_user', { username => $params->{CurrentUser} });
-
-    if ($access_user) {
-      push @where, $access_user->agentnums_sql('table'=>'cust_main');
-    } else {
-      push @where, "1=0";
-    }
-  } else {
-    push @where, $FS::CurrentUser::CurrentUser->agentnums_sql('table'=>'cust_main');
-  }
-
-  my $extra_sql = scalar(@where) ? ' WHERE '. join(' AND ', @where) : '';
-
-  my $addl_from = 'LEFT JOIN part_pkg  USING ( pkgpart  ) '.
-                  'LEFT JOIN pkg_class ON ( part_pkg.classnum = pkg_class.classnum ) '.
-                  'LEFT JOIN cust_location USING ( locationnum ) '.
-                  FS::UI::Web::join_cust_main('cust_pkg', 'cust_pkg');
-
-  my $select;
-  my $count_query;
-  if ( $params->{'select_zip5'} ) {
-    my $zip = 'cust_location.zip';
-
-    $select = "DISTINCT substr($zip,1,5) as zip";
-    $orderby = "ORDER BY substr($zip,1,5)";
-    $count_query = "SELECT COUNT( DISTINCT substr($zip,1,5) )";
-  } else {
-    $select = join(', ',
-                         'cust_pkg.*',
-                         ( map "part_pkg.$_", qw( pkg freq ) ),
-                         'pkg_class.classname',
-                         'cust_main.custnum AS cust_main_custnum',
-                         FS::UI::Web::cust_sql_fields(
-                           $params->{'cust_fields'}
-                         ),
-                  );
-    $count_query = 'SELECT COUNT(*)';
-  }
-
-  $count_query .= " FROM cust_pkg $addl_from $extra_sql";
-
-  my $sql_query = {
-    'table'       => 'cust_pkg',
-    'hashref'     => {},
-    'select'      => $select,
-    'extra_sql'   => $extra_sql,
-    'order_by'    => $orderby,
-    'addl_from'   => $addl_from,
-    'count_query' => $count_query,
-  };
-
-}
-
 =item fcc_477_count
 
 Returns a list of two package counts.  The first is a count of packages
@@ -4652,6 +5311,9 @@ sub _X_show_zero {
 
 =item order CUSTNUM, PKGPARTS_ARYREF, [ REMOVE_PKGNUMS_ARYREF [ RETURN_CUST_PKG_ARRAYREF [ REFNUM ] ] ]
 
+Bulk cancel + order subroutine.  Perhaps slightly deprecated, only used by the
+bulk cancel+order in the web UI and nowhere else (edit/process/cust_pkg.cgi)
+
 CUSTNUM is a customer (see L<FS::cust_main>)
 
 PKGPARTS is a list of pkgparts specifying the the billing item definitions (see
@@ -4681,13 +5343,6 @@ sub order {
   my $conf = new FS::Conf;
 
   # Transactionize this whole mess
-  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;
@@ -4795,7 +5450,7 @@ sub order {
       $dbh->rollback if $oldAutoCommit;
       return "Unable to transfer all services from package ".$old_pkg->pkgnum;
     }
-    $error = $old_pkg->cancel( quiet=>1 );
+    $error = $old_pkg->cancel( quiet=>1, 'no_delay_cancel'=>1 );
     if ($error) {
       $dbh->rollback;
       return $error;
@@ -4827,13 +5482,6 @@ sub bulk_change {
   my ($pkgparts, $remove_pkgnum, $return_cust_pkg) = @_;
 
   # Transactionize this whole mess
-  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;
@@ -4865,6 +5513,95 @@ sub bulk_change {
   '';
 }
 
+=item forward_emails
+
+Returns a hash of svcnums and corresponding email addresses
+for svc_acct services that can be used as source or dest
+for svc_forward services provisioned in this package.
+
+Accepts options I<svc_forward> OR I<svcnum> for a svc_forward
+service;  if included, will ensure the current values of the
+specified service are included in the list, even if for some
+other reason they wouldn't be.  If called as a class method
+with a specified service, returns only these current values.
+
+Caution: does not actually check if svc_forward services are
+available to be provisioned on this package.
+
+=cut
+
+sub forward_emails {
+  my $self = shift;
+  my %opt = @_;
+
+  #load optional service, thoroughly validated
+  die "Use svcnum or svc_forward, not both"
+    if $opt{'svcnum'} && $opt{'svc_forward'};
+  my $svc_forward = $opt{'svc_forward'};
+  $svc_forward ||= qsearchs('svc_forward',{ 'svcnum' => $opt{'svcnum'} })
+    if $opt{'svcnum'};
+  die "Specified service is not a forward service"
+    if $svc_forward && (ref($svc_forward) ne 'FS::svc_forward');
+  die "Specified service not found"
+    if ($opt{'svcnum'} || $opt{'svc_forward'}) && !$svc_forward;
+
+  my %email;
+
+  ## everything below was basically copied from httemplate/edit/svc_forward.cgi 
+  ## with minimal refactoring, not sure why we can't just load all svc_accts for this custnum
+
+  #add current values from specified service, if there was one
+  if ($svc_forward) {
+    foreach my $method (qw( srcsvc_acct dstsvc_acct )) {
+      my $svc_acct = $svc_forward->$method();
+      $email{$svc_acct->svcnum} = $svc_acct->email if $svc_acct;
+    }
+  }
+
+  if (ref($self) eq 'FS::cust_pkg') {
+
+    #and including the rest for this customer
+    my($u_part_svc,@u_acct_svcparts);
+    foreach $u_part_svc ( qsearch('part_svc',{'svcdb'=>'svc_acct'}) ) {
+      push @u_acct_svcparts,$u_part_svc->getfield('svcpart');
+    }
+
+    my $custnum = $self->getfield('custnum');
+    foreach my $i_cust_pkg ( qsearch('cust_pkg',{'custnum'=>$custnum}) ) {
+      my $cust_pkgnum = $i_cust_pkg->getfield('pkgnum');
+      #now find the corresponding record(s) in cust_svc (for this pkgnum!)
+      foreach my $acct_svcpart (@u_acct_svcparts) {
+        foreach my $i_cust_svc (
+          qsearch( 'cust_svc', { 'pkgnum'  => $cust_pkgnum,
+                                 'svcpart' => $acct_svcpart } )
+        ) {
+          my $svc_acct = qsearchs( 'svc_acct', { 'svcnum' => $i_cust_svc->svcnum } );
+          $email{$svc_acct->svcnum} = $svc_acct->email;
+        }  
+      }
+    }
+  }
+
+  return %email;
+}
+
+# Used by FS::Upgrade to migrate to a new database.
+sub _upgrade_schema {  # class method
+  my ($class, %opts) = @_;
+
+  my $sql = '
+    UPDATE cust_pkg SET change_to_pkgnum = NULL
+      WHERE change_to_pkgnum IS NOT NULL
+        AND NOT EXISTS ( SELECT 1 FROM cust_pkg AS ctcp
+                           WHERE ctcp.pkgnum = cust_pkg.change_to_pkgnum
+                       )
+  ';
+
+  my $sth = dbh->prepare($sql) or die dbh->errstr;
+  $sth->execute or die $sth->errstr;
+  '';
+}
+
 # Used by FS::Upgrade to migrate to a new database.
 sub _upgrade_data {  # class method
   my ($class, %opts) = @_;
@@ -4888,6 +5625,49 @@ sub _upgrade_data {  # class method
     my $sth = dbh->prepare($sql);
     $sth->execute or die $sth->errstr;
   }
+
+  # RT31194: supplemental package links that are deleted don't clean up 
+  # linked records
+  my @pkglinknums = qsearch({
+      'select'    => 'DISTINCT cust_pkg.pkglinknum',
+      'table'     => 'cust_pkg',
+      'addl_from' => ' LEFT JOIN part_pkg_link USING (pkglinknum) ',
+      'extra_sql' => ' WHERE cust_pkg.pkglinknum IS NOT NULL 
+                        AND part_pkg_link.pkglinknum IS NULL',
+  });
+  foreach (@pkglinknums) {
+    my $pkglinknum = $_->pkglinknum;
+    warn "cleaning part_pkg_link #$pkglinknum\n";
+    my $part_pkg_link = FS::part_pkg_link->new({pkglinknum => $pkglinknum});
+    my $error = $part_pkg_link->remove_linked;
+    die $error if $error;
+  }
+
+  # RT#73607: canceling a package with billing addons sometimes changes its
+  # pkgpart.
+  # Find records where the last replace_new record for the package before it
+  # was canceled has a different pkgpart from the package itself.
+  my @cust_pkg = qsearch({
+    'table' => 'cust_pkg',
+    'select' => 'cust_pkg.*, h_cust_pkg.pkgpart AS h_pkgpart',
+    'addl_from' => ' JOIN (
+  SELECT pkgnum, MAX(historynum) AS historynum FROM h_cust_pkg
+    WHERE cancel IS NULL
+      AND history_action = \'replace_new\'
+    GROUP BY pkgnum
+  ) AS last_history USING (pkgnum)
+  JOIN h_cust_pkg USING (historynum)',
+    'extra_sql' => ' WHERE cust_pkg.cancel is not null
+                     AND cust_pkg.pkgpart != h_cust_pkg.pkgpart'
+  });
+  foreach my $cust_pkg ( @cust_pkg ) {
+    my $pkgnum = $cust_pkg->pkgnum;
+    warn "fixing pkgpart on canceled pkg#$pkgnum\n";
+    $cust_pkg->set('pkgpart', $cust_pkg->h_pkgpart);
+    my $error = $cust_pkg->replace;
+    die $error if $error;
+  }
+
 }
 
 =back