Browse Source

Merge pull request #13318 from cakephp/3next-entity-nestederror

3.next - Improve nested validator support in EntityContext
Mark Story 6 years ago
parent
commit
0aa0c84ade

+ 10 - 4
src/Datasource/EntityTrait.php

@@ -1080,13 +1080,19 @@ trait EntityTrait
      */
     protected function _nestedErrors($field)
     {
-        $path = explode('.', $field);
-
         // Only one path element, check for nested entity with error.
-        if (count($path) === 1) {
-            return $this->_readError($this->get($path[0]));
+        if (strpos($field, '.') === false) {
+            return $this->_readError($this->get($field));
         }
+        // Try reading the errors data with field as a simple path
+        $error = Hash::get($this->_errors, $field);
+        if ($error !== null) {
+            return $error;
+        }
+        $path = explode('.', $field);
 
+        // Traverse down the related entities/arrays for
+        // the relevant entity.
         $entity = $this;
         $len = count($path);
         while ($len) {

+ 86 - 6
src/View/Form/EntityContext.php

@@ -313,11 +313,12 @@ class EntityContext implements ContextInterface
     }
 
     /**
-     * Fetch the leaf entity for the given path.
+     * Fetch the entity or data value for a given path
      *
-     * This method will traverse the given path and find the leaf
-     * entity. If the path does not contain a leaf entity false
-     * will be returned.
+     * This method will traverse the given path and find the entity
+     * or array value for a given path.
+     *
+     * If you only want the terminal Entity for a path use `leafEntity` instead.
      *
      * @param array|null $path Each one of the parts in a path for a field name
      *  or null to get the entity passed in constructor context.
@@ -349,7 +350,6 @@ class EntityContext implements ContextInterface
             $prop = $path[$i];
             $next = $this->_getProp($entity, $prop);
             $isLast = ($i === $last);
-
             if (!$isLast && $next === null && $prop !== '_ids') {
                 $table = $this->_getTable($path);
 
@@ -373,6 +373,74 @@ class EntityContext implements ContextInterface
     }
 
     /**
+     * Fetch the terminal or leaf entity for the given path.
+     *
+     * Traverse the path until an entity cannot be found. Lists containing
+     * entities will be traversed if the first element contains an entity.
+     * Otherwise the containing Entity will be assumed to be the terminal one.
+     *
+     * @param array|null $path Each one of the parts in a path for a field name
+     *  or null to get the entity passed in constructor context.
+     * @return array Containing the found entity, and remaining un-matched path.
+     * @throws \RuntimeException When properties cannot be read.
+     */
+    protected function leafEntity($path = null)
+    {
+        if ($path === null) {
+            return $this->_context['entity'];
+        }
+
+        $oneElement = count($path) === 1;
+        if ($oneElement && $this->_isCollection) {
+            throw new RuntimeException(sprintf(
+                'Unable to fetch property "%s"',
+                implode('.', $path)
+            ));
+        }
+        $entity = $this->_context['entity'];
+        if ($oneElement) {
+            return [$entity, $path];
+        }
+
+        if ($path[0] === $this->_rootName) {
+            $path = array_slice($path, 1);
+        }
+
+        $len = count($path);
+        $last = $len - 1;
+        $leafEntity = $entity;
+        for ($i = 0; $i < $len; $i++) {
+            $prop = $path[$i];
+            $next = $this->_getProp($entity, $prop);
+
+            // Did not dig into an entity, return the current one.
+            if (is_array($entity) && !($next instanceof EntityInterface || $next instanceof Traversable)) {
+                return [$leafEntity, array_slice($path, $i - 1)];
+            }
+
+            if ($next instanceof EntityInterface) {
+                $leafEntity = $next;
+            }
+
+            // If we are at the end of traversable elements
+            // return the last entity found.
+            $isTraversable = (
+                is_array($next) ||
+                $next instanceof Traversable ||
+                $next instanceof EntityInterface
+            );
+            if (!$isTraversable) {
+                return [$leafEntity, array_slice($path, $i)];
+            }
+            $entity = $next;
+        }
+        throw new RuntimeException(sprintf(
+            'Unable to fetch property "%s"',
+            implode('.', $path)
+        ));
+    }
+
+    /**
      * Read property values or traverse arrays/iterators.
      *
      * @param mixed $target The entity/array/collection to fetch $field from.
@@ -632,9 +700,21 @@ class EntityContext implements ContextInterface
     public function error($field)
     {
         $parts = explode('.', $field);
-        $entity = $this->entity($parts);
+        try {
+            list($entity, $remainingParts) = $this->leafEntity($parts);
+        } catch (RuntimeException $e) {
+            return [];
+        }
+        if (count($remainingParts) === 0) {
+            return $entity->getErrors();
+        }
 
         if ($entity instanceof EntityInterface) {
+            $error = $entity->getError(implode('.', $remainingParts));
+            if ($error) {
+                return $error;
+            }
+
             return $entity->getError(array_pop($parts));
         }
 

+ 20 - 1
tests/TestCase/ORM/EntityTest.php

@@ -1215,7 +1215,7 @@ class EntityTest extends TestCase
      *
      * @return void
      */
-    public function testGetAndSetErrors()
+    public function testGetErrorAndSetError()
     {
         $entity = new Entity();
         $this->assertEmpty($entity->getErrors());
@@ -1241,6 +1241,25 @@ class EntityTest extends TestCase
     }
 
     /**
+     * Tests reading errors from nested validator
+     *
+     * @return void
+     */
+    public function testGetErrorNested()
+    {
+        $entity = new Entity();
+        $entity->setError('options', ['subpages' => ['_empty' => 'required']]);
+
+        $expected = [
+            'subpages' => ['_empty' => 'required']
+        ];
+        $this->assertEquals($expected, $entity->getError('options'));
+
+        $expected = ['_empty' => 'required'];
+        $this->assertEquals($expected, $entity->getError('options.subpages'));
+    }
+
+    /**
      * Tests that it is possible to get errors for nested entities
      *
      * @return void

+ 56 - 0
tests/TestCase/View/Form/EntityContextTest.php

@@ -1312,6 +1312,62 @@ class EntityContextTest extends TestCase
     }
 
     /**
+     * Test error on nested validation
+     *
+     * @return void
+     */
+    public function testErrorNestedValidator()
+    {
+        $this->_setupTables();
+
+        $row = new Article([
+            'title' => 'My title',
+            'options' => ['subpages' => '']
+        ]);
+        $row->setError('options', ['subpages' => ['_empty' => 'required value']]);
+
+        $context = new EntityContext($this->request, [
+            'entity' => $row,
+            'table' => 'Articles',
+        ]);
+        $expected = ['_empty' => 'required value'];
+        $this->assertEquals($expected, $context->error('options.subpages'));
+    }
+
+    /**
+     * Test error on nested validation
+     *
+     * @return void
+     */
+    public function testErrorAssociatedNestedValidator()
+    {
+        $this->_setupTables();
+
+        $tagOne = new Tag(['name' => 'first-post']);
+        $tagTwo = new Tag(['name' => 'second-post']);
+        $tagOne->setError(
+            'metadata',
+            ['description' => ['_empty' => 'required value']]
+        );
+        $row = new Article([
+            'title' => 'My title',
+            'tags' => [
+                $tagOne,
+                $tagTwo
+            ]
+        ]);
+
+        $context = new EntityContext($this->request, [
+            'entity' => $row,
+            'table' => 'Articles',
+        ]);
+        $expected = ['_empty' => 'required value'];
+        $this->assertSame([], $context->error('tags.0.notthere'));
+        $this->assertSame([], $context->error('tags.1.notthere'));
+        $this->assertEquals($expected, $context->error('tags.0.metadata.description'));
+    }
+
+    /**
      * Setup tables for tests.
      *
      * @return void