RT#34289: Flag service fields as mandatory

6 files changed, 63 insertions, 5 deletions

diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm
index 5dcf3c3c2..3a27b741b 100644
--- a/FS/FS/Schema.pm
+++ b/FS/FS/Schema.pm
@@ -2447,6 +2447,7 @@ sub tables_hashref {
         'columnlabel', 'varchar', 'NULL', $char_d, '', '',
         'columnvalue', 'varchar', 'NULL',     512, '', '', 
         'columnflag',  'char',    'NULL',       1, '', '', 
+        'required',    'char',    'NULL',       1, '', '', 
       ],
       'primary_key' => 'columnnum',
       'unique' => [ [ 'svcpart', 'columnname' ] ],
diff --git a/FS/FS/part_svc.pm b/FS/FS/part_svc.pm
index c2003c4aa..abeb28bd2 100644
--- a/FS/FS/part_svc.pm
+++ b/FS/FS/part_svc.pm
@@ -96,8 +96,12 @@ the part_svc_column table appropriately (see L<FS::part_svc_column>).
 
 =item I<svcdb>__I<field> - Default or fixed value for I<field> in I<svcdb>.
 
+=item I<svcdb>__I<field>_label
+
 =item I<svcdb>__I<field>_flag - defines I<svcdb>__I<field> action: null or empty (no default), `D' for default, `F' for fixed (unchangeable), , `S' for selectable choice, `M' for manual selection from inventory, or `A' for automatic selection from inventory.  For virtual fields, can also be 'X' for excluded.
 
+=item I<svcdb>__I<field>_required - I<field> should always have a true value
+
 =back
 
 If you want to add part_svc_column records for fields that do not exist as
@@ -146,6 +150,7 @@ sub insert {
   foreach my $field (
     grep { $_ ne 'svcnum'
            && ( defined( $self->getfield($svcdb.'__'.$_.'_flag') )
+                || defined($self->getfield($svcdb.'__'.$_.'_required'))
                 || $self->getfield($svcdb.'__'.$_.'_label') !~ /^\s*$/ )
          } (fields($svcdb), @fields)
   ) {
@@ -157,6 +162,7 @@ sub insert {
 
     my $flag  = $self->getfield($svcdb.'__'.$field.'_flag');
     my $label = $self->getfield($svcdb.'__'.$field.'_label');
+    my $required = $self->getfield($svcdb.'__'.$field.'_required') ? 'Y' : '';
     if ( uc($flag) =~ /^([A-Z])$/ || $label !~ /^\s*$/ ) {
 
       if ( uc($flag) =~ /^([A-Z])$/ ) {
@@ -171,6 +177,8 @@ sub insert {
       $part_svc_column->setfield('columnlabel', $label)
         if $label !~ /^\s*$/;
 
+      $part_svc_column->setfield('required', $required);
+
       if ( $previous ) {
         $error = $part_svc_column->replace($previous);
       } else {
@@ -280,6 +288,7 @@ sub replace {
     foreach my $field (
       grep { $_ ne 'svcnum'
              && ( defined( $new->getfield($svcdb.'__'.$_.'_flag') )
+                  || defined($new->getfield($svcdb.'__'.$_.'_required'))
                   || $new->getfield($svcdb.'__'.$_.'_label') !~ /^\s*$/ )
            } (fields($svcdb),@fields)
     ) {
@@ -292,6 +301,7 @@ sub replace {
 
       my $flag  = $new->getfield($svcdb.'__'.$field.'_flag');
       my $label = $new->getfield($svcdb.'__'.$field.'_label');
+      my $required = $new->getfield($svcdb.'__'.$field.'_required') ? 'Y' : '';
  
       if ( uc($flag) =~ /^([A-Z])$/ || $label !~ /^\s*$/ ) {
 
@@ -310,6 +320,8 @@ sub replace {
         $part_svc_column->setfield('columnlabel', $label)
           if $label !~ /^\s*$/;
 
+        $part_svc_column->setfield('required', $required);
+
         if ( $previous ) {
           $error = $part_svc_column->replace($previous);
         } else {
@@ -700,6 +712,8 @@ some components specified by "select-.*.html", and a bunch more...
 
 =item select_allow_empty - Used with select_table, adds an empty option
 
+=item required - This field should always have a true value (do not use with type checkbox or disabled)
+
 =back
 
 =cut
@@ -777,7 +791,7 @@ sub process {
                          and ref($param->{ $f }) ) {
                       $param->{ $f } = join(',', @{ $param->{ $f } });
 		    }
-                    ( $f, $f.'_flag', $f.'_label' );
+                    ( $f, $f.'_flag', $f.'_label', $f.'_required' );
                   }
                   @fields;
 
diff --git a/FS/FS/part_svc_column.pm b/FS/FS/part_svc_column.pm
index 38ce1fa80..75a2dfb1a 100644
--- a/FS/FS/part_svc_column.pm
+++ b/FS/FS/part_svc_column.pm
@@ -45,6 +45,8 @@ fields are currently supported:
 
 =item columnflag - null or empty (no default), `D' for default, `F' for fixed (unchangeable), `S' for selectable choice, `M' for manual selection from inventory, `A' for automatic selection from inventory, or `H' for selection from a hardware class.  For virtual fields, can also be 'X' for excluded.
 
+=item required - column value expected to be true
+
 =back
 
 =head1 METHODS
@@ -91,6 +93,7 @@ sub check {
     || $self->ut_alpha('columnname')
     || $self->ut_textn('columnlabel')
     || $self->ut_anything('columnvalue')
+    || $self->ut_flag('required')
   ;
   return $error if $error;
 
diff --git a/FS/FS/svc_Common.pm b/FS/FS/svc_Common.pm
index 61a5f6aa8..34ae2195e 100644
--- a/FS/FS/svc_Common.pm
+++ b/FS/FS/svc_Common.pm
@@ -155,13 +155,46 @@ sub cust_linked {
 
 Checks the validity of fields in this record.
 
-At present, this does nothing but call FS::Record::check (which, in turn, 
-does nothing but run virtual field checks).
+Only checks fields marked as required in table_info or 
+part_svc_column definition.  Should be invoked by service-specific
+check using SUPER.  Invokes FS::Record::check using SUPER.
 
 =cut
 
 sub check {
   my $self = shift;
+
+  ## Checking required fields
+
+  # get fields marked as required in table_info
+  my $required = {};
+  my $labels = {};
+  my $tinfo = $self->can('table_info') ? $self->table_info : {};
+  my $fields = $tinfo->{'fields'} || {};
+  foreach my $field (keys %$fields) {
+    if (ref($fields->{$field}) && $fields->{$field}->{'required'}) {
+      $required->{$field} = 1;
+      $labels->{$field} = $fields->{$field}->{'label'};
+    }
+  }
+  # add fields marked as required in database
+  foreach my $column (
+    qsearch('part_svc_column',{
+      'svcpart' => $self->svcpart,
+      'required' => 'Y'
+    })
+  ) {
+    $required->{$column->columnname} = 1;
+    $labels->{$column->columnname} = $column->columnlabel;
+  }
+  # do the actual checking
+  foreach my $field (keys %$required) {
+    unless ($self->$field) {
+      my $name = $labels->{$field} || $field;
+      return "Field $name is required\n"
+    }
+  }
+
   $self->SUPER::check;
 }
 
diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm
index 93e3f2c6a..ffcadc9cb 100644
--- a/FS/FS/svc_acct.pm
+++ b/FS/FS/svc_acct.pm
@@ -284,6 +284,7 @@ sub table_info {
                          disable_default => 1,
                          disable_fixed => 1,
                          disable_select => 1,
+                         required => 1,
                        },
         'password_selfchange' => { label => 'Password modification',
                                    type  => 'checkbox',
@@ -311,7 +312,9 @@ sub table_info {
                          type => 'text',
                          disable_inventory => 1,
                        },
-        '_password' => 'Password',
+        '_password' => { label => 'Password',
+                         required => 1
+                       },
         'gid'       => {
                          label    => 'GID',
 		         def_info => 'when blank, defaults to UID',
@@ -334,6 +337,7 @@ sub table_info {
                          select_key   => 'svcnum',
                          select_label => 'domain',
                          disable_inventory => 1,
+                         required => 1,
                        },
         'pbxsvc'    => { label => 'PBX',
                          type  => 'select-svc_pbx.html',
diff --git a/FS/FS/svc_domain.pm b/FS/FS/svc_domain.pm
index b01d67310..78556cf8b 100644
--- a/FS/FS/svc_domain.pm
+++ b/FS/FS/svc_domain.pm
@@ -134,7 +134,10 @@ sub table_info {
     'display_weight' => 20,
     'cancel_weight'  => 60,
     'fields' => {
-      'domain' => 'Domain',
+      'domain' => {
+                  label => 'Domain',
+                  required => 1,
+                },
       'parent_svcnum' => { 
                          label => 'Parent domain / Communigate administrator domain',
                          type  => 'select',