NEW Add FieldsValidator to ensure fields get validated

silverstripe · Aug 8, 2023 · 3bfe760 · 3bfe760
1 parent 2a56cc3
commit 3bfe760
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 5 deletions.
diff --git a/src/Forms/FieldsValidator.php b/src/Forms/FieldsValidator.php
@@ -0,0 +1,26 @@
+<?php
+
+namespace SilverStripe\Forms;
+
+/**
+ * Validates the internal state of all fields in the form.
+ */
+class FieldsValidator extends Validator
+{
+    public function php($data): bool
+    {
+        $valid = true;
+        $fields = $this->form->Fields();
+
+        foreach ($fields as $field) {
+            $valid = ($field->validate($this) && $valid);
+        }
+
+        return $valid;
+    }
+
+    public function canBeCached(): bool
+    {
+        return true;
+    }
+}
diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php
@@ -16,6 +16,7 @@
 use SilverStripe\Forms\FormField;
 use SilverStripe\Forms\FormScaffolder;
 use SilverStripe\Forms\CompositeValidator;
+use SilverStripe\Forms\FieldsValidator;
 use SilverStripe\Forms\HiddenField;
 use SilverStripe\i18n\i18n;
 use SilverStripe\i18n\i18nEntityProvider;
@@ -2600,7 +2601,7 @@ public function getCMSActions()
      */
     public function getCMSCompositeValidator(): CompositeValidator
     {
-        $compositeValidator = CompositeValidator::create();
+        $compositeValidator = CompositeValidator::create([FieldsValidator::create()]);
 
         // Support for the old method during the deprecation period
         if ($this->hasMethod('getCMSValidator')) {
@@ -3735,7 +3736,7 @@ private function getDatabaseBackedField(string $fieldPath): ?string
                 return null;
             }
             $fieldParts[] = $nextPart;
-            
+
             if ($component instanceof Relation || $component instanceof DataList) {
                 if ($component->hasMethod($nextPart)) {
                     // If the next part is a method, we don't have a database-backed field.

diff --git a/tests/php/Forms/FieldsValidatorTest.php b/tests/php/Forms/FieldsValidatorTest.php
@@ -0,0 +1,79 @@
+<?php
+
+namespace SilverStripe\Forms\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Forms\EmailField;
+use SilverStripe\Forms\FieldList;
+use SilverStripe\Forms\FieldsValidator;
+use SilverStripe\Forms\Form;
+
+class FieldsValidatorTest extends SapphireTest
+{
+    protected $usesDatabase = false;
+
+    public function provideValidation()
+    {
+        return [
+            'missing values arent invalid' => [
+                'values' => [],
+                'isValid' => true,
+            ],
+            'empty values arent invalid' => [
+                'values' => [
+                    'EmailField1' => '',
+                    'EmailField2' => null,
+                ],
+                'isValid' => true,
+            ],
+            'any invalid is invalid' => [
+                'values' => [
+                    'EmailField1' => '[email protected]',
+                    'EmailField2' => 'not email',
+                ],
+                'isValid' => false,
+            ],
+            'all invalid is invalid' => [
+                'values' => [
+                    'EmailField1' => 'not email',
+                    'EmailField2' => 'not email',
+                ],
+                'isValid' => false,
+            ],
+            'all valid is valid' => [
+                'values' => [
+                    'EmailField1' => '[email protected]',
+                    'EmailField2' => '[email protected]',
+                ],
+                'isValid' => true,
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideValidation
+     */
+    public function testValidation(array $values, bool $isValid)
+    {
+        $fieldList = new FieldList([
+            $field1 = new EmailField('EmailField1'),
+            $field2 = new EmailField('EmailField2'),
+        ]);
+        if (array_key_exists('EmailField1', $values)) {
+            $field1->setValue($values['EmailField1']);
+        }
+        if (array_key_exists('EmailField2', $values)) {
+            $field2->setValue($values['EmailField2']);
+        }
+        $form = new Form(null, 'testForm', $fieldList, new FieldList([/* no actions */]), new FieldsValidator());
+
+        $result = $form->validationResult();
+        $this->assertSame($isValid, $result->isValid());
+        $messages = $result->getMessages();
+        if ($isValid) {
+            $this->assertEmpty($messages);
+        } else {
+            $this->assertNotEmpty($messages);
+        }
+    }
+}
diff --git a/tests/php/Security/GroupTest.php b/tests/php/Security/GroupTest.php
@@ -290,10 +290,11 @@ public function testGroupTitleValidation()
 
         $newGroup = new Group();
 
-        $validators = $newGroup->getCMSCompositeValidator()->getValidators();
+        $validators = $newGroup->getCMSCompositeValidator()->getValidatorsByType(RequiredFields::class);
         $this->assertCount(1, $validators);
-        $this->assertInstanceOf(RequiredFields::class, $validators[0]);
-        $this->assertTrue(in_array('Title', $validators[0]->getRequired() ?? []));
+        $validator = array_shift($validators);
+        $this->assertInstanceOf(RequiredFields::class, $validator);
+        $this->assertTrue(in_array('Title', $validator->getRequired() ?? []));
 
         $newGroup->Title = $group1->Title;
         $result = $newGroup->validate();