From a2c836650f0ab094e0265026404c8efe97c7ec9f Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Thu, 28 Nov 2024 12:24:17 +1300 Subject: [PATCH] ENH Use FieldValidators for FormField validation --- .../FieldValidation/BigIntFieldValidator.php | 1 - .../FieldValidation/BooleanFieldValidator.php | 1 - .../CompositeFieldValidator.php | 1 - .../FieldValidation/DateFieldValidator.php | 108 ++- .../DatetimeFieldValidator.php | 2 - .../FieldValidation/DecimalFieldValidator.php | 3 +- .../FieldValidation/EmailFieldValidator.php | 5 +- .../FieldValidation/FieldValidationTrait.php | 34 +- .../FieldValidation/IntFieldValidator.php | 3 +- .../FieldValidation/IpFieldValidator.php | 3 - .../FieldValidation/LocaleFieldValidator.php | 3 - .../MultiOptionFieldValidator.php | 11 +- .../FieldValidation/NumericFieldValidator.php | 10 +- .../NumericNonStringFieldValidator.php | 21 + .../FieldValidation/OptionFieldValidator.php | 9 +- .../FieldValidation/StringFieldValidator.php | 3 +- .../SymfonyFieldValidatorTrait.php | 1 - .../FieldValidation/TimeFieldValidator.php | 2 - .../FieldValidation/UrlFieldValidator.php | 3 - src/Forms/CompositeField.php | 29 +- src/Forms/ConfirmedPasswordField.php | 255 +++--- src/Forms/CurrencyField.php | 28 +- src/Forms/DateField.php | 144 +-- src/Forms/DateField_Disabled.php | 35 +- src/Forms/DatetimeField.php | 145 +--- src/Forms/EmailField.php | 28 +- src/Forms/FieldGroup.php | 2 +- .../FieldValidatorConverterInterface.php | 10 + .../FieldValidatorConverterTrait.php | 24 + src/Forms/FieldsValidator.php | 7 +- src/Forms/FileField.php | 68 +- src/Forms/FormField.php | 81 +- src/Forms/GridField/GridField.php | 3 - src/Forms/ListboxField.php | 13 +- src/Forms/LookupField.php | 16 +- src/Forms/MoneyField.php | 97 ++- src/Forms/MultiSelectField.php | 76 +- src/Forms/NumericField.php | 124 ++- src/Forms/OptionsetField.php | 12 - src/Forms/RequiredFields.php | 4 +- src/Forms/SearchableDropdownField.php | 11 + src/Forms/SearchableDropdownTrait.php | 8 - src/Forms/SearchableMultiDropdownField.php | 5 + src/Forms/SelectField.php | 33 +- src/Forms/SelectionGroup_Item.php | 2 +- src/Forms/SingleLookupField.php | 18 +- src/Forms/SingleSelectField.php | 52 +- src/Forms/TextField.php | 34 +- src/Forms/TextareaField.php | 33 +- src/Forms/TimeField.php | 107 +-- src/Forms/UrlField.php | 40 +- src/ORM/FieldType/DBDecimal.php | 5 +- src/ORM/FieldType/DBEnum.php | 4 +- src/ORM/FieldType/DBField.php | 13 - src/ORM/FieldType/DBFloat.php | 4 +- src/ORM/FieldType/DBForeignKey.php | 4 +- src/ORM/FieldType/DBMultiEnum.php | 4 +- src/ORM/FieldType/DBPercentage.php | 7 +- src/ORM/FieldType/DBVarchar.php | 5 +- src/ORM/FieldType/DBYear.php | 5 +- .../DateFieldValidatorTest.php | 152 +++- .../MultiOptionFieldValidatorTest.php | 23 +- .../NumericFieldValidatorTest.php | 114 ++- .../NumericNonStringFieldValidatorTest.php | 40 + tests/php/Forms/CheckboxFieldTest.php | 9 +- tests/php/Forms/CheckboxSetFieldTest.php | 58 +- tests/php/Forms/CompositeFieldTest.php | 16 +- .../php/Forms/ConfirmedPasswordFieldTest.php | 88 +- tests/php/Forms/CurrencyFieldTest.php | 43 +- tests/php/Forms/DateFieldTest.php | 103 ++- tests/php/Forms/DatetimeFieldTest.php | 118 ++- tests/php/Forms/DropdownFieldTest.php | 31 +- tests/php/Forms/EmailFieldTest.php | 5 +- tests/php/Forms/FileFieldTest.php | 3 +- tests/php/Forms/FormFieldTest.php | 405 ++++++++- .../FieldValidationExtension.php | 15 +- tests/php/Forms/FormTest.php | 19 +- tests/php/Forms/GroupedDropdownFieldTest.php | 31 +- tests/php/Forms/ListboxFieldTest.php | 139 +-- tests/php/Forms/LookupFieldTest.php | 27 + tests/php/Forms/MoneyFieldTest.php | 21 +- tests/php/Forms/MultiSelectFieldTest.php | 122 +++ tests/php/Forms/NumericFieldTest.php | 819 ++++++++++++++---- tests/php/Forms/OptionsetFieldTest.php | 21 +- .../php/Forms/SearchableDropdownFieldTest.php | 43 + .../php/Forms/SearchableDropdownTraitTest.php | 1 + tests/php/Forms/SelectFieldTest.php | 100 +++ tests/php/Forms/SingleLookupFieldTest.php | 24 + tests/php/Forms/SingleSelectFieldTest.php | 80 ++ tests/php/Forms/TextFieldTest.php | 42 +- tests/php/Forms/TextareaFieldTest.php | 42 +- tests/php/Forms/TimeFieldTest.php | 63 +- tests/php/Forms/UrlFieldTest.php | 4 +- tests/php/ORM/DBFieldTest.php | 5 +- .../php/Security/MemberTest/ValidatorForm.php | 3 +- 95 files changed, 3106 insertions(+), 1542 deletions(-) create mode 100644 src/Core/Validation/FieldValidation/NumericNonStringFieldValidator.php create mode 100644 src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php create mode 100644 src/Forms/FieldValidatorConverter/FieldValidatorConverterTrait.php create mode 100644 tests/php/Core/Validation/FieldValidation/NumericNonStringFieldValidatorTest.php create mode 100644 tests/php/Forms/MultiSelectFieldTest.php create mode 100644 tests/php/Forms/SearchableDropdownFieldTest.php create mode 100644 tests/php/Forms/SelectFieldTest.php create mode 100644 tests/php/Forms/SingleSelectFieldTest.php diff --git a/src/Core/Validation/FieldValidation/BigIntFieldValidator.php b/src/Core/Validation/FieldValidation/BigIntFieldValidator.php index e9d97161e1f..ee9d8c5b12b 100644 --- a/src/Core/Validation/FieldValidation/BigIntFieldValidator.php +++ b/src/Core/Validation/FieldValidation/BigIntFieldValidator.php @@ -3,7 +3,6 @@ namespace SilverStripe\Core\Validation\FieldValidation; use RunTimeException; -use SilverStripe\Core\Validation\FieldValidation\IntFieldValidator; use SilverStripe\Core\Validation\ValidationResult; /** diff --git a/src/Core/Validation/FieldValidation/BooleanFieldValidator.php b/src/Core/Validation/FieldValidation/BooleanFieldValidator.php index b1ddd3edd91..d8c56d2d57c 100644 --- a/src/Core/Validation/FieldValidation/BooleanFieldValidator.php +++ b/src/Core/Validation/FieldValidation/BooleanFieldValidator.php @@ -3,7 +3,6 @@ namespace SilverStripe\Core\Validation\FieldValidation; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\FieldValidator; /** * Validates that a value is a boolean diff --git a/src/Core/Validation/FieldValidation/CompositeFieldValidator.php b/src/Core/Validation/FieldValidation/CompositeFieldValidator.php index 1fe3f36dd49..2add885e7df 100644 --- a/src/Core/Validation/FieldValidation/CompositeFieldValidator.php +++ b/src/Core/Validation/FieldValidation/CompositeFieldValidator.php @@ -4,7 +4,6 @@ use InvalidArgumentException; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\FieldValidator; use SilverStripe\Core\Validation\FieldValidation\FieldValidationInterface; /** diff --git a/src/Core/Validation/FieldValidation/DateFieldValidator.php b/src/Core/Validation/FieldValidation/DateFieldValidator.php index 0a2a3de22b3..28bbbd8ffd6 100644 --- a/src/Core/Validation/FieldValidation/DateFieldValidator.php +++ b/src/Core/Validation/FieldValidation/DateFieldValidator.php @@ -2,29 +2,106 @@ namespace SilverStripe\Core\Validation\FieldValidation; -use SilverStripe\Core\Validation\FieldValidation\FieldValidator; +use InvalidArgumentException; use SilverStripe\Core\Validation\ValidationResult; /** * Validates that a value is a valid date, which means that it follows the equivalent formats: * - PHP date format Y-m-d - * - SO format y-MM-dd i.e. DBDate::ISO_DATE + * - ISO format y-MM-dd i.e. DBDate::ISO_DATE * * Blank string values are allowed */ -class DateFieldValidator extends FieldValidator +class DateFieldValidator extends StringFieldValidator { + /** + * Minimum value as a date string + */ + private ?string $minValue = null; + + /** + * Minimum value as a unix timestamp + */ + private ?int $minTimestamp = null; + + /** + * Maximum value as a date string + */ + private ?string $maxValue = null; + + /** + * Maximum value as a unix timestamp + */ + private ?int $maxTimestamp = null; + + /** + * Converter to convert date strings to another format for display in error messages + * + * @var callable + */ + private $converter; + + public function __construct( + string $name, + mixed $value, + ?string $minValue = null, + ?string $maxValue = null, + ?callable $converter = null, + ) { + // Convert Y-m-d args to timestamps + if (!is_null($minValue)) { + $this->minTimestamp = $this->dateToTimestamp($minValue); + } + if (!is_null($maxValue)) { + $this->maxTimestamp = $this->dateToTimestamp($maxValue); + } + if (!is_null($this->minTimestamp) && !is_null($this->maxTimestamp) + && $this->maxTimestamp < $this->minTimestamp + ) { + throw new InvalidArgumentException('maxValue cannot be less than minValue'); + } + $this->minValue = $minValue; + $this->maxValue = $maxValue; + $this->converter = $converter; + parent::__construct($name, $value); + } + protected function validateValue(): ValidationResult { - $result = ValidationResult::create(); - // Allow empty strings - if ($this->value === '') { + $result = parent::validateValue(); + // If the value is not a string, we can't validate it as a date + if (!$result->isValid()) { return $result; } - // Not using symfony/validator because it was allowing d-m-Y format strings - $date = date_parse_from_format($this->getFormat(), $this->value ?? ''); - if ($date === false || $date['error_count'] > 0 || $date['warning_count'] > 0) { + // Validate value is a valid date + $timestamp = $this->dateToTimestamp($this->value ?? ''); + if ($timestamp === false) { $result->addFieldError($this->name, $this->getMessage()); + return $result; + } + // Validate value is within range + if (!is_null($this->minTimestamp) && $timestamp < $this->minTimestamp) { + $minValue = $this->minValue; + if (!is_null($this->converter)) { + $minValue = call_user_func($this->converter, $this->minValue) ?: $this->minValue; + } + $message = _t( + __CLASS__ . '.TOOSMALL', + 'Value cannot be older than {minValue}', + ['minValue' => $minValue] + ); + $result->addFieldError($this->name, $message); + } elseif (!is_null($this->maxTimestamp) && $timestamp > $this->maxTimestamp) { + $maxValue = $this->maxValue; + if (!is_null($this->converter)) { + $maxValue = call_user_func($this->converter, $this->maxValue) ?: $this->maxValue; + } + $message = _t( + __CLASS__ . '.TOOLARGE', + 'Value cannot be newer than {maxValue}', + ['maxValue' => $maxValue] + ); + $result->addFieldError($this->name, $message); } return $result; } @@ -38,4 +115,17 @@ protected function getMessage(): string { return _t(__CLASS__ . '.INVALID', 'Invalid date'); } + + /** + * Parse a date string into a unix timestamp using the format specified by getFormat() + */ + private function dateToTimestamp(string $date): int|false + { + // Not using symfony/validator because it was allowing d-m-Y format strings + $date = date_parse_from_format($this->getFormat(), $date); + if ($date === false || $date['error_count'] > 0 || $date['warning_count'] > 0) { + return false; + } + return mktime($date['hour'], $date['minute'], $date['second'], $date['month'], $date['day'], $date['year']); + } } diff --git a/src/Core/Validation/FieldValidation/DatetimeFieldValidator.php b/src/Core/Validation/FieldValidation/DatetimeFieldValidator.php index 9b7368067fc..7f4db4e76cf 100644 --- a/src/Core/Validation/FieldValidation/DatetimeFieldValidator.php +++ b/src/Core/Validation/FieldValidation/DatetimeFieldValidator.php @@ -2,8 +2,6 @@ namespace SilverStripe\Core\Validation\FieldValidation; -use SilverStripe\Core\Validation\FieldValidation\DateFieldValidator; - /** * Validates that a value is a valid date/time, which means that it follows the equivalent formats: * - PHP date format Y-m-d H:i:s diff --git a/src/Core/Validation/FieldValidation/DecimalFieldValidator.php b/src/Core/Validation/FieldValidation/DecimalFieldValidator.php index 17888f4d942..9731164fcb2 100644 --- a/src/Core/Validation/FieldValidation/DecimalFieldValidator.php +++ b/src/Core/Validation/FieldValidation/DecimalFieldValidator.php @@ -3,7 +3,6 @@ namespace SilverStripe\Core\Validation\FieldValidation; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\NumericFieldValidator; /** * Validates that a value is a valid decimal @@ -23,7 +22,7 @@ * 1234.9 - 4 digits the before the decimal point * 999.999 - would be rounded to 1000.00 which exceeds 5 total digits */ -class DecimalFieldValidator extends NumericFieldValidator +class DecimalFieldValidator extends NumericNonStringFieldValidator { /** * Whole number size e.g. For Decimal(9,2) this would be 9 diff --git a/src/Core/Validation/FieldValidation/EmailFieldValidator.php b/src/Core/Validation/FieldValidation/EmailFieldValidator.php index 2f39288ad34..84f8505cacc 100644 --- a/src/Core/Validation/FieldValidation/EmailFieldValidator.php +++ b/src/Core/Validation/FieldValidation/EmailFieldValidator.php @@ -4,9 +4,6 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\Email; -use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorTrait; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorInterface; /** * Validates that a value is a valid email address @@ -19,6 +16,6 @@ class EmailFieldValidator extends StringFieldValidator implements SymfonyFieldVa public function getConstraint(): Constraint|array { $message = _t(__CLASS__ . '.INVALID', 'Invalid email address'); - return new Email(message: $message); + return new Email(message: $message, mode: Email::VALIDATION_MODE_STRICT); } } diff --git a/src/Core/Validation/FieldValidation/FieldValidationTrait.php b/src/Core/Validation/FieldValidation/FieldValidationTrait.php index 649c350e64e..e655d5e0110 100644 --- a/src/Core/Validation/FieldValidation/FieldValidationTrait.php +++ b/src/Core/Validation/FieldValidation/FieldValidationTrait.php @@ -5,7 +5,6 @@ use RuntimeException; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Config\Configurable; -use SilverStripe\Core\Validation\FieldValidation\FieldValidationInterface; use SilverStripe\Core\Validation\ValidationResult; /** @@ -21,13 +20,17 @@ trait FieldValidationTrait * * Each item in the array can be one of the following * a) MyFieldValidator::class, - * b) MyFieldValidator::class => [null, 'getSomething'], - * c) MyFieldValidator::class => null, + * b) MyFieldValidator::class => ['argNameA' => null, 'argNameB' => 'getSomething'], + * d) MyFieldValidator::class => null, * * a) Will create a MyFieldValidator and pass the name and value of the field as args to the constructor * b) Will create a MyFieldValidator and pass the name, value, and pass additional args, where each null values * will be passed as null, and non-null values will call a method on the field e.g. will pass null for the first * additional arg and call $field->getSomething() to get a value for the second additional arg + * Keys are used to speicify the arg name, which is done to prevents duplicate + * args being add to config when a subclass defines the same FieldValidator as a parent class. + * Note that keys are not named args, they are simply arbitary keys - though best practice is + * for the keys to match constructor argument names. * c) Will disable a previously set MyFieldValidator. This is useful to disable a FieldValidator that was set * on a parent class * @@ -49,6 +52,18 @@ public function validate(): ValidationResult return $result; } + /** + * Get the value of this field for use in validation via FieldValidators + * + * Intended to be overridden in subclasses when there is a need to provide something different + * from the value of the field itself, for instance DBComposite and CompositeField which need to + * provide a value that is a combination of the values of their children + */ + public function getValueForValidation(): mixed + { + return $this->getValue(); + } + /** * Get instantiated FieldValidators based on `field_validators` configuration */ @@ -62,11 +77,9 @@ private function getFieldValidators(): array } /** @var FieldValidationInterface|Configurable $this */ $name = $this->getName(); + // For composite fields e.g. MyCompositeField[MySubField] we want to use the name of the composite field + $name = preg_replace('#\[[^\]]+\]$#', '', $name); $value = $this->getValueForValidation(); - // Field name is required for FieldValidators when called ValidationResult::addFieldMessage() - if ($name === '') { - throw new RuntimeException('Field name is blank'); - } $classes = $this->getClassesFromConfig(); foreach ($classes as $class => $argCalls) { $args = [$name, $value]; @@ -122,10 +135,9 @@ private function getClassesFromConfig(): array if (!is_array($argCalls)) { throw new RuntimeException("argCalls for FieldValidator $class is not an array"); } - // array_unique() is used to dedupe any cases where a subclass defines the same FieldValidator - // this config can happens when a subclass defines a FieldValidator that was already defined on a parent - // class, though it calls different methods - $argCalls = array_unique($argCalls); + // Ensure that argCalls is a numerically indexed array + // as they may have been defined with string keys to prevent duplicate args + $argCalls = array_values($argCalls); $classes[$class] = $argCalls; } foreach (array_keys($disabledClasses) as $class) { diff --git a/src/Core/Validation/FieldValidation/IntFieldValidator.php b/src/Core/Validation/FieldValidation/IntFieldValidator.php index 4dfc6bb3db4..28c677b036d 100644 --- a/src/Core/Validation/FieldValidation/IntFieldValidator.php +++ b/src/Core/Validation/FieldValidation/IntFieldValidator.php @@ -3,12 +3,11 @@ namespace SilverStripe\Core\Validation\FieldValidation; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\NumericFieldValidator; /** * Validates that a value is a 32-bit signed integer */ -class IntFieldValidator extends NumericFieldValidator +class IntFieldValidator extends NumericNonStringFieldValidator { /** * The minimum value for a signed 32-bit integer. diff --git a/src/Core/Validation/FieldValidation/IpFieldValidator.php b/src/Core/Validation/FieldValidation/IpFieldValidator.php index 96715da5b05..57ebc989cad 100644 --- a/src/Core/Validation/FieldValidation/IpFieldValidator.php +++ b/src/Core/Validation/FieldValidation/IpFieldValidator.php @@ -4,9 +4,6 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\Ip; -use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorTrait; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorInterface; /** * Validates that a value is a valid IP address diff --git a/src/Core/Validation/FieldValidation/LocaleFieldValidator.php b/src/Core/Validation/FieldValidation/LocaleFieldValidator.php index 0ff2c1d98a8..d73b76c400c 100644 --- a/src/Core/Validation/FieldValidation/LocaleFieldValidator.php +++ b/src/Core/Validation/FieldValidation/LocaleFieldValidator.php @@ -4,9 +4,6 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\Locale; -use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorTrait; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorInterface; /** * Validates that a value is a valid locale diff --git a/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php b/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php index dd770784dfa..c043efb065b 100644 --- a/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php +++ b/src/Core/Validation/FieldValidation/MultiOptionFieldValidator.php @@ -2,9 +2,7 @@ namespace SilverStripe\Core\Validation\FieldValidation; -use InvalidArgumentException; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; /** * Validates that that all values are one of a set of options @@ -12,19 +10,20 @@ class MultiOptionFieldValidator extends OptionFieldValidator { /** - * @param mixed $value - an array of values to be validated + * @param mixed $value - an iterable set of values to be validated */ public function __construct(string $name, mixed $value, array $options) { - if (!is_iterable($value) && !is_null($value)) { - throw new InvalidArgumentException('Value must be iterable'); - } parent::__construct($name, $value, $options); } protected function validateValue(): ValidationResult { $result = ValidationResult::create(); + if (!is_iterable($this->value) && !is_null($this->value)) { + $result->addFieldError($this->name, $this->getMessage()); + return $result; + } foreach ($this->value as $value) { $this->checkValueInOptions($value, $result); } diff --git a/src/Core/Validation/FieldValidation/NumericFieldValidator.php b/src/Core/Validation/FieldValidation/NumericFieldValidator.php index 398d9d7234c..a7e524ed008 100644 --- a/src/Core/Validation/FieldValidation/NumericFieldValidator.php +++ b/src/Core/Validation/FieldValidation/NumericFieldValidator.php @@ -2,9 +2,8 @@ namespace SilverStripe\Core\Validation\FieldValidation; -use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\FieldValidator; use InvalidArgumentException; +use SilverStripe\Core\Validation\ValidationResult; /** * Validates that a value is a numeric value @@ -26,7 +25,7 @@ public function __construct( string $name, mixed $value, ?int $minValue = null, - ?int $maxValue = null + ?int $maxValue = null, ) { if (!is_null($minValue) && !is_null($maxValue) && $maxValue < $minValue) { throw new InvalidArgumentException('maxValue cannot be less than minValue'); @@ -39,9 +38,8 @@ public function __construct( protected function validateValue(): ValidationResult { $result = ValidationResult::create(); - if (!is_numeric($this->value) || is_string($this->value)) { - // Must be a numeric value, though not as a numeric string - $message = _t(__CLASS__ . '.WRONGTYPE', 'Must be numeric, and not a string'); + if (!is_numeric($this->value)) { + $message = _t(__CLASS__ . '.NOTNUMERIC', 'Must be numeric'); $result->addFieldError($this->name, $message); } elseif (!is_null($this->minValue) && $this->value < $this->minValue) { $result->addFieldError($this->name, $this->getTooSmallMessage()); diff --git a/src/Core/Validation/FieldValidation/NumericNonStringFieldValidator.php b/src/Core/Validation/FieldValidation/NumericNonStringFieldValidator.php new file mode 100644 index 00000000000..262b0cd3d54 --- /dev/null +++ b/src/Core/Validation/FieldValidation/NumericNonStringFieldValidator.php @@ -0,0 +1,21 @@ +value) && is_string($this->value)) { + $message = _t(__CLASS__ . '.WRONGTYPE', 'Must be numeric and not a string'); + $result->addFieldError($this->name, $message); + } + return $result; + } +} diff --git a/src/Core/Validation/FieldValidation/OptionFieldValidator.php b/src/Core/Validation/FieldValidation/OptionFieldValidator.php index dd514570743..96f024ab886 100644 --- a/src/Core/Validation/FieldValidation/OptionFieldValidator.php +++ b/src/Core/Validation/FieldValidation/OptionFieldValidator.php @@ -3,7 +3,6 @@ namespace SilverStripe\Core\Validation\FieldValidation; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\FieldValidator; /** * Validates that a value is one of a set of options @@ -31,8 +30,12 @@ protected function validateValue(): ValidationResult protected function checkValueInOptions(mixed $value, ValidationResult $result): void { if (!in_array($value, $this->options, true)) { - $message = _t(__CLASS__ . '.NOTALLOWED', 'Not an allowed value'); - $result->addFieldError($this->name, $message); + $result->addFieldError($this->name, $this->getMessage()); } } + + protected function getMessage(): string + { + return _t(__CLASS__ . '.NOTALLOWED', 'Not an allowed value'); + } } diff --git a/src/Core/Validation/FieldValidation/StringFieldValidator.php b/src/Core/Validation/FieldValidation/StringFieldValidator.php index 9878bf8a60c..34494cbd255 100644 --- a/src/Core/Validation/FieldValidation/StringFieldValidator.php +++ b/src/Core/Validation/FieldValidation/StringFieldValidator.php @@ -4,7 +4,6 @@ use InvalidArgumentException; use SilverStripe\Core\Validation\ValidationResult; -use SilverStripe\Core\Validation\FieldValidation\FieldValidator; /** * Validates that a value is a string and optionally checks its multi-byte length. @@ -61,7 +60,7 @@ protected function validateValue(): ValidationResult if (!is_null($this->maxLength) && $len > $this->maxLength) { $message = _t( __CLASS__ . '.TOOLONG', - 'Can not have more than {maxLength} characters', + 'Cannot have more than {maxLength} characters', ['maxLength' => $this->maxLength] ); $result->addFieldError($this->name, $message); diff --git a/src/Core/Validation/FieldValidation/SymfonyFieldValidatorTrait.php b/src/Core/Validation/FieldValidation/SymfonyFieldValidatorTrait.php index c44fb3d75aa..f80cf78f1b7 100644 --- a/src/Core/Validation/FieldValidation/SymfonyFieldValidatorTrait.php +++ b/src/Core/Validation/FieldValidation/SymfonyFieldValidatorTrait.php @@ -5,7 +5,6 @@ use LogicException; use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\Core\Validation\ConstraintValidator; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorInterface; /** * Trait for FieldValidators which validate using Symfony constraints diff --git a/src/Core/Validation/FieldValidation/TimeFieldValidator.php b/src/Core/Validation/FieldValidation/TimeFieldValidator.php index 3af1f83b009..3b1e0727289 100644 --- a/src/Core/Validation/FieldValidation/TimeFieldValidator.php +++ b/src/Core/Validation/FieldValidation/TimeFieldValidator.php @@ -2,8 +2,6 @@ namespace SilverStripe\Core\Validation\FieldValidation; -use SilverStripe\Core\Validation\FieldValidation\DateFieldValidator; - /** * Validates that a value is a valid time, which means that it follows the equivalent formats: * - PHP date format H:i:s diff --git a/src/Core/Validation/FieldValidation/UrlFieldValidator.php b/src/Core/Validation/FieldValidation/UrlFieldValidator.php index 748ed16b9c3..0c7ef8beaaf 100644 --- a/src/Core/Validation/FieldValidation/UrlFieldValidator.php +++ b/src/Core/Validation/FieldValidation/UrlFieldValidator.php @@ -4,9 +4,6 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\Url; -use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorTrait; -use SilverStripe\Core\Validation\FieldValidation\SymfonyFieldValidatorInterface; /** * Validates that a value is a valid URL diff --git a/src/Forms/CompositeField.php b/src/Forms/CompositeField.php index d4f2aef64ce..11c48c34544 100644 --- a/src/Forms/CompositeField.php +++ b/src/Forms/CompositeField.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms; +use SilverStripe\Core\Validation\FieldValidation\CompositeFieldValidator; use SilverStripe\Dev\Debug; /** @@ -13,6 +14,9 @@ */ class CompositeField extends FormField { + private static array $field_validators = [ + CompositeFieldValidator::class, + ]; /** * @var FieldList @@ -63,7 +67,6 @@ public function __construct($children = null) $children = new FieldList($children); } $this->setChildren($children); - parent::__construct(null, false); } @@ -115,12 +118,17 @@ public function getChildren() return $this->children; } + public function getValueForValidation(): mixed + { + return $this->getChildren()->toArray(); + } + /** * Returns the name (ID) for the element. * If the CompositeField doesn't have a name, but we still want the ID/name to be set. * This code generates the ID from the nested children. */ - public function getName() + public function getName(): string { if ($this->name) { return $this->name; @@ -266,8 +274,6 @@ public function setForm($form) return $this; } - - public function setDisabled($disabled) { parent::setDisabled($disabled); @@ -522,19 +528,4 @@ public function debug(): string $result .= ""; return $result; } - - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - $valid = true; - foreach ($this->children as $child) { - $valid = ($child && $child->validate($validator) && $valid); - } - return $this->extendValidationResult($valid, $validator); - } } diff --git a/src/Forms/ConfirmedPasswordField.php b/src/Forms/ConfirmedPasswordField.php index cef12f589d1..34a084be5a8 100644 --- a/src/Forms/ConfirmedPasswordField.php +++ b/src/Forms/ConfirmedPasswordField.php @@ -12,6 +12,7 @@ use Closure; use SilverStripe\Core\Validation\ConstraintValidator; use Symfony\Component\Validator\Constraints\PasswordStrength; +use SilverStripe\Core\Validation\ValidationResult; /** * Two masked input fields, checks for matching passwords. @@ -420,155 +421,145 @@ public function isSaveable() || ($this->showOnClick && $this->hiddenField && $this->hiddenField->Value()); } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) + public function validate(): ValidationResult { - $name = $this->name; - - // if field isn't visible, don't validate + // If the field isn't visible, then do not validate it if (!$this->isSaveable()) { - return $this->extendValidationResult(true, $validator); - } - - $this->getPasswordField()->setValue($this->value); - $this->getConfirmPasswordField()->setValue($this->confirmValue); - $value = $this->getPasswordField()->Value(); - - // both password-fields should be the same - if ($value != $this->getConfirmPasswordField()->Value()) { - $validator->validationError( - $name, - _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSDONTMATCH', "Passwords don't match"), - "validation" - ); - - return $this->extendValidationResult(false, $validator); - } - - if (!$this->canBeEmpty) { - // both password-fields shouldn't be empty - if (!$value || !$this->getConfirmPasswordField()->Value()) { - $validator->validationError( - $name, - _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSNOTEMPTY', "Passwords can't be empty"), - "validation" - ); - - return $this->extendValidationResult(false, $validator); - } + return parent::validate(); } - - // lengths - $minLength = $this->getMinLength(); - $maxLength = $this->getMaxLength(); - if ($minLength || $maxLength) { - $errorMsg = null; - $limit = null; - if ($minLength && $maxLength) { - $limit = "{{$minLength},{$maxLength}}"; - $errorMsg = _t( - __CLASS__ . '.BETWEEN', - 'Passwords must be {min} to {max} characters long.', - ['min' => $minLength, 'max' => $maxLength] - ); - } elseif ($minLength) { - $limit = "{{$minLength}}.*"; - $errorMsg = _t( - __CLASS__ . '.ATLEAST', - 'Passwords must be at least {min} characters long.', - ['min' => $minLength] - ); - } elseif ($maxLength) { - $limit = "{0,{$maxLength}}"; - $errorMsg = _t( - __CLASS__ . '.MAXIMUM', - 'Passwords must be at most {max} characters long.', - ['max' => $maxLength] - ); - } - $limitRegex = '/^.' . $limit . '$/'; - if (!empty($value) && !preg_match($limitRegex ?? '', $value ?? '')) { - $validator->validationError( + $this->beforeExtending('updateValidate', function (ValidationResult $result) { + $name = $this->name; + + $this->getPasswordField()->setValue($this->value); + $this->getConfirmPasswordField()->setValue($this->confirmValue); + $value = $this->getPasswordField()->Value(); + + // both password-fields should be the same + if ($value != $this->getConfirmPasswordField()->Value()) { + $result->addFieldError( $name, - $errorMsg, + _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSDONTMATCH', "Passwords don't match"), "validation" ); - - return $this->extendValidationResult(false, $validator); + return; } - } - - if ($this->getRequireStrongPassword()) { - $strongEnough = ConstraintValidator::validate( - $value, - new PasswordStrength(minScore: $this->getMinPasswordStrength()) - )->isValid(); - if (!$strongEnough) { - $validator->validationError( - $name, - _t( - __CLASS__ . '.VALIDATIONSTRONGPASSWORD', - 'The password strength is too low. Please use a stronger password.' - ), - 'validation' - ); - - return $this->extendValidationResult(false, $validator); + + if (!$this->canBeEmpty) { + // both password-fields shouldn't be empty + if (!$value || !$this->getConfirmPasswordField()->Value()) { + $result->addFieldError( + $name, + _t('SilverStripe\\Forms\\Form.VALIDATIONPASSWORDSNOTEMPTY', "Passwords can't be empty"), + "validation" + ); + return; + } } - } - - // Check if current password is valid - if (!empty($value) && $this->getRequireExistingPassword()) { - if (!$this->currentPasswordValue) { - $validator->validationError( - $name, - _t( - __CLASS__ . '.CURRENT_PASSWORD_MISSING', - 'You must enter your current password.' - ), - "validation" - ); - return $this->extendValidationResult(false, $validator); + + // lengths + $minLength = $this->getMinLength(); + $maxLength = $this->getMaxLength(); + if ($minLength || $maxLength) { + $errorMsg = null; + $limit = null; + if ($minLength && $maxLength) { + $limit = "{{$minLength},{$maxLength}}"; + $errorMsg = _t( + __CLASS__ . '.BETWEEN', + 'Passwords must be {min} to {max} characters long.', + ['min' => $minLength, 'max' => $maxLength] + ); + } elseif ($minLength) { + $limit = "{{$minLength}}.*"; + $errorMsg = _t( + __CLASS__ . '.ATLEAST', + 'Passwords must be at least {min} characters long.', + ['min' => $minLength] + ); + } elseif ($maxLength) { + $limit = "{0,{$maxLength}}"; + $errorMsg = _t( + __CLASS__ . '.MAXIMUM', + 'Passwords must be at most {max} characters long.', + ['max' => $maxLength] + ); + } + $limitRegex = '/^.' . $limit . '$/'; + if (!empty($value) && !preg_match($limitRegex ?? '', $value ?? '')) { + $result->addFieldError( + $name, + $errorMsg, + "validation" + ); + return; + } } - - // Check this password is valid for the current user - $member = Security::getCurrentUser(); - if (!$member) { - $validator->validationError( - $name, - _t( - __CLASS__ . '.LOGGED_IN_ERROR', - "You must be logged in to change your password." - ), - "validation" - ); - return $this->extendValidationResult(false, $validator); + + if ($this->getRequireStrongPassword()) { + $strongEnough = ConstraintValidator::validate( + $value, + new PasswordStrength(minScore: $this->getMinPasswordStrength()) + )->isValid(); + if (!$strongEnough) { + $result->addFieldError( + $name, + _t( + __CLASS__ . '.VALIDATIONSTRONGPASSWORD', + 'The password strength is too low. Please use a stronger password.' + ), + 'validation' + ); + return; + } } - - // With a valid user and password, check the password is correct - $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHECK_PASSWORD); - foreach ($authenticators as $authenticator) { - $checkResult = $authenticator->checkPassword($member, $this->currentPasswordValue); - if (!$checkResult->isValid()) { - $validator->validationError( + + // Check if current password is valid + if (!empty($value) && $this->getRequireExistingPassword()) { + if (!$this->currentPasswordValue) { + $result->addFieldError( $name, _t( - __CLASS__ . '.CURRENT_PASSWORD_ERROR', - "The current password you have entered is not correct." + __CLASS__ . '.CURRENT_PASSWORD_MISSING', + 'You must enter your current password.' ), "validation" ); - return $this->extendValidationResult(false, $validator); + return; + } + + // Check this password is valid for the current user + $member = Security::getCurrentUser(); + if (!$member) { + $result->addFieldError( + $name, + _t( + __CLASS__ . '.LOGGED_IN_ERROR', + "You must be logged in to change your password." + ), + "validation" + ); + return; + } + + // With a valid user and password, check the password is correct + $authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::CHECK_PASSWORD); + foreach ($authenticators as $authenticator) { + $checkResult = $authenticator->checkPassword($member, $this->currentPasswordValue); + if (!$checkResult->isValid()) { + $result->addFieldError( + $name, + _t( + __CLASS__ . '.CURRENT_PASSWORD_ERROR', + "The current password you have entered is not correct." + ), + "validation" + ); + return; + } } } - } - - return $this->extendValidationResult(true, $validator); + }); + return parent::validate(); } /** diff --git a/src/Forms/CurrencyField.php b/src/Forms/CurrencyField.php index 5d36cf8edb3..4546075dff3 100644 --- a/src/Forms/CurrencyField.php +++ b/src/Forms/CurrencyField.php @@ -3,6 +3,7 @@ namespace SilverStripe\Forms; use SilverStripe\ORM\FieldType\DBCurrency; +use SilverStripe\Core\Validation\ValidationResult; /** * Renders a text field, validating its input as a currency. @@ -54,21 +55,20 @@ public function performReadonlyTransformation() return $this->castedCopy(CurrencyField_Readonly::class); } - public function validate($validator) + public function validate(): ValidationResult { - $result = true; - $currencySymbol = preg_quote(DBCurrency::config()->uninherited('currency_symbol') ?? ''); - $regex = '/^\s*(\-?' . $currencySymbol . '?|' . $currencySymbol . '\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/'; - if (!empty($this->value) && !preg_match($regex ?? '', $this->value ?? '')) { - $validator->validationError( - $this->name, - _t('SilverStripe\\Forms\\Form.VALIDCURRENCY', "Please enter a valid currency"), - "validation" - ); - $result = false; - } - - return $this->extendValidationResult($result, $validator); + $this->beforeExtending('updateValidate', function (ValidationResult $result) { + $currencySymbol = preg_quote(DBCurrency::config()->uninherited('currency_symbol') ?? ''); + $regex = '/^\s*(\-?' . $currencySymbol . '?|' . $currencySymbol . '\-?)?(\d{1,3}(\,\d{3})*|(\d+))(\.\d{2})?\s*$/'; + if (!empty($this->value) && !preg_match($regex ?? '', $this->value ?? '')) { + $result->addFieldError( + $this->name, + _t('SilverStripe\\Forms\\Form.VALIDCURRENCY', "Please enter a valid currency"), + "validation" + ); + } + }); + return parent::validate(); } public function getSchemaValidation() diff --git a/src/Forms/DateField.php b/src/Forms/DateField.php index 4bbc9ed4d5d..914b20f72ea 100644 --- a/src/Forms/DateField.php +++ b/src/Forms/DateField.php @@ -8,6 +8,10 @@ use SilverStripe\ORM\FieldType\DBDate; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Core\Validation\ValidationResult; +use SilverStripe\Core\Validation\FieldValidation\DateFieldValidator; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterInterface; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterTrait; +use PHPUnit\Framework\Attributes\DataProvider; /** * Form used for editing a date string @@ -54,8 +58,17 @@ * @see https://unicode-org.github.io/icu/userguide/format_parse/datetime * @see http://api.jqueryui.com/datepicker/#utility-formatDate */ -class DateField extends TextField +class DateField extends TextField implements FieldValidatorConverterInterface { + use FieldValidatorConverterTrait; + + private static array $field_validators = [ + DateFieldValidator::class => [ + 'minValue' => 'getMinDate', + 'maxValue' => 'getMaxDate', + 'converter' => 'getFieldValidatorConverter', + ], + ]; protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DATE; /** @@ -96,15 +109,6 @@ class DateField extends TextField */ protected $maxDate = null; - /** - * Unparsed value, used exclusively for comparing with internal value - * to detect invalid values. - * - * @var mixed - * @deprecated 5.4.0 Use $value instead - */ - protected $rawValue = null; - /** * Use HTML5-based input fields (and force ISO 8601 date formats). * @@ -307,9 +311,6 @@ public function Type() */ public function setSubmittedValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - // Null case if (!$value) { $this->value = null; @@ -332,17 +333,13 @@ public function setSubmittedValue($value, $data = null) */ public function setValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - - // Null case - if (!$value) { + if (is_null($value)) { $this->value = null; return $this; } // Re-run through formatter to tidy up (e.g. remove time component) - $this->value = $this->tidyInternal($value); + $this->value = $this->tidyInternal($value, false); return $this; } @@ -351,6 +348,16 @@ public function Value() return $this->internalToFrontend($this->value); } + public function getValueForValidation(): mixed + { + // new DateField('MyDate') ... getValue() will return empty string + // return null so that validation is skipped + if ($this->getValue() === '') { + return null; + } + return $this->value; + } + public function performReadonlyTransformation() { $field = $this @@ -362,83 +369,6 @@ public function performReadonlyTransformation() return $field; } - /** - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - // Don't validate empty fields - if (empty($this->rawValue)) { - return $this->extendValidationResult(true, $validator); - } - - // We submitted a value, but it couldn't be parsed - if (empty($this->value)) { - $validator->validationError( - $this->name, - _t( - 'SilverStripe\\Forms\\DateField.VALIDDATEFORMAT2', - "Please enter a valid date format ({format})", - ['format' => $this->getDateFormat()] - ) - ); - return $this->extendValidationResult(false, $validator); - } - - // Check min date - $min = $this->getMinDate(); - if ($min) { - $oops = strtotime($this->value ?? '') < strtotime($min ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - 'SilverStripe\\Forms\\DateField.VALIDDATEMINDATE', - "Your date has to be newer or matching the minimum allowed date ({date})", - [ - 'date' => sprintf( - '', - $min, - $this->internalToFrontend($min) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - // Check max date - $max = $this->getMaxDate(); - if ($max) { - $oops = strtotime($this->value ?? '') > strtotime($max ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - 'SilverStripe\\Forms\\DateField.VALIDDATEMAXDATE', - "Your date has to be older or matching the maximum allowed date ({date})", - [ - 'date' => sprintf( - '', - $max, - $this->internalToFrontend($max) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - return $this->extendValidationResult(true, $validator); - } - /** * Get locale to use for this field * @@ -488,7 +418,7 @@ public function getMinDate() */ public function setMinDate($minDate) { - $this->minDate = $this->tidyInternal($minDate); + $this->minDate = $this->tidyInternal($minDate, true); return $this; } @@ -506,7 +436,7 @@ public function getMaxDate() */ public function setMaxDate($maxDate) { - $this->maxDate = $this->tidyInternal($maxDate); + $this->maxDate = $this->tidyInternal($maxDate, true); return $this; } @@ -536,13 +466,12 @@ protected function frontendToInternal($date) * as defined by {@link $dateFormat}. With $html5=true, the frontend date will also be * in ISO 8601. * - * @param string $date * @return string The formatted date, or null if not a valid date */ - protected function internalToFrontend($date) + public function internalToFrontend(mixed $value): ?string { - $date = $this->tidyInternal($date); - if (!$date) { + $date = $this->tidyInternal($value, true); + if (is_null($date)) { return null; } $fromFormatter = $this->getInternalFormatter(); @@ -558,12 +487,13 @@ protected function internalToFrontend($date) * Tidy up the internal date representation (ISO 8601), * and fall back to strtotime() if there's parsing errors. * - * @param string $date Date in ISO 8601 or approximate form - * @return string ISO 8601 date, or null if not valid + * @param mixed $date Date in ISO 8601 or approximate form + * @param bool $returnNullOnFailure return null if the date is not valid, or the original date + * @return null|string ISO 8601 date or null */ - protected function tidyInternal($date) + protected function tidyInternal(mixed $date, bool $returnNullOnFailure): ?string { - if (!$date) { + if (is_null($date)) { return null; } // Re-run through formatter to tidy up (e.g. remove time component) @@ -573,7 +503,7 @@ protected function tidyInternal($date) // Fallback to strtotime $timestamp = strtotime($date ?? '', DBDatetime::now()->getTimestamp()); if ($timestamp === false) { - return null; + return $returnNullOnFailure ? null : $date; } } return $formatter->format($timestamp); diff --git a/src/Forms/DateField_Disabled.php b/src/Forms/DateField_Disabled.php index 6bdc64f882e..be56d92ea5a 100644 --- a/src/Forms/DateField_Disabled.php +++ b/src/Forms/DateField_Disabled.php @@ -22,26 +22,27 @@ public function Field($properties = []) $value = $this->dataValue(); if ($value) { - $value = $this->tidyInternal($value); + $value = $this->tidyInternal($value, true); $df = new DBDate($this->name); $df->setValue($value); - - if ($df->IsToday()) { - // e.g. 2018-06-01 (today) - $format = '%s (%s)'; - $infoComplement = _t('SilverStripe\\Forms\\DateField.TODAY', 'today'); - } else { - // e.g. 2018-06-01, 5 days ago - $format = '%s, %s'; - $infoComplement = $df->Ago(); + // if the date is not valid, $df->getValue() will return false + if ($df->getValue()) { + if ($df->IsToday()) { + // e.g. 2018-06-01 (today) + $format = '%s (%s)'; + $infoComplement = _t('SilverStripe\\Forms\\DateField.TODAY', 'today'); + } else { + // e.g. 2018-06-01, 5 days ago + $format = '%s, %s'; + $infoComplement = $df->Ago(); + } + // Render the display value with some complement of info + $displayValue = Convert::raw2xml(sprintf( + $format ?? '', + $this->Value(), + $infoComplement + )); } - - // Render the display value with some complement of info - $displayValue = Convert::raw2xml(sprintf( - $format ?? '', - $this->Value(), - $infoComplement - )); } return sprintf( diff --git a/src/Forms/DatetimeField.php b/src/Forms/DatetimeField.php index b8e4a7b0985..cfa1beb012c 100644 --- a/src/Forms/DatetimeField.php +++ b/src/Forms/DatetimeField.php @@ -7,7 +7,9 @@ use SilverStripe\i18n\i18n; use SilverStripe\ORM\FieldType\DBDate; use SilverStripe\ORM\FieldType\DBDatetime; -use SilverStripe\Core\Validation\ValidationResult; +use SilverStripe\Core\Validation\FieldValidation\DatetimeFieldValidator; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterTrait; +use SilverStripe\Forms\FieldValidatorConverter\FieldValidatorConverterInterface; /** * Form field used for editing date time strings. @@ -16,8 +18,17 @@ * Data is passed on via {@link dataValue()} with whitespace separators. * The {@link $value} property is always in ISO 8601 format, in the server timezone. */ -class DatetimeField extends TextField +class DatetimeField extends TextField implements FieldValidatorConverterInterface { + use FieldValidatorConverterTrait; + + private static array $field_validators = [ + DatetimeFieldValidator::class => [ + 'minValue' => 'getMinDatetime', + 'maxValue' => 'getMaxDatetime', + 'converter' => 'getFieldValidatorConverter' + ], + ]; /** * @var bool @@ -70,15 +81,6 @@ class DatetimeField extends TextField */ protected $timeLength = null; - /** - * Unparsed value, used exclusively for comparing with internal value - * to detect invalid values. - * - * @var mixed - * @deprecated 5.4.0 Use $value instead - */ - protected $rawValue = null; - /** * @inheritDoc */ @@ -159,9 +161,6 @@ public function setHTML5($bool) */ public function setSubmittedValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - // Null case if (!$value) { $this->value = null; @@ -325,11 +324,7 @@ protected function getInternalFormatter($timezone = null) */ public function setValue($value, $data = null) { - // Save raw value for later validation - $this->rawValue = $value; - - // Empty value - if (empty($value)) { + if (is_null($value)) { $this->value = null; return $this; } @@ -347,6 +342,7 @@ public function setValue($value, $data = null) } if ($timestamp === false) { + $this->value = $value; return $this; } @@ -371,17 +367,26 @@ public function Value() return $this->internalToFrontend($this->value); } + public function getValueForValidation(): mixed + { + // new DatetimeField('MyDatetime') ... getValue() will return empty string + // return null so that validation is skipped + if ($this->getValue() === '') { + return null; + } + return $this->value; + } + /** * Convert the internal date representation (ISO 8601) to a format used by the frontend, * as defined by {@link $dateFormat}. With $html5=true, the frontend date will also be * in ISO 8601. * - * @param string $datetime - * @return string The formatted date and time, or null if not a valid date and time + * @return null|string The formatted date and time, or null if not a valid date and time */ - public function internalToFrontend($datetime) + public function internalToFrontend(mixed $value): ?string { - $datetime = $this->tidyInternal($datetime); + $datetime = $this->tidyInternal($value, true); if (!$datetime) { return null; } @@ -399,22 +404,23 @@ public function internalToFrontend($datetime) * Tidy up the internal date representation (ISO 8601), * and fall back to strtotime() if there's parsing errors. * - * @param string $datetime Date in ISO 8601 or approximate form - * @return string ISO 8601 date, or null if not valid + * @param mixed $datetime Date in ISO 8601 or approximate form + * @param bool $returnNullOnFailure return null if the datetime is not valid, or the original datetime + * @return null|string ISO 8601 date or null */ - public function tidyInternal($datetime) + public function tidyInternal(mixed $datetime, bool $returnNullOnFailure): ?string { - if (!$datetime) { + if (is_null($datetime)) { return null; } - // Re-run through formatter to tidy up (e.g. remove time component) + // Re-run through formatter to tidy up $formatter = $this->getInternalFormatter(); $timestamp = $formatter->parse($datetime); if ($timestamp === false) { // Fallback to strtotime $timestamp = strtotime($datetime ?? '', DBDatetime::now()->getTimestamp()); if ($timestamp === false) { - return null; + return $returnNullOnFailure ? null : $datetime; } } return $formatter->format($timestamp); @@ -536,7 +542,7 @@ public function getMinDatetime() */ public function setMinDatetime($minDatetime) { - $this->minDatetime = $this->tidyInternal($minDatetime); + $this->minDatetime = $this->tidyInternal($minDatetime, true); return $this; } @@ -554,87 +560,10 @@ public function getMaxDatetime() */ public function setMaxDatetime($maxDatetime) { - $this->maxDatetime = $this->tidyInternal($maxDatetime); + $this->maxDatetime = $this->tidyInternal($maxDatetime, true); return $this; } - /** - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - // Don't validate empty fields - if (empty($this->rawValue)) { - return $this->extendValidationResult(true, $validator); - } - - // We submitted a value, but it couldn't be parsed - if (empty($this->value)) { - $validator->validationError( - $this->name, - _t( - __CLASS__ . '.VALIDDATETIMEFORMAT', - "Please enter a valid date and time format ({format})", - ['format' => $this->getDatetimeFormat()] - ) - ); - return $this->extendValidationResult(false, $validator); - } - - // Check min date (in server timezone) - $min = $this->getMinDatetime(); - if ($min) { - $oops = strtotime($this->value ?? '') < strtotime($min ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - __CLASS__ . '.VALIDDATETIMEMINDATE', - "Your date has to be newer or matching the minimum allowed date and time ({datetime})", - [ - 'datetime' => sprintf( - '', - $min, - $this->internalToFrontend($min) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - // Check max date (in server timezone) - $max = $this->getMaxDatetime(); - if ($max) { - $oops = strtotime($this->value ?? '') > strtotime($max ?? ''); - if ($oops) { - $validator->validationError( - $this->name, - _t( - __CLASS__ . '.VALIDDATEMAXDATETIME', - "Your date has to be older or matching the maximum allowed date and time ({datetime})", - [ - 'datetime' => sprintf( - '', - $max, - $this->internalToFrontend($max) - ) - ] - ), - ValidationResult::TYPE_ERROR, - ValidationResult::CAST_HTML - ); - return $this->extendValidationResult(false, $validator); - } - } - - return $this->extendValidationResult(true, $validator); - } - public function performReadonlyTransformation() { $field = clone $this; diff --git a/src/Forms/EmailField.php b/src/Forms/EmailField.php index 4426d67f8c6..752f047756b 100644 --- a/src/Forms/EmailField.php +++ b/src/Forms/EmailField.php @@ -2,14 +2,17 @@ namespace SilverStripe\Forms; -use SilverStripe\Core\Validation\ConstraintValidator; -use Symfony\Component\Validator\Constraints\Email as EmailConstraint; +use SilverStripe\Core\Validation\FieldValidation\EmailFieldValidator; /** * Text input field with validation for correct email format according to the relevant RFC. */ class EmailField extends TextField { + private static array $field_validators = [ + EmailFieldValidator::class, + ]; + protected $inputType = 'email'; public function Type() @@ -17,27 +20,6 @@ public function Type() return 'email text'; } - /** - * Validates for RFC compliant email addresses. - * - * @param Validator $validator - */ - public function validate($validator) - { - $this->value = trim($this->value ?? ''); - - $message = _t('SilverStripe\\Forms\\EmailField.VALIDATION', 'Please enter an email address'); - $result = ConstraintValidator::validate( - $this->value, - new EmailConstraint(message: $message, mode: EmailConstraint::VALIDATION_MODE_STRICT), - $this->getName() - ); - $validator->getResult()->combineAnd($result); - $isValid = $result->isValid(); - - return $this->extendValidationResult($isValid, $validator); - } - public function getSchemaValidation() { $rules = parent::getSchemaValidation(); diff --git a/src/Forms/FieldGroup.php b/src/Forms/FieldGroup.php index c61de2136b9..7e1e77b04d6 100644 --- a/src/Forms/FieldGroup.php +++ b/src/Forms/FieldGroup.php @@ -106,7 +106,7 @@ public function __construct($titleOrField = null, $otherFields = null) * In some cases the FieldGroup doesn't have a title, but we still want * the ID / name to be set. This code, generates the ID from the nested children */ - public function getName() + public function getName(): string { if ($this->name) { return $this->name; diff --git a/src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php b/src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php new file mode 100644 index 00000000000..29fd085602e --- /dev/null +++ b/src/Forms/FieldValidatorConverter/FieldValidatorConverterInterface.php @@ -0,0 +1,10 @@ +internalToFrontend($value); + }; + } +} diff --git a/src/Forms/FieldsValidator.php b/src/Forms/FieldsValidator.php index 0de96225fdf..504cf6157f8 100644 --- a/src/Forms/FieldsValidator.php +++ b/src/Forms/FieldsValidator.php @@ -9,14 +9,11 @@ class FieldsValidator extends Validator { public function php($data): bool { - $valid = true; $fields = $this->form->Fields(); - foreach ($fields as $field) { - $valid = ($field->validate($this) && $valid); + $this->result->combineAnd($field->validate()); } - - return $valid; + return $this->result->isValid(); } public function canBeCached(): bool diff --git a/src/Forms/FileField.php b/src/Forms/FileField.php index 8040d1fd594..a4db0fba7c3 100644 --- a/src/Forms/FileField.php +++ b/src/Forms/FileField.php @@ -7,6 +7,7 @@ use SilverStripe\Core\Injector\Injector; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; +use SilverStripe\Core\Validation\ValidationResult; /** * Represents a file type which can be added to a form. @@ -176,59 +177,48 @@ public function Value() return isset($_FILES[$this->getName()]) ? $_FILES[$this->getName()] : null; } - public function validate($validator) + public function validate(): ValidationResult { - // FileField with the name multi_file_syntax[] or multi_file_syntax[key] will have the brackets trimmed in - // $_FILES super-global so it will be stored as $_FILES['mutli_file_syntax'] - // multi-file uploads, which are not officially supported by Silverstripe, though may be - // implemented in custom code, so we should still ensure they are at least validated - $isMultiFileUpload = strpos($this->name ?? '', '[') !== false; - $fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name ?? ''); - - if (!isset($_FILES[$fieldName])) { - return $this->extendValidationResult(true, $validator); - } - - if ($isMultiFileUpload) { - $isValid = true; - foreach (array_keys($_FILES[$fieldName]['name'] ?? []) as $key) { - $fileData = [ - 'name' => $_FILES[$fieldName]['name'][$key], - 'type' => $_FILES[$fieldName]['type'][$key], - 'tmp_name' => $_FILES[$fieldName]['tmp_name'][$key], - 'error' => $_FILES[$fieldName]['error'][$key], - 'size' => $_FILES[$fieldName]['size'][$key], - ]; - if (!$this->validateFileData($validator, $fileData)) { - $isValid = false; + $this->beforeExtending('updateValidate', function (ValidationResult $result) { + // FileField with the name multi_file_syntax[] or multi_file_syntax[key] will have the brackets trimmed in + // $_FILES super-global so it will be stored as $_FILES['mutli_file_syntax'] + // multi-file uploads, which are not officially supported by Silverstripe, though may be + // implemented in custom code, so we should still ensure they are at least validated + $isMultiFileUpload = strpos($this->name ?? '', '[') !== false; + $fieldName = preg_replace('#\[(.*?)\]$#', '', $this->name ?? ''); + if (!isset($_FILES[$fieldName])) { + return; + } + if ($isMultiFileUpload) { + foreach (array_keys($_FILES[$fieldName]['name'] ?? []) as $key) { + $fileData = [ + 'name' => $_FILES[$fieldName]['name'][$key], + 'type' => $_FILES[$fieldName]['type'][$key], + 'tmp_name' => $_FILES[$fieldName]['tmp_name'][$key], + 'error' => $_FILES[$fieldName]['error'][$key], + 'size' => $_FILES[$fieldName]['size'][$key], + ]; + $this->validateFileData($result, $fileData); } + return; } - return $this->extendValidationResult($isValid, $validator); - } - - // regular single-file upload - $result = $this->validateFileData($validator, $_FILES[$this->name]); - return $this->extendValidationResult($result, $validator); + // regular single-file upload + $this->validateFileData($result, $_FILES[$this->name]); + }); + return parent::validate(); } - /** - * @param Validator $validator - * @param array $fileData - * @return bool - */ - private function validateFileData($validator, array $fileData): bool + private function validateFileData(ValidationResult $result, array $fileData): void { $valid = $this->upload->validate($fileData); if (!$valid) { $errors = $this->upload->getErrors(); if ($errors) { foreach ($errors as $error) { - $validator->validationError($this->name, $error, "validation"); + $result->addFieldError($this->name, $error, "validation"); } } - return false; } - return true; } /** diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index fe953afba8d..3ec6d06eb7a 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -16,7 +16,8 @@ use SilverStripe\View\SSViewer; use SilverStripe\Model\ModelData; use SilverStripe\ORM\DataObject; -use SilverStripe\Dev\Deprecation; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationTrait; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationInterface; /** * Represents a field in a form. @@ -42,10 +43,11 @@ * including both structure (name, id, attributes, etc.) and state (field value). * Can be used by for JSON data which is consumed by a front-end application. */ -class FormField extends RequestHandler +class FormField extends RequestHandler implements FieldValidationInterface { use AttributesHTML; use FormMessage; + use FieldValidationTrait; /** @see $schemaDataType */ const SCHEMA_DATA_TYPE_STRING = 'String'; @@ -429,9 +431,9 @@ public function getTemplateHelper() /** * Returns the field name. */ - public function getName() + public function getName(): string { - return $this->name; + return (string) $this->name; } /** @@ -445,9 +447,17 @@ public function getInputType() } /** - * Returns the field value. + * Returns the internal value of the field + */ + public function getValue(): mixed + { + return $this->value; + } + + /** + * Returns the value of the field which may be modified for display purposes + * for instance to add localisation or formatting. * - * @see FormField::setSubmittedValue() * @return mixed */ public function Value() @@ -455,6 +465,16 @@ public function Value() return $this->value; } + /** + * Returns the value of the field suitable for insertion into the database. + * + * @return mixed + */ + public function dataValue() + { + return $this->value; + } + /** * Method to save this form field into the given record. * @@ -483,16 +503,6 @@ public function saveInto(DataObjectInterface $record) } } - /** - * Returns the field value suitable for insertion into the data object. - * @see Formfield::setValue() - * @return mixed - */ - public function dataValue() - { - return $this->value; - } - /** * Returns the field label - used by templates. * @@ -706,6 +716,9 @@ public function attrValue() */ public function setValue($value, $data = null) { + // Note that unlike setSubmittedValue(), we are explicity not casting blank strings to null here. + // This is because setValue() is used when programatically setting a value on a field + // and we want to enforce type strictness $this->value = $value; return $this; } @@ -721,6 +734,13 @@ public function setValue($value, $data = null) */ public function setSubmittedValue($value, $data = null) { + // Most form sumissions will be strings, this includes for fields whose backing DBField + // has a numeric type, such as DBInt and DBFloat. + // FormFields are not aware of the DBField type they are backed by + // Cast blank strings to null so they don't fail DBField validation on numeric DBFields + if ($value === '') { + $value = null; + } return $this->setValue($value, $data); } @@ -1222,32 +1242,19 @@ public function Type() } /** - * 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 - * @deprecated 5.4.0 Use extend() directly instead + * Determines whether the field is valid or not based on its value */ - protected function extendValidationResult(bool $result, Validator $validator): bool + public function validate(): ValidationResult { - Deprecation::notice('5.4.0', 'Use extend() directly instead'); - $this->extend('updateValidationResult', $result, $validator); + $result = ValidationResult::create(); + $fieldValidators = $this->getFieldValidators(); + foreach ($fieldValidators as $fieldValidator) { + $result->combineAnd($fieldValidator->validate()); + } + $this->extend('updateValidate', $result); return $result; } - /** - * Abstract method each {@link FormField} subclass must implement, determines whether the field - * is valid or not based on the value. - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - return $this->extendValidationResult(true, $validator); - } - /** * Describe this field, provide help text for it. * diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index a00aab521cf..563586e70ef 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -131,9 +131,6 @@ class GridField extends FormField */ protected $customDataFields = []; - /** - * @var string - */ protected $name = ''; /** diff --git a/src/Forms/ListboxField.php b/src/Forms/ListboxField.php index 5ff18afff0c..0a2112cae08 100644 --- a/src/Forms/ListboxField.php +++ b/src/Forms/ListboxField.php @@ -165,6 +165,17 @@ public function getDisabledItems() return $this->disabledItems; } + public function getValueForValidation(): mixed + { + // Unlike other MultiSelectField's, ListboxField allows setting a single value without wrapping it in an array. + // Ensure values are wrapped in an array here so that the validation logic can treat it as a multi-select + $value = parent::getValueForValidation(); + if (!is_array($value) && !is_null($value)) { + $value = [$value]; + } + return $value; + } + /** * Provide ListboxField data to the JSON schema for the frontend component * @@ -256,7 +267,7 @@ public function getValueArray() $replaced = []; foreach ($value as $item) { if (!is_array($item)) { - $item = json_decode($item, true); + $item = json_decode($item ?? '', true); } if ($targetType === gettype($item)) { diff --git a/src/Forms/LookupField.php b/src/Forms/LookupField.php index ac388fc394b..1cb0b3e256d 100644 --- a/src/Forms/LookupField.php +++ b/src/Forms/LookupField.php @@ -6,6 +6,7 @@ use SilverStripe\Core\ArrayLib; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\Core\Validation\FieldValidation\MultiOptionFieldValidator; /** * Read-only complement of {@link MultiSelectField}. @@ -15,6 +16,10 @@ */ class LookupField extends MultiSelectField { + private static $field_validators = [ + MultiOptionFieldValidator::class => null, + ]; + protected $schemaComponent = 'LookupField'; /** @@ -65,17 +70,6 @@ public function Field($properties = []) return parent::Field($properties); } - /** - * Ignore validation as the field is readonly - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - return $this->extendValidationResult(true, $validator); - } - /** * Stubbed so invalid data doesn't save into the DB * diff --git a/src/Forms/MoneyField.php b/src/Forms/MoneyField.php index 1d8e4688911..75e70d61f6a 100644 --- a/src/Forms/MoneyField.php +++ b/src/Forms/MoneyField.php @@ -4,8 +4,10 @@ use InvalidArgumentException; use SilverStripe\Core\ArrayLib; +use SilverStripe\Core\Validation\FieldValidation\CompositeFieldValidator; use SilverStripe\ORM\FieldType\DBMoney; use SilverStripe\ORM\DataObjectInterface; +use SilverStripe\Core\Validation\ValidationResult; /** * A form field that can save into a {@link Money} database field. @@ -16,6 +18,9 @@ */ class MoneyField extends FormField { + private static array $field_validators = [ + CompositeFieldValidator::class, + ]; protected $schemaDataType = 'MoneyField'; @@ -88,26 +93,30 @@ protected function buildAmountField(): void * * @return FormField */ - protected function buildCurrencyField() + protected function buildCurrencyField(bool $forceTextField = false) { $name = $this->getName(); + $field = null; // Validate allowed currencies $currencyValue = $this->fieldCurrency ? $this->fieldCurrency->dataValue() : null; - $allowedCurrencies = $this->getAllowedCurrencies(); - if (count($allowedCurrencies ?? []) === 1) { - // Hidden field for single currency - $field = HiddenField::create("{$name}[Currency]"); - reset($allowedCurrencies); - $currencyValue = key($allowedCurrencies ?? []); - } elseif ($allowedCurrencies) { - // Dropdown field for multiple currencies - $field = DropdownField::create( - "{$name}[Currency]", - _t('SilverStripe\\Forms\\MoneyField.FIELDLABELCURRENCY', 'Currency'), - $allowedCurrencies - ); - } else { + if (!$forceTextField) { + $allowedCurrencies = $this->getAllowedCurrencies(); + if (count($allowedCurrencies ?? []) === 1) { + // Hidden field for single currency + $field = HiddenField::create("{$name}[Currency]"); + reset($allowedCurrencies); + $currencyValue = key($allowedCurrencies ?? []); + } elseif ($allowedCurrencies) { + // Dropdown field for multiple currencies + $field = DropdownField::create( + "{$name}[Currency]", + _t('SilverStripe\\Forms\\MoneyField.FIELDLABELCURRENCY', 'Currency'), + $allowedCurrencies + ); + } + } + if ($field === null) { // Free-text entry for currency value $field = TextField::create( "{$name}[Currency]", @@ -175,8 +184,16 @@ public function setValue($value, $data = null) throw new InvalidArgumentException("Invalid currency format"); } + // Check that currency is allowed, if not, swap out currency field for a text field so + // that user can alter the value as it will fail validation + $allowedCurrencies = $this->getAllowedCurrencies() ?? []; + $currency = $value['Currency']; + if ($currency && count($allowedCurrencies) && !in_array($currency, $allowedCurrencies)) { + $this->fieldCurrency = $this->buildCurrencyField(true); + } + // Save value - $this->fieldCurrency->setValue($value['Currency'], $value); + $this->fieldCurrency->setValue($currency, $value); $this->fieldAmount->setValue($value['Amount'], $value); $this->value = $this->dataValue(); return $this; @@ -321,32 +338,32 @@ public function getLocale() return $this->fieldAmount->getLocale(); } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) + public function getValueForValidation(): mixed { - // Validate currency - $currencies = $this->getAllowedCurrencies(); - $currency = $this->fieldCurrency->dataValue(); - if ($currency && $currencies && !in_array($currency, $currencies ?? [])) { - $validator->validationError( - $this->getName(), - _t( - __CLASS__ . '.INVALID_CURRENCY', - 'Currency {currency} is not in the list of allowed currencies', - ['currency' => $currency] - ) - ); - return $this->extendValidationResult(false, $validator); - } + return [$this->getAmountField(), $this->getCurrencyField()]; + } - // Field-specific validation - $result = $this->fieldAmount->validate($validator) && $this->fieldCurrency->validate($validator); - return $this->extendValidationResult($result, $validator); + public function validate(): ValidationResult + { + $this->beforeExtending('updateValidate', function (ValidationResult $result) { + // Validate currency + $currencies = $this->getAllowedCurrencies(); + $currency = $this->fieldCurrency->dataValue(); + if ($currency && $currencies && !in_array($currency, $currencies ?? [])) { + $result->addFieldError( + $this->getName(), + _t( + __CLASS__ . '.INVALID_CURRENCY_2', + 'Currency {currency} is not in the list of allowed currencies. Allowed currencies are: {currencies}.', + [ + 'currency' => $currency, + 'currencies' => implode(', ', $currencies), + ] + ) + ); + } + }); + return parent::validate(); } public function setForm($form) diff --git a/src/Forms/MultiSelectField.php b/src/Forms/MultiSelectField.php index 7fe0a9b5999..e3adb61c32e 100644 --- a/src/Forms/MultiSelectField.php +++ b/src/Forms/MultiSelectField.php @@ -6,6 +6,8 @@ use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBMultiEnum; use SilverStripe\ORM\Relation; +use SilverStripe\Core\Validation\FieldValidation\MultiOptionFieldValidator; +use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; /** * Represents a SelectField that may potentially have multiple selections, and may have @@ -13,6 +15,12 @@ */ abstract class MultiSelectField extends SelectField { + private static array $field_validators = [ + OptionFieldValidator::class => null, + MultiOptionFieldValidator::class => [ + 'options' => 'getValidValues', + ], + ]; /** * List of items to mark as checked, and may not be unchecked @@ -72,11 +80,34 @@ public function setValue($value, $obj = null) if ($obj instanceof DataObject) { $this->loadFrom($obj); } else { - parent::setValue($value); + parent::setValue($value, $obj); } return $this; } + public function getValueForValidation(): mixed + { + $value = $this->getValue(); + if (is_array($value)) { + foreach ($value as $key => $val) { + if (is_numeric($val) && is_string($val)) { + $value[$key] = $this->castSubmittedValue($val); + } + } + } + return $value; + } + + public function setSubmittedValue($value, $data = null) + { + // When no value is selected and a regular edit form is submitted, + // usually an empty string will be POSTed e.g. `MyField[]: ""` + if ($value === ['']) { + $value = null; + } + return parent::setSubmittedValue($value, $data); + } + /** * Load the value from the dataobject into this field * @@ -223,49 +254,6 @@ protected function csvDecode($value) return preg_split('/\s*,\s*/', trim($value ?? '')); } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - $values = $this->getValueArray(); - $validValues = $this->getValidValues(); - - // Filter out selected values not in the data source - $self = $this; - $invalidValues = array_filter( - $values ?? [], - function ($userValue) use ($self, $validValues) { - foreach ($validValues as $formValue) { - if ($self->isSelectedValue($formValue, $userValue)) { - return false; - } - } - 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" - ); - } - - return $this->extendValidationResult($result, $validator); - } - /** * Transforms the source data for this CheckboxSetField * into a comma separated list of values. diff --git a/src/Forms/NumericField.php b/src/Forms/NumericField.php index 2e93e3e9c30..dbe1c502217 100644 --- a/src/Forms/NumericField.php +++ b/src/Forms/NumericField.php @@ -3,7 +3,11 @@ namespace SilverStripe\Forms; use NumberFormatter; +use SilverStripe\Core\Validation\FieldValidation\NumericFieldValidator; use SilverStripe\i18n\i18n; +use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator; +use Error; +use SilverStripe\Core\Validation\ValidationResult; /** * Text input field with validation for numeric values. Supports validating @@ -12,6 +16,12 @@ */ class NumericField extends TextField { + private static array $field_validators = [ + NumericFieldValidator::class => [ + 'minValue' => null, + 'maxValue' => null, + ], + ]; protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DECIMAL; @@ -48,6 +58,14 @@ class NumericField extends TextField */ protected $scale = 0; + public function __construct($name, $title = null, $value = null, $maxLength = null, $form = null) + { + // This constructor has a default value of null for the $value param, as opposed to the parent + // TextField constructor which has a default value of blank string. This is done to prevent passing + // a blank string to the NumericFieldValidator which which will cause a validation error. + parent::__construct($name, $title, $value, $maxLength, $form); + } + /** * Get number formatter for localising this field * @@ -104,15 +122,26 @@ protected function getNumberType() /** * In some cases and locales, validation expects non-breaking spaces. * This homogenises regular, narrow and thin non-breaking spaces to a regular space character. - * */ private function clean(?string $value): string { return trim(str_replace(["\u{00A0}", "\u{202F}", "\u{2009}"], ' ', $value ?? '')); } + public function setValue($value, $data = null) + { + $this->originalValue = $value; + $this->value = $this->cast($value); + return $this; + } + public function setSubmittedValue($value, $data = null) { + if (is_null($value)) { + $this->value = null; + return $this; + } + // Save original value in case parse fails $value = $this->clean($value); $this->originalValue = $value; @@ -126,11 +155,12 @@ public function setSubmittedValue($value, $data = null) // Format number $formatter = $this->getFormatter(); $parsed = 0; - $this->value = $formatter->parse($value, $this->getNumberType(), $parsed); // Note: may store literal `false` for invalid values + $value = $formatter->parse($value, $this->getNumberType(), $parsed); // Note: may store literal `false` for invalid values // Ensure that entire string is parsed - if ($parsed < strlen($value ?? '')) { - $this->value = false; + if ($parsed < strlen($this->originalValue ?? '')) { + $value = false; } + $this->value = $this->cast($value); return $this; } @@ -149,28 +179,54 @@ public function Value() return $formatter->format($this->value, $this->getNumberType()); } - public function setValue($value, $data = null) + public function getValueForValidation(): mixed { - $this->originalValue = $value; - $this->value = $this->cast($value); - return $this; + $value = $this->getValue(); + // If the submitted value failed to parse in the localisation formatter + // return null so that FieldValidation is skipped + // NumericField::validate() will "manually" add a validation message + if ($value === false) { + return null; + } + return $value; + } + + public function validate(): ValidationResult + { + $this->beforeExtending('updateValidate', function (ValidationResult $result) { + // Value will be false if the submitted value failed to parse in the localisation formatter + if ($this->getValue() === false) { + $result->addFieldError( + $this->getName(), + _t( + __CLASS__ . '.INVALID', + 'Invalid number' + ) + ); + } + }); + return parent::validate(); } /** - * Helper to cast non-localised strings to their native type - * - * @param string $value - * @return float|int + * Helper to cast values to numeric strings using the current scale */ - protected function cast($value) + protected function cast(mixed $value): mixed { - if (strlen($value ?? '') === 0) { + // If the value is false, it means the value failed to parse in the localisation formatter + if ($value === false) { + return false; + } + if (is_null($value) || is_string($value) && strlen($value) === 0) { return null; } - if ($this->getScale() === 0) { - return (int)$value; + if (is_numeric($value)) { + if ($this->getScale() === 0) { + return (string) (int) $value; + } + return (string) (float) $value; } - return (float)$value; + return $value; } /** @@ -193,31 +249,6 @@ public function getAttributes() return $attributes; } - /** - * Validate this field - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - $result = true; - // false signifies invalid value due to failed parse() - if ($this->value === false) { - $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; - } - - return $this->extendValidationResult($result, $validator); - } - public function getSchemaValidation() { $rules = parent::getSchemaValidation(); @@ -232,7 +263,14 @@ public function getSchemaValidation() */ public function dataValue() { - return $this->cast($this->value); + $value = $this->getValue(); + if ($value === false) { + return 0; + } + // Cast to string before passing on to model we don't know the DBField used by the model, + // as it may be a DBString field which can't handle numeric values + // DBInt and DBFloat can both handle numeric strings + return $this->cast($value); } /** diff --git a/src/Forms/OptionsetField.php b/src/Forms/OptionsetField.php index 72655646c36..a966aebe375 100644 --- a/src/Forms/OptionsetField.php +++ b/src/Forms/OptionsetField.php @@ -134,18 +134,6 @@ public function Field($properties = []) return FormField::Field($properties); } - /** - * {@inheritdoc} - */ - public function validate($validator) - { - if (!$this->Value()) { - return $this->extendValidationResult(true, $validator); - } - - return parent::validate($validator); - } - public function getAttributes() { $attributes = array_merge( diff --git a/src/Forms/RequiredFields.php b/src/Forms/RequiredFields.php index 395c18a9647..6e2068aba82 100644 --- a/src/Forms/RequiredFields.php +++ b/src/Forms/RequiredFields.php @@ -87,7 +87,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 (!$this->required) { diff --git a/src/Forms/SearchableDropdownField.php b/src/Forms/SearchableDropdownField.php index eb7c7d5beb6..5f459ad6634 100644 --- a/src/Forms/SearchableDropdownField.php +++ b/src/Forms/SearchableDropdownField.php @@ -38,4 +38,15 @@ public function setEmptyString($string) Deprecation::notice('5.2.0', 'Use setPlaceholder() instead'); return parent::setEmptyString($string); } + + public function getValueForValidation(): mixed + { + $arr = $this->getValueArray(); + if (count($arr) === 0) { + return null; + } elseif (count($arr) === 1) { + return $arr[0]; + } + return $arr; + } } diff --git a/src/Forms/SearchableDropdownTrait.php b/src/Forms/SearchableDropdownTrait.php index 56e80821c62..de0240bff58 100644 --- a/src/Forms/SearchableDropdownTrait.php +++ b/src/Forms/SearchableDropdownTrait.php @@ -416,14 +416,6 @@ public function saveInto(DataObjectInterface $record): void } } - /** - * @param Validator $validator - */ - public function validate($validator): bool - { - return $this->extendValidationResult(true, $validator); - } - public function getSchemaDataType(): string { if ($this->isMultiple) { diff --git a/src/Forms/SearchableMultiDropdownField.php b/src/Forms/SearchableMultiDropdownField.php index 90608a40de5..541c114a7fd 100644 --- a/src/Forms/SearchableMultiDropdownField.php +++ b/src/Forms/SearchableMultiDropdownField.php @@ -25,4 +25,9 @@ public function __construct( $this->addExtraClass('ss-searchable-dropdown-field'); $this->setIsMultiple(true); } + + public function getValueForValidation(): mixed + { + return $this->getValueArray(); + } } diff --git a/src/Forms/SelectField.php b/src/Forms/SelectField.php index 22485590db9..447c951d6cc 100644 --- a/src/Forms/SelectField.php +++ b/src/Forms/SelectField.php @@ -5,12 +5,18 @@ use SilverStripe\Model\List\SS_List; use SilverStripe\Model\List\Map; use ArrayAccess; +use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; /** * Represents a field that allows users to select one or more items from a list */ abstract class SelectField extends FormField { + private static array $field_validators = [ + OptionFieldValidator::class => [ + 'options' => 'getValidValues', + ], + ]; /** * Associative or numeric array of all dropdown items, @@ -121,6 +127,24 @@ protected function getSourceValues() return array_keys($this->getSource() ?? []); } + /** + * Convert a submitted value, which should probalby always be a string, to the correct type + * Currently this will only convert string int to int + */ + protected function castSubmittedValue(mixed $value): mixed + { + if (!is_string($value) || !is_numeric($value) || !ctype_digit($value)) { + return $value; + } + $sourceValues = $this->getSourceValues(); + foreach ($sourceValues as $sourceValue) { + if ((string) $sourceValue === $value) { + return $sourceValue; + } + } + return $value; + } + /** * Gets all valid values for this field. * @@ -130,9 +154,9 @@ protected function getSourceValues() */ public function getValidValues() { - $valid = array_diff($this->getSourceValues() ?? [], $this->getDisabledItems()); + $values = array_diff($this->getSourceValues() ?? [], $this->getDisabledItems()); // Renumber indexes from 0 - return array_values($valid ?? []); + return array_values($values); } /** @@ -203,7 +227,10 @@ protected function getListValues($values) return $values->column('ID'); } - return [trim($values ?? '')]; + if (is_string($values)) { + $values = trim($values); + } + return is_array($values) ? $values : [$values]; } /** diff --git a/src/Forms/SelectionGroup_Item.php b/src/Forms/SelectionGroup_Item.php index 55d021859e7..ab23c0cc83d 100644 --- a/src/Forms/SelectionGroup_Item.php +++ b/src/Forms/SelectionGroup_Item.php @@ -43,7 +43,7 @@ function setTitle($title) return $this; } - function getValue() + function getValue(): mixed { return $this->value; } diff --git a/src/Forms/SingleLookupField.php b/src/Forms/SingleLookupField.php index 5590574caa1..1bf54b8f24e 100644 --- a/src/Forms/SingleLookupField.php +++ b/src/Forms/SingleLookupField.php @@ -3,9 +3,12 @@ namespace SilverStripe\Forms; use SilverStripe\Core\Convert; +use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\Model\List\Map; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\Core\Validation\FieldValidation\OptionFieldValidator; +use PHPUnit\Framework\Attributes\DataProvider; /** * Read-only complement of {@link DropdownField}. @@ -15,6 +18,10 @@ */ class SingleLookupField extends SingleSelectField { + private static $field_validators = [ + OptionFieldValidator::class => null, + ]; + /** * @var bool */ @@ -36,17 +43,6 @@ protected function valueToLabel() return null; } - /** - * Ignore validation as the field is readonly - * - * @param Validator $validator - * @return bool - */ - public function validate($validator) - { - return $this->extendValidationResult(true, $validator); - } - /** * Stubbed so invalid data doesn't save into the DB * diff --git a/src/Forms/SingleSelectField.php b/src/Forms/SingleSelectField.php index 1dcd6b502ed..1cb8176783d 100644 --- a/src/Forms/SingleSelectField.php +++ b/src/Forms/SingleSelectField.php @@ -9,7 +9,6 @@ */ abstract class SingleSelectField extends SelectField { - /** * Show the first