Skip to content

[Form] *_SET_DATA events are now guaranteed to be fired *after* the initial children were added #7878

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.

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

Merged
merged 1 commit into from
Apr 30, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 60 additions & 0 deletions UPGRADE-2.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,66 @@ Form
}
```

* The *_SET_DATA events are now guaranteed to be fired *after* the children
were added by the FormBuilder (unless setData() is called manually). Before,
the *_SET_DATA events were sometimes thrown before adding child forms,
which made it impossible to remove child forms dynamically.

A consequence of this change is that you need to set the "auto_initialize"
option to `false` for `FormInterface` instances that you pass to
`FormInterface::add()`:

Before:

```
$form = $factory->create('form');
$form->add($factory->createNamed('field', 'text'));
```

This code will now throw an exception with the following message:

Automatic initialization is only supported on root forms. You should set the
"auto_initialize" option to false on the field "field".

Consequently, you need to set the "auto_initialize" option:

After (Alternative 1):

```
$form = $factory->create('form');
$form->add($factory->createNamed('field', 'text', array(
'auto_initialize' => false,
)));
```

The problem also disappears if you work with `FormBuilder` instances instead
of `Form` instances:

After (Alternative 2):

```
$builder = $factory->createBuilder('form');
$builder->add($factory->createBuilder('field', 'text'));
$form = $builder->getForm();
```

The best solution is in most cases to let `add()` handle the field creation:

After (Alternative 3):

```
$form = $factory->create('form');
$form->add('field', 'text');
```

After (Alternative 4):

```
$builder = $factory->createBuilder('form');
$builder->add('field', 'text');
$form = $builder->getForm();
```

PropertyAccess
--------------

Expand Down
12 changes: 11 additions & 1 deletion src/Symfony/Component/Form/Button.php
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,16 @@ public function isSynchronized()
return true;
}

/**
* Unsupported method.
*
* @throws BadMethodCallException
*/
public function initialize()
{
throw new BadMethodCallException('Buttons cannot be initialized. Call initialized() on the root form instead.');
Copy link
Member

Choose a reason for hiding this comment

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

shouldn'tit be initialize() here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in #7899

}

/**
* Unsupported method.
*
Expand All @@ -352,7 +362,7 @@ public function isSynchronized()
*/
public function handleRequest($request = null)
{
throw new BadMethodCallException('Buttons cannot be processed. Call process() on the root form instead.');
throw new BadMethodCallException('Buttons cannot handle requests. Call handleRequest() on the root form instead.');
}

/**
Expand Down
26 changes: 26 additions & 0 deletions src/Symfony/Component/Form/ButtonBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,22 @@ public function setRequestHandler(RequestHandlerInterface $requestHandler)
throw new BadMethodCallException('Buttons do not support form processors.');
}

/**
* Unsupported method.
*
* @param Boolean $initialize
*
* @throws BadMethodCallException
*/
public function setAutoInitialize($initialize)
{
if (true === $initialize) {
throw new BadMethodCallException('Buttons do not support automatic initialization.');
}

return $this;
}

/**
* Unsupported method.
*
Expand Down Expand Up @@ -771,6 +787,16 @@ public function getRequestHandler()
return null;
}

/**
* Unsupported method.
*
* @return null Always returns null.
*/
public function getAutoInitialize()
{
return null;
Copy link
Member

Choose a reason for hiding this comment

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

@bschussek shoudln't it return false instead as autoInitialize is a boolean ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in #7899

}

/**
* Unsupported method.
*
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/Form/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ CHANGELOG
* deprecated bind() and isBound() in Form
* deprecated AlreadyBoundException in favor of AlreadySubmittedException
* added support for PATCH requests
* [BC BREAK] added initialize() to FormInterface
* [BC BREAK] added getAutoInitialize() to FormConfigInterface
* [BC BREAK] added setAutoInitialize() to FormConfigBuilderInterface
* [BC BREAK] initialization for Form instances added to a form tree must be manually disabled
* PRE_SET_DATA is now guaranteed to be called after children were added by the form builder,
unless FormInterface::setData() is called manually

2.2.0
-----
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/Form/Extension/Core/Type/FormType.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public function buildForm(FormBuilderInterface $builder, array $options)
->setDataMapper($options['compound'] ? new PropertyPathMapper($this->propertyAccessor) : null)
->setMethod($options['method'])
->setAction($options['action'])
->setAutoInitialize($options['auto_initialize'])
;

if ($options['trim']) {
Expand Down Expand Up @@ -188,6 +189,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
// According to RFC 2396 (http://www.ietf.org/rfc/rfc2396.txt)
// section 4.2., empty URIs are considered same-document references
'action' => '',
'auto_initialize' => true,
));

$resolver->setAllowedTypes(array(
Expand Down
36 changes: 34 additions & 2 deletions src/Symfony/Component/Form/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,25 @@ public function getExtraData()
return $this->extraData;
}

/**
* {@inheritdoc}
*/
public function initialize()
{
if (null !== $this->parent) {
throw new RuntimeException('Only root forms should be initialized.');
}

// Guarantee that the *_SET_DATA events have been triggered once the
// form is initialized. This makes sure that dynamically added or
// removed fields are already visible after initialization.
if (!$this->defaultDataSet) {
$this->setData($this->config->getData());
}

return $this;
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -774,7 +793,11 @@ public function add($child, $type = null, array $options = array())
// * getViewData() is called
// * setData() is called since the form is not initialized yet
// * ... endless recursion ...
if (!$this->lockSetData && !$this->config->getInheritData()) {
//
// Also skip data mapping if setData() has not been called yet.
// setData() will be called upon form initialization and data mapping
// will take place by then.
if (!$this->lockSetData && $this->defaultDataSet && !$this->config->getInheritData()) {
$viewData = $this->getViewData();
}

Expand All @@ -787,18 +810,27 @@ public function add($child, $type = null, array $options = array())
throw new UnexpectedTypeException($type, 'string or Symfony\Component\Form\FormTypeInterface');
}

// Never initialize child forms automatically
$options['auto_initialize'] = false;

if (null === $type) {
$child = $this->config->getFormFactory()->createForProperty($this->config->getDataClass(), $child, null, $options);
} else {
$child = $this->config->getFormFactory()->createNamed($child, $type, null, $options);
}
} elseif ($child->getConfig()->getAutoInitialize()) {
throw new RuntimeException(sprintf(
'Automatic initialization is only supported on root forms. You '.
'should set the "auto_initialize" option to false on the field "%s".',
$child->getName()
));
}

$this->children[$child->getName()] = $child;

$child->setParent($this);

if (!$this->lockSetData && !$this->config->getInheritData()) {
if (!$this->lockSetData && $this->defaultDataSet && !$this->config->getInheritData()) {
$childrenIterator = new InheritDataAwareIterator(array($child));
$childrenIterator = new \RecursiveIteratorIterator($childrenIterator);
$this->config->getDataMapper()->mapDataToForms($viewData, $childrenIterator);
Expand Down
10 changes: 8 additions & 2 deletions src/Symfony/Component/Form/FormBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class FormBuilder extends FormConfigBuilder implements \IteratorAggregate, FormB
/**
* The children of the form builder.
*
* @var array
* @var FormBuilderInterface[]
*/
private $children = array();

Expand Down Expand Up @@ -220,7 +220,13 @@ public function getForm()
$form = new Form($this->getFormConfig());

foreach ($this->children as $child) {
$form->add($child->getForm());
// Automatic initialization is only supported on root forms
$form->add($child->setAutoInitialize(false)->getForm());
}

if ($this->getAutoInitialize()) {
// Automatically initialize the form if it is configured so
$form->initialize();
}

return $form;
Expand Down
28 changes: 23 additions & 5 deletions src/Symfony/Component/Form/FormConfigBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,6 @@ class FormConfigBuilder implements FormConfigBuilderInterface
*/
private $dataMapper;

/**
* @var array
*/
private $validators = array();

/**
* @var Boolean
*/
Expand Down Expand Up @@ -172,6 +167,11 @@ class FormConfigBuilder implements FormConfigBuilderInterface
*/
private $requestHandler;

/**
* @var Boolean
*/
private $autoInitialize = false;

/**
* @var array
*/
Expand Down Expand Up @@ -521,6 +521,14 @@ public function getRequestHandler()
return $this->requestHandler;
}

/**
* {@inheritdoc}
*/
public function getAutoInitialize()
{
return $this->autoInitialize;
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -843,6 +851,16 @@ public function setRequestHandler(RequestHandlerInterface $requestHandler)
return $this;
}

/**
* {@inheritdoc}
*/
public function setAutoInitialize($initialize)
{
$this->autoInitialize = (Boolean) $initialize;

return $this;
}

/**
* {@inheritdoc}
*/
Expand Down
16 changes: 16 additions & 0 deletions src/Symfony/Component/Form/FormConfigBuilderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,28 @@ public function setAction($action);
public function setMethod($method);

/**
* Sets the request handler used by the form.
*
* @param RequestHandlerInterface $requestHandler
*
* @return self The configuration object.
*/
public function setRequestHandler(RequestHandlerInterface $requestHandler);

/**
* Sets whether the form should be initialized automatically.
*
* Should be set to true only for root forms.
*
* @param Boolean $initialize True to initialize the form automatically,
* false to suppress automatic initialization.
* In the second case, you need to call
* {@link FormInterface::initialize()} manually.
*
* @return self The configuration object.
*/
public function setAutoInitialize($initialize);

/**
* Builds and returns the form configuration.
*
Expand Down
12 changes: 11 additions & 1 deletion src/Symfony/Component/Form/FormConfigInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,20 @@ public function getAction();
public function getMethod();

/**
* @return RequestHandlerInterface The form processor.
* Returns the request handler used by the form.
*
* @return RequestHandlerInterface The request handler.
*/
public function getRequestHandler();

/**
* Returns whether the form should be initialized upon creation.
*
* @return Boolean Returns true if the form should be initialized
* when created, false otherwise.
*/
public function getAutoInitialize();

/**
* Returns all options passed during the construction of the form.
*
Expand Down
20 changes: 15 additions & 5 deletions src/Symfony/Component/Form/FormInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,23 @@ public function isEmpty();
public function isSynchronized();

/**
* Processes the given request and calls {@link submit()} if it was submitted.
* Initializes the form tree.
*
* Internally, the request is forwarded to a {@link RequestHandlerInterface}
* instance. This instance determines the allowed value of the
* $request parameter.
* Should be called on the root form after constructing the tree.
*
* @param mixed $request The request to check.
* @return FormInterface The form instance.
*/
public function initialize();

/**
* Inspects the given request and calls {@link submit()} if the form was
* submitted.
*
* Internally, the request is forwarded to the configured
* {@link RequestHandlerInterface} instance, which determines whether to
* submit the form or not.
*
* @param mixed $request The request to handle.
*
* @return FormInterface The form instance.
*/
Expand Down
Loading