bug #488 Return 422 status code when the form fails (belmeopmenieuwes…

…im, Zales0123)

This PR was merged into the 1.10 branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Bug fix?        | no
| New feature?    | yes
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | replaces #469
| License         | MIT

I\'ve taken over the #469 PR, added some tests and clean up the code. Thank you, @belmeopmenieuwesim, for the contribution 🖖 

Commits
-------

1127c3d Return a proper 422 HTTP status code when the form submission fails
cf06855 Test validation error code
2f3dfc2 Remove additional function for rendering the form from ControllerTrait

src/Bundle/Controller/ControllerTrait.php

@@ -225,8 +225,12 @@ protected function renderView(string $view, array $parameters = []): string
      *
      * @final
      */
-    protected function render(string $view, array $parameters = [], Response $response = null): Response
-    {
+    protected function render(
+        string $view,
+        array $parameters = [],
+        Response $response = null,
+        ?int $responseCode = null
+    ): Response {
         if ($this->container->has('templating')) {
             @trigger_error('Using the "templating" service is deprecated since Symfony 4.3 and will be removed in 5.0; use Twig instead.', \E_USER_DEPRECATED);
 
@@ -242,6 +246,9 @@ protected function render(string $view, array $parameters = [], Response $respon
         }
 
         $response->setContent($content);
+        if ($responseCode !== null) {
+            $response->setStatusCode($responseCode);
+        }
 
         return $response;
     }

src/Bundle/Controller/ResourceController.php

@@ -212,6 +212,9 @@ public function createAction(Request $request): Response
 
             return $this->redirectHandler->redirectToResource($configuration, $newResource);
         }
+        if ($request->isMethod('POST') && $form->isSubmitted() && !$form->isValid()) {
+            $responseCode = Response::HTTP_UNPROCESSABLE_ENTITY;
+        }
 
         if (!$configuration->isHtmlRequest()) {
             return $this->createRestView($configuration, $form, Response::HTTP_BAD_REQUEST);
@@ -229,7 +232,7 @@ public function createAction(Request $request): Response
             'resource' => $newResource,
             $this->metadata->getName() => $newResource,
             'form' => $form->createView(),
-        ]);
+        ], null, $responseCode ?? Response::HTTP_OK);
     }
 
     public function updateAction(Request $request): Response
@@ -299,6 +302,9 @@ public function updateAction(Request $request): Response
 
             return $this->redirectHandler->redirectToResource($configuration, $resource);
         }
+        if (in_array($request->getMethod(), ['POST', 'PUT', 'PATCH'], true) && $form->isSubmitted() && !$form->isValid()) {
+            $responseCode = Response::HTTP_UNPROCESSABLE_ENTITY;
+        }
 
         if (!$configuration->isHtmlRequest()) {
             return $this->createRestView($configuration, $form, Response::HTTP_BAD_REQUEST);
@@ -316,7 +322,7 @@ public function updateAction(Request $request): Response
             'resource' => $resource,
             $this->metadata->getName() => $resource,
             'form' => $form->createView(),
-        ]);
+        ], null, $responseCode ?? Response::HTTP_OK);
     }
 
     public function deleteAction(Request $request): Response

src/Bundle/test/config/services.yaml

@@ -57,6 +57,13 @@ services:
             - '%app.model.book.class%'
         tags: ['form.type']
 
+    app.form.type.science_book:
+        class: App\Form\Type\ScienceBookType
+        arguments:
+            - '%app.model.science_book.class%'
+            - ['sylius']
+        tags: [ 'form.type' ]
+
     app.form.type.book_translation:
         class: App\Form\Type\BookTranslationType
         arguments:

src/Bundle/test/config/sylius/resources.yaml

@@ -27,6 +27,7 @@ sylius_resource:
         app.science_book:
             classes:
                 model: App\Entity\ScienceBook
+                form: App\Form\Type\ScienceBookType
 
         app.gedmo:
             classes:

src/Bundle/test/config/validator/validation.yaml

@@ -0,0 +1,16 @@
+App\Entity\ScienceBook:
+    properties:
+        title:
+            - NotBlank:
+                  groups: [sylius]
+        author:
+            - Valid: ~
+
+App\Entity\Author:
+    properties:
+        firstName:
+            - NotBlank:
+                  groups: [sylius]
+        lastName:
+            - NotBlank:
+                  groups: [sylius]

src/Bundle/test/src/Form/Type/AuthorType.php

@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace App\Form\Type;
+
+use App\Entity\Author;
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\Extension\Core\Type\TextType;
+use Symfony\Component\Form\FormBuilderInterface;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+final class AuthorType extends AbstractType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options): void
+    {
+        $builder
+            ->add('firstName', TextType::class)
+            ->add('lastName', TextType::class)
+        ;
+    }
+
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        $resolver->setDefault('data_class', Author::class);
+        $resolver->setDefault('validation_groups', ['sylius']);
+    }
+}

src/Bundle/test/src/Form/Type/ScienceBookType.php

@@ -0,0 +1,29 @@
+<?php
+
+/*
+ * This file is part of the Sylius package.
+ *
+ * (c) Paweł Jędrzejewski
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace App\Form\Type;
+
+use Sylius\Bundle\ResourceBundle\Form\Type\AbstractResourceType;
+use Symfony\Component\Form\Extension\Core\Type\TextType;
+use Symfony\Component\Form\FormBuilderInterface;
+
+final class ScienceBookType extends AbstractResourceType
+{
+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        $builder
+            ->add('title', TextType::class)
+            ->add('author', AuthorType::class)
+        ;
+    }
+}

src/Bundle/test/src/Tests/Controller/ScienceBookUiTest.php

@@ -68,9 +68,9 @@ public function it_allows_creating_a_book(): void
 
         $this->client->request('GET', '/science-books/new');
         $this->client->submitForm('Create', [
-            'sylius_resource[title]' => $newBookTitle,
-            'sylius_resource[author][firstName]' => $newBookAuthorFirstName,
-            'sylius_resource[author][lastName]' => $newBookAuthorLastName,
+            'science_book[title]' => $newBookTitle,
+            'science_book[author][firstName]' => $newBookAuthorFirstName,
+            'science_book[author][lastName]' => $newBookAuthorLastName,
         ]);
 
         $this->assertResponseRedirects(null, expectedCode: Response::HTTP_FOUND);
@@ -84,6 +84,28 @@ public function it_allows_creating_a_book(): void
         $this->assertSame($newBookAuthorLastName, $book->getAuthorLastName());
     }
 
+    /** @test */
+    public function it_does_not_allow_to_create_a_book_if_there_is_a_validation_error(): void
+    {
+        $newBookTitle = 'The Book of Why';
+        $newBookAuthorLastName = 'Pearl';
+
+        $this->loadFixturesFromFile('science_books.yml');
+
+        $this->client->request('GET', '/science-books/new');
+        $this->client->submitForm('Create', [
+            'science_book[title]' => $newBookTitle,
+            'science_book[author][lastName]' => $newBookAuthorLastName,
+        ]);
+
+        $this->assertResponseCode($this->client->getResponse(), Response::HTTP_UNPROCESSABLE_ENTITY);
+
+        /** @var ScienceBook $book */
+        $book = static::getContainer()->get('app.repository.science_book')->findOneBy(['title' => $newBookTitle]);
+
+        $this->assertNull($book);
+    }
+
     /** @test */
     public function it_allows_updating_a_book(): void
     {
@@ -95,9 +117,9 @@ public function it_allows_updating_a_book(): void
 
         $this->client->request('GET', '/science-books/' . $scienceBooks['science-book1']->getId() . '/edit');
         $this->client->submitForm('Save changes', [
-            'sylius_resource[title]' => $newBookTitle,
-            'sylius_resource[author][firstName]' => $newBookAuthorFirstName,
-            'sylius_resource[author][lastName]' => $newBookAuthorLastName,
+            'science_book[title]' => $newBookTitle,
+            'science_book[author][firstName]' => $newBookAuthorFirstName,
+            'science_book[author][lastName]' => $newBookAuthorLastName,
         ]);
 
         $this->assertResponseRedirects(null, expectedCode: Response::HTTP_FOUND);
@@ -111,6 +133,29 @@ public function it_allows_updating_a_book(): void
         $this->assertSame($newBookAuthorLastName, $book->getAuthorLastName());
     }
 
+    /** @test */
+    public function it_does_not_allow_to_update_a_book_if_there_is_a_validation_error(): void
+    {
+        $newBookTitle = 'The Book of Why';
+        $newBookAuthorLastName = 'Pearl';
+
+        $scienceBooks = $this->loadFixturesFromFile('science_books.yml');
+
+        $this->client->request('GET', '/science-books/' . $scienceBooks['science-book1']->getId() . '/edit');
+        $this->client->submitForm('Save changes', [
+            'science_book[title]' => $newBookTitle,
+            'science_book[author][firstName]' => null,
+            'science_book[author][lastName]' => $newBookAuthorLastName,
+        ]);
+
+        $this->assertResponseCode($this->client->getResponse(), Response::HTTP_UNPROCESSABLE_ENTITY);
+
+        /** @var ScienceBook $book */
+        $book = static::getContainer()->get('app.repository.science_book')->findOneBy(['title' => $newBookTitle]);
+
+        $this->assertNull($book);
+    }
+
     /** @test */
     public function it_allows_deleting_a_book(): void
     {