Skip to content

Commit

Permalink
NEW Show more information in ValidationException message
Browse files Browse the repository at this point in the history
  • Loading branch information
@emteknetnz
emteknetnz committed Jan 17, 2025
1 parent 039f919 commit dd52956
Show file tree
Hide file tree
Showing 11 changed files with 459 additions and 128 deletions.
3 changes: 2 additions & 1 deletion src/Core/Validation/ConstraintValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ class ConstraintValidator
/**
* Validate a value by a constraint
*
* @param $value The value to validate
* @param Constraint|Constraint[] $constraints a constraint or array of constraints to validate against
* @param string $fieldName The field name the value relates to, if relevant
* @param $fieldName The field name the value relates to, if relevant
*/
public static function validate(mixed $value, Constraint|array $constraints, string $fieldName = ''): ValidationResult
{
Expand Down
31 changes: 31 additions & 0 deletions src/Core/Validation/ValidationException.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
use Exception;
use InvalidArgumentException;
use SilverStripe\Core\Injector\Injectable;
use SilverStripe\Control\Director;
use SilverStripe\Dev\DevelopmentAdmin;
use SilverStripe\ORM\DataObject;
use SilverStripe\Control\Controller;

/**
* Exception thrown by {@link DataObject}::write if validation fails. By throwing an
Expand Down Expand Up @@ -48,6 +52,16 @@ public function __construct($result = null, $code = 0)
// Pick first message
foreach ($result->getMessages() as $message) {
$exceptionMessage = $message['message'];
if ($this->doShowAdditionalInfo()) {
if ($message['fieldName']) {
$exceptionMessage .= ' - fieldName: ' . $message['fieldName'];
}
$dataClass = $result->getDataClass();
if (is_subclass_of($dataClass, DataObject::class, true)) {
$exceptionMessage .= ', recordID: ' . $result->getRecordID();
$exceptionMessage .= ', dataClass: ' . $dataClass;
}
}
break;
}
} elseif (is_string($result)) {
Expand All @@ -71,4 +85,21 @@ public function getResult()
{
return $this->result;
}

/**
* Whether to show additional information in the error message depending on the context
*
* We do not want this to show this in a context where it's easy to know the record and value that
* triggered the error e.g. form submission, API calls, etc
*/
private function doShowAdditionalInfo()
{
if (Director::is_cli()) {
return true;
}
// e.g. dev/build
if (is_a(Controller::curr(), DevelopmentAdmin::class)) {
return true;
}
}
}
212 changes: 140 additions & 72 deletions src/Core/Validation/ValidationResult.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace SilverStripe\Core\Validation;

use InvalidArgumentException;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Injector\Injectable;

/**
Expand All @@ -15,6 +16,7 @@
class ValidationResult
{
use Injectable;
use Configurable;

/**
* Standard "error" type
Expand Down Expand Up @@ -49,140 +51,185 @@ class ValidationResult
/**
* Is the result valid or not.
* Note that there can be non-error messages in the list.
*
* @var bool
*/
protected $isValid = true;
protected bool $isValid = true;

/**
* List of messages
*
* @var array
*/
protected $messages = [];
protected array $messages = [];

/**
* The class of the DataObject being validated.
*/
private string $dataClass = '';

/**
* The record ID of the object being validated.
*/
private int $recordID = 0;

public function __construct(string $dataClass = '', int $recordID = 0)
{
$this->dataClass = $dataClass;
$this->recordID = $recordID;
}

public function getDataClass(): string
{
return $this->dataClass;
}

public function setDataClass(string $dataClass): static
{
$this->dataClass = $dataClass;
return $this;
}

public function getRecordID(): int
{
return $this->recordID;
}

public function setRecordID(int $recordID): static
{
$this->recordID = $recordID;
return $this;
}

/**
* Record an error against this validation result,
*
* @param string $message The message string.
* @param string $messageType Passed as a CSS class to the form, so other values can be used if desired.
* Standard types are defined by the TYPE_ constant definitions.
* @param string $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param string|bool $cast Cast type; One of the CAST_ constant definitions.
* Bool values will be treated as plain text flag.
* @param $message The message string.
* @param $messageType The type of message: e.g. "bad", "warning", "good", or "required"
* Passed as a CSS class to the form, so other values can be used if desired
* @param $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param $cast Cast type; One of the CAST_ constant definitions.
* @return $this
*/
public function addError(
$message,
$messageType = ValidationResult::TYPE_ERROR,
$code = null,
$cast = ValidationResult::CAST_TEXT,
) {
return $this->addFieldError(null, $message, $messageType, $code, $cast);
string $message,
string $messageType = ValidationResult::TYPE_ERROR,
string $code = '',
string $cast = ValidationResult::CAST_TEXT,
): static {
return $this->addFieldError(
'',
$message,
$messageType,
$code,
$cast,
);
}

/**
* Record an error against this validation result,
*
* @param string $fieldName The field to link the message to. If omitted; a form-wide message is assumed.
* @param string $message The message string.
* @param string $messageType The type of message: e.g. "bad", "warning", "good", or "required". Passed as a CSS
* class to the form, so other values can be used if desired.
* @param string $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param string|bool $cast Cast type; One of the CAST_ constant definitions.
* Bool values will be treated as plain text flag.
* @param $fieldName The field to link the message to. If omitted; a form-wide message is assumed.
* @param $message The message string.
* @param $messageType The type of message: e.g. "bad", "warning", "good", or "required"
* Passed as a CSS class to the form, so other values can be used if desired
* @param $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param $cast Cast type; One of the CAST_ constant definitions.
* @return $this
*/
public function addFieldError(
$fieldName,
$message,
$messageType = ValidationResult::TYPE_ERROR,
$code = null,
$cast = ValidationResult::CAST_TEXT,
) {
string $fieldName,
string $message,
string $messageType = ValidationResult::TYPE_ERROR,
string $code = '',
string $cast = ValidationResult::CAST_TEXT,
): static {
$this->isValid = false;
return $this->addFieldMessage($fieldName, $message, $messageType, $code, $cast);
return $this->addFieldMessage(
$fieldName,
$message,
$messageType,
$code,
$cast,
);
}

/**
* Add a message to this ValidationResult without necessarily marking it as an error
*
* @param string $message The message string.
* @param string $messageType The type of message: e.g. "bad", "warning", "good", or "required". Passed as a CSS
* class to the form, so other values can be used if desired.
* @param string $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param string|bool $cast Cast type; One of the CAST_ constant definitions.
* Bool values will be treated as plain text flag.
* @param $message The message string.
* @param $messageType The type of message: e.g. "bad", "warning", "good", or "required"
* Passed as a CSS class to the form, so other values can be used if desired
* @param $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param $cast Cast type; One of the CAST_ constant definitions.
* @return $this
*/
public function addMessage(
$message,
$messageType = ValidationResult::TYPE_ERROR,
$code = null,
$cast = ValidationResult::CAST_TEXT,
) {
return $this->addFieldMessage(null, $message, $messageType, $code, $cast);
string $message,
string $messageType = ValidationResult::TYPE_ERROR,
string $code = '',
string $cast = ValidationResult::CAST_TEXT,
): static {
return $this->addFieldMessage(
'',
$message,
$messageType,
$code,
$cast,
);
}

/**
* Add a message to this ValidationResult without necessarily marking it as an error
*
* @param string $fieldName The field to link the message to. If omitted; a form-wide message is assumed.
* @param string $message The message string.
* @param string $messageType The type of message: e.g. "bad", "warning", "good", or "required". Passed as a CSS
* class to the form, so other values can be used if desired.
* @param string $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param string|bool $cast Cast type; One of the CAST_ constant definitions.
* Bool values will be treated as plain text flag.
* @param $fieldName The field to link the message to. If omitted; a form-wide message is assumed.
* @param $message The message string.
* @param $messageType The type of message: e.g. "bad", "warning", "good", or "required"
* Passed as a CSS class to the form, so other values can be used if desired
* @param $code A codename for this error. Only one message per codename will be added.
* This can be usedful for ensuring no duplicate messages
* @param $cast Cast type; One of the CAST_ constant definitions.
* @return $this
*/
public function addFieldMessage(
$fieldName,
$message,
$messageType = ValidationResult::TYPE_ERROR,
$code = null,
$cast = ValidationResult::CAST_TEXT,
) {
string $fieldName,
string $message,
string $messageType = ValidationResult::TYPE_ERROR,
string $code = '',
string $cast = ValidationResult::CAST_TEXT,
): static {
if ($code && is_numeric($code)) {
throw new InvalidArgumentException("Don't use a numeric code '$code'. Use a string.");
}
if (is_bool($cast)) {
$cast = $cast ? ValidationResult::CAST_TEXT : ValidationResult::CAST_HTML;
throw new InvalidArgumentException("Don't use a numeric code '$code'. Use a string.");
}
$metadata = [
'message' => $message,
'fieldName' => $fieldName,
'messageType' => $messageType,
'messageCast' => $cast,
'dataClass' => $this->dataClass,
'recordID' => $this->recordID,
];
if ($code) {
$this->messages[$code] = $metadata;
} else {
$this->messages[] = $metadata;
}

return $this;
}

/**
* Returns true if the result is valid.
* @return boolean
*/
public function isValid()
public function isValid(): bool
{
return $this->isValid;
}

/**
* Return the full error meta-data, suitable for combining with another ValidationResult.
*
* @return array Array of messages, where each item is an array of data for that message.
* @return Array of messages, where each item is an array of data for that message.
*/
public function getMessages()
public function getMessages(): array
{
return $this->messages;
}
Expand All @@ -193,12 +240,12 @@ public function getMessages()
* This object will be modified to contain the new validation information.
*
* @param ValidationResult $other the validation result object to combine
* @return $this
*/
public function combineAnd(ValidationResult $other)
public function combineAnd(ValidationResult $other): static
{
$this->isValid = $this->isValid && $other->isValid();
$this->messages = array_merge($this->messages, $other->getMessages());
$this->combineDataClassAndRecordID($other);
return $this;
}

Expand All @@ -215,4 +262,25 @@ public function __unserialize(array $data): void
$this->messages = $data['messages'];
$this->isValid = $data['isValid'];
}

/**
* Combine the data class and record ID from another ValidationResult object.
*/
private function combineDataClassAndRecordID(ValidationResult $other): void
{
$thisDataClass = $this->getDataClass();
$otherDataClass = $other->getDataClass();
$thisRecordID = $this->getRecordID();
$otherRecordID = $other->getRecordID();
if ($thisDataClass === '' && $otherDataClass !== '') {
$this->setDataClass($otherDataClass);
} elseif ($thisDataClass !== '' && $otherDataClass === '') {
$other->setDataClass($thisDataClass);
}
if ($thisRecordID === 0 && $otherRecordID !== 0) {
$this->setRecordID($otherRecordID);
} elseif ($thisRecordID !== 0 && $otherRecordID === 0) {
$other->setRecordID($thisRecordID);
}
}
}
Loading

0 comments on commit dd52956

Please sign in to comment.