diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index 273a71c0cde5..2035e8f805a5 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG --- * Deprecate not configuring the `default_protocol` option of the `UrlType`, it will default to `null` in 8.0 + * Add a `keep_as_list` option to `CollectionType` 7.0 --- diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php index 482007d53b94..641f16525770 100644 --- a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php @@ -31,8 +31,9 @@ class ResizeFormListener implements EventSubscriberInterface protected bool $allowDelete; private \Closure|bool $deleteEmpty; + private bool $keepAsList; - public function __construct(string $type, array $options = [], bool $allowAdd = false, bool $allowDelete = false, bool|callable $deleteEmpty = false, array $prototypeOptions = null) + public function __construct(string $type, array $options = [], bool $allowAdd = false, bool $allowDelete = false, bool|callable $deleteEmpty = false, array $prototypeOptions = null, bool $keepAsList = false) { $this->type = $type; $this->allowAdd = $allowAdd; @@ -40,6 +41,7 @@ public function __construct(string $type, array $options = [], bool $allowAdd = $this->options = $options; $this->deleteEmpty = \is_bool($deleteEmpty) ? $deleteEmpty : $deleteEmpty(...); $this->prototypeOptions = $prototypeOptions ?? $options; + $this->keepAsList = $keepAsList; } public static function getSubscribedEvents(): array @@ -153,6 +155,20 @@ public function onSubmit(FormEvent $event): void } } + if ($this->keepAsList) { + $formReindex = []; + foreach ($form as $name => $child) { + $formReindex[] = $child; + $form->remove($name); + } + foreach ($formReindex as $index => $child) { + $form->add($index, $this->type, array_replace([ + 'property_path' => '['.$index.']', + ], $this->options)); + } + $data = array_values($data); + } + $event->setData($data); } } diff --git a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php index c9d3ec5b7c6e..3cef931526e0 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/CollectionType.php @@ -45,7 +45,8 @@ public function buildForm(FormBuilderInterface $builder, array $options): void $options['allow_add'], $options['allow_delete'], $options['delete_empty'], - $resizePrototypeOptions + $resizePrototypeOptions, + $options['keep_as_list'] ); $builder->addEventSubscriber($resizeListener); @@ -114,12 +115,14 @@ public function configureOptions(OptionsResolver $resolver): void 'prototype_options' => [], 'delete_empty' => false, 'invalid_message' => 'The collection is invalid.', + 'keep_as_list' => false, ]); $resolver->setNormalizer('entry_options', $entryOptionsNormalizer); $resolver->setAllowedTypes('delete_empty', ['bool', 'callable']); $resolver->setAllowedTypes('prototype_options', 'array'); + $resolver->setAllowedTypes('keep_as_list', ['bool']); } public function getBlockPrefix(): string diff --git a/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php b/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php index 3b4cd77396c6..a1d1a3840289 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Validator/Type/FormTypeValidatorExtensionTest.php @@ -15,9 +15,12 @@ use Symfony\Component\Form\Form; use Symfony\Component\Form\Forms; use Symfony\Component\Form\Test\Traits\ValidatorExtensionTrait; +use Symfony\Component\Form\Tests\Extension\Core\Type\CollectionTypeTest; use Symfony\Component\Form\Tests\Extension\Core\Type\FormTypeTest; use Symfony\Component\Form\Tests\Extension\Core\Type\TextTypeTest; use Symfony\Component\Form\Tests\Fixtures\Author; +use Symfony\Component\Form\Tests\Fixtures\AuthorType; +use Symfony\Component\Form\Tests\Fixtures\Organization; use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException; use Symfony\Component\Validator\Constraints\GroupSequence; use Symfony\Component\Validator\Constraints\Length; @@ -158,4 +161,185 @@ protected function createForm(array $options = []) { return $this->factory->create(FormTypeTest::TESTED_TYPE, null, $options); } + + public function testCollectionTypeKeepAsListOptionFalse() + { + $formMetadata = new ClassMetadata(Form::class); + $authorMetadata = (new ClassMetadata(Author::class)) + ->addPropertyConstraint('firstName', new NotBlank()); + $organizationMetadata = (new ClassMetadata(Organization::class)) + ->addPropertyConstraint('authors', new Valid()); + $metadataFactory = $this->createMock(MetadataFactoryInterface::class); + $metadataFactory->expects($this->any()) + ->method('getMetadataFor') + ->willReturnCallback(static function ($classOrObject) use ($formMetadata, $authorMetadata, $organizationMetadata) { + if (Author::class === $classOrObject || $classOrObject instanceof Author) { + return $authorMetadata; + } + + if (Organization::class === $classOrObject || $classOrObject instanceof Organization) { + return $organizationMetadata; + } + + if (Form::class === $classOrObject || $classOrObject instanceof Form) { + return $formMetadata; + } + + return new ClassMetadata(\is_string($classOrObject) ? $classOrObject : $classOrObject::class); + }); + + $validator = Validation::createValidatorBuilder() + ->setMetadataFactory($metadataFactory) + ->getValidator(); + + $form = Forms::createFormFactoryBuilder() + ->addExtension(new ValidatorExtension($validator)) + ->getFormFactory() + ->create(FormTypeTest::TESTED_TYPE, new Organization([]), [ + 'data_class' => Organization::class, + 'by_reference' => false, + ]) + ->add('authors', CollectionTypeTest::TESTED_TYPE, [ + 'entry_type' => AuthorType::class, + 'allow_add' => true, + 'allow_delete' => true, + 'keep_as_list' => false, + ]) + ; + + $form->submit([ + 'authors' => [ + 0 => [ + 'firstName' => '', // Fires a Not Blank Error + 'lastName' => 'lastName1', + ], + // key "1" could be missing if we add 4 blank form entries and then remove it. + 2 => [ + 'firstName' => '', // Fires a Not Blank Error + 'lastName' => 'lastName3', + ], + 3 => [ + 'firstName' => '', // Fires a Not Blank Error + 'lastName' => 'lastName3', + ], + ], + ]); + + // Form does have 3 not blank errors + $errors = $form->getErrors(true); + $this->assertCount(3, $errors); + + // Form behaves as expected. It has index 0, 2 and 3 (1 has been removed) + // But errors property paths mismatch happening with "keep_as_list" option set to false + $errorPaths = [ + $errors[0]->getCause()->getPropertyPath(), + $errors[1]->getCause()->getPropertyPath(), + $errors[2]->getCause()->getPropertyPath(), + ]; + + $this->assertTrue($form->get('authors')->has('0')); + $this->assertContains('data.authors[0].firstName', $errorPaths); + + $this->assertFalse($form->get('authors')->has('1')); + $this->assertContains('data.authors[1].firstName', $errorPaths); + + $this->assertTrue($form->get('authors')->has('2')); + $this->assertContains('data.authors[2].firstName', $errorPaths); + + $this->assertTrue($form->get('authors')->has('3')); + $this->assertNotContains('data.authors[3].firstName', $errorPaths); + + // As result, root form contain errors + $this->assertCount(1, $form->getErrors(false)); + } + + public function testCollectionTypeKeepAsListOptionTrue() + { + $formMetadata = new ClassMetadata(Form::class); + $authorMetadata = (new ClassMetadata(Author::class)) + ->addPropertyConstraint('firstName', new NotBlank()); + $organizationMetadata = (new ClassMetadata(Organization::class)) + ->addPropertyConstraint('authors', new Valid()); + $metadataFactory = $this->createMock(MetadataFactoryInterface::class); + $metadataFactory->expects($this->any()) + ->method('getMetadataFor') + ->willReturnCallback(static function ($classOrObject) use ($formMetadata, $authorMetadata, $organizationMetadata) { + if (Author::class === $classOrObject || $classOrObject instanceof Author) { + return $authorMetadata; + } + + if (Organization::class === $classOrObject || $classOrObject instanceof Organization) { + return $organizationMetadata; + } + + if (Form::class === $classOrObject || $classOrObject instanceof Form) { + return $formMetadata; + } + + return new ClassMetadata(\is_string($classOrObject) ? $classOrObject : $classOrObject::class); + }); + + $validator = Validation::createValidatorBuilder() + ->setMetadataFactory($metadataFactory) + ->getValidator(); + + $form = Forms::createFormFactoryBuilder() + ->addExtension(new ValidatorExtension($validator)) + ->getFormFactory() + ->create(FormTypeTest::TESTED_TYPE, new Organization([]), [ + 'data_class' => Organization::class, + 'by_reference' => false, + ]) + ->add('authors', CollectionTypeTest::TESTED_TYPE, [ + 'entry_type' => AuthorType::class, + 'allow_add' => true, + 'allow_delete' => true, + 'keep_as_list' => true, + ]) + ; + + $form->submit([ + 'authors' => [ + 0 => [ + 'firstName' => '', // Fires a Not Blank Error + 'lastName' => 'lastName1', + ], + // key "1" could be missing if we add 4 blank form entries and then remove it. + 2 => [ + 'firstName' => '', // Fires a Not Blank Error + 'lastName' => 'lastName3', + ], + 3 => [ + 'firstName' => '', // Fires a Not Blank Error + 'lastName' => 'lastName3', + ], + ], + ]); + + // Form does have 3 not blank errors + $errors = $form->getErrors(true); + $this->assertCount(3, $errors); + + // No property paths mismatch happening with "keep_as_list" option set to true + $errorPaths = [ + $errors[0]->getCause()->getPropertyPath(), + $errors[1]->getCause()->getPropertyPath(), + $errors[2]->getCause()->getPropertyPath(), + ]; + + $this->assertTrue($form->get('authors')->has('0')); + $this->assertContains('data.authors[0].firstName', $errorPaths); + + $this->assertTrue($form->get('authors')->has('1')); + $this->assertContains('data.authors[1].firstName', $errorPaths); + + $this->assertTrue($form->get('authors')->has('2')); + $this->assertContains('data.authors[2].firstName', $errorPaths); + + $this->assertFalse($form->get('authors')->has('3')); + $this->assertNotContains('data.authors[3].firstName', $errorPaths); + + // Root form does NOT contain errors + $this->assertCount(0, $form->getErrors(false)); + } } diff --git a/src/Symfony/Component/Form/Tests/Fixtures/Organization.php b/src/Symfony/Component/Form/Tests/Fixtures/Organization.php new file mode 100644 index 000000000000..db9cee9f96ee --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Fixtures/Organization.php @@ -0,0 +1,32 @@ +authors = $authors; + } + + public function getAuthors(): array + { + return $this->authors; + } + + public function addAuthor(Author $author): self + { + $this->authors[] = $author; + return $this; + } + + public function removeAuthor(Author $author): self + { + if (false !== $key = array_search($author, $this->authors, true)) { + array_splice($this->authors, $key, 1); + } + return $this; + } +}