fix one-time charges and package customization for employees who don't have 'Edit global package definition' ACL, RT#4668

4 files changed, 142 insertions, 87 deletions

diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm
index b53c333ce..b856aea10 100644
--- a/FS/FS/Record.pm
+++ b/FS/FS/Record.pm
@@ -2326,15 +2326,18 @@ sub ut_foreign_keyn {
     : '';
 }
 
-=item ut_agentnum_acl
+=item ut_agentnum_acl COLUMN [ NULL_RIGHT | NULL_RIGHT_LISTREF ]
 
 Checks this column as an agentnum, taking into account the current users's
-ACLs.
+ACLs.  NULL_RIGHT or NULL_RIGHT_LISTREF, if specified, indicates the access
+right or rights allowing no agentnum.
 
 =cut
 
 sub ut_agentnum_acl {
-  my( $self, $field, $null_acl ) = @_;
+  my( $self, $field ) = (shift, shift);
+  my $null_acl = scalar(@_) ? shift : [];
+  $null_acl = [ $null_acl ] unless ref($null_acl);
 
   my $error = $self->ut_foreign_keyn($field, 'agent', 'agentnum');
   return "Illegal agentnum: $error" if $error;
@@ -2349,7 +2352,7 @@ sub ut_agentnum_acl {
   } else {
 
     return "Access denied"
-      unless $curuser->access_right($null_acl);
+      unless grep $curuser->access_right($_), @$null_acl;
 
   }
 
diff --git a/FS/FS/part_pkg.pm b/FS/FS/part_pkg.pm
index 38fc03e42..563a5cfc5 100644
--- a/FS/FS/part_pkg.pm
+++ b/FS/FS/part_pkg.pm
@@ -432,6 +432,12 @@ sub check {
     $self->freq($1);
   }
 
+  my @null_agentnum_right = ( 'Edit global package definitions' );
+  push @null_agentnum_right, 'One-time charge'
+    if $self->freq =~ /^0/;
+  push @null_agentnum_right, 'Customize customer package'
+    if $self->disabled eq 'Y'; #good enough
+
   my $error = $self->ut_numbern('pkgpart')
     || $self->ut_text('pkg')
     || $self->ut_text('comment')
@@ -448,7 +454,7 @@ sub check {
                               'part_pkg_taxproduct',
                               'taxproductnum'
                              )
-    || $self->ut_agentnum_acl('agentnum', 'Edit global package definitions')
+    || $self->ut_agentnum_acl('agentnum', \@null_agentnum_right)
     || $self->SUPER::check
   ;
   return $error if $error;
diff --git a/httemplate/edit/elements/edit.html b/httemplate/edit/elements/edit.html
index 617855888..f4e4195de 100644
--- a/httemplate/edit/elements/edit.html
+++ b/httemplate/edit/elements/edit.html
@@ -93,32 +93,18 @@ Example:
     #agent virtualization
     'agent_virt'       => 1,
     'agent_null_right' => 'Access Right Name',
-   
-    #run when re-displaying with an error
-    'error_callback' => sub { my( $cgi, $object, $fields_listref, $opt_hashref ) = @_; },
-   
-    #run when editing
-    'edit_callback' => sub { my( $cgi, $object, $fields_listref ) = @_; },
-   
-    # returns a hashref for the new object
-    'new_hashref_callback'
 
-    # returns the new object iself (otherwise, ->new is called)
-    'new_object_callback'
-   
-    #run when adding
-    'new_callback' => sub { my( $cgi, $object, $fields_listref, $optref ) = @_; },
+    'viewall_dir' => '', #'search' or 'browse', defaults to 'search'
 
-    #run when cloning
-    'clone_callback' => sub { my( $cgi, $object, $fields_listref, $optref ) = @_; },
-   
-    #run before display to return a different value
-    'value_callback' => sub { my( $columname, $value ) = @_; },
+    # overrides default popurl(1)."process/$table.html"
+    'post_url' => popurl(1).'process/something', 
 
-    #run before display to manipulate element of the 'fields' arrayref
-    'field_callback' => sub { my( $cgi, $object, $field_hashref ) = @_; },
+    #we're in a popup (no title/menu/searchboxes)
+    'popup' => 1,
 
-    'viewall_dir' => '', #'search' or 'browse', defaults to 'search'
+    ###
+    # HTML callbacks
+    ###
 
     'html_init'   => '', #after the header/menubar
 
@@ -136,12 +122,50 @@ Example:
     #at the very bottom (well, as low as you can go from here)
     'html_foot'  => '',
 
-    # overrides default popurl(1)."process/$table.html"
-    'post_url' => popurl(1).'process/something', 
+    ###
+    # initialization callbacks
+    ###
 
-    #we're in a popup (no title/menu/searchboxes)
-    'popup' => 1,
+    ###global callbacks
+
+    #always run if provided, after decoding long CGI "redirect=" responses but
+    # before object creation/search
+    # (useful if you have a long form that might trigger redirect= and you need
+    #  to do things with $cgi params - they're not decoded in the calling
+    #  <%init> block yet)
+    'begin_callback' = sub { my( $cgi, $fields_listref, $opt_hashref ) = @_; },
+
+    #always run, after the mode-specific object creation/search
+    'end_callback' = sub { my( $cgi, $object, $fields_listref, $opt_hashref ) = @_; },
+
+    ###mode-specific callbacks
+   
+    #run when re-displaying with an error
+    'error_callback' => sub { my( $cgi, $object, $fields_listref, $opt_hashref ) = @_; },
+   
+    #run when editing
+    'edit_callback' => sub { my( $cgi, $object, $fields_listref ) = @_; },
+   
+    # returns a hashref for the new object
+    'new_hashref_callback'
+
+    # returns the new object iself (otherwise, ->new is called)
+    'new_object_callback'
    
+    #run when adding
+    'new_callback' => sub { my( $cgi, $object, $fields_listref ) = @_; },
+
+    #run when cloning
+    'clone_callback' => sub { my( $cgi, $object, $fields_listref, $opt_hashref ) = @_; },
+
+    ###display callbacks
+
+    #run before display to return a different value
+    'value_callback' => sub { my( $columname, $value ) = @_; },
+
+    #run before display to manipulate element of the 'fields' arrayref
+    'field_callback' => sub { my( $cgi, $object, $field_hashref ) = @_; },
+
   );
 
 </%doc>
@@ -563,6 +587,9 @@ if ( $cgi->param('redirect') ) {
   $cgi = new CGI($pref);
 }
 
+&{$opt{'begin_callback'}}( $cgi, $fields, \%opt )
+  if $opt{'begin_callback'};
+
 my %qsearch = (
     'table'     => $table,
     'extra_sql' => ( $opt{'agent_virt'}
@@ -639,6 +666,9 @@ if ( $cgi->param('error') ) {
 
 }
 
+&{$opt{'end_callback'}}( $cgi, $object, $fields, \%opt )
+  if $opt{'end_callback'};
+
 $opt{action} ||= $object->$pkey() ? 'Edit' : 'Add';
 
 my $title = $opt{action}. ' '. $opt{name};
diff --git a/httemplate/edit/part_pkg.cgi b/httemplate/edit/part_pkg.cgi
index 6de0b3211..4b519f5fa 100755
--- a/httemplate/edit/part_pkg.cgi
+++ b/httemplate/edit/part_pkg.cgi
@@ -7,6 +7,9 @@
               'html_init'   => include('/elements/init_overlib.html').
                                $freq_changed,
               'html_bottom' => $html_bottom,
+
+              'begin_callback'       => $begin_callback,
+              'end_callback'         => $end_callback,
               'new_hashref_callback' => $new_hashref_callback,
               'new_object_callback'  => $new_object_callback,
               'new_callback'         => $new_callback,
@@ -55,7 +58,7 @@
                               },
                               {field=>'comment',  type=>'text', size=>40 }, #32
                               {field=>'classnum', type=>'select-pkg_class' },
-                              {field=>'disabled', type=>'checkbox', value=>'Y'},
+                              {field=>'disabled', type=>$disabled_type, value=>'Y'},
 
                               { type  => 'tablebreak-tr-title',
                                 value => 'Pricing', #better name?
@@ -172,10 +175,19 @@
 
 my $curuser = $FS::CurrentUser::CurrentUser;
 
-die "access denied"
-  unless $curuser->access_right('Edit package definitions')
-      || $curuser->access_right('Edit global package definitions')
-      || ( $cgi->param('pkgnum') && $curuser->access_right('Customize customer package') );
+my $edit_right = $curuser->access_right('Edit package definitions')
+              || $curuser->access_right('Edit global package definitions');
+
+my $begin_callback = sub {
+  my( $cgi, $fields, $opt ) = @_;
+  die "access denied"
+    unless $edit_right
+        || ( $cgi->param('pkgnum')
+             && $curuser->access_right('Customize customer package')
+           );
+};
+
+my $disabled_type = $edit_right ? 'checkbox' : 'hidden';
 
 my $conf = new FS::Conf;
 my $taxproducts = $conf->exists('enable_taxproducts');
@@ -197,38 +209,25 @@ my %tax_override = ();
 my %taxproductnums = map { ($_->classnum => 1) }
                      qsearch('usage_class', { 'disabled' => '' });
 
-if ( $cgi->param('error') ) {  # oh well
-  foreach ($cgi->param) {
-    /^usage_taxproductnum_(\d+)$/ && ($taxproductnums{$1} = 1);
-  }
-} elsif ( my $pkgpart = $cgi->keywords || $cgi->param('pkgpart') ) {
-  $pkgpart =~ /^(\d+)$/ or die "illegal pkgpart";
-  my $part_pkg = qsearchs( 'part_pkg', { pkgpart => $pkgpart } );
-  die "no part_pkg for pkgpart $pkgpart" unless $pkgpart;
-  foreach ($part_pkg->options) {
-    /^usage_taxproductnum_(\d+)$/ && ($taxproductnums{$1} = 1);
-  }
-  foreach ($part_pkg->part_pkg_taxoverride) {
-    $taxproductnums{$_->usage_class} = 1
-      if $_->usage_class;
-  }
-} else {
-  # do nothing
-}
-my @taxproductnums = ( qw( setup recur ), sort (keys %taxproductnums) );
-
 my %options = ();
 my $recur_disabled = 1;
+
 my $error_callback = sub {
   my($cgi, $object, $fields, $opt ) = @_;
+
   (@agent_type) = $cgi->param('agent_type');
-  $tax_override{''} = $cgi->param('tax_override');
-  $tax_override{$_} = $cgi->param('tax_override_$_')
-    foreach(grep { /^tax_override_(\w+)$/ } $cgi->param);
+
   $opt->{action} = 'Custom' if $cgi->param('clone');
 
   $recur_disabled = $cgi->param('freq') ? 0 : 1;
 
+  foreach ($cgi->param) {
+    /^usage_taxproductnum_(\d+)$/ && ($taxproductnums{$1} = 1);
+  }
+  $tax_override{''} = $cgi->param('tax_override');
+  $tax_override{$_} = $cgi->param('tax_override_$_')
+    foreach(grep { /^tax_override_(\w+)$/ } $cgi->param);
+
   #some false laziness w/process
   $cgi->param('plan') =~ /^(\w+)$/ or die 'unparsable plan';
   my $plan = $1;
@@ -267,19 +266,14 @@ my $edit_callback = sub {
   $recur_disabled = $object->freq ? 0 : 1;
 
   (@agent_type) = map {$_->typenum} qsearch('type_pkgs',{'pkgpart'=>$1});
-  $tax_override{$_} =
-    join (",", map {$_->taxclassnum}
-               qsearch( 'part_pkg_taxoverride', { 'pkgpart' => $object->pkgpart,
-                                                  'usage_class' => $_,
-                                                }
-                      )
-         )
-    foreach ( '', @taxproductnums );
-
-#    join (",", map {$_->taxclassnum}
-#               $part_pkg->part_pkg_taxrate( 'cch', $conf->config('defaultloc')
-#         );
-#      unless $tax_override;
+
+  foreach ($object->options) {
+    /^usage_taxproductnum_(\d+)$/ && ($taxproductnums{$1} = 1);
+  }
+  foreach ($object->part_pkg_taxoverride) {
+    $taxproductnums{$_->usage_class} = 1
+      if $_->usage_class;
+  }
 
   %options = $object->options;
 
@@ -512,21 +506,43 @@ my %usage_class = map { ($_->classnum => $_->classname) }
 $usage_class{setup} = 'Setup';
 $usage_class{recur} = 'Recurring';
 
-my %taxproduct_fields = map { $_ => [ "taxproductnum_$_", 
-                                      { type  => 'select-taxproduct',
-                                        #label => "$usage_class{$_} tax product",
-                                      },
-                                      "tax_override_$_", 
-                                      { type  => 'select-taxoverride' }
-                                    ]
-                            }
-                         @taxproductnums;
-$taxproduct_fields{'(default)'} =
-  [ 'taxproductnum', { type => 'select-taxproduct',
-                       #label => 'Default tax product',
-                     },
-    'tax_override',  { type => 'select-taxoverride' },
-  ];
+my @taxproductnums = ();
+my %taxproduct_fields = ();
+my $end_callback = sub {
+  my( $cgi, $object, $fields, $opt ) = @_;
+
+  @taxproductnums = ( qw( setup recur ), sort (keys %taxproductnums) );
+
+  if ( $object->pkgpart ) {
+    foreach my $usage_class ( '', @taxproductnums ) {
+      $tax_override{$usage_class} =
+        join (",", map $_->taxclassnum,
+                       qsearch( 'part_pkg_taxoverride', {
+                                  'pkgpart'     => $object->pkgpart,
+                                  'usage_class' => $usage_class,
+                              })
+             );
+    }
+  }
+
+  %taxproduct_fields =
+    map { $_ => [ "taxproductnum_$_", 
+                  { type  => 'select-taxproduct',
+                    #label => "$usage_class{$_} tax product",
+                  },
+                  "tax_override_$_", 
+                  { type  => 'select-taxoverride' }
+                ]
+        }
+        @taxproductnums;
+
+  $taxproduct_fields{'(default)'} =
+    [ 'taxproductnum', { type => 'select-taxproduct',
+                         #label => 'Default tax product',
+                       },
+      'tax_override',  { type => 'select-taxoverride' },
+    ];
+};
 
 my $taxproduct_values = sub {
   my ($cgi, $object, $flags) = @_;