Improve error handling in Ajax

UPGRADE-4.x.md

@@ -1,6 +1,53 @@
 UPGRADE 4.x
 ===========
 
+UPGRADE FROM 4.x to 4.x
+=======================
+
+## Form errors retrieved via ajax calls
+
+This change will most likely not affect you since normally the admin is not used
+directly as an api for create or edit objects.
+
+Previously ajax form errors that happen on creation / edit of an admin object
+were outputted as a custom json that didn't had the information about which field
+had the error. This was a problem because the form was not able to highlight the
+field with the error.
+
+Now the ajax form errors are outputted with the standard Symfony json format for
+validation errors.
+
+To be able to output errors with that new format, you will also need to have
+`symfony/serializer` installed.
+
+Before:
+
+```json
+    {
+        "result":"error",
+        "errors": [
+            "Form error message"
+        ]
+    }
+```
+
+After:
+
+```json
+    {
+        "type":"https://symfony.com/errors/validation",
+        "title":"Validation Failed",
+        "detail":"name: Form error message",
+        "violations": [
+            {
+                "propertyPath":"name",
+                "title":"Form error message",
+                "parameters":[]
+            }
+        ]
+    }
+```
+
 UPGRADE FROM 4.18 to 4.19
 =========================
 

composer.json

@@ -80,16 +80,18 @@
         "psalm/plugin-phpunit": "^0.17",
         "psalm/plugin-symfony": "^3.0",
         "psr/event-dispatcher": "^1.0",
-        "rector/rector": "^0.13",
+        "rector/rector": "^0.14",
         "symfony/browser-kit": "^4.4 || ^5.4 || ^6.0",
         "symfony/css-selector": "^4.4 || ^5.4 || ^6.0",
         "symfony/filesystem": "^4.4 || ^5.4 || ^6.0",
         "symfony/maker-bundle": "^1.25",
         "symfony/phpunit-bridge": "^6.1",
+        "symfony/serializer": "^4.4 || ^5.4 || ^6.0",
         "symfony/yaml": "^4.4 || ^5.4 || ^6.0",
         "vimeo/psalm": "^4.9.2"
     },
     "suggest": {
+        "symfony/serializer": "To render errors coming from form validation on ajax calls",
         "twig/extra-bundle": "Auto configures the Twig Intl extension"
     },
     "autoload": {

src/Controller/CRUDController.php

@@ -24,6 +24,7 @@
 use Sonata\AdminBundle\Exception\LockException;
 use Sonata\AdminBundle\Exception\ModelManagerException;
 use Sonata\AdminBundle\Exception\ModelManagerThrowable;
+use Sonata\AdminBundle\Form\FormErrorIteratorToConstraintViolationList;
 use Sonata\AdminBundle\Model\AuditManagerInterface;
 use Sonata\AdminBundle\Request\AdminFetcherInterface;
 use Sonata\AdminBundle\Templating\TemplateRegistryInterface;
@@ -1359,15 +1360,10 @@ protected function handleXmlHttpRequestErrorResponse(Request $request, FormInter
             return $this->renderJson([], Response::HTTP_NOT_ACCEPTABLE);
         }
 
-        $errors = [];
-        foreach ($form->getErrors(true) as $error) {
-            $errors[] = $error->getMessage();
-        }
-
-        return $this->renderJson([
-            'result' => 'error',
-            'errors' => $errors,
-        ], Response::HTTP_BAD_REQUEST);
+        return $this->json(
+            FormErrorIteratorToConstraintViolationList::transform($form->getErrors(true)),
+            Response::HTTP_BAD_REQUEST
+        );
     }
 
     /**

src/Form/FormErrorIteratorToConstraintViolationList.php

@@ -0,0 +1,80 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Sonata Project package.
+ *
+ * (c) Thomas Rabaix <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Sonata\AdminBundle\Form;
+
+use Symfony\Component\Form\FormError;
+use Symfony\Component\Form\FormErrorIterator;
+use Symfony\Component\Form\FormInterface;
+use Symfony\Component\Validator\ConstraintViolation;
+use Symfony\Component\Validator\ConstraintViolationInterface;
+use Symfony\Component\Validator\ConstraintViolationList;
+use Symfony\Component\Validator\ConstraintViolationListInterface;
+
+/**
+ * @author Jordi Sala <[email protected]>
+ */
+final class FormErrorIteratorToConstraintViolationList
+{
+    /**
+     * @param FormErrorIterator<FormError> $errors
+     */
+    public static function transform(FormErrorIterator $errors): ConstraintViolationListInterface
+    {
+        $form = $errors->getForm();
+        $list = new ConstraintViolationList();
+
+        foreach ($errors as $error) {
+            $violation = static::buildViolation($error, $form);
+
+            if (null === $violation) {
+                continue;
+            }
+
+            $list->add($violation);
+        }
+
+        return $list;
+    }
+
+    private static function buildViolation(FormError $error, FormInterface $form): ?ConstraintViolationInterface
+    {
+        $cause = $error->getCause();
+
+        if (!$cause instanceof ConstraintViolationInterface) {
+            return null;
+        }
+
+        return new ConstraintViolation(
+            $cause->getMessage(),
+            $cause->getMessageTemplate(),
+            $cause->getParameters(),
+            $cause->getRoot(),
+            self::buildName($error->getOrigin() ?? $form),
+            $cause->getInvalidValue(),
+            $cause->getPlural(),
+            $cause->getCode(),
+        );
+    }
+
+    private static function buildName(FormInterface $form): string
+    {
+        $parent = $form->getParent();
+
+        if (null === $parent) {
+            return $form->getName();
+        }
+
+        return self::buildName($parent).'['.$form->getName().']';
+    }
+}

tests/Controller/CRUDControllerTest.php

@@ -67,6 +67,10 @@
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 use Symfony\Component\Security\Csrf\CsrfToken;
 use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
+use Symfony\Component\Serializer\Encoder\JsonEncoder;
+use Symfony\Component\Serializer\Normalizer\ConstraintViolationListNormalizer;
+use Symfony\Component\Serializer\Serializer;
+use Symfony\Component\Validator\ConstraintViolation;
 use Symfony\Contracts\Translation\TranslatorInterface;
 use Twig\Environment;
 
@@ -218,6 +222,11 @@ protected function setUp(): void
         $this->container->set('sonata.admin.request.fetcher', $this->adminFetcher);
         $this->container->set('parameter_bag', $this->parameterBag);
         $this->container->set('http_kernel', $this->httpKernel);
+        $this->container->set('serializer', new Serializer([
+            new ConstraintViolationListNormalizer(),
+        ], [
+            new JsonEncoder(),
+        ]));
         $this->container->set('controller_resolver', $this->controllerResolver);
 
         $this->parameterBag->set(
@@ -1707,14 +1716,21 @@ public function testEditActionAjaxError(): void
             ->method('isSubmitted')
             ->willReturn(true);
 
+        $form->expects(static::once())
+            ->method('getName')
+            ->willReturn('name');
+
         $form->expects(static::once())
             ->method('isValid')
             ->willReturn(false);
 
         $formError = $this->createMock(FormError::class);
-        $formError->expects(static::atLeastOnce())
-            ->method('getMessage')
-            ->willReturn('Form error message');
+        $formError->expects(static::once())
+            ->method('getCause')
+            ->willReturn(new ConstraintViolation('Form error message', '', [], 'root', 'foo', 'bar'));
+        $formError->expects(static::once())
+            ->method('getOrigin')
+            ->willReturn($form);
 
         $form->expects(static::once())
             ->method('getErrors')
@@ -1728,7 +1744,7 @@ public function testEditActionAjaxError(): void
         static::assertInstanceOf(JsonResponse::class, $response = $this->controller->editAction($this->request));
         $content = $response->getContent();
         static::assertNotFalse($content);
-        static::assertJsonStringEqualsJsonString('{"result":"error","errors":["Form error message"]}', $content);
+        static::assertJsonStringEqualsJsonString('{"type":"https:\/\/symfony.com\/errors\/validation","title":"Validation Failed","detail":"name: Form error message","violations":[{"propertyPath":"name","title":"Form error message","parameters":[]}]}', $content);
     }
 
     public function testEditActionAjaxErrorWithoutAcceptApplicationJson(): void
@@ -2377,14 +2393,21 @@ public function testCreateActionAjaxError(): void
             ->method('isSubmitted')
             ->willReturn(true);
 
+        $form->expects(static::once())
+            ->method('getName')
+            ->willReturn('name');
+
         $form->expects(static::once())
             ->method('isValid')
             ->willReturn(false);
 
         $formError = $this->createMock(FormError::class);
-        $formError->expects(static::atLeastOnce())
-            ->method('getMessage')
-            ->willReturn('Form error message');
+        $formError->expects(static::once())
+            ->method('getCause')
+            ->willReturn(new ConstraintViolation('Form error message', '', [], 'root', 'foo', 'bar'));
+        $formError->expects(static::once())
+            ->method('getOrigin')
+            ->willReturn($form);
 
         $form->expects(static::once())
             ->method('getErrors')
@@ -2399,7 +2422,7 @@ public function testCreateActionAjaxError(): void
 
         $content = $response->getContent();
         static::assertNotFalse($content);
-        static::assertJsonStringEqualsJsonString('{"result":"error","errors":["Form error message"]}', $content);
+        static::assertJsonStringEqualsJsonString('{"type":"https:\/\/symfony.com\/errors\/validation","title":"Validation Failed","detail":"name: Form error message","violations":[{"propertyPath":"name","title":"Form error message","parameters":[]}]}', $content);
     }
 
     public function testCreateActionAjaxErrorWithoutAcceptApplicationJson(): void

tests/Form/FormErrorIteratorToConstraintViolationListTest.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Sonata Project package.
+ *
+ * (c) Thomas Rabaix <[email protected]>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Sonata\AdminBundle\Tests\Form;
+
+use PHPUnit\Framework\TestCase;
+use Sonata\AdminBundle\Form\FormErrorIteratorToConstraintViolationList;
+use Symfony\Component\Form\FormError;
+use Symfony\Component\Form\FormErrorIterator;
+use Symfony\Component\Form\FormInterface;
+use Symfony\Component\Validator\ConstraintViolation;
+use Symfony\Component\Validator\ConstraintViolationList;
+
+/**
+ * @author Jordi Sala <[email protected]>
+ */
+final class FormErrorIteratorToConstraintViolationListTest extends TestCase
+{
+    /**
+     * @param FormErrorIterator<FormError> $formErrors
+     *
+     * @dataProvider provideFormErrorIterators
+     */
+    public function testTransform(int $expectedCount, FormErrorIterator $formErrors): void
+    {
+        $violationList = FormErrorIteratorToConstraintViolationList::transform($formErrors);
+
+        static::assertInstanceOf(ConstraintViolationList::class, $violationList);
+        static::assertCount($expectedCount, $violationList);
+    }
+
+    /**
+     * @phpstan-return iterable<array{int, FormErrorIterator<FormError>}>
+     * @psalm-return iterable<array{int, FormErrorIterator<FormError|FormErrorIterator>}>
+     */
+    public function provideFormErrorIterators(): iterable
+    {
+        $form = $this->createStub(FormInterface::class);
+        $form->method('getName')->willReturn('name');
+
+        yield [0, new FormErrorIterator($form, [])];
+
+        yield [0, new FormErrorIterator($form, [
+            new FormError('error'),
+        ])];
+
+        yield [1, new FormErrorIterator($form, [
+            new FormError(
+                'error',
+                null,
+                [],
+                null,
+                new ConstraintViolation('error', null, [], $form, 'path', 'invalid value')
+            ),
+        ])];
+    }
+}

tests/Maker/AdminMakerTest.php

@@ -23,7 +23,6 @@
 use Symfony\Bundle\MakerBundle\Generator;
 use Symfony\Bundle\MakerBundle\Util\AutoloaderUtil;
 use Symfony\Bundle\MakerBundle\Util\MakerFileLinkFormatter;
-use Symfony\Bundle\MakerBundle\Util\PhpCompatUtil;
 use Symfony\Component\Console\Input\ArrayInput;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputDefinition;
@@ -113,8 +112,7 @@ public function testExecute(): void
 
         $this->generator = new Generator(
             $fileManager,
-            'Sonata\AdminBundle\Tests',
-            new PhpCompatUtil($fileManager)
+            'Sonata\AdminBundle\Tests'
         );
         $maker->generate($this->input, $this->io, $this->generator);
     }

tests/Twig/Extension/RenderElementExtensionTest.php

@@ -1486,7 +1486,6 @@ class="x-editable"
             [],
         ];
 
-        // @phpstan-ignore-next-line https://github.com/phpstan/phpstan/issues/7963
         return $elements;
     }
 
@@ -2005,7 +2004,6 @@ class="sonata-readmore"
             [],
         ];
 
-        // @phpstan-ignore-next-line https://github.com/phpstan/phpstan/issues/7963
         return $elements;
     }
 

tests/Twig/RenderElementRuntimeTest.php

@@ -1482,7 +1482,6 @@ class="x-editable"
             [],
         ];
 
-        // @phpstan-ignore-next-line https://github.com/phpstan/phpstan/issues/7963
         return $elements;
     }