diff --git a/src/Core/Validation/FieldValidation/DateFieldValidator.php b/src/Core/Validation/FieldValidation/DateFieldValidator.php index 0a2a3de22b3..21c3adce792 100644 --- a/src/Core/Validation/FieldValidation/DateFieldValidator.php +++ b/src/Core/Validation/FieldValidation/DateFieldValidator.php @@ -2,18 +2,76 @@ namespace SilverStripe\Core\Validation\FieldValidation; +use Exception; +use InvalidArgumentException; use SilverStripe\Core\Validation\FieldValidation\FieldValidator; 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 { + /** + * Minimum value as a date string + */ + private ?string $minValue; + + /** + * Minimum value as a unix timestamp + */ + private ?int $minTimestamp; + + /** + * Maximum value as a date string + */ + private ?string $maxValue; + + /** + * Maximum value as a unix timestamp + */ + private ?int $maxTimestamp; + + /** + * 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 + // Intermiediate variables are used to prevent "must not be accessed before initialization" PHP errors + // when reading properties in the constructor + $minTimestamp = null; + $maxTimestamp = null; + if (!is_null($minValue)) { + $minTimestamp = $this->dateToTimestamp($minValue); + } + if (!is_null($maxValue)) { + $maxTimestamp = $this->dateToTimestamp($maxValue); + } + if (!is_null($minTimestamp) && !is_null($maxTimestamp) && $minTimestamp < $maxTimestamp) { + throw new InvalidArgumentException('maxValue cannot be less than minValue'); + } + $this->minValue = $minValue; + $this->maxValue = $maxValue; + $this->minTimestamp = $minTimestamp; + $this->maxTimestamp = $maxTimestamp; + $this->converter = $converter; + parent::__construct($name, $value); + } + protected function validateValue(): ValidationResult { $result = ValidationResult::create(); @@ -21,10 +79,36 @@ protected function validateValue(): ValidationResult if ($this->value === '') { 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 + try { + $timestamp = $this->dateToTimestamp($this->value ?? ''); + } catch (Exception) { $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 less 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 greater than {maxValue}', + ['maxValue' => $maxValue] + ); + $result->addFieldError($this->name, $message); } return $result; } @@ -38,4 +122,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 + { + // 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) { + throw new InvalidArgumentException('Invalid date'); + } + return mktime($date['hour'], $date['minute'], $date['second'], $date['month'], $date['day'], $date['year']); + } } diff --git a/src/Core/Validation/FieldValidation/FieldValidationTrait.php b/src/Core/Validation/FieldValidation/FieldValidationTrait.php index 649c350e64e..70519f58b9f 100644 --- a/src/Core/Validation/FieldValidation/FieldValidationTrait.php +++ b/src/Core/Validation/FieldValidation/FieldValidationTrait.php @@ -63,11 +63,11 @@ private function getFieldValidators(): array /** @var FieldValidationInterface|Configurable $this */ $name = $this->getName(); $value = $this->getValueForValidation(); + $classes = $this->getClassesFromConfig(); // Field name is required for FieldValidators when called ValidationResult::addFieldMessage() - if ($name === '') { + if (count($classes) > 0 && $name === '') { throw new RuntimeException('Field name is blank'); } - $classes = $this->getClassesFromConfig(); foreach ($classes as $class => $argCalls) { $args = [$name, $value]; foreach ($argCalls as $i => $argCall) { @@ -122,10 +122,7 @@ 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); + $argCalls = $this->makeUnique($argCalls); $classes[$class] = $argCalls; } foreach (array_keys($disabledClasses) as $class) { @@ -133,4 +130,22 @@ private function getClassesFromConfig(): array } return $classes; } + + /** + * Dedupe any cases where a subclass defines the same FieldValidator + * his config can happens when a subclass defines a FieldValidator that was already defined + * on a parent class, though it calls different methods + */ + private function makeUnique(array $argCalls): array + { + // there may be multiple null values, which we need to retain so make them unqiue first + // note that we can't use a short function in combination with $n++ in array_map as $n will always be 0 + $n = 0; + $argCalls = array_map(function ($v) use (&$n) { + return is_null($v) ? '{null}-' . $n++ : $v; + }, $argCalls); + $argCalls = array_unique($argCalls); + $argCalls = array_map(fn($v) => str_contains($v, '{null}-') ? null : $v, $argCalls); + return $argCalls; + } } diff --git a/src/Core/Validation/FieldValidation/NumericFieldValidator.php b/src/Core/Validation/FieldValidation/NumericFieldValidator.php index 398d9d7234c..2ba9d81cd09 100644 --- a/src/Core/Validation/FieldValidation/NumericFieldValidator.php +++ b/src/Core/Validation/FieldValidation/NumericFieldValidator.php @@ -39,8 +39,14 @@ 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 + if (!is_numeric($this->value)) { + $message = _t(__CLASS__ . '.NOTNUMERIC', 'Must be numeric'); + $result->addFieldError($this->name, $message); + } elseif (is_numeric($this->value) && is_string($this->value)) { + // This is a separate check from the the one above because for DBField the type is important + // though for FormField form submissions values will usually have a string type + // though should cast to the correct int or float type before validation + // i.e. we don't want to tell CMS users to not use a string when they're using a TextField $message = _t(__CLASS__ . '.WRONGTYPE', 'Must be numeric, and not a string'); $result->addFieldError($this->name, $message); } elseif (!is_null($this->minValue) && $this->value < $this->minValue) { diff --git a/src/Forms/CompositeField.php b/src/Forms/CompositeField.php index d4f2aef64ce..96a45a91c09 100644 --- a/src/Forms/CompositeField.php +++ b/src/Forms/CompositeField.php @@ -2,7 +2,9 @@ namespace SilverStripe\Forms; +use SilverStripe\Core\Validation\FieldValidation\CompositeFieldValidator; use SilverStripe\Dev\Debug; +use SilverStripe\Core\Validation\ValidationResult; /** * Base class for all fields that contain other fields. @@ -13,6 +15,9 @@ */ class CompositeField extends FormField { + private static array $field_validators = [ + CompositeFieldValidator::class, + ]; /** * @var FieldList @@ -52,6 +57,13 @@ class CompositeField extends FormField protected $schemaComponent = 'CompositeField'; + /** + * Iterator used to name the field so that FieldValidators won't complain that the field is not named. + * + * @internal + */ + private static int $nameCounter = 0; + public function __construct($children = null) { // Normalise $children to a FieldList @@ -63,8 +75,8 @@ public function __construct($children = null) $children = new FieldList($children); } $this->setChildren($children); - - parent::__construct(null, false); + $name = 'CompositeField_' . self::$nameCounter++; + parent::__construct($name, false); } /** @@ -115,12 +127,17 @@ public function getChildren() return $this->children; } + public function getValueForValidation(): mixed + { + return $this->getChildren(); + } + /** * 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 +283,6 @@ public function setForm($form) return $this; } - - public function setDisabled($disabled) { parent::setDisabled($disabled); @@ -522,19 +537,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 1856a057eff..2b889709edd 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,141 @@ 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; - + $result = ValidationResult::create(); // if field isn't visible, don't validate 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 $result; } - - // 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 () use ($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); } - } - - 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" + ); + } } - } - - // 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" + ); + } } - - // 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' + ); + } } - - // 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); + } + + // 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 $result->combineAnd(parent::validate()); } /** diff --git a/src/Forms/CurrencyField.php b/src/Forms/CurrencyField.php index 5d36cf8edb3..f0a00fa3fd3 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,22 @@ 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); + $result = ValidationResult::create(); + $this->beforeExtending('updateValidate', function () use ($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" + ); + $result = false; + } + }); + return $result->combineAnd(parent::validate()); } public function getSchemaValidation() diff --git a/src/Forms/DateField.php b/src/Forms/DateField.php index 5649f8039e1..9015f59696d 100644 --- a/src/Forms/DateField.php +++ b/src/Forms/DateField.php @@ -8,6 +8,9 @@ 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; /** * Form used for editing a date string @@ -54,8 +57,14 @@ * @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 => ['getMinDate', 'getMaxDate', 'getFieldValidatorConverter'], + ]; + protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DATE; /** @@ -96,14 +105,6 @@ class DateField extends TextField */ protected $maxDate = null; - /** - * Unparsed value, used exclusively for comparing with internal value - * to detect invalid values. - * - * @var mixed - */ - protected $rawValue = null; - /** * Use HTML5-based input fields (and force ISO 8601 date formats). * @@ -306,9 +307,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; @@ -331,12 +329,8 @@ 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) { - $this->value = null; + if (is_null($value)) { + $this->value = $value; return $this; } @@ -350,6 +344,11 @@ public function Value() return $this->internalToFrontend($this->value); } + public function getValueForValidation(): mixed + { + return $this->value; + } + public function performReadonlyTransformation() { $field = $this @@ -361,83 +360,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 * @@ -535,12 +457,11 @@ 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); + $date = $this->tidyInternal($value); if (!$date) { return null; } @@ -562,8 +483,8 @@ protected function internalToFrontend($date) */ protected function tidyInternal($date) { - if (!$date) { - return null; + if (is_null($date)) { + return $date; } // Re-run through formatter to tidy up (e.g. remove time component) $formatter = $this->getInternalFormatter(); diff --git a/src/Forms/DatetimeField.php b/src/Forms/DatetimeField.php index c822dbf7b42..9ccb579a412 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,13 @@ * 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 => ['getMinDatetime', 'getMaxDatetime', 'getFieldValidatorConverter'], + ]; /** * @var bool @@ -70,14 +77,6 @@ class DatetimeField extends TextField */ protected $timeLength = null; - /** - * Unparsed value, used exclusively for comparing with internal value - * to detect invalid values. - * - * @var mixed - */ - protected $rawValue = null; - /** * @inheritDoc */ @@ -158,9 +157,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; @@ -324,12 +320,8 @@ 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)) { - $this->value = null; + if (is_null($value)) { + $this->value = $value; return $this; } @@ -346,6 +338,7 @@ public function setValue($value, $data = null) } if ($timestamp === false) { + $this->value = $value; return $this; } @@ -370,17 +363,21 @@ public function Value() return $this->internalToFrontend($this->value); } + public function getValueForValidation(): mixed + { + 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); if (!$datetime) { return null; } @@ -557,83 +554,6 @@ public function setMaxDatetime($maxDatetime) 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..6ff9b7317a8 100644 --- a/src/Forms/FieldsValidator.php +++ b/src/Forms/FieldsValidator.php @@ -13,7 +13,9 @@ public function php($data): bool $fields = $this->form->Fields(); foreach ($fields as $field) { - $valid = ($field->validate($this) && $valid); + $result = $field->validate(); + $valid = $result->isValid() && $valid; + $this->result->combineAnd($result); } return $valid; diff --git a/src/Forms/FileField.php b/src/Forms/FileField.php index 8040d1fd594..b0ab8352adc 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,49 @@ 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; + $result = ValidationResult::create(); + $this->beforeExtending('updateValidate', function () use ($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 $result->combineAnd(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 b6f336b8972..e44766fc86b 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -16,6 +16,9 @@ use SilverStripe\View\SSViewer; use SilverStripe\Model\ModelData; use SilverStripe\ORM\DataObject; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationTrait; +use SilverStripe\Core\Validation\FieldValidation\FieldValidationInterface; +use InvalidArgumentException; /** * Represents a field in a form. @@ -41,10 +44,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'; @@ -428,7 +432,7 @@ public function getTemplateHelper() /** * Returns the field name. */ - public function getName() + public function getName(): string { return $this->name; } @@ -444,7 +448,28 @@ public function getInputType() } /** - * Returns the field value. + * Returns the field value + * + * Alias of Value() + */ + public function getValue(): mixed + { + // This alias is here to meet the FieldValidationInterface API + // While this is a newer method than Value(), this calls Value() rather than the other way + // around as there are many FormField subclasses that override Value() + return $this->Value(); + } + + /** + * Get the value of this field for field validation + */ + public function getValueForValidation(): mixed + { + return $this->getValue(); + } + + /** + * Returns the field value * * @see FormField::setSubmittedValue() * @return mixed @@ -1221,30 +1246,30 @@ 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 + * Determines whether the field is valid or not based on its value */ - protected function extendValidationResult(bool $result, Validator $validator): bool + public function validate(): ValidationResult { - $this->extend('updateValidationResult', $result, $validator); + if (count(func_get_args()) !== 0) { + // In prior versions, FormField::validate() had a method signature of + // validate(Validator $validator): bool + // Old code may still call this method this without throwing an error, however the + // return type is now ValidationResult which may be mistakenly interpreted as truthy + // While we cannot stop people from interpreting the return value as a boolean, we can + // at least throw an exception so they might notice something is wrong. + $message = 'validate() does not accept any arguments.' + . ' Note that the return type is ValidationResult, not boolean.'; + throw new InvalidArgumentException($message); + } + $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/LookupField.php b/src/Forms/LookupField.php index ac388fc394b..086e80a1b0f 100644 --- a/src/Forms/LookupField.php +++ b/src/Forms/LookupField.php @@ -4,8 +4,10 @@ use SilverStripe\Core\Convert; use SilverStripe\Core\ArrayLib; +use SilverStripe\Core\Validation\ValidationResult; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\Core\Validation\FieldValidation\MultiOptionFieldValidator; /** * Read-only complement of {@link MultiSelectField}. @@ -15,6 +17,10 @@ */ class LookupField extends MultiSelectField { + private static $field_validators = [ + MultiOptionFieldValidator::class => null, + ]; + protected $schemaComponent = 'LookupField'; /** @@ -65,17 +71,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..325c2764dca 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'; @@ -321,32 +326,30 @@ 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 + { + $result = ValidationResult::create(); + $this->beforeExtending('updateValidate', function () use ($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', + 'Currency {currency} is not in the list of allowed currencies', + ['currency' => $currency] + ) + ); + } + }); + return $result->combineAnd(parent::validate()); } public function setForm($form) diff --git a/src/Forms/MultiSelectField.php b/src/Forms/MultiSelectField.php index 7fe0a9b5999..1e0a0bd179c 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,10 @@ */ abstract class MultiSelectField extends SelectField { + private static array $field_validators = [ + OptionFieldValidator::class => null, + MultiOptionFieldValidator::class => ['getValidValues'], + ]; /** * List of items to mark as checked, and may not be unchecked @@ -72,11 +78,28 @@ 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 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; + } + if (is_array($value) && $this->getAllSourceValuesAreIntType()) { + foreach ($value as $key => $val) { + if (is_numeric($val) && is_string($val)) { + $value[$key] = (int) $val; + } + } + } + return parent::setSubmittedValue($value, $data); + } + /** * Load the value from the dataobject into this field * @@ -223,49 +246,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..3afc864d88a 100644 --- a/src/Forms/NumericField.php +++ b/src/Forms/NumericField.php @@ -3,7 +3,10 @@ namespace SilverStripe\Forms; use NumberFormatter; +use SilverStripe\Core\Validation\FieldValidation\NumericFieldValidator; use SilverStripe\i18n\i18n; +use SilverStripe\Core\Validation\FieldValidation\StringFieldValidator; +use SilverStripe\Core\Validation\ValidationResult; /** * Text input field with validation for numeric values. Supports validating @@ -12,6 +15,11 @@ */ class NumericField extends TextField { + private static array $field_validators = [ + // Disable parent validator as value for validation will be cast to int or float + StringFieldValidator::class => null, + NumericFieldValidator::class, + ]; protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_DECIMAL; @@ -48,6 +56,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,7 +120,6 @@ 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 { @@ -113,6 +128,11 @@ private function clean(?string $value): string public function setSubmittedValue($value, $data = null) { + if (is_null($value)) { + $this->value = $value; + return $this; + } + // Save original value in case parse fails $value = $this->clean($value); $this->originalValue = $value; @@ -156,6 +176,15 @@ public function setValue($value, $data = null) return $this; } + public function getValueForValidation(): mixed + { + $value = $this->value; + if (is_numeric($value) && is_string($value)) { + return $this->cast($value); + } + return $value; + } + /** * Helper to cast non-localised strings to their native type * @@ -168,9 +197,9 @@ protected function cast($value) return null; } if ($this->getScale() === 0) { - return (int)$value; + return (int) $value; } - return (float)$value; + return (float) $value; } /** @@ -193,31 +222,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(); 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 fa751c49a2f..f49d0e878a8 100644 --- a/src/Forms/SearchableDropdownTrait.php +++ b/src/Forms/SearchableDropdownTrait.php @@ -417,14 +417,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..abdae9c5302 100644 --- a/src/Forms/SelectField.php +++ b/src/Forms/SelectField.php @@ -5,12 +5,16 @@ 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 => ['getValidValues'], + ]; /** * Associative or numeric array of all dropdown items, @@ -121,6 +125,16 @@ protected function getSourceValues() return array_keys($this->getSource() ?? []); } + /** + * Whether all source values are typed as int + */ + protected function getAllSourceValuesAreIntType(): bool + { + $sourceValues = $this->getSourceValues(); + $intCount = count(array_filter(array_map('is_int', $sourceValues))); + return $intCount === count($sourceValues); + } + /** * Gets all valid values for this field. * @@ -130,9 +144,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 +217,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..93d47c6eba0 100644 --- a/src/Forms/SingleLookupField.php +++ b/src/Forms/SingleLookupField.php @@ -3,9 +3,11 @@ 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; /** * Read-only complement of {@link DropdownField}. @@ -15,6 +17,10 @@ */ class SingleLookupField extends SingleSelectField { + private static $field_validators = [ + OptionFieldValidator::class => null, + ]; + /** * @var bool */ @@ -36,17 +42,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..b2d6f81c229 100644 --- a/src/Forms/SingleSelectField.php +++ b/src/Forms/SingleSelectField.php @@ -9,7 +9,6 @@ */ abstract class SingleSelectField extends SelectField { - /** * Show the first