From 3d1f8d4d5a3cac8c861aa44a7d813c267e357acc Mon Sep 17 00:00:00 2001 From: Cristoforo Cervino Date: Thu, 18 Mar 2021 22:16:25 +0100 Subject: [PATCH 1/2] allow using form events as expected with inherit_data option --- src/Symfony/Component/Form/Form.php | 77 +++++++++++++------ .../Extension/Core/Type/FormTypeTest.php | 48 ++++++++++++ 2 files changed, 103 insertions(+), 22 deletions(-) diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 1901524562c6a..059cf7c8648e9 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -333,13 +333,18 @@ public function setData($modelData) } $this->lockSetData = true; - $dispatcher = $this->config->getEventDispatcher(); + + // Collecting $this + all children with "inherit_data" option + $thisModelDataAwareForms = $this->getSameModelAwareForms($this); // Hook to change content of the model data before transformation and mapping children - if ($dispatcher->hasListeners(FormEvents::PRE_SET_DATA)) { - $event = new PreSetDataEvent($this, $modelData); - $dispatcher->dispatch($event, FormEvents::PRE_SET_DATA); - $modelData = $event->getData(); + foreach ($thisModelDataAwareForms as $form) { + $dispatcher = $form->getConfig()->getEventDispatcher(); + if ($dispatcher->hasListeners(FormEvents::PRE_SET_DATA)) { + $event = new PreSetDataEvent($form, $modelData); + $dispatcher->dispatch($event, FormEvents::PRE_SET_DATA); + $modelData = $event->getData(); + } } // Treat data as strings unless a transformer exists @@ -359,7 +364,7 @@ public function setData($modelData) if (null !== $dataClass && !$viewData instanceof $dataClass) { $actualType = get_debug_type($viewData); - throw new LogicException('The form\'s view data is expected to be a "'.$dataClass.'", but it is a "'.$actualType.'". You can avoid this error by setting the "data_class" option to null or by adding a view transformer that transforms "'.$actualType.'" to an instance of "'.$dataClass.'".'); + throw new LogicException('The form\'s view data is expected to be a ".'.$dataClass.'", but it is a "'.$actualType.'". You can avoid this error by setting the "data_class" option to null or by adding a view transformer that transforms "'.$actualType.'" to an instance of "'.$dataClass.'".'); } } @@ -374,10 +379,12 @@ public function setData($modelData) // Update child forms from the data (unless their config data is locked) $this->config->getDataMapper()->mapDataToForms($viewData, new \RecursiveIteratorIterator(new InheritDataAwareIterator($this->children))); } - - if ($dispatcher->hasListeners(FormEvents::POST_SET_DATA)) { - $event = new PostSetDataEvent($this, $modelData); - $dispatcher->dispatch($event, FormEvents::POST_SET_DATA); + foreach ($thisModelDataAwareForms as $form) { + $dispatcher = $form->getConfig()->getEventDispatcher(); + if ($dispatcher->hasListeners(FormEvents::POST_SET_DATA)) { + $event = new PostSetDataEvent($form, $modelData); + $dispatcher->dispatch($event, FormEvents::POST_SET_DATA); + } } return $this; @@ -538,7 +545,8 @@ public function submit($submittedData, bool $clearMissing = true) $this->transformationFailure = new TransformationFailedException('Submitted data was expected to be text or number, array given.'); } - $dispatcher = $this->config->getEventDispatcher(); + // Collecting $this + all children with "inherit_data" option only if $this is not using "inherit_data" option + $thisModelDataAwareForms = !$this->getConfig()->getInheritData() ? $this->getSameModelAwareForms($this) : []; $modelData = null; $normData = null; @@ -550,10 +558,13 @@ public function submit($submittedData, bool $clearMissing = true) } // Hook to change content of the data submitted by the browser - if ($dispatcher->hasListeners(FormEvents::PRE_SUBMIT)) { - $event = new PreSubmitEvent($this, $submittedData); - $dispatcher->dispatch($event, FormEvents::PRE_SUBMIT); - $submittedData = $event->getData(); + foreach ($thisModelDataAwareForms as $form) { + $dispatcher = $form->getConfig()->getEventDispatcher(); + if ($dispatcher->hasListeners(FormEvents::PRE_SUBMIT)) { + $event = new PreSubmitEvent($form, $submittedData); + $dispatcher->dispatch($event, FormEvents::PRE_SUBMIT); + $submittedData = $event->getData(); + } } // Check whether the form is compound. @@ -636,10 +647,13 @@ public function submit($submittedData, bool $clearMissing = true) // Hook to change content of the data in the normalized // representation - if ($dispatcher->hasListeners(FormEvents::SUBMIT)) { - $event = new SubmitEvent($this, $normData); - $dispatcher->dispatch($event, FormEvents::SUBMIT); - $normData = $event->getData(); + foreach ($thisModelDataAwareForms as $form) { + $dispatcher = $form->getConfig()->getEventDispatcher(); + if ($dispatcher->hasListeners(FormEvents::SUBMIT)) { + $event = new SubmitEvent($form, $normData); + $dispatcher->dispatch($event, FormEvents::SUBMIT); + $normData = $event->getData(); + } } // Synchronize representations - must not change the content! @@ -663,14 +677,33 @@ public function submit($submittedData, bool $clearMissing = true) $this->normData = $normData; $this->viewData = $viewData; - if ($dispatcher->hasListeners(FormEvents::POST_SUBMIT)) { - $event = new PostSubmitEvent($this, $viewData); - $dispatcher->dispatch($event, FormEvents::POST_SUBMIT); + foreach ($thisModelDataAwareForms as $form) { + $dispatcher = $form->getConfig()->getEventDispatcher(); + if ($dispatcher->hasListeners(FormEvents::POST_SUBMIT)) { + $event = new PostSubmitEvent($form, $viewData); + $dispatcher->dispatch($event, FormEvents::POST_SUBMIT); + } } return $this; } + /** + * @return array + */ + private function getSameModelAwareForms(self $form): array + { + return array_reduce(iterator_to_array($form->children), + static function (array $children, FormInterface $child): array { + if ($child->getConfig()->getInheritData()) { + $children[] = $child; + } + + return $children; + }, [$form] + ); + } + /** * {@inheritdoc} */ diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php index 3701b653f855e..eb9a24c863693 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php @@ -13,6 +13,7 @@ use Symfony\Component\Form\CallbackTransformer; use Symfony\Component\Form\DataMapperInterface; +use Symfony\Component\Form\Event\PostSubmitEvent; use Symfony\Component\Form\Exception\InvalidArgumentException; use Symfony\Component\Form\Exception\LogicException; use Symfony\Component\Form\Exception\TransformationFailedException; @@ -23,6 +24,8 @@ use Symfony\Component\Form\Form; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormError; +use Symfony\Component\Form\FormEvent; +use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\Forms; use Symfony\Component\Form\Tests\Fixtures\Author; use Symfony\Component\Form\Tests\Fixtures\FixedDataTransformer; @@ -859,6 +862,51 @@ public function testSortingViewChildrenBasedOnPriorityOption() ]; $this->assertSame($expected, array_keys($view->children)); } + + public function testSetDataEventsDispatchedToChildrenWithInheritDataConfigured() + { + $data = ['child' => 'child_value']; + $calledEvents = []; + $form = $this->factory->createNamedBuilder('form', self::TESTED_TYPE, $data) + ->add( + $this->factory->createNamedBuilder('inherit_data_type', self::TESTED_TYPE, null, [ + 'inherit_data' => true, + ]) + ->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) use (&$calledEvents) { + $calledEvents[] = FormEvents::PRE_SET_DATA; + $event->getForm()->add('child', self::TESTED_TYPE); + }) + ->addEventListener(FormEvents::POST_SET_DATA, function (FormEvent $event) use (&$calledEvents) { + $calledEvents[] = FormEvents::POST_SET_DATA; + $event->getForm()->add('child2', self::TESTED_TYPE, ['data' => $event->getData()['child']]); + }) + ->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use (&$calledEvents) { + $calledEvents[] = FormEvents::PRE_SUBMIT; + $this->assertSame($event->getData(), $event->getForm()->getData()); + }) + ->addEventListener(FormEvents::SUBMIT, function (FormEvent $event) use (&$calledEvents) { + $calledEvents[] = FormEvents::SUBMIT; + $this->assertNotNull($event->getData()); + }) + ->addEventListener(FormEvents::POST_SUBMIT, function (PostSubmitEvent $event) use (&$calledEvents) { + $calledEvents[] = FormEvents::POST_SUBMIT; + $this->assertNotNull($event->getData()); + }) + ) + ->getForm(); + $this->assertTrue($form->get('inherit_data_type')->has('child')); + $this->assertTrue($form->get('inherit_data_type')->has('child2')); + $this->assertSame('child_value', $form->get('inherit_data_type')->get('child')->getData()); + $this->assertSame('child_value', $form->get('inherit_data_type')->get('child2')->getData()); + $errorMessage = '"%s" event has not been called on form child with "inherit_data" option.'; + $form->submit($data); + $this->assertContains(FormEvents::PRE_SET_DATA, $calledEvents, sprintf($errorMessage, FormEvents::PRE_SET_DATA)); + $this->assertContains(FormEvents::POST_SET_DATA, $calledEvents, sprintf($errorMessage, FormEvents::POST_SET_DATA)); + $this->assertContains(FormEvents::PRE_SUBMIT, $calledEvents, sprintf($errorMessage, FormEvents::PRE_SUBMIT)); + $this->assertContains(FormEvents::SUBMIT, $calledEvents, sprintf($errorMessage, FormEvents::SUBMIT)); + $this->assertContains(FormEvents::POST_SUBMIT, $calledEvents, sprintf($errorMessage, FormEvents::POST_SUBMIT)); + $this->assertCount(5, $calledEvents); + } } class Money From e1ccfe32ca006bb8999d2fe374ea134695ab5c00 Mon Sep 17 00:00:00 2001 From: Cristoforo Cervino Date: Fri, 21 Oct 2022 14:09:34 +0200 Subject: [PATCH 2/2] fix PRE_SET_DATA error --- src/Symfony/Component/Form/Form.php | 51 +++++++++---------- .../Extension/Core/Type/FormTypeTest.php | 8 +-- .../Form/Util/InheritDataAwareFilter.php | 23 +++++++++ 3 files changed, 52 insertions(+), 30 deletions(-) create mode 100644 src/Symfony/Component/Form/Util/InheritDataAwareFilter.php diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 0601805d4a34c..140f05242e1d3 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -24,6 +24,7 @@ use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\Util\FormUtil; +use Symfony\Component\Form\Util\InheritDataAwareFilter; use Symfony\Component\Form\Util\InheritDataAwareIterator; use Symfony\Component\Form\Util\OrderedHashMap; use Symfony\Component\PropertyAccess\PropertyPath; @@ -282,11 +283,13 @@ public function setData(mixed $modelData): static $this->lockSetData = true; - // Collecting $this + all children with "inherit_data" option - $thisModelDataAwareForms = $this->getSameModelAwareForms($this); + // Collecting $this + all children with "inherit_data" option TRUE + $thisModelDataAwareFormsIterator = new \AppendIterator(); + $thisModelDataAwareFormsIterator->append(new \ArrayIterator([$this])); + $thisModelDataAwareFormsIterator->append(new InheritDataAwareFilter(new InheritDataAwareIterator($this->children))); // Hook to change content of the model data before transformation and mapping children - foreach ($thisModelDataAwareForms as $form) { + foreach ($thisModelDataAwareFormsIterator as $form) { $dispatcher = $form->getConfig()->getEventDispatcher(); if ($dispatcher->hasListeners(FormEvents::PRE_SET_DATA)) { $event = new PreSetDataEvent($form, $modelData); @@ -327,7 +330,8 @@ public function setData(mixed $modelData): static // Update child forms from the data (unless their config data is locked) $this->config->getDataMapper()->mapDataToForms($viewData, new \RecursiveIteratorIterator(new InheritDataAwareIterator($this->children))); } - foreach ($thisModelDataAwareForms as $form) { + + foreach ($thisModelDataAwareFormsIterator as $form) { $dispatcher = $form->getConfig()->getEventDispatcher(); if ($dispatcher->hasListeners(FormEvents::POST_SET_DATA)) { $event = new PostSetDataEvent($form, $modelData); @@ -472,8 +476,13 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static $this->transformationFailure = new TransformationFailedException('Submitted data was expected to be text or number, array given.'); } - // Collecting $this + all children with "inherit_data" option only if $this is not using "inherit_data" option - $thisModelDataAwareForms = !$this->getConfig()->getInheritData() ? $this->getSameModelAwareForms($this) : []; + $thisModelDataAwareFormsIterator = new \AppendIterator(); + + // Collecting $this + all children with "inherit_data" option TRUE only if $this has "inherit_data" option FALSE to avoid to dispatch events two times. + if (!$this->getConfig()->getInheritData()) { + $thisModelDataAwareFormsIterator->append(new \ArrayIterator(['' => $this])); + $thisModelDataAwareFormsIterator->append(new InheritDataAwareFilter(new InheritDataAwareIterator($this->children))); + } $modelData = null; $normData = null; @@ -485,10 +494,16 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static } // Hook to change content of the data submitted by the browser - foreach ($thisModelDataAwareForms as $form) { + foreach ($thisModelDataAwareFormsIterator as $name => $form) { $dispatcher = $form->getConfig()->getEventDispatcher(); if ($dispatcher->hasListeners(FormEvents::PRE_SUBMIT)) { - $event = new PreSubmitEvent($form, $submittedData); + $eventSubmittedData = null; + if (empty($name)) { + $eventSubmittedData = $submittedData; + } elseif (\array_key_exists($name, $submittedData)) { + $eventSubmittedData = $submittedData[$name]; + } + $event = new PreSubmitEvent($form, $eventSubmittedData); $dispatcher->dispatch($event, FormEvents::PRE_SUBMIT); $submittedData = $event->getData(); } @@ -574,7 +589,7 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static // Hook to change content of the data in the normalized // representation - foreach ($thisModelDataAwareForms as $form) { + foreach ($thisModelDataAwareFormsIterator as $form) { $dispatcher = $form->getConfig()->getEventDispatcher(); if ($dispatcher->hasListeners(FormEvents::SUBMIT)) { $event = new SubmitEvent($form, $normData); @@ -604,7 +619,7 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static $this->normData = $normData; $this->viewData = $viewData; - foreach ($thisModelDataAwareForms as $form) { + foreach ($thisModelDataAwareFormsIterator as $form) { $dispatcher = $form->getConfig()->getEventDispatcher(); if ($dispatcher->hasListeners(FormEvents::POST_SUBMIT)) { $event = new PostSubmitEvent($form, $viewData); @@ -615,22 +630,6 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static return $this; } - /** - * @return array - */ - private function getSameModelAwareForms(self $form): array - { - return array_reduce(iterator_to_array($form->children), - static function (array $children, FormInterface $child): array { - if ($child->getConfig()->getInheritData()) { - $children[] = $child; - } - - return $children; - }, [$form] - ); - } - public function addError(FormError $error): static { if (null === $error->getOrigin()) { diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php index eb9a24c863693..11b1410d80fc9 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php @@ -865,7 +865,7 @@ public function testSortingViewChildrenBasedOnPriorityOption() public function testSetDataEventsDispatchedToChildrenWithInheritDataConfigured() { - $data = ['child' => 'child_value']; + $data = ['child' => 'foo']; $calledEvents = []; $form = $this->factory->createNamedBuilder('form', self::TESTED_TYPE, $data) ->add( @@ -882,7 +882,7 @@ public function testSetDataEventsDispatchedToChildrenWithInheritDataConfigured() }) ->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use (&$calledEvents) { $calledEvents[] = FormEvents::PRE_SUBMIT; - $this->assertSame($event->getData(), $event->getForm()->getData()); + $this->assertNotNull($event->getData()); }) ->addEventListener(FormEvents::SUBMIT, function (FormEvent $event) use (&$calledEvents) { $calledEvents[] = FormEvents::SUBMIT; @@ -896,8 +896,8 @@ public function testSetDataEventsDispatchedToChildrenWithInheritDataConfigured() ->getForm(); $this->assertTrue($form->get('inherit_data_type')->has('child')); $this->assertTrue($form->get('inherit_data_type')->has('child2')); - $this->assertSame('child_value', $form->get('inherit_data_type')->get('child')->getData()); - $this->assertSame('child_value', $form->get('inherit_data_type')->get('child2')->getData()); + $this->assertSame('foo', $form->get('inherit_data_type')->get('child')->getData()); + $this->assertSame('foo', $form->get('inherit_data_type')->get('child2')->getData()); $errorMessage = '"%s" event has not been called on form child with "inherit_data" option.'; $form->submit($data); $this->assertContains(FormEvents::PRE_SET_DATA, $calledEvents, sprintf($errorMessage, FormEvents::PRE_SET_DATA)); diff --git a/src/Symfony/Component/Form/Util/InheritDataAwareFilter.php b/src/Symfony/Component/Form/Util/InheritDataAwareFilter.php new file mode 100644 index 0000000000000..916e4e0efb2e6 --- /dev/null +++ b/src/Symfony/Component/Form/Util/InheritDataAwareFilter.php @@ -0,0 +1,23 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +/** + * @author Cristoforo Cervino + */ +namespace Symfony\Component\Form\Util; + +class InheritDataAwareFilter extends \FilterIterator +{ + public function accept(): bool + { + return (bool) $this->current()->getConfig()->getInheritData(); + } +}