don't wipe out part_pkg options on replace, related to #18503
[freeside.git] / FS / FS / part_pkg.pm
index 9f23178..1b887a2 100644 (file)
@@ -103,6 +103,16 @@ inherits from FS::Record.  The following fields are currently supported:
 
 =item fcc_ds0s - Optional DS0 equivalency number for FCC form 477
 
+=item fcc_voip_class - Which column of FCC form 477 part II.B this package 
+belongs in.
+
+=item successor - Foreign key for the part_pkg that replaced this record.
+If this record is not obsolete, will be null.
+
+=item family_pkgpart - Foreign key for the part_pkg that was the earliest
+ancestor of this record.  If this record is not a successor to another 
+part_pkg, will be equal to pkgpart.
+
 =back
 
 =head1 METHODS
@@ -192,6 +202,16 @@ sub insert {
     return $error;
   }
 
+  # set family_pkgpart
+  if ( $self->get('family_pkgpart') eq '' ) {
+    $self->set('family_pkgpart' => $self->pkgpart);
+    $error = $self->SUPER::replace;
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return $error;
+    }
+  }
+
   my $conf = new FS::Conf;
   if ( $conf->exists('agent_defaultpkg') ) {
     warn "  agent_defaultpkg set; allowing all agents to purchase package"
@@ -277,23 +297,24 @@ sub insert {
   }
 
   if ( $options{'part_pkg_vendor'} ) {
-      my($exportnum,$vendor_pkg_id);
-      my %options_part_pkg_vendor = $options{'part_pkg_vendor'};
-      while(($exportnum,$vendor_pkg_id) = each %options_part_pkg_vendor){
-           my $ppv = new FS::part_pkg_vendor( {
-                   'pkgpart' => $self->pkgpart,
-                   'exportnum' => $exportnum,
-                   'vendor_pkg_id' => $vendor_pkg_id, 
-               } );
-           my $error = $ppv->insert;
-           if ( $error ) {
-             $dbh->rollback if $oldAutoCommit;
-             return "Error inserting part_pkg_vendor record: $error";
-           }
+      while ( my ($exportnum, $vendor_pkg_id) =
+                each %{ $options{part_pkg_vendor} }
+            )
+      {
+            my $ppv = new FS::part_pkg_vendor( {
+                    'pkgpart' => $self->pkgpart,
+                    'exportnum' => $exportnum,
+                    'vendor_pkg_id' => $vendor_pkg_id, 
+                } );
+            my $error = $ppv->insert;
+            if ( $error ) {
+              $dbh->rollback if $oldAutoCommit;
+              return "Error inserting part_pkg_vendor record: $error";
+            }
       }
   }
 
-  warn "  commiting transaction" if $DEBUG;
+  warn "  committing transaction" if $DEBUG and $oldAutoCommit;
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
 
   '';
@@ -343,7 +364,7 @@ sub replace {
       ? shift
       : { @_ };
 
-  $options->{options} = {} unless defined($options->{options});
+  $options->{options} = { $old->options } unless defined($options->{options});
 
   warn "FS::part_pkg::replace called on $new to replace $old with options".
        join(', ', map "$_ => ". $options->{$_}, keys %$options)
@@ -359,6 +380,28 @@ sub replace {
   my $oldAutoCommit = $FS::UID::AutoCommit;
   local $FS::UID::AutoCommit = 0;
   my $dbh = dbh;
+  
+  my $conf = new FS::Conf;
+  if ( $conf->exists('part_pkg-lineage') ) {
+    if ( grep { $options->{options}->{$_} ne $old->option($_, 1) }
+          qw(setup_fee recur_fee) #others? config?
+        ) { 
+    
+      warn "  superseding package" if $DEBUG;
+
+      my $error = $new->supersede($old, %$options);
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return $error;
+      }
+      else {
+        warn "  committing transaction" if $DEBUG and $oldAutoCommit;
+        $dbh->commit if $oldAutoCommit;
+        return $error;
+      }
+    }
+    #else nothing
+  }
 
   #plandata shit stays in replace for upgrades until after 2.0 (or edit
   #_upgrade_data)
@@ -403,105 +446,117 @@ sub replace {
   }
 
   warn "  replacing pkg_svc records" if $DEBUG;
-  my $pkg_svc = $options->{'pkg_svc'} || {};
+  my $pkg_svc = $options->{'pkg_svc'};
   my $hidden_svc = $options->{'hidden_svc'} || {};
-  foreach my $part_svc ( qsearch('part_svc', {} ) ) {
-    my $quantity = $pkg_svc->{$part_svc->svcpart} || 0;
-    my $hidden = $hidden_svc->{$part_svc->svcpart} || '';
-    my $primary_svc =
-      ( defined($options->{'primary_svc'}) && $options->{'primary_svc'}
-        && $options->{'primary_svc'} == $part_svc->svcpart
-      )
-        ? 'Y'
-        : '';
+  if ( $pkg_svc ) { # if it wasn't passed, don't change existing pkg_svcs
+    foreach my $part_svc ( qsearch('part_svc', {} ) ) {
+      my $quantity = $pkg_svc->{$part_svc->svcpart} || 0;
+      my $hidden = $hidden_svc->{$part_svc->svcpart} || '';
+      my $primary_svc =
+        ( defined($options->{'primary_svc'}) && $options->{'primary_svc'}
+          && $options->{'primary_svc'} == $part_svc->svcpart
+        )
+          ? 'Y'
+          : '';
 
-    my $old_pkg_svc = qsearchs('pkg_svc', {
-        'pkgpart' => $old->pkgpart,
-        'svcpart' => $part_svc->svcpart,
+      my $old_pkg_svc = qsearchs('pkg_svc', {
+          'pkgpart' => $old->pkgpart,
+          'svcpart' => $part_svc->svcpart,
+        }
+      );
+      my $old_quantity = 0;
+      my $old_primary_svc = '';
+      my $old_hidden = '';
+      if ( $old_pkg_svc ) {
+        $old_quantity = $old_pkg_svc->quantity;
+        $old_primary_svc = $old_pkg_svc->primary_svc 
+          if $old_pkg_svc->dbdef_table->column('primary_svc'); # is this needed?
+        $old_hidden = $old_pkg_svc->hidden;
       }
-    );
-    my $old_quantity = 0;
-    my $old_primary_svc = '';
-    my $old_hidden = '';
-    if ( $old_pkg_svc ) {
-      $old_quantity = $old_pkg_svc->quantity;
-      $old_primary_svc = $old_pkg_svc->primary_svc 
-        if $old_pkg_svc->dbdef_table->column('primary_svc'); # is this needed?
-      $old_hidden = $old_pkg_svc->hidden;
-    }
-    next unless $old_quantity != $quantity || 
-                $old_primary_svc ne $primary_svc ||
-                $old_hidden ne $hidden;
-  
-    my $new_pkg_svc = new FS::pkg_svc( {
-      'pkgsvcnum'   => ( $old_pkg_svc ? $old_pkg_svc->pkgsvcnum : '' ),
-      'pkgpart'     => $new->pkgpart,
-      'svcpart'     => $part_svc->svcpart,
-      'quantity'    => $quantity, 
-      'primary_svc' => $primary_svc,
-      'hidden'      => $hidden,
-    } );
-    my $error = $old_pkg_svc
-                  ? $new_pkg_svc->replace($old_pkg_svc)
-                  : $new_pkg_svc->insert;
-    if ( $error ) {
-      $dbh->rollback if $oldAutoCommit;
-      return $error;
-    }
-  }
+   
+      next unless $old_quantity != $quantity || 
+                  $old_primary_svc ne $primary_svc ||
+                  $old_hidden ne $hidden;
+    
+      my $new_pkg_svc = new FS::pkg_svc( {
+        'pkgsvcnum'   => ( $old_pkg_svc ? $old_pkg_svc->pkgsvcnum : '' ),
+        'pkgpart'     => $new->pkgpart,
+        'svcpart'     => $part_svc->svcpart,
+        'quantity'    => $quantity, 
+        'primary_svc' => $primary_svc,
+        'hidden'      => $hidden,
+      } );
+      my $error = $old_pkg_svc
+                    ? $new_pkg_svc->replace($old_pkg_svc)
+                    : $new_pkg_svc->insert;
+      if ( $error ) {
+        $dbh->rollback if $oldAutoCommit;
+        return $error;
+      }
+    } #foreach $part_svc
+  } #if $options->{pkg_svc}
   
   my @part_pkg_vendor = $old->part_pkg_vendor;
   my @current_exportnum = ();
   if ( $options->{'part_pkg_vendor'} ) {
       my($exportnum,$vendor_pkg_id);
       while ( ($exportnum,$vendor_pkg_id) 
-                               = each %{$options->{'part_pkg_vendor'}} ) {
-         my $noinsert = 0;
-         foreach my $part_pkg_vendor ( @part_pkg_vendor ) {
-           if($exportnum == $part_pkg_vendor->exportnum
-               && $vendor_pkg_id ne $part_pkg_vendor->vendor_pkg_id) {
-               $part_pkg_vendor->vendor_pkg_id($vendor_pkg_id);
-               my $error = $part_pkg_vendor->replace;
-               if ( $error ) {
-                 $dbh->rollback if $oldAutoCommit;
-                 return "Error replacing part_pkg_vendor record: $error";
-               }
-               $noinsert = 1;
-               last;
-           }
-           elsif($exportnum == $part_pkg_vendor->exportnum
-               && $vendor_pkg_id eq $part_pkg_vendor->vendor_pkg_id) {
-               $noinsert = 1;
-               last;
-           }
-         }
-         unless ( $noinsert ) {
-           my $ppv = new FS::part_pkg_vendor( {
-                   'pkgpart' => $new->pkgpart,
-                   'exportnum' => $exportnum,
-                   'vendor_pkg_id' => $vendor_pkg_id, 
-               } );
-           my $error = $ppv->insert;
-           if ( $error ) {
-             $dbh->rollback if $oldAutoCommit;
-             return "Error inserting part_pkg_vendor record: $error";
-           }
-         }
-         push @current_exportnum, $exportnum;
+                                = each %{$options->{'part_pkg_vendor'}} ) {
+          my $noinsert = 0;
+          foreach my $part_pkg_vendor ( @part_pkg_vendor ) {
+            if($exportnum == $part_pkg_vendor->exportnum
+                && $vendor_pkg_id ne $part_pkg_vendor->vendor_pkg_id) {
+                $part_pkg_vendor->vendor_pkg_id($vendor_pkg_id);
+                my $error = $part_pkg_vendor->replace;
+                if ( $error ) {
+                  $dbh->rollback if $oldAutoCommit;
+                  return "Error replacing part_pkg_vendor record: $error";
+                }
+                $noinsert = 1;
+                last;
+            }
+            elsif($exportnum == $part_pkg_vendor->exportnum
+                && $vendor_pkg_id eq $part_pkg_vendor->vendor_pkg_id) {
+                $noinsert = 1;
+                last;
+            }
+          }
+          unless ( $noinsert ) {
+            my $ppv = new FS::part_pkg_vendor( {
+                    'pkgpart' => $new->pkgpart,
+                    'exportnum' => $exportnum,
+                    'vendor_pkg_id' => $vendor_pkg_id, 
+                } );
+            my $error = $ppv->insert;
+            if ( $error ) {
+              $dbh->rollback if $oldAutoCommit;
+              return "Error inserting part_pkg_vendor record: $error";
+            }
+          }
+          push @current_exportnum, $exportnum;
       }
   }
   foreach my $part_pkg_vendor ( @part_pkg_vendor ) {
       unless ( grep($_ eq $part_pkg_vendor->exportnum, @current_exportnum) ) {
-       my $error = $part_pkg_vendor->delete;
-       if ( $error ) {
-         $dbh->rollback if $oldAutoCommit;
-         return "Error deleting part_pkg_vendor record: $error";
-       }
+        my $error = $part_pkg_vendor->delete;
+        if ( $error ) {
+          $dbh->rollback if $oldAutoCommit;
+          return "Error deleting part_pkg_vendor record: $error";
+        }
       }
   }
+  
+  # propagate changes to certain core fields
+  if ( $conf->exists('part_pkg-lineage') ) {
+    warn "  propagating changes to family" if $DEBUG;
+    my $error = $new->propagate($old);
+    if ( $error ) {
+      $dbh->rollback if $oldAutoCommit;
+      return $error;
+    }
+  }
 
-  warn "  commiting transaction" if $DEBUG;
+  warn "  committing transaction" if $DEBUG and $oldAutoCommit;
   $dbh->commit or die $dbh->errstr if $oldAutoCommit;
   '';
 }
@@ -552,6 +607,8 @@ sub check {
     || $self->ut_enum('disabled', [ '', 'Y' ] )
     || $self->ut_enum('custom', [ '', 'Y' ] )
     || $self->ut_enum('no_auto', [ '', 'Y' ])
+    || $self->ut_enum('recur_show_zero', [ '', 'Y' ])
+    || $self->ut_enum('setup_show_zero', [ '', 'Y' ])
     #|| $self->ut_moneyn('setup_cost')
     #|| $self->ut_moneyn('recur_cost')
     || $self->ut_floatn('setup_cost')
@@ -570,6 +627,9 @@ sub check {
            : $self->ut_agentnum_acl('agentnum', \@null_agentnum_right)
        )
     || $self->ut_numbern('fcc_ds0s')
+    || $self->ut_numbern('fcc_voip_class')
+    || $self->ut_foreign_keyn('successor', 'part_pkg', 'pkgpart')
+    || $self->ut_foreign_keyn('family_pkgpart', 'part_pkg', 'pkgpart')
     || $self->SUPER::check
   ;
   return $error if $error;
@@ -584,6 +644,76 @@ sub check {
   '';
 }
 
+=item supersede OLD [, OPTION => VALUE ... ]
+
+Inserts this package as a successor to the package OLD.  All options are as
+for C<insert>.  After inserting, disables OLD and sets the new package as its
+successor.
+
+=cut
+
+sub supersede {
+  my ($new, $old, %options) = @_;
+  my $error;
+
+  $new->set('pkgpart' => '');
+  $new->set('family_pkgpart' => $old->family_pkgpart);
+  warn "    inserting successor package\n" if $DEBUG;
+  $error = $new->insert(%options);
+  return $error if $error;
+  warn "    disabling superseded package\n" if $DEBUG; 
+  $old->set('successor' => $new->pkgpart);
+  $old->set('disabled' => 'Y');
+  $error = $old->SUPER::replace; # don't change its options/pkg_svc records
+  return $error if $error;
+
+  warn "  propagating changes to family" if $DEBUG;
+  $new->propagate($old);
+}
+
+=item propagate OLD
+
+If any of certain fields have changed from OLD to this package, then,
+for all packages in the same lineage as this one, sets those fields 
+to their values in this package.
+
+=cut
+
+my @propagate_fields = (
+  qw( pkg classnum setup_cost recur_cost taxclass
+  setuptax recurtax pay_weight credit_weight
+  )
+);
+
+sub propagate {
+  my $new = shift;
+  my $old = shift;
+  my %fields = (
+    map { $_ => $new->get($_) }
+    grep { $new->get($_) ne $old->get($_) }
+    @propagate_fields
+  );
+
+  my @part_pkg = qsearch('part_pkg', { 
+      'family_pkgpart' => $new->family_pkgpart 
+  });
+  my @error;
+  foreach my $part_pkg ( @part_pkg ) {
+    my $pkgpart = $part_pkg->pkgpart;
+    next if $pkgpart == $new->pkgpart; # don't modify $new
+    warn "    propagating to pkgpart $pkgpart\n" if $DEBUG;
+    foreach ( keys %fields ) {
+      $part_pkg->set($_, $fields{$_});
+    }
+    # SUPER::replace to avoid changing non-core fields
+    my $error = $part_pkg->SUPER::replace;
+    push @error, "pkgpart $pkgpart: $error"
+      if $error;
+  }
+  join("\n", @error);
+}
+
 =item pkg_comment [ OPTION => VALUE... ]
 
 Returns an (internal) string representing this package.  Currently,
@@ -605,6 +735,10 @@ sub pkg_comment {
   $pre. $self->pkg. ' - '. $self->custom_comment;
 }
 
+sub price_info { # safety, in case a part_pkg hasn't defined price_info
+    '';
+}
+
 sub custom_comment {
   my $self = shift;
   ( $self->custom ? '(CUSTOM) ' : '' ). $self->comment . ' ' . $self->price_info;
@@ -844,10 +978,7 @@ Returns true if this package is free.
 
 sub is_free {
   my $self = shift;
-  unless ( $self->plan ) {
-    $self->setup =~ /^\s*0+(\.0*)?\s*$/
-      && $self->recur =~ /^\s*0+(\.0*)?\s*$/;
-  } elsif ( $self->can('is_free_options') ) {
+  if ( $self->can('is_free_options') ) {
     not grep { $_ !~ /^\s*0*(\.0*)?\s*$/ }
          map { $self->option($_) } 
              $self->is_free_options;
@@ -860,6 +991,8 @@ sub is_free {
 
 sub can_discount { 0; }
 
+sub can_start_date { 1; }
+
 sub freqs_href {
   # moved to FS::Misc to make this accessible to other packages
   # at initialization
@@ -1044,6 +1177,17 @@ sub svc_part_pkg_link {
   shift->_part_pkg_link('svc', @_);
 }
 
+=item supp_part_pkg_link
+
+Returns the associated part_pkg_link records of type 'supp' (supplemental
+packages).
+
+=cut
+
+sub supp_part_pkg_link {
+  shift->_part_pkg_link('supp', @_);
+}
+
 sub _part_pkg_link {
   my( $self, $type ) = @_;
   qsearch({ table    => 'part_pkg_link',
@@ -1060,6 +1204,10 @@ sub self_and_bill_linked {
   shift->_self_and_linked('bill', @_);
 }
 
+sub self_and_svc_linked {
+  shift->_self_and_linked('svc', @_);
+}
+
 sub _self_and_linked {
   my( $self, $type, $hidden ) = @_;
   $hidden ||= '';
@@ -1267,52 +1415,31 @@ sub _rebless {
   }
   return $self if ref($self) =~ /::$plan$/; #already blessed into plan subclass
   my $class = ref($self). "::$plan";
-  warn "reblessing $self into $class" if $DEBUG;
+  warn "reblessing $self into $class" if $DEBUG > 1;
   eval "use $class;";
   die $@ if $@;
   bless($self, $class) unless $@;
   $self;
 }
 
-#fallbacks that eval the setup and recur fields, for backwards compat
-
-sub calc_setup {
-  my $self = shift;
-  warn 'no price plan class for '. $self->plan. ", eval-ing setup\n";
-  $self->_calc_eval('setup', @_);
-}
-
-sub calc_recur {
-  my $self = shift;
-  warn 'no price plan class for '. $self->plan. ", eval-ing recur\n";
-  $self->_calc_eval('recur', @_);
-}
-
-use vars qw( $sdate @details );
-sub _calc_eval {
-  #my( $self, $field, $cust_pkg ) = @_;
-  my( $self, $field, $cust_pkg, $sdateref, $detailsref ) = @_;
-  *sdate = $sdateref;
-  *details = $detailsref;
-  $self->$field() =~ /^(.*)$/
-    or die "Illegal $field (pkgpart ". $self->pkgpart. '): '.
-            $self->$field(). "\n";
-  my $prog = $1;
-  return 0 if $prog =~ /^\s*$/;
-  my $value = eval $prog;
-  die $@ if $@;
-  $value;
-}
+#fatal fallbacks
+sub calc_setup { die 'no calc_setup for '. shift->plan. "\n"; }
+sub calc_recur { die 'no calc_recur for '. shift->plan. "\n"; }
 
 #fallback that return 0 for old legacy packages with no plan
-
 sub calc_remain { 0; }
-sub calc_cancel { 0; }
 sub calc_units  { 0; }
 
+#fallback for everything not based on flat.pm
+sub recur_temporality { 'upcoming'; }
+sub calc_cancel { 0; }
+
 #fallback for everything except bulk.pm
 sub hide_svc_detail { 0; }
 
+#fallback for packages that can't/won't summarize usage
+sub sum_usage { 0; }
+
 =item recur_cost_permonth CUST_PKG
 
 recur_cost divided by freq (only supported for monthly and longer frequencies)
@@ -1325,6 +1452,29 @@ sub recur_cost_permonth {
   sprintf('%.2f', $self->recur_cost / $self->freq );
 }
 
+=item cust_bill_pkg_recur CUST_PKG
+
+Actual recurring charge for the specified customer package from customer's most
+recent invoice
+
+=cut
+
+sub cust_bill_pkg_recur {
+  my($self, $cust_pkg) = @_;
+  my $cust_bill_pkg = qsearchs({
+    'table'     => 'cust_bill_pkg',
+    'addl_from' => 'LEFT JOIN cust_bill USING ( invnum )',
+    'hashref'   => { 'pkgnum' => $cust_pkg->pkgnum,
+                     'recur'  => { op=>'>', value=>'0' },
+                   },
+    'order_by'  => 'ORDER BY cust_bill._date     DESC,
+                             cust_bill_pkg.sdate DESC
+                     LIMIT 1
+                   ',
+  }) or return 0; #die "use cust_bill_pkg_recur credits with once_perinv condition";
+  $cust_bill_pkg->recur;
+}
+
 =item format OPTION DATA
 
 Returns data formatted according to the function 'format' described
@@ -1379,8 +1529,6 @@ sub _upgrade_data { # class method
   my @part_pkg = qsearch({
     'table'     => 'part_pkg',
     'extra_sql' => "WHERE ". join(' OR ',
-                     ( map "($_ IS NOT NULL AND $_ != '' )",
-                           qw( plandata setup recur ) ),
                      'plan IS NULL', "plan = '' ",
                    ),
   });
@@ -1391,43 +1539,7 @@ sub _upgrade_data { # class method
       $part_pkg->plan('flat');
     }
 
-    if ( length($part_pkg->option('setup_fee')) == 0 
-         && $part_pkg->setup =~ /^\s*([\d\.]+)\s*$/ ) {
-
-      my $opt = new FS::part_pkg_option {
-        'pkgpart'     => $part_pkg->pkgpart,
-        'optionname'  => 'setup_fee',
-        'optionvalue' => $1,
-      };
-      my $error = $opt->insert;
-      die $error if $error;
-
-
-      #} else {
-      #  die "Can't parse part_pkg.setup for fee; convert pkgnum ".
-      #      $part_pkg->pkgnum. " manually: ". $part_pkg->setup. "\n";
-    }
-    $part_pkg->setup('');
-
-    if ( length($part_pkg->option('recur_fee')) == 0
-         && $part_pkg->recur =~ /^\s*([\d\.]+)\s*$/ ) {
-
-        my $opt = new FS::part_pkg_option {
-          'pkgpart'     => $part_pkg->pkgpart,
-          'optionname'  => 'recur_fee',
-          'optionvalue' => $1,
-        };
-        my $error = $opt->insert;
-        die $error if $error;
-
-
-      #} else {
-      #  die "Can't parse part_pkg.setup for fee; convert pkgnum ".
-      #      $part_pkg->pkgnum. " manually: ". $part_pkg->setup. "\n";
-    }
-    $part_pkg->recur('');
-
-    $part_pkg->replace; #this should take care of plandata, right?
+    $part_pkg->replace;
 
   }
 
@@ -1459,6 +1571,14 @@ sub _upgrade_data { # class method
     die $error if $error;
   }
 
+  # set family_pkgpart on any packages that don't have it
+  @part_pkg = qsearch('part_pkg', { 'family_pkgpart' => '' });
+  foreach my $part_pkg (@part_pkg) {
+    $part_pkg->set('family_pkgpart' => $part_pkg->pkgpart);
+    my $error = $part_pkg->SUPER::replace;
+    die $error if $error;
+  }
+
   my @part_pkg_option = qsearch('part_pkg_option',
     { 'optionname'  => 'unused_credit',
       'optionvalue' => 1,
@@ -1478,6 +1598,117 @@ sub _upgrade_data { # class method
       die $error if $error;
     }
   }
+
+  # migrate use_disposition_taqua and use_disposition to disposition_in
+  @part_pkg_option = qsearch('part_pkg_option',
+    { 'optionname'  => { op => 'LIKE',
+                         value => 'use_disposition%',
+                       },
+      'optionvalue' => 1,
+    });
+  my %newopts = map { $_->pkgpart => $_ } 
+    qsearch('part_pkg_option',  { 'optionname'  => 'disposition_in', } );
+  foreach my $old_opt (@part_pkg_option) {
+        my $pkgpart = $old_opt->pkgpart;
+        my $newval = $old_opt->optionname eq 'use_disposition_taqua' ? '100' 
+                                                                  : 'ANSWERED';
+        my $error = $old_opt->delete;
+        die $error if $error;
+
+        if ( exists($newopts{$pkgpart}) ) {
+            my $opt = $newopts{$pkgpart};
+            $opt->optionvalue($opt->optionvalue.",$newval");
+            $error = $opt->replace;
+            die $error if $error;
+        } else {
+            my $new_opt = new FS::part_pkg_option {
+                'pkgpart'     => $pkgpart,
+                'optionname'  => 'disposition_in',
+                'optionvalue' => $newval,
+              };
+              $error = $new_opt->insert;
+              die $error if $error;
+              $newopts{$pkgpart} = $new_opt;
+        }
+  }
+
+  # set any package with FCC voice lines to the "VoIP with broadband" category
+  # for backward compatibility
+  #
+  # recover from a bad upgrade bug
+  my $upgrade = 'part_pkg_fcc_voip_class_FIX';
+  if (!FS::upgrade_journal->is_done($upgrade)) {
+    my $bad_upgrade = qsearchs('upgrade_journal', 
+      { upgrade => 'part_pkg_fcc_voip_class' }
+    );
+    if ( $bad_upgrade ) {
+      my $where = 'WHERE history_date <= '.$bad_upgrade->_date.
+                  ' AND  history_date >  '.($bad_upgrade->_date - 3600);
+      my @h_part_pkg_option = map { FS::part_pkg_option->new($_->hashref) }
+        qsearch({
+          'select'    => '*',
+          'table'     => 'h_part_pkg_option',
+          'hashref'   => {},
+          'extra_sql' => "$where AND history_action = 'delete'",
+          'order_by'  => 'ORDER BY history_date ASC',
+        });
+      my @h_pkg_svc = map { FS::pkg_svc->new($_->hashref) }
+        qsearch({
+          'select'    => '*',
+          'table'     => 'h_pkg_svc',
+          'hashref'   => {},
+          'extra_sql' => "$where AND history_action = 'replace_old'",
+          'order_by'  => 'ORDER BY history_date ASC',
+        });
+      my %opt;
+      foreach my $deleted (@h_part_pkg_option, @h_pkg_svc) {
+        my $pkgpart ||= $deleted->pkgpart;
+        $opt{$pkgpart} ||= {
+          options => {},
+          pkg_svc => {},
+          primary_svc => '',
+          hidden_svc => {},
+        };
+        if ( $deleted->isa('FS::part_pkg_option') ) {
+          $opt{$pkgpart}{options}{ $deleted->optionname } = $deleted->optionvalue;
+        } else { # pkg_svc
+          my $svcpart = $deleted->svcpart;
+          $opt{$pkgpart}{pkg_svc}{$svcpart} = $deleted->quantity;
+          $opt{$pkgpart}{hidden_svc}{$svcpart} ||= $deleted->hidden;
+          $opt{$pkgpart}{primary_svc} = $svcpart if $deleted->primary_svc;
+        }
+      }
+      foreach my $pkgpart (keys %opt) {
+        my $part_pkg = FS::part_pkg->by_key($pkgpart);
+        my $error = $part_pkg->replace( $part_pkg->replace_old, $opt{$pkgpart} );
+        if ( $error ) {
+          die "error recovering damaged pkgpart $pkgpart:\n$error\n";
+        }
+      }
+    } # $bad_upgrade exists
+    else { # do the original upgrade, but correctly this time
+      @part_pkg = qsearch('part_pkg', {
+          fcc_ds0s        => { op => '>', value => 0 },
+          fcc_voip_class  => ''
+      });
+      foreach my $part_pkg (@part_pkg) {
+        $part_pkg->set(fcc_voip_class => 2);
+        my @pkg_svc = $part_pkg->pkg_svc;
+        my %quantity = map {$_->svcpart, $_->quantity} @pkg_svc;
+        my %hidden   = map {$_->svcpart, $_->hidden  } @pkg_svc;
+        my $error = $part_pkg->replace(
+          $part_pkg->replace_old,
+          options     => { $part_pkg->options },
+          pkg_svc     => \%quantity,
+          hidden_svc  => \%hidden,
+          primary_svc => ($part_pkg->svcpart || ''),
+        );
+        die $error if $error;
+      }
+    }
+    FS::upgrade_journal->set_done($upgrade);
+  }
+
 }
 
 =item curuser_pkgs_sql
@@ -1590,6 +1821,10 @@ foreach my $name (keys(%info)) {
   my $parents = $info{$name}->{'inherit_fields'} || [];
   my (%fields, %field_exists, @fieldorder);
   foreach my $parent ($name, @$parents) {
+    if ( !exists($info{$parent}) ) {
+      warn "$name tried to inherit from nonexistent '$parent'\n";
+      next;
+    }
     %fields = ( # avoid replacing existing fields
       %{ $info{$parent}->{'fields'} || {} },
       %fields
@@ -1599,7 +1834,8 @@ foreach my $name (keys(%info)) {
       next if $field_exists{$_};
       $field_exists{$_} = 1;
       # allow inheritors to remove inherited fields from the fieldorder
-      push @fieldorder, $_ if !exists($fields{$_}->{'disabled'});
+      push @fieldorder, $_ if !exists($fields{$_}) or
+                              !exists($fields{$_}->{'disabled'});
     }
   }
   $plans{$name}->{'fields'} = \%fields;