Merge pull request #909 from greezybacon/issue/new-form-validation

forms: Don't save on validation errors

Reviewed-By: Peter Rotich <peter@osticket.com>

include/staff/dynamic-form.inc.php

@@ -4,21 +4,24 @@ $info=array();
 if($form && $_REQUEST['a']!='add') {
     $title = 'Update custom form section';
     $action = 'update';
+    $url = "?id=".urlencode($_REQUEST['id']);
     $submit_text='Save Changes';
     $info = $form->ht;
     $newcount=2;
 } else {
     $title = 'Add new custom form section';
     $action = 'add';
+    $url = '?a=add';
     $submit_text='Add Form';
     $newcount=4;
 }
 $info=Format::htmlchars(($errors && $_POST)?$_POST:$info);
 
 ?>
-<form action="?id=<?php echo urlencode($_REQUEST['id']); ?>" method="post" id="save">
+<form action="<?php echo $url ?>" method="post" id="save">
     <?php csrf_token(); ?>
     <input type="hidden" name="do" value="<?php echo $action; ?>">
+    <input type="hidden" name="a" value="<?php echo $action; ?>">
     <input type="hidden" name="id" value="<?php echo $info['id']; ?>">
     <h2>Custom Form</h2>
     <table class="form_table" width="940" border="0" cellspacing="0" cellpadding="2">

scp/forms.php

@@ -10,6 +10,7 @@ if($_POST) {
     $fields = array('title', 'notes', 'instructions');
     $required = array('title');
     $max_sort = 0;
+    $form_fields = array();
     switch(strtolower($_POST['do'])) {
         case 'update':
             foreach ($fields as $f)
@@ -48,7 +49,7 @@ if($_POST) {
                 if ($field->get('name'))
                     $names[] = $field->get('name');
                 if ($field->isValid())
-                    $field->save();
+                    $form_fields[] = $field;
                 else
                     # notrans (not shown)
                     $errors["field-$id"] = 'Field has validation errors';
@@ -57,11 +58,14 @@ if($_POST) {
             }
             break;
         case 'add':
-            $form = DynamicForm::create(array(
-                'title'=>$_POST['title'],
-                'instructions'=>$_POST['instructions'],
-                'notes'=>$_POST['notes']));
-            $form->save(true);
+            $form = DynamicForm::create();
+            foreach ($fields as $f) {
+                if (in_array($f, $required) && !$_POST[$f])
+                    $errors[$f] = sprintf('%s is required',
+                        mb_convert_case($f, MB_CASE_TITLE));
+                elseif (isset($_POST[$f]))
+                    $form->set($f, $_POST[$f]);
+            }
             break;
 
         case 'mass_process':
@@ -93,7 +97,6 @@ if($_POST) {
             if (!$_POST["label-new-$i"])
                 continue;
             $field = DynamicFormField::create(array(
-                'form_id'=>$form->get('id'),
                 'sort'=>$_POST["sort-new-$i"] ? $_POST["sort-new-$i"] : ++$max_sort,
                 'label'=>$_POST["label-new-$i"],
                 'type'=>$_POST["type-new-$i"],
@@ -103,13 +106,19 @@ if($_POST) {
             ));
             $field->setForm($form);
             if ($field->isValid())
-                $field->save();
+                $form_fields[] = $field;
             else
                 $errors["new-$i"] = $field->errors();
         }
         // XXX: Move to an instrumented list that can handle this better
-        if (!$errors)
+        if (!$errors) {
             $form->_dfields = $form->_fields = null;
+            $form->save(true);
+            foreach ($form_fields as $field) {
+                $field->set('form_id', $form->get('id'));
+                $field->save();
+            }
+        }
     }
     if ($errors)
         $errors['err'] = 'Unable to commit form. Check validation errors';