new package def editor

7 files changed, 281 insertions, 59 deletions

diff --git a/FS/FS.pm b/FS/FS.pm
index 6fa6faa52..26f8c18d3 100644
--- a/FS/FS.pm
+++ b/FS/FS.pm
@@ -165,6 +165,8 @@ L<FS::pkg_class> - Package class class
 
 L<FS::part_pkg> - Package definition class
 
+L<FS::part_pkg_link> - Package definition link class
+
 L<FS::part_pkg_taxclass> - Tax class class
 
 L<FS::part_pkg_option> - Package definition option class
diff --git a/FS/FS/m2m_Common.pm b/FS/FS/m2m_Common.pm
index 5dc2a8ec8..fdd008f7c 100644
--- a/FS/FS/m2m_Common.pm
+++ b/FS/FS/m2m_Common.pm
@@ -25,8 +25,9 @@ use FS::m2m_Common;
 FS::m2m_Common is intended as a mixin class for classes which have a
 many-to-many relationship with another table (via a linking table).
 
-Note: It is currently assumed that the link table contains two fields
-named the same as the primary keys of ths base and target tables.
+It is currently assumed that the link table contains two fields named the same
+as the primary keys of the base and target tables, but you can ovverride this
+assumption if your table is different.
 
 =head1 METHODS
 
@@ -36,26 +37,45 @@ named the same as the primary keys of ths base and target tables.
 
 Available options:
 
-link_table (required) - 
+=over 4
+
+=item link_table (required)
+
+=item target_table (required)
 
-target_table (required) - 
+=item params (required)
 
-params (required) - hashref; keys are primary key values in target_table (values are boolean).  For convenience, keys may optionally be prefixed with the name
-of the primary key, as in agentnum54 instead of 54, or passed as an arrayref
+hashref; keys are primary key values in target_table (values are boolean).  For convenience, keys may optionally be prefixed with the name
+of the primary key, as in "agentnum54" instead of "54", or passed as an arrayref
 of values.
 
+=item base_field (optional)
+
+base field, defaults to primary key of this base table
+
+=item target_field (optional)
+
+target field, defaults to the primary key of the target table
+
+=item hashref (optional)
+
+static hashref further qualifying the m2m fields
+
 =cut
 
 sub process_m2m {
   my( $self, %opt ) = @_;
 
   my $self_pkey = $self->dbdef_table->primary_key;
-  my %hash = ( $self_pkey => $self->$self_pkey() );
+  my $base_field = $opt{'base_field'} || $self_pkey;
+  my %hash = $opt{'hashref'} || {};
+  $hash{$base_field} = $self->$self_pkey();
 
   my $link_table = $self->_load_table($opt{'link_table'});
 
   my $target_table = $self->_load_table($opt{'target_table'});
-  my $target_pkey = dbdef->table($target_table)->primary_key;
+  my $target_field = $opt{'target_field'}
+                     || dbdef->table($target_table)->primary_key;
 
   if ( ref($opt{'params'}) eq 'ARRAY' ) {
     $opt{'params'} = { map { $_=>1 } @{$opt{'params'}} };
@@ -74,9 +94,9 @@ sub process_m2m {
 
   foreach my $del_obj (
     grep { 
-           my $targetnum = $_->$target_pkey();
+           my $targetnum = $_->$target_field();
            (    ! $opt{'params'}->{$targetnum}
-             && ! $opt{'params'}->{"$target_pkey$targetnum"}
+             && ! $opt{'params'}->{"$target_field$targetnum"}
            );
          }
          qsearch( $link_table, \%hash )
@@ -89,16 +109,16 @@ sub process_m2m {
   }
 
   foreach my $add_targetnum (
-    grep { ! qsearchs( $link_table, { %hash, $target_pkey => $_ } ) }
-    map  { /^($target_pkey)?(\d+)$/; $2; }
-    grep { /^($target_pkey)?(\d+)$/ }
+    grep { ! qsearchs( $link_table, { %hash, $target_field => $_ } ) }
+    map  { /^($target_field)?(\d+)$/; $2; }
+    grep { /^($target_field)?(\d+)$/ }
     grep { $opt{'params'}->{$_} }
     keys %{ $opt{'params'} }
   ) {
 
     my $add_obj = "FS::$link_table"->new( {
       %hash, 
-      $target_pkey => $add_targetnum,
+      $target_field => $add_targetnum,
     });
     my $error = $add_obj->insert;
     if ( $error ) {
diff --git a/FS/FS/part_pkg_link.pm b/FS/FS/part_pkg_link.pm
new file mode 100644
index 000000000..f51736055
--- /dev/null
+++ b/FS/FS/part_pkg_link.pm
@@ -0,0 +1,157 @@
+package FS::part_pkg_link;
+
+use strict;
+use vars qw( @ISA );
+use FS::Record qw( qsearchs );
+use FS::part_pkg;
+
+@ISA = qw(FS::Record);
+
+=head1 NAME
+
+FS::part_pkg_link - Object methods for part_pkg_link records
+
+=head1 SYNOPSIS
+
+  use FS::part_pkg_link;
+
+  $record = new FS::part_pkg_link \%hash;
+  $record = new FS::part_pkg_link { 'column' => 'value' };
+
+  $error = $record->insert;
+
+  $error = $new_record->replace($old_record);
+
+  $error = $record->delete;
+
+  $error = $record->check;
+
+=head1 DESCRIPTION
+
+An FS::part_pkg_link object represents an link from one package definition to
+another.  FS::part_pkg_link inherits from FS::Record.  The following fields are
+currently supported:
+
+=over 4
+
+=item pkglinknum
+
+primary key
+
+=item src_pkgpart
+
+Source package (see L<FS::part_pkg>)
+
+=item dst_pkgpart
+
+Destination package (see L<FS::part_pkg>)
+
+=item link_type
+
+Link type - currently, "bill" (source package bills a line item from target
+package), or "svc" (source package includes services from target package).
+
+=back
+
+=head1 METHODS
+
+=over 4
+
+=item new HASHREF
+
+Creates a new link.  To add the link to the database, see L<"insert">.
+
+Note that this stores the hash reference, not a distinct copy of the hash it
+points to.  You can ask the object for a copy with the I<hash> method.
+
+=cut
+
+# the new method can be inherited from FS::Record, if a table method is defined
+
+sub table { 'part_pkg_link'; }
+
+=item insert
+
+Adds this record to the database.  If there is an error, returns the error,
+otherwise returns false.
+
+=cut
+
+# the insert method can be inherited from FS::Record
+
+=item delete
+
+Delete this record from the database.
+
+=cut
+
+# the delete method can be inherited from FS::Record
+
+=item replace OLD_RECORD
+
+Replaces the OLD_RECORD with this one in the database.  If there is an error,
+returns the error, otherwise returns false.
+
+=cut
+
+# the replace method can be inherited from FS::Record
+
+=item check
+
+Checks all fields to make sure this is a valid link.  If there is
+an error, returns the error, otherwise returns false.  Called by the insert
+and replace methods.
+
+=cut
+
+# the check method should currently be supplied - FS::Record contains some
+# data checking routines
+
+sub check {
+  my $self = shift;
+
+  my $error = 
+    $self->ut_numbern('pkglinknum')
+    || $self->ut_foreign_key('src_pkgpart', 'part_pkg', 'pkgpart')
+    || $self->ut_foreign_key('dst_pkgpart', 'part_pkg', 'pkgpart')
+    || $self->ut_text('link_type', [ 'bill', 'svc' ] )
+  ;
+  return $error if $error;
+
+  $self->SUPER::check;
+}
+
+=item src_pkg
+
+Returns the source part_pkg object (see L<FS::part_pkg>).
+
+=cut
+
+sub src_pkg {
+  my $self = shift;
+  qsearchs('part_pkg', { 'pkgpart' => $self->src_pkgpart } );
+}
+
+=item dst_pkg
+
+Returns the source part_pkg object (see L<FS::part_pkg>).
+
+=cut
+
+sub dst_pkg {
+  my $self = shift;
+  qsearchs('part_pkg', { 'pkgpart' => $self->dst_pkgpart } );
+}
+
+=back
+
+=head1 BUGS
+
+=head1 SEE ALSO
+
+L<FS::Record>, schema.html from the base documentation.
+
+=cut
+
+1;
+
diff --git a/FS/t/part_pkg_link.t b/FS/t/part_pkg_link.t
new file mode 100644
index 000000000..f5ada88dd
--- /dev/null
+++ b/FS/t/part_pkg_link.t
@@ -0,0 +1,5 @@
+BEGIN { $| = 1; print "1..1\n" }
+END {print "not ok 1\n" unless $loaded;}
+use FS::part_pkg_link;
+$loaded=1;
+print "ok 1\n";
diff --git a/httemplate/edit/elements/edit.html b/httemplate/edit/elements/edit.html
index 150ed8583..4a62ba316 100644
--- a/httemplate/edit/elements/edit.html
+++ b/httemplate/edit/elements/edit.html
@@ -41,10 +41,9 @@ Example:
                     'm2name_table'   => 'table_name',
                     'm2name_namecol' => 'name_column',
                     #OR#
-                    'm2m_table'        =>
-                    'm2m_target_table' =>
-                    'm2m_srccol'  => #opt, if not the same as this table
-                    'm2m_dstcol'  => #opt, if not the same as target table
+                    'm2m_method'       =>
+                    #'m2m_srccol'  => #opt, if not the same as this table
+                    'm2m_dstcol'  => #required for now, eventuaully opt, if not the same as target table
 
                     'm2_label'   => 'Label',        #
                     'm2_new_default' => \@table_name_objects, #default
@@ -196,7 +195,7 @@ Example:
 %   $layer_values = &{ $f->{'layer_values_callback'} }( $cgi, $object )
 %     if $f->{'layer_values_callback'}
 %     && ! $f->{'m2name_table'}
-%     && ! $f->{'m2m_table'};
+%     && ! $f->{'m2m_method'};
 %
 %   warn "layer values: ". Dumper($layer_values)
 %     if $opt{'debug'};
@@ -277,17 +276,15 @@ Example:
 %
 %   my $fieldnum = '';
 %   my $curr_value = '';
-%   if ( $f->{'m2name_table'} || $f->{'m2m_table'} ) { #XXX test this for all
+%   if ( $f->{'m2name_table'} || $f->{'m2m_method'} ) { #XXX test this for all
 %                                                      #types of fields
 %     my($table, $col);
 %     if ( $f->{'m2name_table'} ) {
 %       $table = $f->{'m2name_table'};
 %       $col   = $f->{'m2name_namecol'};
-%     } elsif ( $f->{'m2m_table'} ) {
-%       $table = $f->{'m2m_table'}; #XXX method name
-%       $col   = $f->{'m2m_dstcol'}; #XXX or get it from schema, defualt to
-%                                    #same name as primary key of
-%                                    #m2m_target_table
+%     } elsif ( $f->{'m2m_method'} ) {
+%       $table = $f->{'m2m_method'};
+%       $col   = $f->{'m2m_dstcol'};
 %     }
 %     $fieldnum = 0;
 %     $layer_prefix_on = 1;
@@ -357,7 +354,7 @@ Example:
 
     <% include( @include ) %>
 
-%   if ( $f->{'m2name_table'} || $f->{'m2m_table'} ) {
+%   if ( $f->{'m2name_table'} || $f->{'m2m_method'} ) {
 
       <SCRIPT TYPE="text/javascript">
 
diff --git a/httemplate/edit/part_pkg.cgi b/httemplate/edit/part_pkg.cgi
index 959fb309b..bc165cf9d 100755
--- a/httemplate/edit/part_pkg.cgi
+++ b/httemplate/edit/part_pkg.cgi
@@ -58,7 +58,7 @@
                               },
                               { field    => 'freq',
                                 type     => 'part_pkg_freq',
-                                onchange => 'freq_changed', #XXX enable recurring fee
+                                onchange => 'freq_changed',
                               },
                               { field    => 'recur_fee',
                                 type     => 'money',
@@ -79,12 +79,12 @@
                               {field=>'taxproductnum', type=>'select-taxproduct' },
 
                               { type  => 'tablebreak-tr-title',
-                                value => 'Promotions', #XXX better name?
+                                value => 'Promotions', #better name?
                               },
                               { field=>'promo_code', type=>'text', size=>15 },
 
                               { type  => 'tablebreak-tr-title',
-                                value => 'Line-item revenue recogition', #XXX better name?
+                                value => 'Line-item revenue recogition', #better name?
                               },
                               { field=>'pay_weight',    type=>'text', size=>6 },
                               { field=>'credit_weight', type=>'text', size=>6 },
@@ -105,14 +105,13 @@
                             { 'type'  => 'tablebreak-tr-title',
                               'value' => 'Pricing add-ons',
                             },
-                            { 'field'    => 'bill_dst_pkgpart',
-                              'type'     => 'select-part_pkg',
-                              'm2_label' => 'Include line item(s) from package',
-                              'm2m_table'        => 'part_pkg_link',
-                              'm2m_target_table' => 'part_pkg', #XXX actually just the method name...
-                              'm2m_dstcol'       => 'dst_pkgpart',
-                              'm2m_static_or_something' => { 'link_type' => 'bill' }, #XXX
-                              'm2_error_callback' => sub { (); }, #XXX existing!
+                            { 'field'      => 'bill_dst_pkgpart',
+                              'type'       => 'select-part_pkg',
+                              'm2_label'   => 'Include line item(s) from package',
+                              'm2m_method' => 'bill_part_pkg_link',
+                              'm2m_dstcol' => 'dst_pkgpart',
+                              'm2_error_callback' =>
+                                &{$m2_error_callback_maker}('bill'),
                             },
 
                             { type  => 'tablebreak-tr-title',
@@ -120,15 +119,14 @@
                             },
                             { type => 'pkg_svc', },
 
-                            { 'field'    => 'svc_dst_pkgpart',
-                              'label'    => 'Also include services from package: ',
-                              'type'     => 'select-part_pkg',
-                              'm2_label' => 'Include services of package: ',
-                              'm2m_table'        => 'part_pkg_link',
-                              'm2m_target_table' => 'part_pkg', #XXX actually just the method name...
-                              'm2m_dstcol'       => 'dst_pkgpart',
-                              'm2m_static_or_something' => { 'link_type' => 'svc' }, #XXX
-                              'm2_error_callback' => sub { (); }, #XXX existing!
+                            { 'field'      => 'svc_dst_pkgpart',
+                              'label'      => 'Also include services from package: ',
+                              'type'       => 'select-part_pkg',
+                              'm2_label'   => 'Include services of package: ',
+                              'm2m_method' => 'svc_part_pkg_link',
+                              'm2m_dstcol' => 'dst_pkgpart',
+                              'm2_error_callback' =>
+                                &{$m2_error_callback_maker}('svc'),
                             },
 
                             { type  => 'tablebreak-tr-title',
@@ -149,10 +147,10 @@ die "access denied"
       || ( $cgi->param('pkgnum') && $curuser->access_right('Customize customer package') );
 
 #XXX
-# - part_pkg.pm bits (need separate access methods not just part_pkg_link)
 # - tr-part_pkg_freq: month_increments_only (from price plans)
 # - test editing 
-#   - write edit bits for m2ms
+#   - write edit bits for new m2ms
+# - display add-ons in (existing) edit
 # - display add-ons in browse... yeah
 # -QIS- thank goodness
 # - test cloning
@@ -192,7 +190,9 @@ my $error_callback = sub {
         }
         @options;
 
-  $cgi->param($_, $options{$_}) foreach (qw( setup_fee recur_fee ));
+  #$cgi->param($_, $options{$_}) foreach (qw( setup_fee recur_fee ));
+  $object->set($_ => scalar($cgi->param($_)) )
+    foreach (qw( setup_fee recur_fee ));
 
 };
 
@@ -201,17 +201,22 @@ my $new_hashref_callback = sub { { 'plan' => 'flat' }; };
 my $new_object_callback = sub {
   my( $cgi, $hashref, $fields, $opt ) = @_;
 
+  my $part_pkg = '';
   if ( $cgi->param('clone') ) {
     $opt->{action} = 'Custom';
-    $clone_part_pkg = qsearchs('part_pkg', { 'pkgpart' => $cgi->param('clone') } );
-    my $part_pkg = $clone_part_pkg->clone;
+    $clone_part_pkg = qsearchs('part_pkg', { pkgpart=>$cgi->param('clone') } );
+    $part_pkg = $clone_part_pkg->clone;
     $part_pkg->disabled('Y');
     %options = $clone_part_pkg->options;
-    $part_pkg;
   } else {
-    FS::part_pkg->new( $hashref );
+    $part_pkg = FS::part_pkg->new( $hashref );
   }
 
+  $part_pkg->set($_ => '0')
+    foreach (qw( setup_fee recur_fee ));
+
+  $part_pkg;
+
 };
 
 my $edit_callback = sub {
@@ -232,6 +237,9 @@ my $edit_callback = sub {
 
   %options = $object->options;
 
+  $object->set($_ => $object->option($_))
+    foreach (qw( setup_fee recur_fee ));
+
 };
 
 my $new_callback = sub {
@@ -245,6 +253,23 @@ my $new_callback = sub {
 
 };
 
+my $m2_error_callback_maker = sub {
+  my $link_type = shift; #yay closures
+  return sub {
+    my( $cgi, $object ) = @_;
+      map  {
+             new FS::part_pkg_link {
+               'link_type'   => $link_type,
+               'src_pkgpart' => $object->pkgpart,
+               'dst_pkgpart' => $_,
+             };
+           }
+      grep $_,
+      map  $cgi->param($_),
+      grep /^${link_type}_dst_pkgpart(\d+)$/, $cgi->param;
+  };
+};
+
 my $freq_changed = <<'END';
   <SCRIPT TYPE="text/javascript">
 
@@ -341,7 +366,7 @@ my $html_bottom = sub {
           ) {
             my $value = $record->getfield($href->{$field}{'select_key'});
             $html .= qq!<OPTION VALUE="$value"!.
-                     (  $options{$field} =~ /(^|, *)$value *(,|$)/ #XXX fix?
+                     (  $options{$field} =~ /(^|, *)$value *(,|$)/ #?
                           ? ' SELECTED'
                           : ''
                      ).
@@ -351,7 +376,7 @@ my $html_bottom = sub {
           foreach my $key ( keys %{ $href->{$field}{'select_options'} } ) {
             my $label = $href->{$field}{'select_options'}{$key};
             $html .= qq!<OPTION VALUE="$key"!.
-                     ( $options{$field} =~ /(^|, *)$key *(,|$)/ #XXX fix?
+                     ( $options{$field} =~ /(^|, *)$key *(,|$)/ #?
                          ? ' SELECTED'
                          : ''
                      ).
@@ -373,7 +398,7 @@ my $html_bottom = sub {
         foreach my $key ( keys %{ $href->{$field}{'options'} } ) {
           my $label = $href->{$field}{'options'}{$key};
           $html .= qq!$radio VALUE="$key"!.
-                   ( $options{$field} =~ /(^|, *)$key *(,|$)/ #XXX fix?
+                   ( $options{$field} =~ /(^|, *)$key *(,|$)/ #?
                        ? ' CHECKED'
                        : ''
                    ).
diff --git a/httemplate/edit/process/part_pkg.cgi b/httemplate/edit/process/part_pkg.cgi
index be6e2f85b..290c6f7bc 100755
--- a/httemplate/edit/process/part_pkg.cgi
+++ b/httemplate/edit/process/part_pkg.cgi
@@ -84,7 +84,8 @@ my $args_callback = sub {
         }
         @options;
 
-  $options{$_} = scalar($cgi->param($_)) for (qw( setup_fee recur_fee ));
+  $options{$_} = scalar( $cgi->param($_) )
+    for (qw( setup_fee recur_fee ));
   
   push @args, 'options' => \%options;
 
@@ -110,6 +111,8 @@ my $args_callback = sub {
 
 };
 
+#these should probably move to @args above and be processed by part_pkg.pm...
+
 $cgi->param('tax_override') =~ /^([\d,]+)$/;
 my (@tax_overrides) = (grep "$_", split (",", $1));
 
@@ -118,7 +121,21 @@ my @process_m2m = (
     'link_table'   => 'part_pkg_taxoverride',
     'target_table' => 'tax_class',
     'params'       => \@tax_overrides,
-  }
+  },
+  { 'link_table'   => 'part_pkg_link',
+    'target_table' => 'part_pkg',
+    'base_field'   => 'src_pkgpart',
+    'target_field' => 'dst_pkgpart',
+    'hashref'      => { 'link_type' => 'bill' },
+    'params'       => [ map $cgi->param($_), grep /^bill_dst_pkgpart/, $cgi->param ],
+  },
+  { 'link_table'   => 'part_pkg_link',
+    'target_table' => 'part_pkg',
+    'base_field'   => 'src_pkgpart',
+    'target_field' => 'dst_pkgpart',
+    'hashref'      => { 'link_type' => 'svc' },
+    'params'       => [ map $cgi->param($_), grep /^svc_dst_pkgpart/, $cgi->param ],
+  },
 );
 
 my $conf = new FS::Conf;
@@ -129,7 +146,6 @@ if ( $cgi->param('pkgpart') || ! $conf->exists('agent_defaultpkg') ) {
     /^(\d+)$/;
     push @agents, $1 if $1;
   }
-  warn "AGENTS: @agents";
   push @process_m2m, {
     'link_table'   => 'type_pkgs',
     'target_table' => 'agent_type',