Browse Source

Improve error messages when persistence fails

When persistence fails error messages should include all the validation
errors recursively.

Fixes #12931
Mark Story 7 years ago
parent
commit
73ee9ed8cd
2 changed files with 29 additions and 22 deletions
  1. 4 21
      src/ORM/Exception/PersistenceFailedException.php
  2. 25 1
      tests/TestCase/ORM/TableTest.php

+ 4 - 21
src/ORM/Exception/PersistenceFailedException.php

@@ -14,6 +14,7 @@ namespace Cake\ORM\Exception;
 
 use Cake\Core\Exception\Exception;
 use Cake\Datasource\EntityInterface;
+use Cake\Utility\Hash;
 
 /**
  * Used when a strict save or delete fails
@@ -47,36 +48,18 @@ class PersistenceFailedException extends Exception
         $this->_entity = $entity;
         if (is_array($message)) {
             $errors = [];
-            foreach ($entity->getErrors() as $field => $error) {
-                $errors[] = $field . ': "' . $this->buildError($error) . '"';
+            foreach (Hash::flatten($entity->getErrors()) as $field => $error) {
+                $errors[] = $field . ': "' . $error . '"';
             }
             if ($errors) {
                 $message[] = implode(', ', $errors);
-                $this->_messageTemplate = 'Entity %s failure (%s).';
+                $this->_messageTemplate = 'Entity %s failure. Found the following errors (%s).';
             }
         }
         parent::__construct($message, $code, $previous);
     }
 
     /**
-     * @param string|array $error Error message.
-     * @return string
-     */
-    protected function buildError($error)
-    {
-        if (!is_array($error)) {
-            return $error;
-        }
-
-        $errors = [];
-        foreach ($error as $key => $message) {
-            $errors[] = is_int($key) ? $message : $key;
-        }
-
-        return implode(', ', $errors);
-    }
-
-    /**
      * Get the passed in entity
      *
      * @return \Cake\Datasource\EntityInterface

+ 25 - 1
tests/TestCase/ORM/TableTest.php

@@ -6414,7 +6414,7 @@ class TableTest extends TestCase
     public function testSaveOrFailErrorDisplay()
     {
         $this->expectException(\Cake\ORM\Exception\PersistenceFailedException::class);
-        $this->expectExceptionMessage('Entity save failure (field: "Some message", multiple: "one, two")');
+        $this->expectExceptionMessage('Entity save failure. Found the following errors (field.0: "Some message", multiple.one: "One", multiple.two: "Two")');
 
         $entity = new Entity([
             'foo' => 'bar'
@@ -6427,6 +6427,30 @@ class TableTest extends TestCase
     }
 
     /**
+     * Tests that saveOrFail with nested errors
+     *
+     * @return void
+     */
+    public function testSaveOrFailNestedError()
+    {
+        $this->expectException(\Cake\ORM\Exception\PersistenceFailedException::class);
+        $this->expectExceptionMessage('Entity save failure. Found the following errors (articles.0.title.0: "Bad value")');
+
+        $entity = new Entity([
+            'username' => 'bad',
+            'articles' => [
+                new Entity(['title' => 'not an entity'])
+            ]
+        ]);
+        $entity->articles[0]->setError('title', 'Bad value');
+
+        $table = $this->getTableLocator()->get('Users');
+        $table->hasMany('Articles');
+
+        $table->saveOrFail($entity);
+    }
+
+    /**
      * Tests that saveOrFail returns the right entity
      *
      * @return void