diff --git a/src/Forms/FieldsValidator.php b/src/Forms/FieldsValidator.php deleted file mode 100644 index 9a49e1aa2b7..00000000000 --- a/src/Forms/FieldsValidator.php +++ /dev/null @@ -1,38 +0,0 @@ -form->Fields(); - foreach ($fields as $field) { - $this->result->combineAnd($field->validate()); - } - return $this->result->isValid(); - } - - public function canBeCached(): bool - { - return true; - } -} diff --git a/src/Forms/Form.php b/src/Forms/Form.php index e2c4ed8751e..aa74dad0dfa 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -299,8 +299,10 @@ public function __construct( $this->setName($name); // Form validation - $this->validator = ($validator) ? $validator : new RequiredFieldsValidator(); - $this->validator->setForm($this); + if ($validator) { + $this->validator = $validator; + $this->validator->setForm($this); + } // Form error controls $this->restoreFormState(); @@ -1264,17 +1266,23 @@ public function getLegend() */ public function validationResult() { + $result = ValidationResult::create(); // Automatically pass if there is no validator, or the clicked button is exempt // Note: Soft support here for validation with absent request handler $handler = $this->getRequestHandler(); $action = $handler ? $handler->buttonClicked() : null; - $validator = $this->getValidator(); - if (!$validator || $this->actionIsValidationExempt($action)) { - return ValidationResult::create(); + if ($this->actionIsValidationExempt($action)) { + return $result; + } + // Invoke FormField validation + foreach ($this->Fields() as $field) { + $result->combineAnd($field->validate()); } - // Invoke validator - $result = $validator->validate(); + $validator = $this->getValidator(); + if ($validator) { + $result->combineAnd($validator->validate()); + } $this->loadMessagesFrom($result); return $result; } diff --git a/src/Forms/GridField/GridFieldDetailForm.php b/src/Forms/GridField/GridFieldDetailForm.php index cff6eaddfd4..d80961e16dd 100644 --- a/src/Forms/GridField/GridFieldDetailForm.php +++ b/src/Forms/GridField/GridFieldDetailForm.php @@ -14,7 +14,6 @@ use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\FieldList; -use SilverStripe\Forms\FieldsValidator; use SilverStripe\Forms\Validation\Validator; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; @@ -147,10 +146,8 @@ public function handleItem($gridField, $request) if (!$this->getValidator()) { if ($record->hasMethod('getCMSCompositeValidator')) { $validator = $record->getCMSCompositeValidator(); - } else { - $validator = FieldsValidator::create(); + $this->setValidator($validator); } - $this->setValidator($validator); } return $handler->handleRequest($request); diff --git a/src/Forms/Validation/RequiredFieldsValidator.php b/src/Forms/Validation/RequiredFieldsValidator.php index 0ff690f3dbe..aeadf8974b9 100644 --- a/src/Forms/Validation/RequiredFieldsValidator.php +++ b/src/Forms/Validation/RequiredFieldsValidator.php @@ -116,12 +116,6 @@ public function php($data) $valid = true; $fields = $this->form->Fields(); - foreach ($fields as $field) { - $result = $field->validate(); - $valid = $result->isValid() && $valid; - $this->result->combineAnd($result); - } - if (!$this->required) { return $valid; } diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index f0d735e51e3..8cbf4f9575d 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -19,7 +19,6 @@ use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormScaffolder; use SilverStripe\Forms\Validation\CompositeValidator; -use SilverStripe\Forms\FieldsValidator; use SilverStripe\Forms\GridField\GridField; use SilverStripe\Forms\GridField\GridFieldConfig_RelationEditor; use SilverStripe\Forms\HiddenField; @@ -2686,7 +2685,7 @@ public function getCMSActions() */ public function getCMSCompositeValidator(): CompositeValidator { - $compositeValidator = CompositeValidator::create([FieldsValidator::create()]); + $compositeValidator = CompositeValidator::create(); // Support for the old method during the deprecation period if ($this->hasMethod('getCMSValidator')) { diff --git a/tests/php/Forms/EmailFieldTest.php b/tests/php/Forms/EmailFieldTest.php index d46ceae1214..9c5064812dd 100644 --- a/tests/php/Forms/EmailFieldTest.php +++ b/tests/php/Forms/EmailFieldTest.php @@ -4,7 +4,6 @@ use SilverStripe\Dev\FunctionalTest; use SilverStripe\Forms\EmailField; -use SilverStripe\Forms\FieldsValidator; class EmailFieldTest extends FunctionalTest { diff --git a/tests/php/Forms/FieldsValidatorTest.php b/tests/php/Forms/FieldsValidatorTest.php deleted file mode 100644 index d5eac24d9c4..00000000000 --- a/tests/php/Forms/FieldsValidatorTest.php +++ /dev/null @@ -1,78 +0,0 @@ - [ - 'values' => [], - 'isValid' => true, - ], - 'empty values arent invalid' => [ - 'values' => [ - 'EmailField1' => '', - 'EmailField2' => null, - ], - 'isValid' => true, - ], - 'any invalid is invalid' => [ - 'values' => [ - 'EmailField1' => 'email@example.com', - '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@example.com', - 'EmailField2' => 'email@example.com', - ], - '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/Forms/FormFieldTest.php b/tests/php/Forms/FormFieldTest.php index ba8438c3ffc..4ccc0039400 100644 --- a/tests/php/Forms/FormFieldTest.php +++ b/tests/php/Forms/FormFieldTest.php @@ -41,7 +41,6 @@ use PHPUnit\Framework\Attributes\DataProvider; use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\ConfirmedPasswordField; -use SilverStripe\Forms\FieldsValidator; use SilverStripe\Forms\NumericField; use SilverStripe\Forms\CheckboxField_Readonly; use SilverStripe\VersionedAdmin\Forms\DiffField; @@ -548,7 +547,7 @@ public function testValidationExtensionHooks() /** @var TextField|FieldValidationExtension $field */ $field = new TextField('Test'); $field->setMaxLength(5); - $form = new Form(null, 'test', new FieldList($field), new FieldList(), new FieldsValidator()); + $form = new Form(null, 'test', new FieldList($field), new FieldList()); $form->disableSecurityToken(); $field->setValue('IAmLongerThan5Characters'); diff --git a/tests/php/Forms/FormTest.php b/tests/php/Forms/FormTest.php index 30816b0feb9..7e30a1b5fe3 100644 --- a/tests/php/Forms/FormTest.php +++ b/tests/php/Forms/FormTest.php @@ -34,6 +34,7 @@ use SilverStripe\Model\ArrayData; use SilverStripe\View\SSViewer; use PHPUnit\Framework\Attributes\DataProvider; +use SilverStripe\Forms\EmailField; class FormTest extends FunctionalTest { @@ -1309,4 +1310,67 @@ protected function clean($input) trim($input ?? '') ); } + + public static function provideValidateFormFields() + { + 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@example.com', + '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@example.com', + 'EmailField2' => 'email@example.com', + ], + 'isValid' => true, + ], + ]; + } + + #[DataProvider('provideValidateFormFields')] + public function testValidateFormFields(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 */])); + + $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/Forms/GridField/GridFieldTest.php b/tests/php/Forms/GridField/GridFieldTest.php index 7dc479356e1..8847ee255f8 100644 --- a/tests/php/Forms/GridField/GridFieldTest.php +++ b/tests/php/Forms/GridField/GridFieldTest.php @@ -551,7 +551,7 @@ public function testValidationMessageInOutput() $gridField->setMessage(null); // A form that passes validation should not display a validation error in the FieldHolder output. - $form->setValidator(new RequiredFieldsValidator()); + $form->setValidator(null); $form->validationResult(); $gridfieldOutput = $gridField->FieldHolder(); $this->assertStringNotContainsString('

', $gridfieldOutput);