NEW: Add extension hook for field-specific validation

This commit is contained in:
Loz Calver 2022-11-04 11:59:52 +00:00 committed by Loz Calver
parent 6a6a7849c0
commit 97f7be502f
19 changed files with 248 additions and 67 deletions

View File

@ -545,6 +545,6 @@ class CompositeField extends FormField
/** @var FormField $child */ /** @var FormField $child */
$valid = ($child && $child->validate($validator) && $valid); $valid = ($child && $child->validate($validator) && $valid);
} }
return $valid; return $this->extendValidationResult($valid, $validator);
} }
} }

View File

@ -416,7 +416,7 @@ class ConfirmedPasswordField extends FormField
// if field isn't visible, don't validate // if field isn't visible, don't validate
if (!$this->isSaveable()) { if (!$this->isSaveable()) {
return true; return $this->extendValidationResult(true, $validator);
} }
$this->getPasswordField()->setValue($this->value); $this->getPasswordField()->setValue($this->value);
@ -431,7 +431,7 @@ class ConfirmedPasswordField extends FormField
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
if (!$this->canBeEmpty) { if (!$this->canBeEmpty) {
@ -443,7 +443,7 @@ class ConfirmedPasswordField extends FormField
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
@ -483,7 +483,7 @@ class ConfirmedPasswordField extends FormField
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
@ -498,7 +498,7 @@ class ConfirmedPasswordField extends FormField
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
@ -513,7 +513,7 @@ class ConfirmedPasswordField extends FormField
), ),
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
// Check this password is valid for the current user // Check this password is valid for the current user
@ -527,7 +527,7 @@ class ConfirmedPasswordField extends FormField
), ),
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
// With a valid user and password, check the password is correct // With a valid user and password, check the password is correct
@ -543,12 +543,12 @@ class ConfirmedPasswordField extends FormField
), ),
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
} }
return true; return $this->extendValidationResult(true, $validator);
} }
/** /**

View File

@ -58,6 +58,7 @@ class CurrencyField extends TextField
public function validate($validator) public function validate($validator)
{ {
$result = true;
$currencySymbol = preg_quote(DBCurrency::config()->uninherited('currency_symbol') ?? ''); $currencySymbol = preg_quote(DBCurrency::config()->uninherited('currency_symbol') ?? '');
$regex = '/^\s*(\-?' . $currencySymbol . '?|' . $currencySymbol . '\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/'; $regex = '/^\s*(\-?' . $currencySymbol . '?|' . $currencySymbol . '\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/';
if (!empty($this->value) && !preg_match($regex ?? '', $this->value ?? '')) { if (!empty($this->value) && !preg_match($regex ?? '', $this->value ?? '')) {
@ -66,9 +67,10 @@ class CurrencyField extends TextField
_t('SilverStripe\\Forms\\Form.VALIDCURRENCY', "Please enter a valid currency"), _t('SilverStripe\\Forms\\Form.VALIDCURRENCY', "Please enter a valid currency"),
"validation" "validation"
); );
return false; $result = false;
} }
return true;
return $this->extendValidationResult($result, $validator);
} }
public function getSchemaValidation() public function getSchemaValidation()

View File

@ -369,7 +369,7 @@ class DateField extends TextField
{ {
// Don't validate empty fields // Don't validate empty fields
if (empty($this->rawValue)) { if (empty($this->rawValue)) {
return true; return $this->extendValidationResult(true, $validator);
} }
// We submitted a value, but it couldn't be parsed // We submitted a value, but it couldn't be parsed
@ -382,7 +382,7 @@ class DateField extends TextField
['format' => $this->getDateFormat()] ['format' => $this->getDateFormat()]
) )
); );
return false; return $this->extendValidationResult(false, $validator);
} }
// Check min date // Check min date
@ -406,7 +406,7 @@ class DateField extends TextField
ValidationResult::TYPE_ERROR, ValidationResult::TYPE_ERROR,
ValidationResult::CAST_HTML ValidationResult::CAST_HTML
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
@ -431,11 +431,11 @@ class DateField extends TextField
ValidationResult::TYPE_ERROR, ValidationResult::TYPE_ERROR,
ValidationResult::CAST_HTML ValidationResult::CAST_HTML
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
return true; return $this->extendValidationResult(true, $validator);
} }
/** /**

View File

@ -565,7 +565,7 @@ class DatetimeField extends TextField
{ {
// Don't validate empty fields // Don't validate empty fields
if (empty($this->rawValue)) { if (empty($this->rawValue)) {
return true; return $this->extendValidationResult(true, $validator);
} }
// We submitted a value, but it couldn't be parsed // We submitted a value, but it couldn't be parsed
@ -578,7 +578,7 @@ class DatetimeField extends TextField
['format' => $this->getDatetimeFormat()] ['format' => $this->getDatetimeFormat()]
) )
); );
return false; return $this->extendValidationResult(false, $validator);
} }
// Check min date (in server timezone) // Check min date (in server timezone)
@ -602,7 +602,7 @@ class DatetimeField extends TextField
ValidationResult::TYPE_ERROR, ValidationResult::TYPE_ERROR,
ValidationResult::CAST_HTML ValidationResult::CAST_HTML
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
@ -627,11 +627,11 @@ class DatetimeField extends TextField
ValidationResult::TYPE_ERROR, ValidationResult::TYPE_ERROR,
ValidationResult::CAST_HTML ValidationResult::CAST_HTML
); );
return false; return $this->extendValidationResult(false, $validator);
} }
} }
return true; return $this->extendValidationResult(true, $validator);
} }
public function performReadonlyTransformation() public function performReadonlyTransformation()

View File

@ -29,6 +29,7 @@ class EmailField extends TextField
*/ */
public function validate($validator) public function validate($validator)
{ {
$result = true;
$this->value = trim($this->value ?? ''); $this->value = trim($this->value ?? '');
$pattern = '^[a-z0-9!#$%&\'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&\'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$'; $pattern = '^[a-z0-9!#$%&\'*+/=?^_`{|}~-]+(?:\\.[a-z0-9!#$%&\'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$';
@ -43,10 +44,10 @@ class EmailField extends TextField
'validation' 'validation'
); );
return false; $result = false;
} }
return true; return $this->extendValidationResult($result, $validator);
} }
public function getSchemaValidation() public function getSchemaValidation()

View File

@ -187,7 +187,7 @@ class FileField extends FormField implements FileHandleField
$fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name ?? ''); $fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name ?? '');
if (!isset($_FILES[$fieldName])) { if (!isset($_FILES[$fieldName])) {
return true; return $this->extendValidationResult(true, $validator);
} }
if ($isMultiFileUpload) { if ($isMultiFileUpload) {
@ -204,11 +204,12 @@ class FileField extends FormField implements FileHandleField
$isValid = false; $isValid = false;
} }
} }
return $isValid; return $this->extendValidationResult($isValid, $validator);
} }
// regular single-file upload // regular single-file upload
return $this->validateFileData($validator, $_FILES[$this->name]); $result = $this->validateFileData($validator, $_FILES[$this->name]);
return $this->extendValidationResult($result, $validator);
} }
/** /**

View File

@ -1219,18 +1219,29 @@ class FormField extends RequestHandler
return strtolower(preg_replace('/Field$/', '', $type->getShortName() ?? '') ?? ''); return strtolower(preg_replace('/Field$/', '', $type->getShortName() ?? '') ?? '');
} }
/**
* Utility method to call an extension hook which allows the result of validate() calls to be adjusted
*
* @param bool $result
* @param Validator $validator
* @return bool
*/
protected function extendValidationResult(bool $result, Validator $validator): bool
{
$this->extend('updateValidationResult', $result, $validator);
return $result;
}
/** /**
* Abstract method each {@link FormField} subclass must implement, determines whether the field * Abstract method each {@link FormField} subclass must implement, determines whether the field
* is valid or not based on the value. * is valid or not based on the value.
* *
* @todo Make this abstract.
*
* @param Validator $validator * @param Validator $validator
* @return bool * @return bool
*/ */
public function validate($validator) public function validate($validator)
{ {
return true; return $this->extendValidationResult(true, $validator);
} }
/** /**

View File

@ -73,7 +73,7 @@ class LookupField extends MultiSelectField
*/ */
public function validate($validator) public function validate($validator)
{ {
return true; return $this->extendValidationResult(true, $validator);
} }
/** /**

View File

@ -334,11 +334,12 @@ class MoneyField extends FormField
['currency' => $currency] ['currency' => $currency]
) )
); );
return false; return $this->extendValidationResult(false, $validator);
} }
// Field-specific validation // Field-specific validation
return $this->fieldAmount->validate($validator) && $this->fieldCurrency->validate($validator); $result = $this->fieldAmount->validate($validator) && $this->fieldCurrency->validate($validator);
return $this->extendValidationResult($result, $validator);
} }
public function setForm($form) public function setForm($form)

View File

@ -247,21 +247,23 @@ abstract class MultiSelectField extends SelectField
return true; return true;
} }
); );
if (empty($invalidValues)) {
return true; $result = true;
if (!empty($invalidValues)) {
$result = false;
// List invalid items
$validator->validationError(
$this->getName(),
_t(
'SilverStripe\\Forms\\MultiSelectField.SOURCE_VALIDATION',
"Please select values within the list provided. Invalid option(s) {value} given",
['value' => implode(',', $invalidValues)]
),
"validation"
);
} }
// List invalid items return $this->extendValidationResult($result, $validator);
$validator->validationError(
$this->getName(),
_t(
'SilverStripe\\Forms\\MultiSelectField.SOURCE_VALIDATION',
"Please select values within the list provided. Invalid option(s) {value} given",
['value' => implode(',', $invalidValues)]
),
"validation"
);
return false;
} }
/** /**

View File

@ -191,20 +191,21 @@ class NumericField extends TextField
*/ */
public function validate($validator) public function validate($validator)
{ {
$result = true;
// false signifies invalid value due to failed parse() // false signifies invalid value due to failed parse()
if ($this->value !== false) { if ($this->value === false) {
return true; $validator->validationError(
$this->name,
_t(
'SilverStripe\\Forms\\NumericField.VALIDATION',
"'{value}' is not a number, only numbers can be accepted for this field",
['value' => $this->originalValue]
)
);
$result = false;
} }
$validator->validationError( return $this->extendValidationResult($result, $validator);
$this->name,
_t(
'SilverStripe\\Forms\\NumericField.VALIDATION',
"'{value}' is not a number, only numbers can be accepted for this field",
['value' => $this->originalValue]
)
);
return false;
} }
public function getSchemaValidation() public function getSchemaValidation()

View File

@ -140,7 +140,7 @@ class OptionsetField extends SingleSelectField
public function validate($validator) public function validate($validator)
{ {
if (!$this->Value()) { if (!$this->Value()) {
return true; return $this->extendValidationResult(true, $validator);
} }
return parent::validate($validator); return parent::validate($validator);

View File

@ -44,7 +44,7 @@ class SingleLookupField extends SingleSelectField
*/ */
public function validate($validator) public function validate($validator)
{ {
return true; return $this->extendValidationResult(true, $validator);
} }
/** /**

View File

@ -137,13 +137,13 @@ abstract class SingleSelectField extends SelectField
// Use selection rules to check which are valid // Use selection rules to check which are valid
foreach ($validValues as $formValue) { foreach ($validValues as $formValue) {
if ($this->isSelectedValue($formValue, $selected)) { if ($this->isSelectedValue($formValue, $selected)) {
return true; return $this->extendValidationResult(true, $validator);
} }
} }
} else { } else {
if ($this->getHasEmptyDefault() || !$validValues || in_array('', $validValues ?? [])) { if ($this->getHasEmptyDefault() || !$validValues || in_array('', $validValues ?? [])) {
// Check empty value // Check empty value
return true; return $this->extendValidationResult(true, $validator);
} }
$selected = '(none)'; $selected = '(none)';
} }
@ -158,7 +158,7 @@ abstract class SingleSelectField extends SelectField
), ),
"validation" "validation"
); );
return false; return $this->extendValidationResult(false, $validator);
} }
public function castedCopy($classOrCopy) public function castedCopy($classOrCopy)

View File

@ -125,6 +125,7 @@ class TextField extends FormField implements TippableFieldInterface
*/ */
public function validate($validator) public function validate($validator)
{ {
$result = true;
if (!is_null($this->maxLength) && mb_strlen($this->value ?? '') > $this->maxLength) { if (!is_null($this->maxLength) && mb_strlen($this->value ?? '') > $this->maxLength) {
$name = strip_tags($this->Title() ? $this->Title() : $this->getName()); $name = strip_tags($this->Title() ? $this->Title() : $this->getName());
$validator->validationError( $validator->validationError(
@ -136,9 +137,9 @@ class TextField extends FormField implements TippableFieldInterface
), ),
"validation" "validation"
); );
return false; $result = false;
} }
return true; return $this->extendValidationResult($result, $validator);
} }
public function getSchemaValidation() public function getSchemaValidation()

View File

@ -324,9 +324,10 @@ class TimeField extends TextField
{ {
// Don't validate empty fields // Don't validate empty fields
if (empty($this->rawValue)) { if (empty($this->rawValue)) {
return true; return $this->extendValidationResult(true, $validator);
} }
$result = true;
// We submitted a value, but it couldn't be parsed // We submitted a value, but it couldn't be parsed
if (empty($this->value)) { if (empty($this->value)) {
$validator->validationError( $validator->validationError(
@ -337,9 +338,11 @@ class TimeField extends TextField
['format' => $this->getTimeFormat()] ['format' => $this->getTimeFormat()]
) )
); );
return false; $result = false;
} }
return true;
$this->extendValidationResult($result, $validator);
return $result;
} }
/** /**

View File

@ -2,21 +2,40 @@
namespace SilverStripe\Forms\Tests; namespace SilverStripe\Forms\Tests;
use Exception;
use LogicException; use LogicException;
use ReflectionClass; use ReflectionClass;
use SilverStripe\Core\ClassInfo; use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\SapphireTest;
use SilverStripe\Dev\TestOnly;
use SilverStripe\Forms\CompositeField; use SilverStripe\Forms\CompositeField;
use SilverStripe\Forms\FieldGroup;
use SilverStripe\Forms\FieldList; use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\Form; use SilverStripe\Forms\Form;
use SilverStripe\Forms\FormField; use SilverStripe\Forms\FormField;
use SilverStripe\Forms\GridField\GridField;
use SilverStripe\Forms\GridField\GridField_FormAction;
use SilverStripe\Forms\GridField\GridState;
use SilverStripe\Forms\NullableField; use SilverStripe\Forms\NullableField;
use SilverStripe\Forms\PopoverField;
use SilverStripe\Forms\PrintableTransformation_TabSet;
use SilverStripe\Forms\RequiredFields; use SilverStripe\Forms\RequiredFields;
use SilverStripe\Forms\SelectionGroup;
use SilverStripe\Forms\SelectionGroup_Item;
use SilverStripe\Forms\Tab;
use SilverStripe\Forms\Tests\FormFieldTest\FieldValidationExtension;
use SilverStripe\Forms\Tests\FormFieldTest\TestExtension; use SilverStripe\Forms\Tests\FormFieldTest\TestExtension;
use SilverStripe\Forms\TextField; use SilverStripe\Forms\TextField;
use SilverStripe\Forms\Tip; use SilverStripe\Forms\Tip;
use SilverStripe\Forms\ToggleCompositeField;
use SilverStripe\Forms\TreeDropdownField;
use SilverStripe\Forms\TreeDropdownField_Readonly;
use SilverStripe\ORM\ValidationResult; use SilverStripe\ORM\ValidationResult;
use SilverStripe\Security\Group;
use SilverStripe\Security\Permission;
use SilverStripe\Security\PermissionCheckboxSetField;
use SilverStripe\Security\PermissionCheckboxSetField_Readonly;
class FormFieldTest extends SapphireTest class FormFieldTest extends SapphireTest
{ {
@ -24,6 +43,7 @@ class FormFieldTest extends SapphireTest
protected static $required_extensions = [ protected static $required_extensions = [
FormField::class => [ FormField::class => [
TestExtension::class, TestExtension::class,
FieldValidationExtension::class,
], ],
]; ];
@ -455,6 +475,106 @@ class FormFieldTest extends SapphireTest
); );
} }
public function testValidationExtensionHooks()
{
/** @var TextField|FieldValidationExtension $field */
$field = new TextField('Test');
$field->setMaxLength(5);
$field->setValue('IAmLongerThan5Characters');
$result = $field->validate(new RequiredFields('Test'));
$this->assertFalse($result);
// Call extension method in FieldValidationExtension
$field->setExcludeFromValidation(true);
$result = $field->validate(new RequiredFields('Test'));
$this->assertTrue($result);
// Call extension methods in FieldValidationExtension
$field->setValue('1234');
$field->setExcludeFromValidation(false);
$field->setTriggerTestValidationError(true);
// Ensure messages set via updateValidationResult() propagate through to form fields after validation
$form = new Form(null, 'TestForm', new FieldList($field), new FieldList(), new RequiredFields());
$form->validationResult();
$schema = $field->getSchemaState();
$this->assertEquals(
'A test error message',
$schema['message']['value']
);
}
public function testValidationExtensionHooksAreCalledOnFormFieldSubclasses()
{
// Can't use a dataProvider for this as dataProviders are fetched very early by phpunit,
// and the ClassManifest isn't ready then
$formFieldClasses = ClassInfo::subclassesFor(FormField::class, false);
foreach ($formFieldClasses as $formFieldClass) {
$reflection = new ReflectionClass($formFieldClass);
// Skip abstract classes, like MultiSelectField, and fields that only exist for unit tests
if ($reflection->isAbstract() || is_a($formFieldClass, TestOnly::class, true)) {
continue;
}
switch ($formFieldClass) {
case NullableField::class:
case CompositeField::class:
case FieldGroup::class:
case PopoverField::class:
$args = [TextField::create('Test2')];
break;
case SelectionGroup_Item::class:
$args = ['Test', [TextField::create('Test2')]];
break;
case ToggleCompositeField::class:
$args = ['Test', 'Test', TextField::create('Test2')];
break;
case PrintableTransformation_TabSet::class:
$args = [Tab::create('TestTab', 'Testtab', TextField::create('Test2'))];
break;
case TreeDropdownField::class:
case TreeDropdownField_Readonly::class:
$args = ['Test', 'Test', Group::class];
break;
case PermissionCheckboxSetField::class:
case PermissionCheckboxSetField_Readonly::class:
$args = ['Test', 'Test', Permission::class, 'Test'];
break;
case SelectionGroup::class:
$args = ['Test', []];
break;
case GridField_FormAction::class:
$args = [GridField::create('GF'), 'Test', 'Test label', 'Test action name', []];
break;
case GridState::class:
$args = [GridField::create('GF')];
break;
default:
$args = ['Test', 'Test label'];
}
// Assert that extendValidationResult is called once each time ->validate() is called
$mock = $this->getMockBuilder($formFieldClass)
->setConstructorArgs($args)
->onlyMethods(['extendValidationResult'])
->getMock();
$mock->expects($invocationRule = $this->once())
->method('extendValidationResult')
->will($this->returnValue(true));
$isValid = $mock->validate(new RequiredFields());
$this->assertTrue($isValid, "$formFieldClass should be valid");
// This block is not essential and only exists to make test debugging easier - without this,
// the error message on failure is generic and doesn't include the class name that failed
try {
$invocationRule->verify();
} catch (Exception $e) {
$this->fail("Expectation failed for '$formFieldClass' class: {$e->getMessage()}");
}
}
}
public function testHasClass() public function testHasClass()
{ {
$field = new FormField('Test'); $field = new FormField('Test');

View File

@ -0,0 +1,38 @@
<?php
namespace SilverStripe\Forms\Tests\FormFieldTest;
use SilverStripe\Core\Extension;
use SilverStripe\Dev\TestOnly;
use SilverStripe\Forms\Validator;
class FieldValidationExtension extends Extension implements TestOnly
{
protected bool $excludeFromValidation = false;
protected bool $triggerTestValidationError = false;
public function updateValidationResult(bool &$result, Validator $validator)
{
if ($this->excludeFromValidation) {
$result = true;
return;
}
if ($this->triggerTestValidationError) {
$result = false;
$validator->validationError($this->owner->getName(), 'A test error message');
return;
}
}
public function setExcludeFromValidation(bool $exclude)
{
$this->excludeFromValidation = $exclude;
}
public function setTriggerTestValidationError(bool $triggerTestValidationError)
{
$this->triggerTestValidationError = $triggerTestValidationError;
}
}