Navigation Menu

Skip to content

Commit

Permalink
[Form] Fixed: Empty forms can be compound too
Browse files Browse the repository at this point in the history
  • Loading branch information
webmozart committed Jul 3, 2012
1 parent 8597764 commit 45d967e
Show file tree
Hide file tree
Showing 12 changed files with 204 additions and 108 deletions.
Expand Up @@ -24,7 +24,7 @@ class PropertyPathMapper implements DataMapperInterface
public function mapDataToForms($data, array $forms)
{
if (!empty($data) && !is_array($data) && !is_object($data)) {
throw new UnexpectedTypeException($data, 'Object, array or empty');
throw new UnexpectedTypeException($data, 'object, array or empty');
}

if (!empty($data)) {
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/Form/Extension/Core/Type/FormType.php
Expand Up @@ -42,6 +42,7 @@ public function buildForm(FormBuilderInterface $builder, array $options)
->setMapped($options['mapped'])
->setByReference($options['by_reference'])
->setVirtual($options['virtual'])
->setCompound($options['compound'])
->setData($options['data'])
->setDataMapper(new PropertyPathMapper())
;
Expand Down Expand Up @@ -116,7 +117,7 @@ public function buildView(FormViewInterface $view, FormInterface $form, array $o
'multipart' => false,
'attr' => $options['attr'],
'label_attr' => $options['label_attr'],
'compound' => $options['compound'],
'compound' => $form->getConfig()->getCompound(),
'types' => $types,
'translation_domain' => $translationDomain,
));
Expand Down Expand Up @@ -160,7 +161,7 @@ public function setDefaultOptions(OptionsResolverInterface $resolver)
}

return function (FormInterface $form) {
return count($form) > 0 ? array() : '';
return $form->getConfig()->getCompound() ? array() : '';
};
};

Expand Down
50 changes: 29 additions & 21 deletions src/Symfony/Component/Form/Form.php
Expand Up @@ -345,7 +345,7 @@ public function setData($modelData)
$viewData = $this->normToView($normData);

// Validate if view data matches data class (unless empty)
if (!empty($viewData)) {
if ('' !== $viewData && null !== $viewData) {
$dataClass = $this->config->getDataClass();

$actualType = is_object($viewData) ? 'an instance of class ' . get_class($viewData) : ' a(n) ' . gettype($viewData);
Expand Down Expand Up @@ -378,7 +378,7 @@ public function setData($modelData)
$this->viewData = $viewData;
$this->synchronized = true;

if (count($this->children) > 0 && $this->config->getDataMapper()) {
if ($this->config->getCompound() && $this->config->getDataMapper()) {
// Update child forms from the data
$this->config->getDataMapper()->mapDataToForms($viewData, $this->children);
}
Expand Down Expand Up @@ -477,37 +477,42 @@ public function bind($submittedData)
$this->config->getEventDispatcher()->dispatch(FormEvents::BIND_CLIENT_DATA, $event);
$submittedData = $event->getData();

// Build the data in the view format
$viewData = $submittedData;

if (count($this->children) > 0) {
if (null === $viewData || '' === $viewData) {
$viewData = array();
// Check whether the form is compound.
// This check is preferrable over checking the number of children,
// since forms without children may also be compound.
// (think of empty collection forms)
if ($this->config->getCompound()) {

This comment has been minimized.

Copy link
@fran6co

fran6co Jul 4, 2012

Contributor

@bschussek shouldn't it set by default the compound to the number of children? After this commit my forms are failing because every form is set as compound even if they are not.

This comment has been minimized.

Copy link
@webmozart

webmozart Jul 4, 2012

Author Contributor

Example? Maybe your forms should be set to non-compound?

This comment has been minimized.

Copy link
@fran6co

fran6co Jul 4, 2012

Contributor

I'm trying to generate a reproducible case for you, but in any case my code was working fine before this change and in 2.0. I think this commit introduced a BC change with the compound flag.

This comment has been minimized.

Copy link
@fran6co

fran6co Jul 4, 2012

Contributor

Yes, you were right if I add setCompound(false) to my formType it works. I didn't see in the UPGRADE-2.1 any reference to that change.

My formType has as a parent 'field' and it translates a Doctrine entity to an id and viceversa.

if (null === $submittedData || '' === $submittedData) {
$submittedData = array();
}

if (!is_array($viewData)) {
throw new UnexpectedTypeException($viewData, 'array');
if (!is_array($submittedData)) {
throw new UnexpectedTypeException($submittedData, 'array');
}

foreach ($this->children as $name => $child) {
if (!isset($viewData[$name])) {
$viewData[$name] = null;
if (!isset($submittedData[$name])) {
$submittedData[$name] = null;
}
}

foreach ($viewData as $name => $value) {
foreach ($submittedData as $name => $value) {
if ($this->has($name)) {
$this->children[$name]->bind($value);
} else {
$extraData[$name] = $value;
}
}
}

// If we have a data mapper, use old view data and merge
// data from the children into it later
if ($this->config->getDataMapper()) {
$viewData = $this->getViewData();
}
// By default, the submitted data is also the data in view format
$viewData = $submittedData;

// If the form is compound, the default data in view format
// is reused. The data of the children is merged into this
// default data using the data mapper.
if ($this->config->getCompound() && $this->config->getDataMapper()) {
$viewData = $this->getViewData();
}

if (null === $viewData || '' === $viewData) {
Expand All @@ -522,7 +527,7 @@ public function bind($submittedData)
}

// Merge form data from children into existing view data
if (count($this->children) > 0 && $this->config->getDataMapper() && null !== $viewData) {
if ($this->config->getCompound() && $this->config->getDataMapper() && null !== $viewData) {
$this->config->getDataMapper()->mapFormsToData($this->children, $viewData);
}

Expand All @@ -542,7 +547,6 @@ public function bind($submittedData)
$this->config->getEventDispatcher()->dispatch(FormEvents::BIND_NORM_DATA, $event);
$normData = $event->getData();


// Synchronize representations - must not change the content!
$modelData = $this->normToModel($normData);
$viewData = $this->normToView($normData);
Expand Down Expand Up @@ -591,7 +595,7 @@ public function bindRequest(Request $request)
// Form bound without name
$params = $request->request->all();
$files = $request->files->all();
} elseif (count($this->children) > 0) {
} elseif ($this->config->getCompound()) {
// Form bound with name and children
$params = $request->request->get($name, array());
$files = $request->files->get($name, array());
Expand Down Expand Up @@ -841,6 +845,10 @@ public function add(FormInterface $child)
throw new AlreadyBoundException('You cannot add children to a bound form');
}

if (!$this->config->getCompound()) {
throw new FormException('You cannot add children to a simple form. Maybe you should set the option "compound" to true?');
}

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

$child->setParent($this);
Expand Down
23 changes: 23 additions & 0 deletions src/Symfony/Component/Form/FormConfig.php
Expand Up @@ -53,6 +53,11 @@ class FormConfig implements FormConfigEditorInterface
*/
private $virtual = false;

/**
* @var Boolean
*/
private $compound = true;

/**
* @var array
*/
Expand Down Expand Up @@ -356,6 +361,14 @@ public function getVirtual()
return $this->virtual;
}

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

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -632,6 +645,16 @@ public function setVirtual($virtual)
return $this;
}

/**
* {@inheritdoc}
*/
public function setCompound($compound)
{
$this->compound = $compound;

return $this;
}

/**
* {@inheritdoc}
*/
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Form/FormConfigEditorInterface.php
Expand Up @@ -199,6 +199,17 @@ function setByReference($byReference);
*/
function setVirtual($virtual);

/**
* Sets whether the form should be compound.
*
* @param Boolean $compound Whether the form should be compound.
*
* @return self The configuration object.
*
* @see FormConfigInterface::getCompound()
*/
function setCompound($compound);

/**
* Set the types.
*
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Form/FormConfigInterface.php
Expand Up @@ -65,6 +65,17 @@ function getByReference();
*/
function getVirtual();

/**
* Returns whether the form is compound.
*
* This property is independent of whether the form actually has
* children. A form can be compound and have no children at all, like
* for example an empty collection form.
*
* @return Boolean Whether the form is compound.
*/
function getCompound();

/**
* Returns the form types used to construct the form.
*
Expand Down
Expand Up @@ -18,7 +18,7 @@ class CollectionTypeTest extends TypeTestCase
public function testContainsNoChildByDefault()
{
$form = $this->factory->create('collection', null, array(
'type' => 'form',
'type' => 'text',
));

$this->assertCount(0, $form);
Expand All @@ -27,7 +27,7 @@ public function testContainsNoChildByDefault()
public function testSetDataAdjustsSize()
{
$form = $this->factory->create('collection', null, array(
'type' => 'form',
'type' => 'text',
'options' => array(
'max_length' => 20,
),
Expand All @@ -53,7 +53,7 @@ public function testSetDataAdjustsSize()
public function testThrowsExceptionIfObjectIsNotTraversable()
{
$form = $this->factory->create('collection', null, array(
'type' => 'form',
'type' => 'text',
));
$this->setExpectedException('Symfony\Component\Form\Exception\UnexpectedTypeException');
$form->setData(new \stdClass());
Expand All @@ -62,21 +62,21 @@ public function testThrowsExceptionIfObjectIsNotTraversable()
public function testNotResizedIfBoundWithMissingData()
{
$form = $this->factory->create('collection', null, array(
'type' => 'form',
'type' => 'text',
));
$form->setData(array('foo@foo.com', 'bar@bar.com'));
$form->bind(array('foo@bar.com'));

$this->assertTrue($form->has('0'));
$this->assertTrue($form->has('1'));
$this->assertEquals('foo@bar.com', $form[0]->getData());
$this->assertNull($form[1]->getData());
$this->assertEquals('', $form[1]->getData());
}

public function testResizedDownIfBoundWithMissingDataAndAllowDelete()
{
$form = $this->factory->create('collection', null, array(
'type' => 'form',
'type' => 'text',
'allow_delete' => true,
));
$form->setData(array('foo@foo.com', 'bar@bar.com'));
Expand All @@ -91,7 +91,7 @@ public function testResizedDownIfBoundWithMissingDataAndAllowDelete()
public function testNotResizedIfBoundWithExtraData()
{
$form = $this->factory->create('collection', null, array(
'type' => 'form',
'type' => 'text',
));
$form->setData(array('foo@bar.com'));
$form->bind(array('foo@foo.com', 'bar@bar.com'));
Expand All @@ -104,7 +104,7 @@ public function testNotResizedIfBoundWithExtraData()
public function testResizedUpIfBoundWithExtraDataAndAllowAdd()
{
$form = $this->factory->create('collection', null, array(
'type' => 'form',
'type' => 'text',
'allow_add' => true,
));
$form->setData(array('foo@bar.com'));
Expand Down

0 comments on commit 45d967e

Please sign in to comment.