Merge pull request #1345 from creative-commoners/pulls/7/form-field-v…

…alid ENH Use FieldValidator for FormFields
silverstripe · Dec 2, 2024 · 0020f7d · 0020f7d
2 parents 781d65a + 2b9c6cc
commit 0020f7d
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 38 deletions.
diff --git a/code/Form/UserFormsRequiredFields.php b/code/Form/UserFormsRequiredFields.php
@@ -36,7 +36,9 @@ public function php($data)
         $fields = $this->form->Fields();
 
         foreach ($fields as $field) {
-            $valid = ($field->validate($this) && $valid);
+            $result = $field->validate();
+            $valid = $result->isValid() && $valid;
+            $this->result->combineAnd($result);
         }
 
         if (empty($this->required)) {

diff --git a/code/FormField/UserFormsCheckboxSetField.php b/code/FormField/UserFormsCheckboxSetField.php
@@ -2,8 +2,8 @@
 
 namespace SilverStripe\UserForms\FormField;
 
+use SilverStripe\Core\Validation\ValidationResult;
 use SilverStripe\Forms\CheckboxSetField;
-use SilverStripe\UserForms\Model\EditableFormField;
 
 /**
  * @package userforms
@@ -45,33 +45,16 @@ public function getOptions()
         return $options;
     }
 
-    /**
-     * @inheritdoc
-     *
-     * @param Validator $validator
-     *
-     * @return bool
-     */
-    public function validate($validator)
+    public function getValueForValidation(): mixed
     {
-        // get the previous values (could contain comma-delimited list)
-
-        $previous = $value = $this->Value();
-
-        if (is_string($value) && strstr($value ?? '', ",")) {
-            $value = explode(",", $value ?? '');
+        $value = $this->Value();
+        if (is_iterable($value) || is_null($value)) {
+            return $value;
         }
-
-        // set the value as an array for parent validation
-
-        $this->setValue($value);
-
-        $validated = parent::validate($validator);
-
-        // restore previous value after validation
-
-        $this->setValue($previous);
-
-        return $validated;
+        // Value may contain a comma-delimited list of values
+        if (is_string($value) && strstr($value, ',')) {
+            return explode(',', $value);
+        }
+        return [$value];
     }
 }
diff --git a/tests/php/Control/UserDefinedFormControllerTest.php b/tests/php/Control/UserDefinedFormControllerTest.php
@@ -155,7 +155,7 @@ public function testValidation()
             'required-email' => 'invalid',
             'required-text' => 'bob'
         ]);
-        $this->assertStringContainsString('Please enter an email address', $response->getBody());
+        $this->assertStringContainsString('Invalid email address', $response->getBody());
 
         // Post with only required
         $this->get($form->URLSegment);

diff --git a/tests/php/FormField/UserFormsCheckboxSetFieldTest.php b/tests/php/FormField/UserFormsCheckboxSetFieldTest.php
@@ -3,7 +3,6 @@
 namespace SilverStripe\UserForms\Tests\FormField;
 
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\UserForms\Form\UserFormsRequiredFields;
 use SilverStripe\UserForms\FormField\UserFormsCheckboxSetField;
 use SilverStripe\UserForms\Model\EditableFormField\EditableCheckboxGroupField;
 
@@ -14,27 +13,26 @@ class UserFormsCheckboxSetFieldTest extends SapphireTest
     public function testValidate()
     {
         $field = new UserFormsCheckboxSetField('Field', 'My field', ['One' => 'One', 'Two' => 'Two']);
-        $validator = new UserFormsRequiredFields();
 
         // String values
         $field->setValue('One');
-        $this->assertTrue($field->validate($validator));
+        $this->assertTrue($field->validate()->isValid());
         $field->setValue('One,Two');
-        $this->assertTrue($field->validate($validator));
+        $this->assertTrue($field->validate()->isValid());
         $field->setValue('Three,Four');
-        $this->assertFalse($field->validate($validator));
+        $this->assertFalse($field->validate()->isValid());
 
         // Array values
         $field->setValue(array('One'));
-        $this->assertTrue($field->validate($validator));
+        $this->assertTrue($field->validate()->isValid());
         $field->setValue(array('One', 'Two'));
-        $this->assertTrue($field->validate($validator));
+        $this->assertTrue($field->validate()->isValid());
 
         // Invalid
         $field->setValue('Three');
-        $this->assertFalse($field->validate($validator));
+        $this->assertFalse($field->validate()->isValid());
         $field->setValue(array('Three', 'Four'));
-        $this->assertFalse($field->validate($validator));
+        $this->assertFalse($field->validate()->isValid());
     }
 
     public function testCustomErrorMessageValidationAttributesHTML()