Skip to content

[Form]  Allow Form events to be used as expected with inherit_data option #40515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: 7.3
Choose a base branch
from
Open

[Form]  Allow Form events to be used as expected with inherit_data option #40515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d1f8d4
allow using form events as expected with inherit_data option
cristoforocervino Mar 18, 2021
8392534
Merge branch '6.2' into pre-set-data-event-to-inherit-data-true-children
cristoforocervino Oct 21, 2022
e1ccfe3
fix PRE_SET_DATA error
cristoforocervino Oct 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 52 additions & 20 deletions src/Symfony/Component/Form/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -281,13 +282,20 @@ public function setData(mixed $modelData): static
}

$this->lockSetData = true;
$dispatcher = $this->config->getEventDispatcher();

// 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
if ($dispatcher->hasListeners(FormEvents::PRE_SET_DATA)) {
$event = new PreSetDataEvent($this, $modelData);
$dispatcher->dispatch($event, FormEvents::PRE_SET_DATA);
$modelData = $event->getData();
foreach ($thisModelDataAwareFormsIterator 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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would not allow child forms with inherit_data to change the model data of the parent form. This is a very bad idea IMO (and was one of the reasons why this was not supported)

}
}

// Treat data as strings unless a transformer exists
Expand Down Expand Up @@ -323,9 +331,12 @@ public function setData(mixed $modelData): static
$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 ($thisModelDataAwareFormsIterator 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;
Expand Down Expand Up @@ -465,7 +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.');
}

$dispatcher = $this->config->getEventDispatcher();
$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;
Expand All @@ -477,10 +494,19 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static
}

// 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 ($thisModelDataAwareFormsIterator as $name => $form) {
$dispatcher = $form->getConfig()->getEventDispatcher();
if ($dispatcher->hasListeners(FormEvents::PRE_SUBMIT)) {
$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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is broken. It will replace the whole submitted data by the data of the child only.

}
}

// Check whether the form is compound.
Expand Down Expand Up @@ -563,10 +589,13 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static

// 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 ($thisModelDataAwareFormsIterator 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!
Expand All @@ -590,9 +619,12 @@ public function submit(mixed $submittedData, bool $clearMissing = true): static
$this->normData = $normData;
$this->viewData = $viewData;

if ($dispatcher->hasListeners(FormEvents::POST_SUBMIT)) {
$event = new PostSubmitEvent($this, $viewData);
$dispatcher->dispatch($event, FormEvents::POST_SUBMIT);
foreach ($thisModelDataAwareFormsIterator 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;
Expand Down
48 changes: 48 additions & 0 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -859,6 +862,51 @@ public function testSortingViewChildrenBasedOnPriorityOption()
];
$this->assertSame($expected, array_keys($view->children));
}

public function testSetDataEventsDispatchedToChildrenWithInheritDataConfigured()
{
$data = ['child' => 'foo'];
$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->assertNotNull($event->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('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));
$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
Expand Down
23 changes: 23 additions & 0 deletions src/Symfony/Component/Form/Util/InheritDataAwareFilter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

/**
* @author Cristoforo Cervino <cristoforo.cervino@me.com>
*/
namespace Symfony\Component\Form\Util;

class InheritDataAwareFilter extends \FilterIterator
{
public function accept(): bool
{
return (bool) $this->current()->getConfig()->getInheritData();
}
}