NEW Add FieldsValidator to ensure fields get validated

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;
+    }
+}

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.

tests/behat/src/CmsUiContext.php

@@ -6,6 +6,7 @@
 use Behat\Behat\Hook\Scope\AfterStepScope;
 use Behat\Mink\Element\Element;
 use Behat\Mink\Element\NodeElement;
+use Behat\Mink\Exception\ElementNotFoundException;
 use Behat\Mink\Selector\Xpath\Escaper;
 use Behat\Mink\Session;
 use PHPUnit\Framework\Assert;
@@ -86,26 +87,37 @@ public function iShouldSeeANotice($notice)
     }
 
     /**
-     * @Then /^I should see a "(.+)" (\w+) toast$/
+     * @Then /^I should (not |)see a "(.+)" (\w+) toast$/
      */
-    public function iShouldSeeAToast($notice, $type)
+    public function iShouldSeeAToast($not, $notice, $type)
     {
-        $this->getMainContext()->assertElementContains('.toast--' . $type, $notice);
+        if ($not) {
+            try {
+                // If there is a toast of that type, ensure it doesn't contain the notice.
+                $this->getMainContext()->assertElementNotContains('.toast--' . $type, $notice);
+            } catch (ElementNotFoundException $e) {
+                // no-op - if the element doesn't exist at all, then that passes the test.
+            }
+        } else {
+            $this->getMainContext()->assertElementContains('.toast--' . $type, $notice);
+        }
     }
 
     /**
-     * @Then /^I should see a "(.+)" (\w+) toast with these actions: (.+)$/
+     * @Then /^I should (not |)see a "(.+)" (\w+) toast with these actions: (.+)$/
      */
-    public function iShouldSeeAToastWithAction($notice, $type, $actions)
+    public function iShouldSeeAToastWithAction($not, $notice, $type, $actions)
     {
-        $this->iShouldSeeAToast($notice, $type);
+        $this->iShouldSeeAToast($not, $notice, $type);
 
-        $actions = explode(',', $actions ?? '');
-        foreach ($actions as $order => $action) {
-            $this->getMainContext()->assertElementContains(
-                sprintf('.toast--%s .toast__action:nth-child(%s)', $type, $order+1),
-                trim($action ?? '')
-            );
+        if (!$not) {
+            $actions = explode(',', $actions ?? '');
+            foreach ($actions as $order => $action) {
+                $this->getMainContext()->assertElementContains(
+                    sprintf('.toast--%s .toast__action:nth-child(%s)', $type, $order+1),
+                    trim($action ?? '')
+                );
+            }
         }
     }
 

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);
+        }
+    }
+}

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();