ENH Pass invalid value to ValidationResult

silverstripe · Jan 16, 2025 · e148ac2 · e148ac2
1 parent 039f919
commit e148ac2
Show file tree

Hide file tree

Showing 9 changed files with 310 additions and 125 deletions.
diff --git a/src/Core/Validation/ConstraintValidator.php b/src/Core/Validation/ConstraintValidator.php
@@ -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
     {
@@ -35,9 +36,16 @@ public static function validate(mixed $value, Constraint|array $constraints, str
         /** @var ConstraintViolationInterface $violation */
         foreach ($violations as $violation) {
             if ($fieldName) {
-                $result->addFieldError($fieldName, $violation->getMessage());
+                $result->addFieldError(
+                    $fieldName,
+                    $violation->getMessage(),
+                    value: $value,
+                );
             } else {
-                $result->addError($violation->getMessage());
+                $result->addError(
+                    $violation->getMessage(),
+                    value: $value,
+                );
             }
         }
 

diff --git a/src/Core/Validation/ValidationException.php b/src/Core/Validation/ValidationException.php
@@ -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
@@ -48,6 +52,14 @@ public function __construct($result = null, $code = 0)
             // Pick first message
             foreach ($result->getMessages() as $message) {
                 $exceptionMessage = $message['message'];
+                if ($this->getShouldShowAdditionalInfo()) {
+                    $exceptionMessage .= ' - fieldName: ' . $message['fieldName'];
+                    $exceptionMessage .= ', value: ' . (string) $message['value'];
+                    if (is_subclass_of($result->getDataClass(), DataObject::class, true)) {
+                        $exceptionMessage .= ', recordID: ' . $result->getRecordID();
+                        $exceptionMessage .= ', dataClass: ' . $result->getDataClass();
+                    }
+                }
                 break;
             }
         } elseif (is_string($result)) {
@@ -71,4 +83,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 getShouldShowAdditionalInfo()
+    {
+        if (Director::is_cli()) {
+            return true;
+        }
+        // e.g. dev/build
+        if (is_a(Controller::curr(), DevelopmentAdmin::class)) {
+            return true;
+        }
+    }
 }